fix: paths-only skips branch validation, setup-plan preserves existing plan#2672
Open
mnriem wants to merge 4 commits into
Open
fix: paths-only skips branch validation, setup-plan preserves existing plan#2672mnriem wants to merge 4 commits into
mnriem wants to merge 4 commits into
Conversation
…g plan (github#2653) - check-prerequisites.sh/ps1: move branch validation after --paths-only early exit so --paths-only returns paths without requiring a spec branch - setup-plan.sh/ps1: skip template copy when plan.md already exists to prevent overwriting user-authored plans on reruns - setup-plan.sh: send status messages to stderr in --json mode so stdout remains parseable JSON - Add tests for both fixes (bash + PowerShell)
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes two script behaviors affecting automation and reruns: check-prerequisites should honor --paths-only as “no validation”, and setup-plan should avoid overwriting an existing plan.md (while keeping --json output machine-parseable).
Changes:
- Move branch validation to occur after
--paths-onlyearly-exit in both bash and PowerShellcheck-prerequisitesscripts. - Update
setup-plan(bash + PowerShell) to skip copying the plan template whenplan.mdalready exists. - Add pytest coverage for
--paths-onlybehavior and for “do not overwrite plan.md” behavior.
Show a summary per file
| File | Description |
|---|---|
| scripts/bash/check-prerequisites.sh | Defers branch validation until after --paths-only early exit. |
| scripts/powershell/check-prerequisites.ps1 | Same as bash: -PathsOnly exits before branch validation. |
| scripts/bash/setup-plan.sh | Skips template copy if plan.md exists; emits status to stderr in --json mode. |
| scripts/powershell/setup-plan.ps1 | Skips template write if plan.md exists (but currently writes a status line to stdout). |
| tests/test_check_prerequisites_paths_only.py | Adds coverage for paths-only behavior in bash and PowerShell. |
| tests/test_setup_plan_no_overwrite.py | Adds coverage for initial creation + non-overwrite behavior in bash and PowerShell. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 2
…tions Address review: setup-plan.ps1 Write-Output polluted stdout in -Json mode when plan.md already existed. Use [Console]::Error.WriteLine() when -Json is set. Add json.loads + stderr assertions to the PS rerun test to catch regressions.
Bare Test-Path matches directories too, which would silently skip plan creation if a directory existed at the plan.md path.
Comment on lines
+24
to
+60
| 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 _clean_env() -> dict[str, str]: | ||
| env = os.environ.copy() | ||
| for key in list(env): | ||
| if key.startswith("SPECIFY_"): | ||
| env.pop(key) | ||
| return env | ||
|
|
||
|
|
||
| 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 | ||
| ) |
Comment on lines
+23
to
+54
| 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(CHECK_PREREQS_SH, d / "check-prerequisites.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(CHECK_PREREQS_PS, d / "check-prerequisites.ps1") | ||
|
|
||
|
|
||
| def _clean_env() -> dict[str, str]: | ||
| env = os.environ.copy() | ||
| for key in list(env): | ||
| if key.startswith("SPECIFY_"): | ||
| env.pop(key) | ||
| return env | ||
|
|
||
|
|
||
| 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 | ||
| ) | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2653
Summary
Two shell-script bug fixes reported in #2653:
1.
check-prerequisites --paths-onlyno longer validates branch name--paths-only/-PathsOnlyis documented as "no prerequisite validation", butcheck_feature_branchran before thePATHS_ONLYearly exit, causing failures on non-spec branches (e.g.main).Fix: Move branch validation after the
PATHS_ONLYblock in both bash and PowerShell scripts.2.
setup-planpreserves existingplan.mdsetup-plan.sh/setup-plan.ps1unconditionally copied the plan template to$IMPL_PLAN, silently overwriting user-authored plans on reruns.Fix: Skip template copy when
plan.mdalready exists. Also redirect status messages to stderr in--jsonmode so stdout remains parseable JSON.Changes
scripts/bash/check-prerequisites.sh— movecheck_feature_branchafterPATHS_ONLYexitscripts/powershell/check-prerequisites.ps1— moveTest-FeatureBranchafter$PathsOnlyexitscripts/bash/setup-plan.sh— guardcpwith existence check; status messages to stderr in JSON modescripts/powershell/setup-plan.ps1— guard template write with existence checkTests
tests/test_check_prerequisites_paths_only.py— 7 tests (bash + PowerShell): paths-only on non-spec branch, paths-only on spec branch, text mode, normal mode still validatestests/test_setup_plan_no_overwrite.py— 6 tests (bash + PowerShell): creates plan when missing, preserves existing plan, JSON parseable on first run, skip message on stderr