Skip to content

Commit 37745ec

Browse files
authored
fix(plan): use .specify/feature.json to allow /speckit.plan on custom git branches (#2305) (#2349)
* fix: allow plan setup to use feature metadata on custom branches * fix: harden feature metadata validation * fix: use portable feature metadata path Made-with: Cursor * fix: share feature.json parser and make path compare OS aware * test: isolate setup plan subprocess environment * fix: normalize feature metadata paths with pwd -P
1 parent 998f927 commit 37745ec

5 files changed

Lines changed: 335 additions & 14 deletions

File tree

scripts/bash/common.sh

Lines changed: 56 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,59 @@ check_feature_branch() {
153153
return 0
154154
}
155155

156+
# Safely read .specify/feature.json's "feature_directory" value.
157+
# Prints the raw value (possibly relative) to stdout, or empty string if the file
158+
# is missing, unparseable, or does not contain the key. Always returns 0 so callers
159+
# under `set -e` cannot be aborted by parser failure.
160+
# Parser order mirrors the historical get_feature_paths behavior: jq -> python3 -> grep/sed.
161+
read_feature_json_feature_directory() {
162+
local repo_root="$1"
163+
local fj="$repo_root/.specify/feature.json"
164+
[[ -f "$fj" ]] || { printf '%s' ''; return 0; }
165+
166+
local _fd=''
167+
if command -v jq >/dev/null 2>&1; then
168+
if ! _fd=$(jq -r '.feature_directory // empty' "$fj" 2>/dev/null); then
169+
_fd=''
170+
fi
171+
elif command -v python3 >/dev/null 2>&1; then
172+
# Use Python so pretty-printed/multi-line JSON still parses correctly.
173+
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
174+
_fd=''
175+
fi
176+
else
177+
# Last-resort single-line grep/sed fallback. The `|| true` guards against
178+
# grep returning 1 (no match) aborting under `set -e` / `pipefail`.
179+
_fd=$( { grep -E '"feature_directory"[[:space:]]*:' "$fj" 2>/dev/null || true; } \
180+
| head -n 1 \
181+
| sed -E 's/^[^:]*:[[:space:]]*"([^"]*)".*$/\1/' )
182+
fi
183+
184+
printf '%s' "$_fd"
185+
return 0
186+
}
187+
188+
# Returns 0 when .specify/feature.json lists feature_directory that exists as a directory
189+
# and matches the resolved active FEATURE_DIR (so /speckit.plan can skip git branch pattern checks).
190+
# Delegates parsing to read_feature_json_feature_directory, which is safe under `set -e`.
191+
feature_json_matches_feature_dir() {
192+
local repo_root="$1"
193+
local active_feature_dir="$2"
194+
195+
local _fd
196+
_fd=$(read_feature_json_feature_directory "$repo_root")
197+
198+
[[ -n "$_fd" ]] || return 1
199+
[[ "$_fd" != /* ]] && _fd="$repo_root/$_fd"
200+
[[ -d "$_fd" ]] || return 1
201+
202+
local norm_json norm_active
203+
norm_json="$(cd -- "$_fd" 2>/dev/null && pwd -P)" || return 1
204+
norm_active="$(cd -- "$active_feature_dir" 2>/dev/null && pwd -P)" || return 1
205+
206+
[[ "$norm_json" == "$norm_active" ]]
207+
}
208+
156209
# Find feature directory by numeric prefix instead of exact branch match
157210
# This allows multiple branches to work on the same spec (e.g., 004-fix-bug, 004-add-feature)
158211
find_feature_dir_by_prefix() {
@@ -217,16 +270,10 @@ get_feature_paths() {
217270
# Normalize relative paths to absolute under repo root
218271
[[ "$feature_dir" != /* ]] && feature_dir="$repo_root/$feature_dir"
219272
elif [[ -f "$repo_root/.specify/feature.json" ]]; then
273+
# Shared, set -e-safe parser: jq -> python3 -> grep/sed. Returns empty on
274+
# missing/unparseable/unset so we fall through to the branch-prefix lookup.
220275
local _fd
221-
if command -v jq >/dev/null 2>&1; then
222-
_fd=$(jq -r '.feature_directory // empty' "$repo_root/.specify/feature.json" 2>/dev/null)
223-
elif command -v python3 >/dev/null 2>&1; then
224-
# Fallback: use Python to parse JSON so pretty-printed/multi-line files work
225-
_fd=$(python3 -c "import json,sys; d=json.load(open(sys.argv[1])); print(d.get('feature_directory',''))" "$repo_root/.specify/feature.json" 2>/dev/null)
226-
else
227-
# Last resort: single-line grep fallback (won't work on multi-line JSON)
228-
_fd=$(grep -o '"feature_directory"[[:space:]]*:[[:space:]]*"[^"]*"' "$repo_root/.specify/feature.json" 2>/dev/null | sed 's/.*"\([^"]*\)"$/\1/')
229-
fi
276+
_fd=$(read_feature_json_feature_directory "$repo_root")
230277
if [[ -n "$_fd" ]]; then
231278
feature_dir="$_fd"
232279
# Normalize relative paths to absolute under repo root

scripts/bash/setup-plan.sh

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@ _paths_output=$(get_feature_paths) || { echo "ERROR: Failed to resolve feature p
3232
eval "$_paths_output"
3333
unset _paths_output
3434

35-
# Check if we're on a proper feature branch (only for git repos)
36-
check_feature_branch "$CURRENT_BRANCH" "$HAS_GIT" || exit 1
35+
# If feature.json pins an existing feature directory, branch naming is not required.
36+
if ! feature_json_matches_feature_dir "$REPO_ROOT" "$FEATURE_DIR"; then
37+
check_feature_branch "$CURRENT_BRANCH" "$HAS_GIT" || exit 1
38+
fi
3739

3840
# Ensure the feature directory exists
3941
mkdir -p "$FEATURE_DIR"

scripts/powershell/common.ps1

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,74 @@ function Test-FeatureBranch {
164164
return $true
165165
}
166166

167+
# True when .specify/feature.json pins an existing feature directory that matches the
168+
# active FEATURE_DIR from Get-FeaturePathsEnv (so /speckit.plan can skip git branch pattern checks).
169+
function Test-FeatureJsonMatchesFeatureDir {
170+
param(
171+
[Parameter(Mandatory = $true)][string]$RepoRoot,
172+
[Parameter(Mandatory = $true)][string]$ActiveFeatureDir
173+
)
174+
175+
$featureJson = Join-Path (Join-Path $RepoRoot '.specify') 'feature.json'
176+
if (-not (Test-Path -LiteralPath $featureJson -PathType Leaf)) {
177+
return $false
178+
}
179+
180+
try {
181+
$raw = Get-Content -LiteralPath $featureJson -Raw
182+
$cfg = $raw | ConvertFrom-Json
183+
} catch {
184+
return $false
185+
}
186+
187+
$fd = $cfg.feature_directory
188+
if ([string]::IsNullOrWhiteSpace([string]$fd)) {
189+
return $false
190+
}
191+
192+
if (-not [System.IO.Path]::IsPathRooted($fd)) {
193+
$fd = Join-Path $RepoRoot $fd
194+
}
195+
196+
if (-not (Test-Path -LiteralPath $fd -PathType Container)) {
197+
return $false
198+
}
199+
200+
# Resolve both paths to canonical absolute form. Prefer Resolve-Path (follows
201+
# symlinks and is the canonical PS way); fall back to [Path]::GetFullPath when
202+
# Resolve-Path can't produce a value. Mirrors the pattern used by Find-SpecifyRoot.
203+
$resolvedJson = Resolve-Path -LiteralPath $fd -ErrorAction SilentlyContinue
204+
if ($resolvedJson) {
205+
$normJson = $resolvedJson.Path
206+
} else {
207+
$normJson = [System.IO.Path]::GetFullPath($fd)
208+
}
209+
210+
$resolvedActive = Resolve-Path -LiteralPath $ActiveFeatureDir -ErrorAction SilentlyContinue
211+
if ($resolvedActive) {
212+
$normActive = $resolvedActive.Path
213+
} else {
214+
$normActive = [System.IO.Path]::GetFullPath($ActiveFeatureDir)
215+
}
216+
217+
# Use case-insensitive compare only on Windows; POSIX filesystems are case-sensitive.
218+
# PowerShell 5.1 is Windows-only and does not define $IsWindows, so treat its
219+
# absence as "we're on Windows".
220+
if ($null -ne $IsWindows) {
221+
$onWindows = $IsWindows
222+
} else {
223+
$onWindows = $true
224+
}
225+
226+
if ($onWindows) {
227+
$comparison = [System.StringComparison]::OrdinalIgnoreCase
228+
} else {
229+
$comparison = [System.StringComparison]::Ordinal
230+
}
231+
232+
return [string]::Equals($normJson, $normActive, $comparison)
233+
}
234+
167235
# Resolve specs/<feature-dir> by numeric/timestamp prefix (mirrors scripts/bash/common.sh find_feature_dir_by_prefix).
168236
function Find-FeatureDirByPrefix {
169237
param(

scripts/powershell/setup-plan.ps1

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ if ($Help) {
2323
# Get all paths and variables from common functions
2424
$paths = Get-FeaturePathsEnv
2525

26-
# Check if we're on a proper feature branch (only for git repos)
27-
if (-not (Test-FeatureBranch -Branch $paths.CURRENT_BRANCH -HasGit $paths.HAS_GIT)) {
28-
exit 1
26+
# If feature.json pins an existing feature directory, branch naming is not required.
27+
if (-not (Test-FeatureJsonMatchesFeatureDir -RepoRoot $paths.REPO_ROOT -ActiveFeatureDir $paths.FEATURE_DIR)) {
28+
if (-not (Test-FeatureBranch -Branch $paths.CURRENT_BRANCH -HasGit $paths.HAS_GIT)) {
29+
exit 1
30+
}
2931
}
3032

3133
# Ensure the feature directory exists
Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
"""Tests for setup-plan bypassing branch-pattern checks when feature.json is valid."""
2+
3+
import json
4+
import os
5+
import shutil
6+
import subprocess
7+
from pathlib import Path
8+
9+
import pytest
10+
11+
from tests.conftest import requires_bash
12+
13+
PROJECT_ROOT = Path(__file__).resolve().parent.parent
14+
COMMON_SH = PROJECT_ROOT / "scripts" / "bash" / "common.sh"
15+
SETUP_PLAN_SH = PROJECT_ROOT / "scripts" / "bash" / "setup-plan.sh"
16+
COMMON_PS = PROJECT_ROOT / "scripts" / "powershell" / "common.ps1"
17+
SETUP_PLAN_PS = PROJECT_ROOT / "scripts" / "powershell" / "setup-plan.ps1"
18+
PLAN_TEMPLATE = PROJECT_ROOT / "templates" / "plan-template.md"
19+
20+
HAS_PWSH = shutil.which("pwsh") is not None
21+
_POWERSHELL = shutil.which("powershell.exe") or shutil.which("powershell")
22+
23+
24+
def _install_bash_scripts(repo: Path) -> None:
25+
d = repo / ".specify" / "scripts" / "bash"
26+
d.mkdir(parents=True, exist_ok=True)
27+
shutil.copy(COMMON_SH, d / "common.sh")
28+
shutil.copy(SETUP_PLAN_SH, d / "setup-plan.sh")
29+
30+
31+
def _install_ps_scripts(repo: Path) -> None:
32+
d = repo / ".specify" / "scripts" / "powershell"
33+
d.mkdir(parents=True, exist_ok=True)
34+
shutil.copy(COMMON_PS, d / "common.ps1")
35+
shutil.copy(SETUP_PLAN_PS, d / "setup-plan.ps1")
36+
37+
38+
def _minimal_templates(repo: Path) -> None:
39+
tdir = repo / ".specify" / "templates"
40+
tdir.mkdir(parents=True, exist_ok=True)
41+
shutil.copy(PLAN_TEMPLATE, tdir / "plan-template.md")
42+
43+
44+
def _clean_env() -> dict[str, str]:
45+
"""Return a copy of the current environment with any SPECIFY_* vars removed.
46+
47+
setup-plan.{sh,ps1} honors SPECIFY_FEATURE, SPECIFY_FEATURE_DIRECTORY, etc.,
48+
which would otherwise leak from a developer shell or CI runner and make these
49+
tests flaky. Stripping them forces every case to rely purely on git branch +
50+
.specify/feature.json state set up by the fixture.
51+
"""
52+
env = os.environ.copy()
53+
for key in list(env):
54+
if key.startswith("SPECIFY_"):
55+
env.pop(key)
56+
return env
57+
58+
59+
def _git_init(repo: Path) -> None:
60+
subprocess.run(["git", "init", "-q"], cwd=repo, check=True)
61+
subprocess.run(
62+
["git", "config", "user.email", "test@example.com"], cwd=repo, check=True
63+
)
64+
subprocess.run(["git", "config", "user.name", "Test User"], cwd=repo, check=True)
65+
subprocess.run(
66+
["git", "commit", "--allow-empty", "-m", "init", "-q"], cwd=repo, check=True
67+
)
68+
69+
70+
@pytest.fixture
71+
def plan_repo(tmp_path: Path) -> Path:
72+
repo = tmp_path / "proj"
73+
repo.mkdir()
74+
_git_init(repo)
75+
(repo / ".specify").mkdir()
76+
_minimal_templates(repo)
77+
_install_bash_scripts(repo)
78+
_install_ps_scripts(repo)
79+
return repo
80+
81+
82+
@requires_bash
83+
def test_setup_plan_passes_custom_branch_when_feature_json_valid(plan_repo: Path) -> None:
84+
subprocess.run(
85+
["git", "checkout", "-q", "-b", "feature/my-feature-branch"],
86+
cwd=plan_repo,
87+
check=True,
88+
)
89+
feat = plan_repo / "specs" / "001-tiny-notes-app"
90+
feat.mkdir(parents=True)
91+
(feat / "spec.md").write_text("# spec\n", encoding="utf-8")
92+
(plan_repo / ".specify" / "feature.json").write_text(
93+
json.dumps({"feature_directory": "specs/001-tiny-notes-app"}),
94+
encoding="utf-8",
95+
)
96+
script = plan_repo / ".specify" / "scripts" / "bash" / "setup-plan.sh"
97+
result = subprocess.run(
98+
["bash", str(script)],
99+
cwd=plan_repo,
100+
capture_output=True,
101+
text=True,
102+
check=False,
103+
env=_clean_env(),
104+
)
105+
assert result.returncode == 0, result.stderr + result.stdout
106+
assert (feat / "plan.md").is_file()
107+
108+
109+
@requires_bash
110+
def test_setup_plan_fails_custom_branch_without_feature_json(plan_repo: Path) -> None:
111+
subprocess.run(
112+
["git", "checkout", "-q", "-b", "feature/my-feature-branch"],
113+
cwd=plan_repo,
114+
check=True,
115+
)
116+
script = plan_repo / ".specify" / "scripts" / "bash" / "setup-plan.sh"
117+
result = subprocess.run(
118+
["bash", str(script)],
119+
cwd=plan_repo,
120+
capture_output=True,
121+
text=True,
122+
check=False,
123+
env=_clean_env(),
124+
)
125+
assert result.returncode != 0
126+
assert "Not on a feature branch" in result.stderr
127+
128+
129+
@requires_bash
130+
def test_setup_plan_numbered_branch_unchanged_without_feature_json(
131+
plan_repo: Path,
132+
) -> None:
133+
subprocess.run(
134+
["git", "checkout", "-q", "-b", "001-tiny-notes-app"],
135+
cwd=plan_repo,
136+
check=True,
137+
)
138+
feat = plan_repo / "specs" / "001-tiny-notes-app"
139+
feat.mkdir(parents=True)
140+
(feat / "spec.md").write_text("# spec\n", encoding="utf-8")
141+
script = plan_repo / ".specify" / "scripts" / "bash" / "setup-plan.sh"
142+
result = subprocess.run(
143+
["bash", str(script)],
144+
cwd=plan_repo,
145+
capture_output=True,
146+
text=True,
147+
check=False,
148+
env=_clean_env(),
149+
)
150+
assert result.returncode == 0, result.stderr + result.stdout
151+
assert (feat / "plan.md").is_file()
152+
153+
154+
@pytest.mark.skipif(not (HAS_PWSH or _POWERSHELL), reason="no PowerShell available")
155+
def test_setup_plan_ps_passes_custom_branch_when_feature_json_valid(plan_repo: Path) -> None:
156+
subprocess.run(
157+
["git", "checkout", "-q", "-b", "feature/my-feature-branch"],
158+
cwd=plan_repo,
159+
check=True,
160+
)
161+
feat = plan_repo / "specs" / "001-tiny-notes-app"
162+
feat.mkdir(parents=True)
163+
(feat / "spec.md").write_text("# spec\n", encoding="utf-8")
164+
(plan_repo / ".specify" / "feature.json").write_text(
165+
json.dumps({"feature_directory": "specs/001-tiny-notes-app"}),
166+
encoding="utf-8",
167+
)
168+
script = plan_repo / ".specify" / "scripts" / "powershell" / "setup-plan.ps1"
169+
exe = "pwsh" if HAS_PWSH else _POWERSHELL
170+
result = subprocess.run(
171+
[exe, "-NoProfile", "-File", str(script)],
172+
cwd=plan_repo,
173+
capture_output=True,
174+
text=True,
175+
check=False,
176+
env=_clean_env(),
177+
)
178+
assert result.returncode == 0, result.stderr + result.stdout
179+
assert (feat / "plan.md").is_file()
180+
181+
182+
@pytest.mark.skipif(not (HAS_PWSH or _POWERSHELL), reason="no PowerShell available")
183+
def test_setup_plan_ps_fails_custom_branch_without_feature_json(
184+
plan_repo: Path,
185+
) -> None:
186+
subprocess.run(
187+
["git", "checkout", "-q", "-b", "feature/my-feature-branch"],
188+
cwd=plan_repo,
189+
check=True,
190+
)
191+
script = plan_repo / ".specify" / "scripts" / "powershell" / "setup-plan.ps1"
192+
exe = "pwsh" if HAS_PWSH else _POWERSHELL
193+
result = subprocess.run(
194+
[exe, "-NoProfile", "-File", str(script)],
195+
cwd=plan_repo,
196+
capture_output=True,
197+
text=True,
198+
check=False,
199+
env=_clean_env(),
200+
)
201+
assert result.returncode != 0
202+
assert "Not on a feature branch" in result.stderr

0 commit comments

Comments
 (0)