Skip to content

Commit af8f5a4

Browse files
authored
fix(scripts): name setup-plan's feature directory key FEATURE_DIR (#4397)
* fix(scripts): name setup-plan's feature directory key FEATURE_DIR setup-plan emitted a key called SPECS_DIR holding $FEATURE_DIR -- the per-feature subdirectory, not the specs root. The name is already taken elsewhere with the other meaning: create-new-feature.sh sets SPECS_DIR="$REPO_ROOT/specs" and derives FEATURE_DIR="$SPECS_DIR/$BRANCH_NAME". setup-plan was also the only script in the suite using it. setup-tasks and both check-prerequisites payloads already emit FEATURE_DIR for exactly this value, so this brings setup-plan in line rather than inventing a convention. Renamed in all three ports so the payloads stay identical, and in templates/commands/plan.md, which is the only consumer -- it parses the key by name, so it has to move in the same commit. Verified the bash, PowerShell, and Python variants all emit ['BRANCH','FEATURE_DIR','FEATURE_SPEC','IMPL_PLAN']. Fixes #4017 * test(scripts): pin setup-plan's FEATURE_DIR output contract Addresses review feedback. The existing setup-plan tests compare the ports against each other, so all three could regress to SPECS_DIR together and still pass. This asserts the contract absolutely, in JSON and text mode and across bash/Python/PowerShell: the key is FEATURE_DIR, it carries the feature directory rather than the specs root, and SPECS_DIR is absent. The value is matched by suffix rather than full path because the ports legitimately differ in path flavour -- under MSYS bash reports /tmp/... where the Python and PowerShell ports report C:\... . The suffix still separates specs/001-my-feature from a bare specs, which is the regression being guarded; verified it rejects both /tmp/proj/specs and C:\proj\specs.
1 parent db64869 commit af8f5a4

5 files changed

Lines changed: 63 additions & 9 deletions

File tree

‎scripts/bash/setup-plan.sh‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,16 +70,16 @@ if $JSON_MODE; then
7070
jq -cn \
7171
--arg feature_spec "$FEATURE_SPEC" \
7272
--arg impl_plan "$IMPL_PLAN" \
73-
--arg specs_dir "$FEATURE_DIR" \
73+
--arg feature_dir "$FEATURE_DIR" \
7474
--arg branch "$CURRENT_BRANCH" \
75-
'{FEATURE_SPEC:$feature_spec,IMPL_PLAN:$impl_plan,SPECS_DIR:$specs_dir,BRANCH:$branch}'
75+
'{FEATURE_SPEC:$feature_spec,IMPL_PLAN:$impl_plan,FEATURE_DIR:$feature_dir,BRANCH:$branch}'
7676
else
77-
printf '{"FEATURE_SPEC":"%s","IMPL_PLAN":"%s","SPECS_DIR":"%s","BRANCH":"%s"}\n' \
77+
printf '{"FEATURE_SPEC":"%s","IMPL_PLAN":"%s","FEATURE_DIR":"%s","BRANCH":"%s"}\n' \
7878
"$(json_escape "$FEATURE_SPEC")" "$(json_escape "$IMPL_PLAN")" "$(json_escape "$FEATURE_DIR")" "$(json_escape "$CURRENT_BRANCH")"
7979
fi
8080
else
8181
echo "FEATURE_SPEC: $FEATURE_SPEC"
8282
echo "IMPL_PLAN: $IMPL_PLAN"
83-
echo "SPECS_DIR: $FEATURE_DIR"
83+
echo "FEATURE_DIR: $FEATURE_DIR"
8484
echo "BRANCH: $CURRENT_BRANCH"
8585
fi

‎scripts/powershell/setup-plan.ps1‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,13 @@ if ($Json) {
7676
$result = [PSCustomObject]@{
7777
FEATURE_SPEC = $paths.FEATURE_SPEC
7878
IMPL_PLAN = $paths.IMPL_PLAN
79-
SPECS_DIR = $paths.FEATURE_DIR
79+
FEATURE_DIR = $paths.FEATURE_DIR
8080
BRANCH = $paths.CURRENT_BRANCH
8181
}
8282
$result | ConvertTo-Json -Compress
8383
} else {
8484
Write-Output "FEATURE_SPEC: $($paths.FEATURE_SPEC)"
8585
Write-Output "IMPL_PLAN: $($paths.IMPL_PLAN)"
86-
Write-Output "SPECS_DIR: $($paths.FEATURE_DIR)"
86+
Write-Output "FEATURE_DIR: $($paths.FEATURE_DIR)"
8787
Write-Output "BRANCH: $($paths.CURRENT_BRANCH)"
8888
}

