Skip to content

feat(scripts): add SPECIFY_FEATURE_NO_PERSIST env var to suppress feature.json writes (#4128) - #4129

Open
chelsealong wants to merge 4 commits into
github:mainfrom
chelsealong:fix/4128-specify-no-persist
Open

chelsealong wants to merge 4 commits into
github:mainfrom
chelsealong:fix/4128-specify-no-persist

Conversation

@chelsealong

@chelsealong chelsealong commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Problem

Fixes #4128.

When multiple Spec Kit script invocations run concurrently against the same
checkout (e.g. multiple agents/subagents each working on a different
feature), each one typically sets its own SPECIFY_FEATURE_DIRECTORY. Most
core scripts (setup-plan, setup-tasks) call get_feature_paths() without
--no-persist, so every invocation persists its SPECIFY_FEATURE_DIRECTORY
to the shared .specify/feature.json — the last writer wins, silently
overwriting another process's pinned feature directory.

--no-persist (added for #3025) already exists as a per-call opt-out, but
it's a parameter each call site has to pass — scripts that don't know they
need it (like setup-plan/setup-tasks) never do.

Fix

Add SPECIFY_FEATURE_NO_PERSIST as the environment-level equivalent of
--no-persist / -NoPersist / no_persist=True. When set to 1 or true,
it suppresses the .specify/feature.json write in get_feature_paths()
regardless of whether the calling script passes the per-call flag. An
orchestrator can set it once for a whole process tree so no script
invocation in that tree can write feature.json, even ones that don't know
to opt out themselves.

Implemented identically in all three script variants per the project's
parity rule (AGENTS.md):

  • scripts/bash/common.shget_feature_paths()
  • scripts/powershell/common.ps1Get-FeaturePathsEnv
  • scripts/python/common.pyget_feature_paths()

Also documented the new variable in docs/reference/core.md's environment
variables table, next to SPECIFY_FEATURE_DIRECTORY.

Fully backward compatible: existing behavior when SPECIFY_FEATURE_NO_PERSIST is
unset is unchanged in every scenario.

Test plan

tests/test_specify_no_persist.py now has 8 tests, covering all three
script variants at both write sites:

Via setup-plan (which calls get_feature_paths() without --no-persist,
so it's a script that previously had no way to opt out):

  • Baseline: SPECIFY_FEATURE_DIRECTORY set, no SPECIFY_FEATURE_NO_PERSIST
    feature.json is written (existing behavior, unchanged).
  • SPECIFY_FEATURE_NO_PERSIST=1/truefeature.json write is suppressed, for
    bash, PowerShell, and Python.
  • Two-agent race: agent A persists specs/001-a, agent B runs with
    SPECIFY_FEATURE_DIRECTORY=specs/002-b and SPECIFY_FEATURE_NO_PERSIST=1
    feature.json still points at agent A's specs/001-a (the pin isn't
    clobbered).

Via create-new-feature (which writes .specify/feature.json directly,
bypassing get_feature_paths() — the bypass flagged in review and fixed in
bc3341a):

  • SPECIFY_FEATURE_NO_PERSIST=1 suppresses that direct write too, for bash,
    PowerShell, and Python.

Current full run of the 8 tests:

tests/test_specify_no_persist.py::test_bash_persists_by_default PASSED
tests/test_specify_no_persist.py::test_bash_specify_no_persist_suppresses_write PASSED
tests/test_specify_no_persist.py::test_bash_specify_no_persist_does_not_clobber_existing_pin PASSED
tests/test_specify_no_persist.py::test_ps_specify_no_persist_suppresses_write PASSED
tests/test_specify_no_persist.py::test_py_specify_no_persist_suppresses_write PASSED
tests/test_specify_no_persist.py::test_bash_create_new_feature_no_persist_suppresses_write PASSED
tests/test_specify_no_persist.py::test_ps_create_new_feature_no_persist_suppresses_write PASSED
tests/test_specify_no_persist.py::test_py_create_new_feature_no_persist_suppresses_write PASSED
8 passed in 8.41s

Also ran:

  • shellcheck --severity=error on all tracked .sh files — clean.
  • markdownlint-cli2 on docs/reference/core.md — same 2 pre-existing
    MD028 findings as on unmodified main (unrelated blockquotes further
    down the file, confirmed by diffing lint output before/after).
  • Full pytest tests/ -q (6944 tests): 6921 passed, 9 skipped, 4 pre-existing
    failures unrelated to this change (test_presets.py,
    test_resolve_template_python_parity.py — template-composition/locale
    edge cases), confirmed present on main before this change too.

No manual agent-driven testing was done for this change since it's a
core-script/docs-only change with no slash-command template modifications.

AI disclosure

This PR was written primarily by an autonomous AI coding agent (Claude Code,
Claude Sonnet 5), including the code change, tests, and this description.

…n writes (github#4128)

Multi-agent setups running several Spec Kit script invocations concurrently
against the same checkout each set their own SPECIFY_FEATURE_DIRECTORY.
Every invocation that omits --no-persist (e.g. setup-plan, setup-tasks)
still writes that value to the shared .specify/feature.json, so agents can
clobber each other's pinned feature directory. SPECIFY_NO_PERSIST=1|true is
the environment-level equivalent of --no-persist, letting an orchestrator
suppress that write across every call in the process tree without patching
each call site.

Assisted-by: Claude Code (model: claude-sonnet-5, autonomous)
@chelsealong
chelsealong requested a review from mnriem as a code owner August 14, 2026 22:38
@wittkung

Copy link
Copy Markdown

Issue author here. Reviewed the diff — the implementation across all three language variants (bash/ps/python) matches the proposal in #4128 exactly, and the clobber-prevention test (test_bash_specify_no_persist_does_not_clobber_existing_pin) covers the core race scenario we described.

We backported the bash change locally and ran 8 end-to-end scenarios against get_feature_paths() directly — all passing:

  • Default persist ✓
  • --no-persist suppresses ✓
  • SPECIFY_NO_PERSIST=1 suppresses ✓
  • SPECIFY_NO_PERSIST=true suppresses ✓
  • Agent-A pin not clobbered by Agent-B with NO_PERSIST ✓
  • Path resolution unaffected by NO_PERSIST ✓
  • SPECIFY_NO_PERSIST=0 does not suppress (boundary) ✓

Happy to contribute a follow-up docs/guides/multi-agent.md guide once this lands — covering the full isolation protocol with integration examples for Antigravity, Claude Code, Cursor, and CI matrix jobs.

wittkung added a commit to wittkung/ttzip-core that referenced this pull request Aug 15, 2026
- Apply upstream PR #4129 patch to common.sh: add SPECIFY_NO_PERSIST
  env var as process-level equivalent of --no-persist
- Update speckit-multiagent.md rule with NO_PERSIST documentation,
  isolation diagram, and 8/8 local verification results
- Ref: github/spec-kit#4128 (our RFC), github/spec-kit#4129 (impl)
@mnriem
mnriem requested a balanced review from Copilot August 17, 2026 13:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an environment-level opt-out for feature-context persistence across Bash, PowerShell, and Python scripts.

Changes:

  • Implements SPECIFY_NO_PERSIST.
  • Adds cross-script regression tests.
  • Documents the environment variable.
Show a summary per file
File Description
scripts/bash/common.sh Adds Bash persistence suppression.
scripts/powershell/common.ps1 Adds PowerShell persistence suppression.
scripts/python/common.py Adds Python persistence suppression.
tests/test_specify_no_persist.py Tests default, suppression, and pin-preservation behavior.
docs/reference/core.md Documents the new variable.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread docs/reference/core.md Outdated
… feature.json write

The three create-new-feature variants write .specify/feature.json directly
rather than through get_feature_paths(), so they ignored SPECIFY_NO_PERSIST
entirely, leaving the "stops every core script from writing feature.json"
guarantee in docs/reference/core.md broader than the implementation.
@chelsealong

Copy link
Copy Markdown
Contributor Author

Fixed: create-new-feature (bash/ps/py) wrote .specify/feature.json directly via _persist_feature_json/Save-FeatureJson/persist_feature_json, bypassing get_feature_paths() entirely, so SPECIFY_NO_PERSIST had no effect there. All three variants now check SPECIFY_NO_PERSIST before that write, with parity tests added in tests/test_specify_no_persist.py (confirmed failing before the fix, passing after). Full pytest tests/ run: 6937 passed, 9 skipped, 1 pre-existing unrelated failure (verified present without this change too).

@mnriem mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address Copilot feedback and adjust the variable name to SPECIFY_FEATURE_NO_PERSIST

wittkung added a commit to wittkung/ttzip-core that referenced this pull request Aug 18, 2026
- Update common.sh to accept SPECIFY_FEATURE_NO_PERSIST (official maintainer naming)
  while preserving SPECIFY_NO_PERSIST for backward compatibility
- Update speckit-multiagent.md rule documentation
- Ref: github/spec-kit#4128, github/spec-kit#4129
…RSIST

Per review feedback on github#4129, align the env var name with SPECIFY_FEATURE_DIRECTORY
across bash/powershell/python.
@chelsealong

Copy link
Copy Markdown
Contributor Author

Renamed SPECIFY_NO_PERSIST to SPECIFY_FEATURE_NO_PERSIST across bash/PowerShell/Python (common.*, create-new-feature.*, tests, docs) in 7e92dfb. The Copilot inline comment about create-new-feature bypassing the variable was already addressed in bc3341a. tests/test_specify_no_persist.py (8 tests) and shellcheck both pass with the new name.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The PR title and description still advertise the obsolete environment-variable name.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread docs/reference/core.md
@chelsealong chelsealong changed the title feat(scripts): add SPECIFY_NO_PERSIST env var to suppress feature.json writes (#4128) feat(scripts): add SPECIFY_FEATURE_NO_PERSIST env var to suppress feature.json writes (#4128) Sep 18, 2026
@chelsealong

Copy link
Copy Markdown
Contributor Author

Updated the PR title and description to use SPECIFY_FEATURE_NO_PERSIST throughout (the code/docs were already renamed in 7e92dfb; only the PR metadata still referenced the old SPECIFY_NO_PERSIST name). No code changes needed.

@mnriem mnriem added the triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review label Sep 18, 2026
@mnriem

mnriem commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Please resolve conflicts

…-persist

# Conflicts:
#	docs/reference/core.md
@chelsealong

Copy link
Copy Markdown
Contributor Author

Merged upstream/main and resolved the conflict in docs/reference/core.md (upstream had reworded the SPECIFY_FEATURE row; kept upstream's wording and kept our SPECIFY_FEATURE_NO_PERSIST row after it). No other files conflicted. tests/test_specify_no_persist.py (8 tests) and the full pytest tests/ (8208 passed, 12 skipped) pass post-merge; shellcheck clean.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation is consistent across all script variants and includes focused regression coverage for every write path.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)

@mnriem

mnriem commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

The implementation now addresses the original feedback, including the direct create-new-feature write paths across Bash, PowerShell, and Python. Before re-review, please update the PR’s test evidence: tests/test_specify_no_persist.py now contains eight tests, while the description still reports the earlier five-test run and omits the three create-new-feature regression tests. Please include the current eight-test result and then resolve the addressed review threads.

Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol, autonomous); comment fully AI-drafted.

@chelsealong

Copy link
Copy Markdown
Contributor Author

Updated the PR description's test plan to reflect the current 8 tests in tests/test_specify_no_persist.py (including the 3 create-new-feature regression tests), with a fresh passing run: 8 passed in 8.41s. Resolved the two addressed review threads (the create-new-feature bypass, fixed in bc3341a, and the naming inconsistency, fixed in 7e92dfb).

This branch has not been deployed

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

Labels

triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Multi-agent isolation protocol — process-level feature context without shared-state races

4 participants