fix(plan): use .specify/feature.json to allow /speckit.plan on custom git branches (#2305)#2349
fix(plan): use .specify/feature.json to allow /speckit.plan on custom git branches (#2305)#2349Adr1an04 wants to merge 6 commits intogithub:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the /speckit.plan setup scripts so that when .specify/feature.json already pins an existing feature directory, the plan workflow can proceed even if the current Git branch name doesn’t match the usual feature-branch pattern (e.g., custom branches like feature/my-feature-branch).
Changes:
- Added helpers (bash + PowerShell) to verify
.specify/feature.jsonmatches the resolved active feature directory. - Updated
setup-planscripts to skip branch-pattern validation when the pinned directory is valid and matches. - Added regression tests covering the bash and PowerShell custom-branch + valid
feature.jsonscenario.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/bash/common.sh |
Adds feature_json_matches_feature_dir helper used to decide whether to bypass branch checks. |
scripts/bash/setup-plan.sh |
Uses the new helper to conditionally skip check_feature_branch. |
scripts/powershell/common.ps1 |
Adds Test-FeatureJsonMatchesFeatureDir helper to validate pinned feature directory. |
scripts/powershell/setup-plan.ps1 |
Uses the new helper to conditionally skip Test-FeatureBranch. |
tests/test_setup_plan_feature_json.py |
Adds regression tests for custom branches with valid/missing feature.json (bash) and valid (PowerShell). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I addressed the Copilot feedback in by hardening the Bash parsing path, adding the grep/sed fallback, and adding the missing PowerShell negative test. I reran the focused tests after the update: Git Bash |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I’ll leave this decision up to you @mnriem. Copilot is suggesting a shared helper refactor, but I’m happy to make that change if you’d prefer. |
mnriem
left a comment
There was a problem hiding this comment.
Please address the first comment by Copilot. For the second comment I will resolve that one
Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Adr1an04 Please address Copilot feedback |
|
Fixed the copilot comments for the PowerShell path comparison and Bash parsing helper @mnriem . I left the broader Thanks for running copilot for me |
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 1
| 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, | ||
| ) |
There was a problem hiding this comment.
These tests invoke the setup scripts without controlling the child process environment. If a developer/CI runner has SPECIFY_FEATURE, SPECIFY_FEATURE_DIRECTORY, or related env vars set, get_feature_paths can resolve a different feature directory/branch and make the tests flaky. Consider passing an explicit env to subprocess.run that starts from os.environ but removes/overrides those SPECIFY_* variables (and any other variables the scripts consult).
|
No problem. Note Copilot can be a bit annoying at times, but we require everyone to go through the same cycle of reviews until Copilot finally stops giving more feedback (and yes that includes maintainers too ;)) So time to address some more Copilot feedback! |
|
Good to know this is my first PR here so! Thank you, I'll fix the comments copilot gave out. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| norm_json="$(cd -- "$_fd" 2>/dev/null && pwd)" || return 1 | ||
| norm_active="$(cd -- "$active_feature_dir" 2>/dev/null && pwd)" || return 1 |
There was a problem hiding this comment.
feature_json_matches_feature_dir claims to compare “resolved” paths, but it uses pwd (logical path), which does not resolve symlinks. If $FEATURE_DIR and the feature_directory from feature.json refer to the same directory via different symlink representations, this equality check will be false and /speckit.plan will incorrectly require a feature-branch name. Consider using a physical path (pwd -P) or another canonicalization approach to avoid false negatives.
| norm_json="$(cd -- "$_fd" 2>/dev/null && pwd)" || return 1 | |
| norm_active="$(cd -- "$active_feature_dir" 2>/dev/null && pwd)" || return 1 | |
| norm_json="$(cd -- "$_fd" 2>/dev/null && pwd -P)" || return 1 | |
| norm_active="$(cd -- "$active_feature_dir" 2>/dev/null && pwd -P)" || return 1 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This PR Closes #2305.
Summary
This PR is based on the discussion in #2305, with clarification from @mnriem that Git branch creation and spec directory creation are intentionally separate in Spec Kit, this change preserves that behavior.
The issue addressed here is narrower than the original directory naming expectation from #2305. After the discussion with @mnriem, I focused this PR on the later
/speckit.planbehavior:/speckit.plancould fail on a custom Git branch likefeature/my-feature-branchbefore using the existing feature context from.specify/feature.json.That meant a valid setup like this could still be rejected:
{"feature_directory":"specs/001-notes-app"}This PR updates
setup-planso it can continue when.specify/feature.jsonpoints to a valid existing feature directory. Iffeature.jsonis missing, stale, points to a non-existent directory, or does not match the resolved feature directory, the existing branch validation behavior still runs.What changed
.specify/feature.jsonmatches the resolved feature directory.setup-plan.shandsetup-plan.ps1to use that metadata check before failing on the branch-name pattern.feature.jsoncase and fallback behaviorWhat did not change
Based on @mnriem's clarification in #2305, this PR doesn't change
/speckit.specify, branch naming, spec directory naming, or make Git branch names the source of truth./speckit.specifystill uses the existing feature branch and directory behavior. This change is only about letting/speckit.plancontinue when existing feature metadata already resolves the correct feature directory.Test selection reasoning
scripts/bash/setup-plan.sh/speckit.plansetup-plan.shis invoked by the plan workflowscripts/powershell/setup-plan.ps1/speckit.planscripts/bash/common.sh/speckit.plan, shared script consumerssetup-plan.sh; file is shared, so existing consistency/full-suite tests were also runscripts/powershell/common.ps1/speckit.plan, shared script consumerssetup-plan.ps1; file is shared, so existing consistency/full-suite tests were also runtests/test_setup_plan_feature_json.pyfeature.jsonand normal numbered branch behaviorRequired tests
setup-planfeature metadata behavior/speckit.specifymanual prerequisite workflow/speckit.planmanual workflow on a custom branch with valid.specify/feature.json/speckit.tasksdownstream smoke testtests/test_agent_config_consistency.pyTesting
I did focused regression tests from native PowerShell:
also focused regression tests through Git Bash:
Used the existing consistency test:
As well as ran a full test suite:
Normal diff check just in case:
Script level validation
I also tested the setup scripts directly in temporary repos for both Bash and PowerShell:
.specify/feature.json:setup-planexited 0, reused the existing feature directory, createdplan.md, did not create a duplicate specs folder, and did not switch branches..specify/feature.json:setup-plankept the existing behavior and failed withERROR: Not on a feature branch, with noplan.mdcreated.plan.mdwas created normally.Manual testing
I also ran the flow through GitHub Copilot in VS Code on Windows.
I initialized a fresh project with:
Then I ran
/speckit.specify, which created branch001-notes-appandspecs/001-notes-app/spec.md.To reproduce the custom-branch scenario from #2305, I committed that initial output, switched to
feature/my-feature-branch, and added:{"feature_directory":"specs/001-notes-app"}Then I ran
/speckit.planin a fresh Copilot chat. It stayed onfeature/my-feature-branch, reusedspecs/001-notes-app, and createdplan.mdwithout the previousERROR: Not on a feature branchfailure.I also ran
/speckit.tasksas a smoke test. It createdtasks.mdin the same feature directory and did not create a duplicate specs folder.Key evidence after

/speckit.plan:Follow-up review cleanup
After the Copilot review, I pushed a follow up commit to harden the metadata validation path:
jq/python3parsing so parse failures return non-zero from the helper instead of interacting poorly withset -e.feature.jsonfallback path.AI disclosure
I used ChatGPT and GitHub Copilot to help investigate the issue, review test coverage and also helping me polish this description (edit: also to fix the last problem which i reviewed myself and commit it with Cursor). I personally wrote the script changes, ran the repro, tests, and manual validation, and reviewed the final diff before submitting.