‎scripts/python/setup_plan.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,15 @@ def main(argv: list[str] | None = None) -> int:
8282
{
8383
"FEATURE_SPEC": str(paths.feature_spec),
8484
"IMPL_PLAN": str(paths.impl_plan),
85-
"SPECS_DIR": str(paths.feature_dir),
85+
"FEATURE_DIR": str(paths.feature_dir),
8686
"BRANCH": paths.current_branch,
8787
}
8888
)
8989
)
9090
else:
9191
print(f"FEATURE_SPEC: {paths.feature_spec}")
9292
print(f"IMPL_PLAN: {paths.impl_plan}")
93-
print(f"SPECS_DIR: {paths.feature_dir}")
93+
print(f"FEATURE_DIR: {paths.feature_dir}")
9494
print(f"BRANCH: {paths.current_branch}")
9595
return 0
9696

‎templates/commands/plan.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ You **MUST** consider the user input before proceeding (if not empty).
5959
6060
## Outline
6161
62-
1. **Setup**: Run `{SCRIPT}` from repo root and parse JSON for FEATURE_SPEC, IMPL_PLAN, SPECS_DIR, BRANCH. For single quotes in args like "I'm Groot", use escape syntax: e.g 'I'\''m Groot' (or double-quote if possible: "I'm Groot").
62+
1. **Setup**: Run `{SCRIPT}` from repo root and parse JSON for FEATURE_SPEC, IMPL_PLAN, FEATURE_DIR, BRANCH. For single quotes in args like "I'm Groot", use escape syntax: e.g 'I'\''m Groot' (or double-quote if possible: "I'm Groot").
6363
6464
2. **Load context**: Read FEATURE_SPEC and `/memory/constitution.md`. Load IMPL_PLAN template (already copied).
6565

‎tests/test_setup_plan_python_parity.py‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,3 +374,57 @@ def test_python_json_output_matches_powershell(repo: Path) -> None:
374374

375375
assert py.returncode == ps.returncode == 0
376376
assert json_stdout(py) == json_stdout(ps)
377+
378+
379+
@requires_bash
380+
@pytest.mark.parametrize("args", [("--json",), ()], ids=["json", "text"])
381+
def test_all_variants_emit_feature_dir_not_specs_dir(
382+
repo: Path, args: tuple[str, ...]
383+
) -> None:
384+
r"""Pin the output key name, not just cross-port agreement.
385+
386+
The other tests here compare the ports against each other, so all three
387+
could regress to ``SPECS_DIR`` together and still pass. This asserts the
388+
contract absolutely: the key is ``FEATURE_DIR``, it carries the feature
389+
directory rather than the specs root, and the old name is gone.
390+
``SPECS_DIR`` means the specs root in ``create-new-feature.sh``, so
391+
re-emitting it here would reintroduce one name for two paths.
392+
393+
The value is matched by suffix because the ports legitimately differ in
394+
path flavour -- under MSYS bash reports ``/tmp/...`` where the Python and
395+
PowerShell ports report ``C:\...``. The suffix still separates
396+
``specs/001-my-feature`` from a bare ``specs``, which is the regression
397+
this guards.
398+
"""
399+
json_mode = args == ("--json",)
400+
suffix = ("specs", "001-my-feature")
401+
402+
commands = [bash_cmd(repo, SCRIPT, *args), py_cmd(repo, SCRIPT, *args)]
403+
if HAS_POWERSHELL:
404+
commands.append(ps_cmd(repo, SCRIPT, *(("-Json",) if json_mode else ())))
405+
406+
for cmd in commands:
407+
result = run(cmd, repo)
408+
assert result.returncode == 0, result.stderr
409+
assert "SPECS_DIR" not in result.stdout
410+
411+
if json_mode:
412+
payload = json_stdout(result)
413+
assert isinstance(payload, dict)
414+
assert sorted(payload) == [
415+
"BRANCH",
416+
"FEATURE_DIR",
417+
"FEATURE_SPEC",
418+
"IMPL_PLAN",
419+
]
420+
value = payload["FEATURE_DIR"]
421+
else:
422+
lines = dict(
423+
line.split(": ", 1)
424+
for line in result.stdout.splitlines()
425+
if ": " in line
426+
)
427+
assert "FEATURE_DIR" in lines
428+
value = lines["FEATURE_DIR"]
429+
430+
assert tuple(value.replace("\\", "/").rstrip("/").split("/")[-2:]) == suffix

0 commit comments

Comments
 (0)