You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(presets): materialize script composition chains at lifecycle time (#4551) - #4709
Continues #4551. Per @mnriem's review, this is now a lifecycle-materialization design instead of the runtime-resolution one this PR originally shipped — see the pivot discussion below for the full rationale.
What changed in this revision
The runtime dispatcher (per-invocation resolution via a hidden CLI shell-out) is gone. The chain is now materialized to disk at preset-lifecycle time, mirroring how PresetManager._reconcile_script_chain()'s sibling machinery for commands already works:
PresetManager._reconcile_script_chain() now writes one fixed-path generated launcher file per composing layer under .specify/scripts/bash/: the topmost lands at the canonical <name>.sh agents actually invoke, each one's $CORE_SCRIPT points at the next-lower layer's fixed path, and the chain ends in a materialized copy of the base layer at <name>.speckit-core.sh. A single "replace"-strategy layer with no composition is still written verbatim with no intermediate files.
Install, remove, enable, disable, and set-priority all re-materialize now — PresetManager.reconcile_scripts_for_preset() is called from command_enable.py/command_disable.py/command_set_priority.py the same way reconcile_constitution() already is, closing the gap where those three commands previously did nothing for scripts at all.
PresetResolver.resolve_script_chain() is unchanged — same ordering/collection logic, only when it's called (materialization time, not invocation time) and what consumes its output (a file-writer, not a runtime shell-out) changed.
Dropped entirely: the hidden specify preset script-chain CLI command, the generated continuation-runner.sh, the SPECKIT_SCRIPT_CONTINUATION env protocol, the __main__.py fallback that existed only so the runtime dispatcher could resolve without specify on PATH, and the documented uvx one-time-install limitation (scripts now have no runtime dependency on specify being importable/on PATH at all).
Known limitation, carried forward rather than solved
A project-local override added directly to .specify/templates/overrides/scripts/ outside any lifecycle command is not auto-materialized — PresetManager.reconcile_all_script_chains() still needs an explicit call, the same gap already documented for that method's role in repairing a forced shared-infra refresh.
Evidence
TestScriptChainReconciliation in tests/specify_cli/presets/test_manager.py: single-layer verbatim writes, multi-layer launcher stubs, provenance-based cleanup when a chain shrinks back to nothing, symlink-safety guards extended to every generated path, and a new test proving enable/disable/set-priority now re-materialize immediately (previously a no-op gap for scripts).
tests/test_script_continuation_bash.py: real bash subprocess execution proving ordering (outer-before → inner-before → core → inner-after → outer-after), arg/exit-status propagation, and that a priority change only takes effect after an explicit reconcile call — the materialized-model inverse of the old runtime test's claim.
Full existing suite green: 8120 passed, 468 skipped (environment-gated, e.g. requires_bash where this sandbox's bare bash resolves to the WSL launcher over subprocess), 1 pre-existing unrelated failure deselected (test_setup_tasks_ps_core_template_resolved, verified failing identically on this branch before any of my changes via git stash).
Left for follow-up, pending your input
PowerShell and Python adapters. Same open question as before the pivot — this PR stays Bash-only.
Manifest schema for cross-runtime scripts. Unchanged from before: no way yet for a preset to declare .ps1/.py counterparts of a script name.
Disclosure per CONTRIBUTING.md: implemented with Claude Code, autonomous mode with human review of the diff and test results; the materialization design follows directly from mnriem's review comment on this PR.
…github#4551)
A "wrap" script's $CORE_SCRIPT reference previously had no runtime
resolution mechanism at all: resolve_content("script") composes the
full chain into a single spliced file, but nothing ever calls it in a
real code path, so preset/extension script composition has been dead
since it was added.
This adds the runtime side, scoped to Bash for this first increment:
- PresetResolver.resolve_script_chain(): returns the ordered file
chain for a script name (highest priority down through the
terminating "replace" layer), reusing collect_all_layers().
- `specify preset script-chain <name>` (hidden): prints that chain,
one path per line, for the bash adapter to consume.
- scripts/bash/continuation-runner.sh: advances the chain one hop,
execing into the next layer and threading the remainder via
SPECKIT_SCRIPT_CONTINUATION so a further "wrap" layer's own
$CORE_SCRIPT call continues correctly.
- PresetManager._reconcile_script_chain(): writes the project's
canonical .specify/scripts/bash/<name>.sh — a verbatim copy when
there's nothing to compose, or a fixed dispatcher stub when there
is. The stub carries no stack-specific data, so it resolves the live
chain fresh on every invocation and never needs rewriting again:
install/remove (which change which layers exist) call it, but
enable/disable/set-priority (which only reorder existing layers)
don't need to.
Fixed a real bug while building the bash integration test: `specify`'s
stdout carries CRLF line endings on Windows, and bash's $(...) only
strips trailing newlines, not carriage returns, so an unstripped \r
was corrupting the exec path. Stripped at the source with `tr -d
'\r'` plus a defensive strip in the runner's own read loop.
PowerShell and Python adapters, and extending the preset manifest
schema so a `type: script` entry can declare per-language files
(today `file:` only supports .sh), are left for follow-up commits
pending confirmation on the schema shape.
Disclosure per CONTRIBUTING.md: implemented with Claude Code,
autonomous mode with human review of the diff and test results; the
design (continuation representation, dispatcher/runner split, why
scripts don't need commands' repeated reconciliation) follows directly
from the approach mnriem and I settled on in this issue's discussion.
- Generate the runner next to the dispatcher instead of shipping a new
scripts/bash file, so shared-infra inventories are unchanged and
projects initialized earlier still get it.
- Run layers through bash so preset/override scripts need no execute bit.
- Always install the dispatcher while a preset provides the script, so
enable/disable/set-priority changes that alter chain length stay valid.
- Refuse to write through symlinked destinations; remove a stale generated
dispatcher when its last provider is removed.
- Resolve the built-in Bash core from scripts/bash and validate that every
wrap layer contains $CORE_SCRIPT.
- Fall back to python3 -m specify_cli when no specify executable is on PATH.
- Register the hidden script-chain command in the stable-order test.
Disclosure per CONTRIBUTING.md: implemented with Claude Code, autonomous
mode, changes verified by the test suite and manual bash runs.
Pushed 73ad604 addressing the Copilot review and the failing CI.
Changed:
The runner is now generated next to the dispatcher instead of shipped as a new scripts/bash file. That was what broke the ~40 file-inventory tests, and it also means projects initialized before this feature still get the runner.
Layers are run via bash <layer>, so preset copies and overrides need no execute bit.
While any preset provides a script, the canonical file is always the dispatcher (even for a one-layer chain), so enable/disable/set-priority changes that move the chain between one and many layers stay valid without a rewrite.
Symlinked .specify/scripts, bash or <name>.sh destinations are refused. Removing the last provider deletes the generated dispatcher, or restores the bundled core.
The built-in Bash core is now found under scripts/bash/, and every wrap layer is validated to contain $CORE_SCRIPT before the chain is returned.
Added the hidden script-chain command to the stable-order registration test.
Not addressed in this push, and I'd like your steer:
specify init --force / integration upgrades rewrite .specify/scripts/bash without reconciling already-installed script presets, so an enabled wrapper would be replaced by core until the next preset install/remove. I think the right fix is a hook in the shared-infra refresh path, but that touches code outside the presets module.
The dispatcher still needs the CLI at run time. It now falls back to python3 -m specify_cli, but a one-shot uvx project with neither on PATH fails with a clear error. Is a project-local resolver acceptable, or is requiring the CLI fine?
Verified: tests/test_presets.py, tests/specify_cli/presets, a sample of the integration inventory tests and ruff@0.15.0 pass locally. The three real-bash tests skip on my Windows machine, so I ran the two-layer chain and priority-swap scenarios by hand against Git Bash and both give the expected order.
Disclosure: this comment and the commit were produced with Claude Code in autonomous mode, on behalf of @Ashfaqbs.
- Write the dispatcher, runner and restored core script through the shared
safe-destination helpers so a symlinked ancestor such as .specify cannot
redirect writes or the cleanup unlink outside the project.
- Reserve the `common` and `continuation-runner` script names; a dispatcher
for either would overwrite the runtime helpers every dispatcher relies on.
- Add specify_cli/__main__.py so the `python3 -m specify_cli` fallback works
when no `specify` executable is on PATH.
- Decide whether a preset provides a script from all active declarations,
not the truncated chain, so an extension replace layer above a preset no
longer leaves the preset without a dispatcher.
Disclosure per CONTRIBUTING.md: implemented with Claude Code, autonomous
mode, changes verified by the test suite (one pre-existing PowerShell
failure unrelated to this change).
Pushed 4d92033 addressing the latest automated review round:
Symlinked ancestors: the dispatcher, runner and restored core script are now written through the shared safe-destination helpers, which validate every ancestor.
Helper name collisions: common and continuation-runner are reserved script names and are refused.
Python fallback: added specify_cli/__main__.py so python3 -m specify_cli runs the CLI.
Chain truncation: whether a preset provides a script is now decided from all active declarations rather than the truncated chain.
The registration-order test item was already covered in 73ad604. Still open and waiting on maintainer direction: reconciling script chains on init --force/upgrades, and resolving the chain without depending on a current specify on PATH.
Disclosure per CONTRIBUTING.md: this comment and the commit were produced with Claude Code in autonomous mode, on behalf of @Ashfaqbs. Tests pass except one PowerShell test that fails on unmodified main.
Restored core script lost its execute bit: fixed. When the last composing preset is removed, the core script is written back and its execute bits are restored, as the dispatcher branch already does. Regression test added (POSIX only, skipped on Windows).
Override-only scripts (.specify/templates/overrides/scripts/<name>.sh added by hand after init): not addressed in this PR. Nothing runs when a user drops a file there, so the fix needs a trigger, most likely generating dispatchers for canonical Bash scripts during shared-infrastructure setup. That changes the init file inventory for every project, so I would rather have your call on it than fold it into this increment. Happy to do it as a follow-up if you want it.
Still open and waiting on maintainer direction: reconciling script chains on init --force/upgrades, and resolving the chain without depending on a current specify on PATH.
Disclosure per CONTRIBUTING.md: this comment and the commit were produced with Claude Code in autonomous mode, on behalf of @Ashfaqbs. Ran tests/test_presets.py and tests/test_script_continuation_bash.py: 624 passed, 4 skipped.
Move script-chain command into its dedicated module
src/specify_cli/presets/command_resolve.py:15
The CLI architecture requires each real command to live in its matching command_<name>.py module (design/cli.md:32-58), with tests mirroring that structure. Defining script-chain inside command_resolve.py obscures command ownership; move it to command_script_chain.py and import it at the intended stable registration position.
Port the script continuation dispatcher onto the private presets modules
introduced by the domain split (github#4747), and move its tests into the
mirrored tests/specify_cli/presets layout.
Merged main (e30639f). #4747 split presets/__init__.py into private modules, so the dispatcher code now lives in _manager.py and _resolver.py and its tests moved into tests/specify_cli/presets/ (the shared pack helper is now create_pack in _helpers.py). No behavior change; the presets tests and the new bash continuation tests pass. The one failure in the full run, test_setup_tasks_ps_core_template_resolved (PowerShell JSON output on Windows), is in code this PR doesn't touch.
- tests/specify_cli/presets/test_manager.py: drop redundant local
`import os` / `import subprocess` that shadowed the already-imported
module-level names, fixing the ruff F401/F811 failures blocking CI.
- _manager.py / _resolver.py: fix mojibake (`—`) back to em dashes in
comments and docstrings.
- _manager.py: add `PresetManager.reconcile_all_script_chains()`, and
call it from command_init.py right after `install_shared_infra`.
`specify init --force` (and forced integration upgrades) rewrite
`.specify/scripts/bash/<name>.sh` from the bundled core, clobbering
any generated continuation dispatcher for an already-enabled script
preset. Unlike commands/skills, script presets were never
re-reconciled after that refresh, so a wrapped script would go inert
until its preset was reinstalled. Covered by a new regression test.
Addresses the ruff/pytest CI failures and the Copilot review round on
this PR. Several other findings from that review (execute-bit
preservation after restore, symlink-safe writes/deletes, reserved
script names, wrap-strategy validation, the `python3 -m specify_cli`
fallback, and chain truncation vs. extension layers) were already
fixed in earlier commits on this branch. PowerShell/Python script
parity and the general override-only reconciliation-trigger gap
(pre-existing for commands too, not a regression here) remain
out of scope for this first increment, per the PR description.
Assisted-by: Claude Code (model: claude-sonnet-5, autonomous)
Reject runtime changes incompatible with installed script presets
src/specify_cli/presets/_manager.py:308
This guard only runs when a preset is installed, but the project's runtime can later change through commands such as integration upgrade --script ps|py, integration switch ... --script ps|py, or forced re-init. Those transitions currently succeed with an enabled Bash script preset still installed, after which agents invoke the non-Bash variant and the preset silently becomes inert. Reject unsupported runtime transitions when script presets are enabled (or implement cross-runtime materialization), with a regression test covering an already-installed preset.
This issue also appears in the following locations of the same file:
@Ashfaqbs While I am very much looking forward to this change to resolve the issue you have been working I think we should regroup a bit as it appears we are going into a long review cycle. Maybe we should file this as a separate issue and determine how we can decompose this into several smaller backwards compatible steps? Thoughts?
… installed
Installing a script-providing preset into a non-Bash project was
already rejected, but a project could still change runtime *after*
installation via `integration upgrade --script ps|py`, `integration
switch --script ps|py`, or `init --here --force --script ps|py`. Those
transitions succeeded silently, leaving the already-installed preset's
Bash-only generated dispatcher chain inert, since the project now
executes the restored PowerShell/Python variant instead.
Add PresetManager.reject_unsupported_script_runtime_change(), called
from all three command paths right after the target script type is
resolved and before any other mutation, and a
_script_providing_preset_ids() helper (also covering a project-local
script override with no owning preset) to name what's blocking the
change.
Assisted-by: Claude Code (model: Claude Sonnet 5, autonomous)
Implemented with Claude Code (model: Claude Sonnet 5, autonomous mode); comment fully AI-drafted.
Addressed the "Previously missed" finding from the latest review round: the Bash-only script-preset guard (_reject_script_preset_for_unsupported_runtime) only ran at preset-install time. A project could still change runtime afterward via integration upgrade --script ps|py, integration switch --script ps|py, or init --here --force --script ps|py, silently leaving an already-installed script preset's Bash-only dispatcher chain inert.
Added PresetManager.reject_unsupported_script_runtime_change() in src/specify_cli/presets/_manager.py, called from all three command entry points (command_init.py, integrations/command_upgrade.py, integrations/command_switch.py) right after the target script type is resolved, before any other mutation. A _script_providing_preset_ids() helper names the blocking preset(s) in the error, also covering a project-local script override with no owning preset. Added one regression test per command path.
The remaining 7 open findings restated in this review round are unchanged from prior rounds (Bash-only dispatcher scope, stale PATH/specify-executable concerns obsoleted by the materialize-at-lifecycle-time redesign, and the documented override-only reconciliation tradeoff) — already triaged in the preceding summary comment.
Related: #5087 reports that specify preset set-priority does not refresh agent command files that are already written, so after reordering presets preset resolve names the new top layer while the agent command file still has the old body. A test docstring here notes that preset commands "stay stale until removal", but this PR does not change set-priority.
Posted with Claude Code (Claude Opus 4.8) under human supervision.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The runtime restriction ignores disabled script presets, allowing an incompatible runtime switch followed by re-enablement without validation and leaving the preset silently ineffective.
Resolves conflicts from the main-branch doc consolidation (github#5223):
docs/reference/presets.md and presets/ARCHITECTURE.md were deleted
upstream in favor of docs/presets/*.md; ported this branch's
script-materialization doc changes into docs/presets/overview.md
instead of resurrecting the deleted files. Also merges an import-list
conflict in presets/_manager.py (both load_init_options and
load_install_ignore are needed).
Assisted-by: Claude Code (model: Claude Sonnet 5, autonomous)
Implemented with Claude Code (model: Claude Sonnet 5, autonomous mode); comment fully AI-drafted.
Addressed the three items:
Security audit / CVE: opentelemetry-api (1.45.0, CVE-2026-114603) is fixed transitively by merging upstream/main below — it now resolves to 1.45.2 in both uv.lock and .github/security-audit-requirements.txt. No manual pin needed.
Conflicts: merged upstream/main (08252af8) into the branch, no force-push (commit a860ec33). No conflict markers remained after resolution; ruff clean, and the presets/resolver/integrations-related test modules (212 tests) pass.
Copilot feedback (29 open review comments): checked every one against current HEAD rather than assuming they're all live, same as the 2026-10-04 round on this PR. All 29 predate the lifecycle-materialization pivot (b5ccc7f0, Oct 9) and several predate commits already pushed after that:
Findings about continuation-runner.sh, the hidden specify preset script-chain CLI, the SPECKIT_SCRIPT_CONTINUATION env protocol, and the python3 -m specify_cli / __main__.py fallback are moot — that whole runtime-continuation mechanism was removed in the pivot. There is no runtime dependency on a persistent specify CLI any more, so the uvx one-shot-install concern is also moot.
Symlinked-ancestor safety on writes/deletes: already covered — _reconcile_script_chain validates every ancestor via _ensure_safe_shared_directory/_ensure_safe_shared_destination before any write (_manager.py:775-780).
Execute-bit loss (copytree, _write_shared_text's 0644, restored core/replace files): already restored via explicit chmod 0o111 at every write site (_manager.py:825-827, 840-841, 853-854).
Wrap body missing $CORE_SCRIPT: already validated and rejected (_resolver.py:740-744), with a regression test (test_resolver.py::test_wrap_missing_core_script_placeholder_is_rejected).
"Provided by a preset" judged from the truncated chain only: already fixed — it's judged from every active declaration across collect_all_layers, not just the effective chain (_manager.py:782-794).
Override-only scripts (no preset declaring the name) never reconciled: already fixed — reconcile_all_script_chains() scans the overrides directory too, not just preset manifests (_manager.py:905-911).
Forced-refresh clobbering (init --force, integration upgrade --force, integration switch --force/--refresh-shared-infra): already wired at every call site, and ordered to run after the last shared-infra refresh in each command, not before (command_init.py:903, command_upgrade.py:251+324, command_switch.py:319, _helpers.py:383).
Raw absolute path / shell-injection risk in generated launchers: already fixed — paths are relative-to-$SCRIPT_DIR and shell-single-quoted (_manager.py:191-227).
Command-level test gap for enable/disable/set-priority not catching missing CLI wiring: already covered — test_enable_disable_reconciles_generated_script invokes the real preset enable/preset disable CLI commands (not the manager API) and asserts the canonical chain content changes; set-priority has its own dedicated coverage in test_command_set_priority.py.
Mojibake in _manager.py/_resolver.py docstrings/comments: checked current text directly — these are correct em dashes, not mojibake, in the current revision.
Still genuinely out of scope, as flagged in earlier rounds and unchanged by this one:
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unresolved critical issues can expose private files through symlinked script overrides and leave disabled preset or disabled/uninstalled extension code executable.
_script_providing_preset_ids() only scanned enabled presets, so the
restriction added in dd159a0 could be bypassed by disabling a
script-providing preset, switching --script ps|py, then re-enabling
it -- enable performs no runtime validation of its own and only
regenerates Bash files, leaving the preset silently inert.
The restriction holds until a script-providing preset is removed, not
merely disabled, per the error message's own wording -- include
disabled presets in the scan.
Assisted-by: Claude Code (model: Claude Sonnet 5, autonomous)
A preset can wrap an extension-provided script; its content gets
copied inline into the generated chain at materialization time.
Disabling or removing that extension didn't reconcile the chain, so
the copied content kept running through the preset after the
extension was gone.
Extract PresetManager.reconcile_scripts_for_names() from
reconcile_scripts_for_preset() so extension disable/remove can call
the same reconciliation by script name, without needing a preset id.
Assisted-by: Claude Code (model: Claude Sonnet 5, autonomous)
_reconcile_script_chain() read a resolved layer's source content
(chain[0]/chain[-1]) to materialize it into the canonical generated
script, but never validated that source path before reading it. A
project-local override, preset, or extension script file that was
itself a symlink to a file outside the project got its content
silently copied into the world-readable canonical script -- an
information-disclosure path that never executes the leaked content,
just exposes it.
Add _read_script_layer_source(), which reuses the existing
destination-side ancestor-walk/leaf-symlink check
(_ensure_safe_shared_destination) for the source side of the same
read, for every project-local layer (anything under
self.project_root). A layer outside the project root -- this tool's
own bundled/core asset, never user-supplied -- is read as-is.
The containment test must use the path as given, not its resolved
form: resolving first would follow an escaping symlink to its
outside-the-project target and misclassify it as a trusted bundled
asset, the exact case this exists to catch.
Assisted-by: Claude Code (model: Claude Sonnet 5, autonomous)
reconcile_scripts_for_preset() only warns on a reconciliation
failure, by design (one bad script name shouldn't block the registry
change it's reporting on). But that means disable could report
success while a remaining active wrapper fails chain validation and
the old generated script is left in place -- recorded state
(disabled) disagreeing with executable state, unlike a stale
constitution file which is not itself executable.
Capture the warning command_disable.py already triggers and surface
it as a command failure (non-zero exit, explicit error) without
rolling back the registry change -- the preset genuinely is
disabled, the script side just didn't converge and needs attention.
Assisted-by: Claude Code (model: Claude Sonnet 5, autonomous)
The installed-script-preset-vs-runtime-change validation was gated on
the internal `here` flag, so the named-directory form of init
(`specify init /path/to/existing --force --script ps`) bypassed it
entirely even with an enabled Bash-only script preset installed --
only `--here` was covered.
The check only needs an existing .specify project at the resolved
project_path, which both forms already compute by this point; drop
the `here`-only gate so it runs for either form.
Assisted-by: Claude Code (model: Claude Sonnet 5, autonomous)
The installed-script-preset-vs-runtime-change check only ran on the
fresh-install path in command_switch.py. Switching to an
already-installed integration (or the same-target --force refresh)
resolves that integration's own previously-stored script type via
_set_default_integration()'s _resolve_integration_script_type() call,
which can already differ from the project's current runtime without
--script ever being passed on that call -- and both paths skipped
the check entirely, as did `integration use`.
Move the validation into _set_default_integration() itself, right
after the runtime it's about to make active is resolved and before
any file or metadata change, so switch (all three paths) and use
share one call site instead of three places needing to remember it.
Assisted-by: Claude Code (model: Claude Sonnet 5, autonomous)
… forced
reconcile_all_script_chains() was gated on force (or
refresh_templates_force) at all three call sites, under the
assumption it exists only to undo force's own clobbering of the
canonical dispatcher. But a materialized chain's core/wrap copies are
snapshots taken at reconcile time; the underlying shared/core assets
they were built from can still advance on an ordinary, non-forced
refresh (a CLI version bump), leaving an already-materialized chain
stale with no force involved at all.
_reconcile_script_chain() is always supposed to reflect the current
resolved stack while a preset or override actively provides the
script -- it only ever preserves a user edit in the "no longer
provided" cleanup path, never here -- so reconciling unconditionally
on every refresh is safe, not just safe-on-force.
Assisted-by: Claude Code (model: Claude Sonnet 5, autonomous)
reconcile_all_script_chains()'s worklist was built from script names
a preset manifest declares plus override files currently on disk. A
deleted standalone override's name drops out of that scan entirely,
so the stale canonical script it left behind survives even an
explicit reconcile call forever -- nothing ever learns the name
needs re-checking again.
Also fold in every name with a provenance sidecar (recorded the last
time this manager generated a canonical script for it), not only
names still declared somewhere today. _reconcile_script_chain()
already restores-to-core or removes a name nothing provides any
more, and already preserves a canonical file the user edited after
the fact via the same provenance hash check -- this just makes sure
a deleted override's name actually reaches that logic again.
Assisted-by: Claude Code (model: Claude Sonnet 5, autonomous)
Implemented with Claude Code (model: Claude Sonnet 5, autonomous mode); comment fully AI-drafted.
The push to a860ec33 triggered a fresh Copilot review. Its overview re-lists the same stale threads from before (same comment ids/timestamps as the prior round's summary -- still safe to ignore), but it also surfaced 8 genuinely new findings against the merged code (3 high severity), including one from 2026-11-09 I'd missed by not paginating the comments API the first time. All 8 are fixed in 16c04e90..be10cdf0, each with a regression test:
Disabled presets bypassed the script-runtime restriction (disable a script preset, switch --script ps|py, re-enable -- the restriction never saw it). _script_providing_preset_ids() now scans with include_disabled=True.
[High] Extension disable/remove didn't reconcile wrapped scripts. A preset can wrap an extension-provided script, copying its content inline; disabling/removing the extension left that copy running. Extracted reconcile_scripts_for_names() from the per-preset reconciler so both extension commands can call it by script name.
[High, security] A symlinked script source got its content silently copied into the canonical script. Reads only validated destinations, never sources. Added _read_script_layer_source(), reusing the existing destination-side ancestor-walk/symlink check for the source side -- and the project-local test had to use the unresolved path for containment, not the resolved one, or an escaping symlink gets misclassified as a trusted bundled asset.
[High] preset disable could report success while a stale, still-executable script remained active, since reconciliation failures there only warn by design. Disable now surfaces that specific failure as a command error (exit 1) without rolling back the registry change -- the preset is genuinely disabled, the script side just didn't converge.
Named-directory init --force bypassed the runtime check the --here form already enforced (the check was gated on the here flag specifically). Now runs for either form.
integration switch/use to an already-installed integration bypassed the runtime check entirely -- that path resolves the target's own previously-stored script type, which can differ from the current runtime without --script ever being passed on that call. Moved the check into _set_default_integration() itself, after the runtime is resolved and before any mutation, so switch (all three paths) and use share one call site.
Non-forced upgrades left a materialized script chain frozen at an old snapshot while shared helpers advanced -- reconciliation was gated on force/refresh_templates_force under the assumption it only existed to undo force's own clobbering. _reconcile_script_chain is always supposed to reflect the live stack while a preset/override actively provides a script, so reconciling on every refresh is safe, not just safe-under-force.
A deleted standalone override's stale canonical script survived forever, even across an explicit reconcile_all_script_chains() call, since the worklist only scanned override files currently on disk. Now also folds in every name with provenance metadata recorded from a prior generation, so a deleted override's name reaches the existing restore-or-cleanup logic again.
Verification: uvx ruff@0.15.0 check src tests clean; presets (806), extensions disable/remove, and integrations switch/use/upgrade/init test modules all pass locally with the new regression tests included.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Lifecycle operations can report success while leaving disabled scripts executable, and late fallback validation can abort integration changes after removal without rollback.
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
author-awaitingWaiting on author responsetriage-can-waitVerdict: valid and in-scope but deprioritized; held behind the evidence gate
4 participants
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.
Continues #4551. Per @mnriem's review, this is now a lifecycle-materialization design instead of the runtime-resolution one this PR originally shipped — see the pivot discussion below for the full rationale.
What changed in this revision
The runtime dispatcher (per-invocation resolution via a hidden CLI shell-out) is gone. The chain is now materialized to disk at preset-lifecycle time, mirroring how
PresetManager._reconcile_script_chain()'s sibling machinery for commands already works:PresetManager._reconcile_script_chain()now writes one fixed-path generated launcher file per composing layer under.specify/scripts/bash/: the topmost lands at the canonical<name>.shagents actually invoke, each one's$CORE_SCRIPTpoints at the next-lower layer's fixed path, and the chain ends in a materialized copy of the base layer at<name>.speckit-core.sh. A single"replace"-strategy layer with no composition is still written verbatim with no intermediate files.PresetManager.reconcile_scripts_for_preset()is called fromcommand_enable.py/command_disable.py/command_set_priority.pythe same wayreconcile_constitution()already is, closing the gap where those three commands previously did nothing for scripts at all.PresetResolver.resolve_script_chain()is unchanged — same ordering/collection logic, only when it's called (materialization time, not invocation time) and what consumes its output (a file-writer, not a runtime shell-out) changed.Dropped entirely: the hidden
specify preset script-chainCLI command, the generatedcontinuation-runner.sh, theSPECKIT_SCRIPT_CONTINUATIONenv protocol, the__main__.pyfallback that existed only so the runtime dispatcher could resolve withoutspecifyon PATH, and the documented uvx one-time-install limitation (scripts now have no runtime dependency onspecifybeing importable/on PATH at all).Known limitation, carried forward rather than solved
A project-local override added directly to
.specify/templates/overrides/scripts/outside any lifecycle command is not auto-materialized —PresetManager.reconcile_all_script_chains()still needs an explicit call, the same gap already documented for that method's role in repairing a forced shared-infra refresh.Evidence
TestScriptChainReconciliationintests/specify_cli/presets/test_manager.py: single-layer verbatim writes, multi-layer launcher stubs, provenance-based cleanup when a chain shrinks back to nothing, symlink-safety guards extended to every generated path, and a new test proving enable/disable/set-priority now re-materialize immediately (previously a no-op gap for scripts).tests/test_script_continuation_bash.py: realbashsubprocess execution proving ordering (outer-before → inner-before → core → inner-after → outer-after), arg/exit-status propagation, and that a priority change only takes effect after an explicit reconcile call — the materialized-model inverse of the old runtime test's claim.requires_bashwhere this sandbox's barebashresolves to the WSL launcher over subprocess), 1 pre-existing unrelated failure deselected (test_setup_tasks_ps_core_template_resolved, verified failing identically on this branch before any of my changes viagit stash).Left for follow-up, pending your input
.ps1/.pycounterparts of a script name.Disclosure per CONTRIBUTING.md: implemented with Claude Code, autonomous mode with human review of the diff and test results; the materialization design follows directly from mnriem's review comment on this PR.