Fix: scaffold self-contained namespaced preset commands (#4076) - #4082
Fix: scaffold self-contained namespaced preset commands (#4076)#4082mnriem wants to merge 2 commits into
Conversation
Preset command templates named `speckit.<ns>.<cmd>` were silently dropped whenever `.specify/extensions/<ns>/` was absent, while `speckit.<cmd>` always scaffolded. The `_extension_installed_for_command` guard filtered purely on name shape, conflating "override of an installed extension's command" with "a preset shipping its own namespaced command." Because a `type: command` template always ships its own body, such a command is self-contained and must scaffold like any short-named command. Remove the name-shape guard at all four call sites (registration, both reconciliation passes, and skills). The reconciliation loop already skips names that resolve to no layers (`if not layers: continue`), and the composed-None branch still cleans up commands whose base layer disappeared. Convert the command-mode "no base layer to compose onto" hard error into a warn + skip, matching the existing behavior in _reconcile_composed_commands so command-mode install and reconciliation stay consistent. Update the two tests that encoded the old drop behavior to assert the new consistent-scaffold contract, and add coverage proving 2-part and 3-part preset commands scaffold identically with no extension installed. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cfc4f1ce-6acb-465a-aa7b-999f2e4197fb
There was a problem hiding this comment.
Pull request overview
Fixes namespaced preset commands being dropped when no matching extension is installed.
Changes:
- Removes extension-presence filtering for preset-owned commands.
- Aligns command and skill reconciliation behavior.
- Adds regression coverage for namespaced command scaffolding.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/presets/__init__.py |
Allows self-contained namespaced commands and warns on uncomposable commands. |
tests/test_presets.py |
Updates and adds namespaced scaffolding tests. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
| import warnings | ||
| warnings.warn( | ||
| f"Command '{cmd['name']}' uses '{strategy}' " | ||
| f"strategy but no base command layer exists to " | ||
| f"compose onto; skipping. Provide a lower-priority " | ||
| f"preset, extension, or core command for it before " | ||
| f"using composition strategies.", | ||
| stacklevel=2, | ||
| ) | ||
| continue |
There was a problem hiding this comment.
Good catch — fixed in d5c2998.
_register_skills now applies the same skip as _register_commands: for a composition-strategy command (wrap/prepend/append) with no .composed file, it resolves the stack and skips when no base exists (resolve_content is None), so an uncomposable command is no longer materialized as a skill with a literal {CORE_TEMPLATE} (wrap) or as a bare preset fragment (prepend/append). The skip is silent there because _register_commands already warns for the same command in the same pass. Added a regression test (test_uncomposable_wrap_command_skips_skill_in_skills_mode) asserting no skill is written and no {CORE_TEMPLATE} leaks.
Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8, autonomous).
When _register_commands skips an uncomposable composition command (a
wrap/prepend/append with no base layer to compose onto — e.g. the command
it wraps comes from an uninstalled extension), install still passed the
full manifest to _register_skills. For a command-backed integration in
skills mode, _register_skills created the missing skill and fell back to
the raw preset body because no `.composed` file existed, materializing a
broken SKILL.md — a literal `{CORE_TEMPLATE}` for wrap, or just the
preset's own fragment for prepend/append. Previously the raise in
_register_commands aborted before skills ran, so this never surfaced.
Make _register_skills apply the same skip: for a composition-strategy
command with no `.composed` file, resolve the stack and skip when no base
exists (resolve_content is None). The skip is silent because
_register_commands already warned for the same command in the same pass.
Add a regression test proving an uncomposable wrap command renders no skill
and never leaks a literal {CORE_TEMPLATE} in skills mode.
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: cfc4f1ce-6acb-465a-aa7b-999f2e4197fb
Review round 1 — addressedSkills-mode consistency (
Full Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8, autonomous). |
There was a problem hiding this comment.
Review details
Suppressed comments (1)
src/specify_cli/presets/init.py:2733
- The cached
.composedfile makes this check miss a now-uncomposable command. If a wrap/prepend/append command was composed successfully and its base preset or extension is later removed,_reconcile_composed_commandshandlesresolve_content(...) is Nonewithout deleting that cache; this condition then short-circuits, and lines 2682-2684 render the stale composition as the skill. Re-resolve non-replace commands regardless of cache existence, and have reconciliation remove both the stale cache and any managed skill when no base remains.
if (
effective_strategy != "replace"
and not composed_file.exists()
and resolver.resolve_content(cmd_name, "command") is None
):
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Description
Closes #4076.
specify preset addsilently dropped preset command templates namedspeckit.<ns>.<cmd>(3-part) whenever.specify/extensions/<ns>/was absent, while the 2-partspeckit.<cmd>form always scaffolded.The
_extension_installed_for_commandguard filtered purely on name shape, wrongly conflating two different things:Because a
type: commandtemplate always ships its own body, such a command is self-contained and must scaffold like any short-named command. This was a long-standing latent bug: the 3-part form was dropped in every preset-era version, breaking any preset that ships namespaced commands without also delivering a matching extension (a pattern some community presets rely on).Change
if not layers: continue), so an absent extension contributes no layer and nothing broken is materialized; the composed-Nonebranch still cleans up commands whose base layer disappeared._reconcile_composed_commandsso command-mode install and reconciliation behave identically. A genuinely uncomposablewrap/prepend/appendcommand is now reported loudly instead of silently dropped or aborting the whole install.Testing
uv sync && uv run pytest— full suite green (6525 passed, 8 skipped).test_short_and_namespaced_commands_scaffold_consistently,test_selfcontained_namespaced_command_scaffolds_without_extension).AI Disclosure
Investigation, fix, and tests were authored autonomously by GitHub Copilot (model: Claude Opus 4.8) on behalf of @mnriem. Each commit carries an
Assisted-by:trailer.