Skip to content

fix: avoid paths-only feature context writes#3053

Open
luohui1 wants to merge 1 commit into
github:mainfrom
luohui1:fix/3025-feature-context-paths
Open

fix: avoid paths-only feature context writes#3053
luohui1 wants to merge 1 commit into
github:mainfrom
luohui1:fix/3025-feature-context-paths

Conversation

@luohui1

@luohui1 luohui1 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Description

Closes #3025 by preventing check-prerequisites --paths-only / -PathsOnly from rewriting .specify/feature.json when SPECIFY_FEATURE_DIRECTORY is only a temporary read-only override.

Both Bash and PowerShell paths now suppress feature context persistence only for paths-only mode; normal command execution continues to persist explicit feature directory context.

Testing

  • Tested locally with uv run specify --help
  • Ran existing tests with uv sync && uv run pytest
  • Tested with a sample project (if applicable)

Additional validation run locally after splitting this PR:

  • uv run --extra test pytest tests/test_check_prerequisites_paths_only.py -q - 4 passed, 5 skipped
  • uvx ruff check tests/test_check_prerequisites_paths_only.py - passed
  • git diff --check - passed

Manual test results

Agent: Codex (GPT-5) | OS/Shell: Windows 11 / PowerShell 7.5.4

Command tested Notes
.specify/scripts/powershell/check-prerequisites.ps1 -Json -PathsOnly Covered by regression test for read-only .specify/feature.json behavior.

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (describe below)

This PR was implemented by Codex (GPT-5) acting as an AI coding agent on behalf of @luohui1. The agent read the repository contribution/security/code-of-conduct docs and issue templates, inspected the relevant code paths, implemented the changes, ran the validation listed above, and prepared this PR description.

@luohui1 luohui1 requested a review from mnriem as a code owner June 18, 2026 08:26
@mnriem

mnriem commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Please separate out in 3 PRs. Thanks!

@luohui1 luohui1 force-pushed the fix/3025-feature-context-paths branch from 99577f8 to 1c89df3 Compare June 18, 2026 14:06
@luohui1 luohui1 changed the title fix: harden feature context path resolution fix: avoid paths-only feature context writes Jun 18, 2026
@mnriem

mnriem commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this. The fix direction is right — --paths-only is documented as read-only path resolution, so it shouldn't write tracked files. A couple of requests before this lands:

1. Reframe the rationale. The PR/issue describes SPECIFY_FEATURE_DIRECTORY as "only a temporary read-only override," but that's not how it's defined in the code. In get_feature_paths() it's an explicit override that is deliberately persisted ("Persist to feature.json so future sessions without the env var still work"). Can you re-anchor the justification on the defensible claim — "--paths-only must not mutate tracked files" — rather than on the env var being temporary? Also worth a line confirming persistence is still covered by the non-paths-only commands (specify, setup-plan, setup-tasks, etc.), so the "future sessions still work" guarantee holds.

2. Drop the new SPECIFY_NO_PERSIST_FEATURE_JSON env var. It's a global back-channel for one bit of state, and the PowerShell path sets $env:SPECIFY_NO_PERSIST_FEATURE_JSON = '1' without ever unsetting it — so suppression leaks for the rest of the process and would silently disable persistence for any later in-process Save-FeatureJson call. Please pass this as a local parameter/switch instead:

  • Bash: a flag/arg on get_feature_paths threaded down to _persist_feature_json.
  • PowerShell: a [switch]$NoPersist on Get-FeaturePathsEnv passed to Save-FeatureJson.

That keeps it self-contained and removes the leak. (If there's a strong reason to keep the env var, the PS side at least needs to scope/unset it in a finally to match the inline scoping Bash already uses.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: get_feature_paths persists .specify/feature.json during read-only path resolution (check-prerequisites.sh --paths-only)

2 participants