fix: stop check-prerequisites --paths-only from writing feature.json (#3025)#3190
Open
Noor-ul-ain001 wants to merge 1 commit into
Open
fix: stop check-prerequisites --paths-only from writing feature.json (#3025)#3190Noor-ul-ain001 wants to merge 1 commit into
Noor-ul-ain001 wants to merge 1 commit into
Conversation
…ithub#3025) check-prerequisites --paths-only / -PathsOnly is documented as pure, read-only path resolution, but when SPECIFY_FEATURE_DIRECTORY was set it called the persist routine and rewrote .specify/feature.json. That dirtied the working tree and overwrote a pinned feature directory during what should be a no-op. Add an explicit opt-out at the resolver boundary instead of a global env back-channel: - bash: get_feature_paths accepts a leading --no-persist flag that skips _persist_feature_json; check-prerequisites.sh passes it in --paths-only mode. - PowerShell: Get-FeaturePathsEnv gains a -NoPersist switch that skips Save-FeatureJson; check-prerequisites.ps1 passes it in -PathsOnly mode. Normal (non-paths-only) invocations are unchanged and still persist the override, so future sessions without the env var keep working. Add regression tests asserting --paths-only/-PathsOnly leaves a pinned feature.json untouched even when the env override differs, plus a guard that normal mode still persists.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes unintended side effects during “read-only” path resolution: check-prerequisites --paths-only / -PathsOnly now resolves feature paths without persisting SPECIFY_FEATURE_DIRECTORY into .specify/feature.json, preventing working tree dirtiness and accidental overwrites of pinned feature directories (issue #3025).
Changes:
- Bash: add
get_feature_paths --no-persistopt-out and use it fromcheck-prerequisites.sh --paths-only. - PowerShell: add
Get-FeaturePathsEnv -NoPersistopt-out and use it fromcheck-prerequisites.ps1 -PathsOnly. - Add regression tests ensuring paths-only honors the override in output but does not rewrite
.specify/feature.json(bash + PowerShell), and that bash normal mode still persists.
Show a summary per file
| File | Description |
|---|---|
| tests/test_check_prerequisites_paths_only.py | Adds regression coverage for “paths-only does not write feature.json”, plus a bash “normal mode still persists” guard. |
| scripts/powershell/common.ps1 | Adds -NoPersist switch to skip feature.json persistence for read-only callers. |
| scripts/powershell/check-prerequisites.ps1 | Passes -NoPersist to Get-FeaturePathsEnv when -PathsOnly is used. |
| scripts/bash/common.sh | Adds --no-persist flag handling to get_feature_paths to skip _persist_feature_json. |
| scripts/bash/check-prerequisites.sh | Uses get_feature_paths --no-persist in --paths-only mode. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Low
Comment on lines
+167
to
+171
| # Persist to feature.json so future sessions without the env var still | ||
| # work — unless the caller opted out for read-only resolution (#3025). | ||
| if (-not $NoPersist) { | ||
| Save-FeatureJson -RepoRoot $repoRoot -FeatureDirectory $env:SPECIFY_FEATURE_DIRECTORY | ||
| } |
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.
What
check-prerequisites --paths-only(and PowerShell-PathsOnly) is documented as pure, read-only path resolution. But whenSPECIFY_FEATURE_DIRECTORYis set,get_feature_paths/Get-FeaturePathsEnvcalled the persist routine and rewrote.specify/feature.json— dirtying the working tree and overwriting a pinned feature directory during what should be a no-op.Fixes #3025.
How
Opt out of the write at the resolver boundary with an explicit flag, rather than a global env back-channel (per maintainer feedback on #3053):
get_feature_pathsaccepts a leading--no-persistflag that skips_persist_feature_json;check-prerequisites.shpasses it in--paths-onlymode.Get-FeaturePathsEnvgains a-NoPersistswitch that skipsSave-FeatureJson;check-prerequisites.ps1passes it in-PathsOnlymode.Normal (non-paths-only) invocations are unchanged and still persist the override, so future sessions without the env var keep working.
Tests
Added regression tests (bash + PowerShell):
--paths-only/-PathsOnlyleaves a pinnedfeature.jsonuntouched even whenSPECIFY_FEATURE_DIRECTORYdiffers from the pinned value, while still honoring the override in the output.Notes
This is an alternative to #3053, which originally used a
SPECIFY_NO_PERSIST_FEATURE_JSONenv var; that approach was flagged in review as a leaky global back-channel. This PR uses explicit local flags/parameters at the call site instead.