From 491b60ae7bc81f906a0cd3fd3f1e3db439d5b5f5 Mon Sep 17 00:00:00 2001 From: Adrian Osorio Blanchard Date: Thu, 23 Apr 2026 22:59:53 -0400 Subject: [PATCH 1/2] fix: allow plan setup to use feature metadata on custom branches --- scripts/bash/common.sh | 24 ++++ scripts/bash/setup-plan.sh | 6 +- scripts/powershell/common.ps1 | 38 ++++++ scripts/powershell/setup-plan.ps1 | 8 +- tests/test_setup_plan_feature_json.py | 159 ++++++++++++++++++++++++++ 5 files changed, 230 insertions(+), 5 deletions(-) create mode 100644 tests/test_setup_plan_feature_json.py diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index cad10bdb39..e8396400c1 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -153,6 +153,30 @@ check_feature_branch() { return 0 } +# Returns 0 when .specify/feature.json lists feature_directory that exists as a directory +# and matches the resolved active FEATURE_DIR (so /speckit.plan can skip git branch pattern checks). +feature_json_matches_feature_dir() { + local repo_root="$1" + local active_feature_dir="$2" + local fj="$repo_root/.specify/feature.json" + [[ -f "$fj" ]] || return 1 + local _fd + if command -v jq >/dev/null 2>&1; then + _fd=$(jq -r '.feature_directory // empty' "$fj" 2>/dev/null) + elif command -v python3 >/dev/null 2>&1; then + _fd=$(python3 -c "import json,sys; d=json.load(open(sys.argv[1])); v=d.get('feature_directory'); print(v if v else '')" "$fj" 2>/dev/null) + else + return 1 + fi + [[ -n "$_fd" ]] || return 1 + [[ "$_fd" != /* ]] && _fd="$repo_root/$_fd" + [[ -d "$_fd" ]] || return 1 + local norm_json norm_active + norm_json="$(cd -- "$_fd" 2>/dev/null && pwd)" || return 1 + norm_active="$(cd -- "$active_feature_dir" 2>/dev/null && pwd)" || return 1 + [[ "$norm_json" == "$norm_active" ]] +} + # Find feature directory by numeric prefix instead of exact branch match # This allows multiple branches to work on the same spec (e.g., 004-fix-bug, 004-add-feature) find_feature_dir_by_prefix() { diff --git a/scripts/bash/setup-plan.sh b/scripts/bash/setup-plan.sh index 9f5523149e..f2d2f6e6fc 100644 --- a/scripts/bash/setup-plan.sh +++ b/scripts/bash/setup-plan.sh @@ -32,8 +32,10 @@ _paths_output=$(get_feature_paths) || { echo "ERROR: Failed to resolve feature p eval "$_paths_output" unset _paths_output -# Check if we're on a proper feature branch (only for git repos) -check_feature_branch "$CURRENT_BRANCH" "$HAS_GIT" || exit 1 +# If feature.json pins an existing feature directory, branch naming is not required. +if ! feature_json_matches_feature_dir "$REPO_ROOT" "$FEATURE_DIR"; then + check_feature_branch "$CURRENT_BRANCH" "$HAS_GIT" || exit 1 +fi # Ensure the feature directory exists mkdir -p "$FEATURE_DIR" diff --git a/scripts/powershell/common.ps1 b/scripts/powershell/common.ps1 index d799e4f7e7..a715ae368f 100644 --- a/scripts/powershell/common.ps1 +++ b/scripts/powershell/common.ps1 @@ -164,6 +164,44 @@ function Test-FeatureBranch { return $true } +# True when .specify/feature.json pins an existing feature directory that matches the +# active FEATURE_DIR from Get-FeaturePathsEnv (so /speckit.plan can skip git branch pattern checks). +function Test-FeatureJsonMatchesFeatureDir { + param( + [Parameter(Mandatory = $true)][string]$RepoRoot, + [Parameter(Mandatory = $true)][string]$ActiveFeatureDir + ) + + $featureJson = Join-Path $RepoRoot '.specify\feature.json' + if (-not (Test-Path -LiteralPath $featureJson -PathType Leaf)) { + return $false + } + + try { + $raw = Get-Content -LiteralPath $featureJson -Raw + $cfg = $raw | ConvertFrom-Json + } catch { + return $false + } + + $fd = $cfg.feature_directory + if ([string]::IsNullOrWhiteSpace([string]$fd)) { + return $false + } + + if (-not [System.IO.Path]::IsPathRooted($fd)) { + $fd = Join-Path $RepoRoot $fd + } + + if (-not (Test-Path -LiteralPath $fd -PathType Container)) { + return $false + } + + $normJson = [System.IO.Path]::GetFullPath($fd) + $normActive = [System.IO.Path]::GetFullPath($ActiveFeatureDir) + return [string]::Equals($normJson, $normActive, [System.StringComparison]::OrdinalIgnoreCase) +} + # Resolve specs/ by numeric/timestamp prefix (mirrors scripts/bash/common.sh find_feature_dir_by_prefix). function Find-FeatureDirByPrefix { param( diff --git a/scripts/powershell/setup-plan.ps1 b/scripts/powershell/setup-plan.ps1 index ee09094bf7..15ae557544 100644 --- a/scripts/powershell/setup-plan.ps1 +++ b/scripts/powershell/setup-plan.ps1 @@ -23,9 +23,11 @@ if ($Help) { # Get all paths and variables from common functions $paths = Get-FeaturePathsEnv -# Check if we're on a proper feature branch (only for git repos) -if (-not (Test-FeatureBranch -Branch $paths.CURRENT_BRANCH -HasGit $paths.HAS_GIT)) { - exit 1 +# If feature.json pins an existing feature directory, branch naming is not required. +if (-not (Test-FeatureJsonMatchesFeatureDir -RepoRoot $paths.REPO_ROOT -ActiveFeatureDir $paths.FEATURE_DIR)) { + if (-not (Test-FeatureBranch -Branch $paths.CURRENT_BRANCH -HasGit $paths.HAS_GIT)) { + exit 1 + } } # Ensure the feature directory exists diff --git a/tests/test_setup_plan_feature_json.py b/tests/test_setup_plan_feature_json.py new file mode 100644 index 0000000000..a5a57bc8de --- /dev/null +++ b/tests/test_setup_plan_feature_json.py @@ -0,0 +1,159 @@ +"""Tests for setup-plan bypassing branch-pattern checks when feature.json is valid.""" + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +from tests.conftest import requires_bash + +PROJECT_ROOT = Path(__file__).resolve().parent.parent +COMMON_SH = PROJECT_ROOT / "scripts" / "bash" / "common.sh" +SETUP_PLAN_SH = PROJECT_ROOT / "scripts" / "bash" / "setup-plan.sh" +COMMON_PS = PROJECT_ROOT / "scripts" / "powershell" / "common.ps1" +SETUP_PLAN_PS = PROJECT_ROOT / "scripts" / "powershell" / "setup-plan.ps1" +PLAN_TEMPLATE = PROJECT_ROOT / "templates" / "plan-template.md" + +HAS_PWSH = shutil.which("pwsh") is not None +_POWERSHELL = shutil.which("powershell.exe") or shutil.which("powershell") + + +def _install_bash_scripts(repo: Path) -> None: + d = repo / ".specify" / "scripts" / "bash" + d.mkdir(parents=True, exist_ok=True) + shutil.copy(COMMON_SH, d / "common.sh") + shutil.copy(SETUP_PLAN_SH, d / "setup-plan.sh") + + +def _install_ps_scripts(repo: Path) -> None: + d = repo / ".specify" / "scripts" / "powershell" + d.mkdir(parents=True, exist_ok=True) + shutil.copy(COMMON_PS, d / "common.ps1") + shutil.copy(SETUP_PLAN_PS, d / "setup-plan.ps1") + + +def _minimal_templates(repo: Path) -> None: + tdir = repo / ".specify" / "templates" + tdir.mkdir(parents=True, exist_ok=True) + shutil.copy(PLAN_TEMPLATE, tdir / "plan-template.md") + + +def _git_init(repo: Path) -> None: + subprocess.run(["git", "init", "-q"], cwd=repo, check=True) + subprocess.run( + ["git", "config", "user.email", "test@example.com"], cwd=repo, check=True + ) + subprocess.run(["git", "config", "user.name", "Test User"], cwd=repo, check=True) + subprocess.run( + ["git", "commit", "--allow-empty", "-m", "init", "-q"], cwd=repo, check=True + ) + + +@pytest.fixture +def plan_repo(tmp_path: Path) -> Path: + repo = tmp_path / "proj" + repo.mkdir() + _git_init(repo) + (repo / ".specify").mkdir() + _minimal_templates(repo) + _install_bash_scripts(repo) + _install_ps_scripts(repo) + return repo + + +@requires_bash +def test_setup_plan_passes_custom_branch_when_feature_json_valid(plan_repo: Path) -> None: + subprocess.run( + ["git", "checkout", "-q", "-b", "feature/my-feature-branch"], + cwd=plan_repo, + check=True, + ) + feat = plan_repo / "specs" / "001-tiny-notes-app" + feat.mkdir(parents=True) + (feat / "spec.md").write_text("# spec\n", encoding="utf-8") + (plan_repo / ".specify" / "feature.json").write_text( + json.dumps({"feature_directory": "specs/001-tiny-notes-app"}), + encoding="utf-8", + ) + script = plan_repo / ".specify" / "scripts" / "bash" / "setup-plan.sh" + result = subprocess.run( + ["bash", str(script)], + cwd=plan_repo, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + result.stdout + assert (feat / "plan.md").is_file() + + +@requires_bash +def test_setup_plan_fails_custom_branch_without_feature_json(plan_repo: Path) -> None: + subprocess.run( + ["git", "checkout", "-q", "-b", "feature/my-feature-branch"], + cwd=plan_repo, + check=True, + ) + script = plan_repo / ".specify" / "scripts" / "bash" / "setup-plan.sh" + result = subprocess.run( + ["bash", str(script)], + cwd=plan_repo, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode != 0 + assert "Not on a feature branch" in result.stderr + + +@requires_bash +def test_setup_plan_numbered_branch_unchanged_without_feature_json( + plan_repo: Path, +) -> None: + subprocess.run( + ["git", "checkout", "-q", "-b", "001-tiny-notes-app"], + cwd=plan_repo, + check=True, + ) + feat = plan_repo / "specs" / "001-tiny-notes-app" + feat.mkdir(parents=True) + (feat / "spec.md").write_text("# spec\n", encoding="utf-8") + script = plan_repo / ".specify" / "scripts" / "bash" / "setup-plan.sh" + result = subprocess.run( + ["bash", str(script)], + cwd=plan_repo, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + result.stdout + assert (feat / "plan.md").is_file() + + +@pytest.mark.skipif(not (HAS_PWSH or _POWERSHELL), reason="no PowerShell available") +def test_setup_plan_ps_passes_custom_branch_when_feature_json_valid(plan_repo: Path) -> None: + subprocess.run( + ["git", "checkout", "-q", "-b", "feature/my-feature-branch"], + cwd=plan_repo, + check=True, + ) + feat = plan_repo / "specs" / "001-tiny-notes-app" + feat.mkdir(parents=True) + (feat / "spec.md").write_text("# spec\n", encoding="utf-8") + (plan_repo / ".specify" / "feature.json").write_text( + json.dumps({"feature_directory": "specs/001-tiny-notes-app"}), + encoding="utf-8", + ) + script = plan_repo / ".specify" / "scripts" / "powershell" / "setup-plan.ps1" + exe = "pwsh" if HAS_PWSH else _POWERSHELL + result = subprocess.run( + [exe, "-NoProfile", "-File", str(script)], + cwd=plan_repo, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + result.stdout + assert (feat / "plan.md").is_file() From d584e7fd831742b6671db29d1b8a1359169ef58c Mon Sep 17 00:00:00 2001 From: Adrian Osorio Blanchard Date: Fri, 24 Apr 2026 00:16:53 -0400 Subject: [PATCH 2/2] fix: harden feature metadata validation --- scripts/bash/common.sh | 20 +++++++++++++++++--- tests/test_setup_plan_feature_json.py | 22 ++++++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index e8396400c1..f83f4557f2 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -155,25 +155,39 @@ check_feature_branch() { # Returns 0 when .specify/feature.json lists feature_directory that exists as a directory # and matches the resolved active FEATURE_DIR (so /speckit.plan can skip git branch pattern checks). +# Parser fallback order mirrors get_feature_paths: jq -> python3 -> grep/sed. +# All parser failures are treated as "no match" (return 1) so callers under `set -e` +# fall through to the existing branch validation instead of aborting the script. feature_json_matches_feature_dir() { local repo_root="$1" local active_feature_dir="$2" local fj="$repo_root/.specify/feature.json" + [[ -f "$fj" ]] || return 1 + local _fd if command -v jq >/dev/null 2>&1; then - _fd=$(jq -r '.feature_directory // empty' "$fj" 2>/dev/null) + if ! _fd=$(jq -r '.feature_directory // empty' "$fj" 2>/dev/null); then + return 1 + fi elif command -v python3 >/dev/null 2>&1; then - _fd=$(python3 -c "import json,sys; d=json.load(open(sys.argv[1])); v=d.get('feature_directory'); print(v if v else '')" "$fj" 2>/dev/null) + if ! _fd=$(python3 -c "import json,sys; d=json.load(open(sys.argv[1])); v=d.get('feature_directory'); print(v if v else '')" "$fj" 2>/dev/null); then + return 1 + fi else - return 1 + _fd=$(grep -E '"feature_directory"[[:space:]]*:' "$fj" 2>/dev/null \ + | head -n 1 \ + | sed -E 's/^[^:]*:[[:space:]]*"([^"]*)".*$/\1/') fi + [[ -n "$_fd" ]] || return 1 [[ "$_fd" != /* ]] && _fd="$repo_root/$_fd" [[ -d "$_fd" ]] || return 1 + local norm_json norm_active norm_json="$(cd -- "$_fd" 2>/dev/null && pwd)" || return 1 norm_active="$(cd -- "$active_feature_dir" 2>/dev/null && pwd)" || return 1 + [[ "$norm_json" == "$norm_active" ]] } diff --git a/tests/test_setup_plan_feature_json.py b/tests/test_setup_plan_feature_json.py index a5a57bc8de..6f3a143cff 100644 --- a/tests/test_setup_plan_feature_json.py +++ b/tests/test_setup_plan_feature_json.py @@ -157,3 +157,25 @@ def test_setup_plan_ps_passes_custom_branch_when_feature_json_valid(plan_repo: P ) assert result.returncode == 0, result.stderr + result.stdout assert (feat / "plan.md").is_file() + + +@pytest.mark.skipif(not (HAS_PWSH or _POWERSHELL), reason="no PowerShell available") +def test_setup_plan_ps_fails_custom_branch_without_feature_json( + plan_repo: Path, +) -> None: + subprocess.run( + ["git", "checkout", "-q", "-b", "feature/my-feature-branch"], + cwd=plan_repo, + check=True, + ) + script = plan_repo / ".specify" / "scripts" / "powershell" / "setup-plan.ps1" + exe = "pwsh" if HAS_PWSH else _POWERSHELL + result = subprocess.run( + [exe, "-NoProfile", "-File", str(script)], + cwd=plan_repo, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode != 0 + assert "Not on a feature branch" in result.stderr