iframe-proxy

kennedy-whytech · 2026-04-12T14:35:28Z

Description

Implements strategy: wrap for preset command and skill overrides. Presets can now inject content before and after a core command using a {CORE_TEMPLATE} placeholder, without fully replacing it.
Multiple wrap presets compose in deterministic priority order — the lowest-priority number wraps outermost, the highest-priority wraps innermost.

What's new

Core feature

strategy: wrap wired into both register_commands (all agents) and _register_skills (ai-skills path)
{CORE_TEMPLATE} resolves against core templates and extensions, never against installed presets, to prevent accidental nesting
Presets inherit scripts/agent_scripts from the core command when they don't define their own

Multi-preset composability (according to PR comments)

Installing multiple wrap presets for the same command composes them in priority order rather than last-write-wins
_replay_wraps_for_command recomposes all enabled wrap presets after every install and remove
_replay_skill_override keeps SKILL.md in sync with the recomposed body for ai-skills-enabled projects
Registry stores wrap_commands per preset; remove() re-orders operations so replay sees post-removal state before recomposing

Bug fixes

{SCRIPT} and {AGENT_SCRIPT} placeholders were not being resolved in the markdown/YAML agent branch when strategy: wrap was active — fixed by calling resolve_skill_placeholders +
_convert_argument_placeholder after wrap substitution
strategy: wrap key was leaking into the rendered frontmatter of the output file — stripped after substitution
Argument placeholder conversion ({ARGS} → agent-specific format) was running before resolve_skill_placeholders, so $ARGUMENTS injected by skill resolution was never converted — ordering fixed
Removed preset key from init-options.json during project init (was being stored but never used as authoritative state)

Test hygiene

Restored missing "reinstall" assertion in the bundled preset CLI error test
Restored AGENT_CONFIGS on the class (not the instance) after test mutation, preventing state leaking across tests

Test plan
- uvx pytest tests/test_presets.py::TestWrapStrategy — all wrap strategy tests pass
- uvx pytest tests/test_presets.py — full preset suite (209 tests)
- uvx pytest — full suite, no new failures

End-to-end test with my own presets. The wrapping seems doing good. Also, I feel like "wrap" can cover prepend and append already.

Testing

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

AI Disclosure

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

AI assistance disclosure

This PR was implemented with AI assistance (Claude Code) for code generation,
test writing, and commit messages. All changes were reviewed and verified
manually.

Copilot

Pull request overview

Adds support for strategy: wrap on preset-provided commands, enabling preset authors to inject pre/post content around an existing “core” command body via a {CORE_TEMPLATE} placeholder during command/skill registration.

Changes:

Introduces _substitute_core_template() and applies it during preset skill registration (_register_skills) and generic agent command registration (CommandRegistrar.register_commands).
Extends the self-test preset with a new speckit.wrap-test command and updates docs to mark wrap as implemented for commands.
Adds unit + E2E-style tests covering placeholder substitution and install behavior.

Show a summary per file

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 6/6 changed files
Comments generated: 1

mnriem

Please address Copilot feedback. If not applicable, please explain why

kennedy-whytech · 2026-04-13T16:11:16Z

@mnriem that makes sense to me. updated.

Copilot

Pull request overview

Implements support for strategy: wrap on preset command templates by substituting {CORE_TEMPLATE} with the installed core command template body during installation/registration, enabling presets to extend core commands without copy/paste.

Changes:

Added _substitute_core_template() helper and applied it when generating skills from preset commands (Claude skills flow).
Applied the same substitution in CommandRegistrar.register_commands() for non-skill agent command installation.
Added a self-test preset wrap command + unit/E2E tests, and updated preset strategy documentation.

Show a summary per file

File	Description
`src/specify_cli/presets.py`	Adds `{CORE_TEMPLATE}` substitution helper and applies it in the skills generation path for wrap-strategy preset commands.
`src/specify_cli/agents.py`	Applies `{CORE_TEMPLATE}` substitution during normal agent command registration when `strategy: wrap` is set.
`tests/test_presets.py`	Updates self-test manifest expectations and adds unit + E2E coverage for wrap substitution.
`presets/self-test/preset.yml`	Registers a new wrap-strategy command in the bundled self-test preset.
`presets/self-test/commands/speckit.wrap-test.md`	New wrap command template containing `{CORE_TEMPLATE}` placeholder.
`presets/README.md`	Updates roadmap/docs to reflect wrap being implemented for commands/artifacts (not scripts).

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 6/6 changed files
Comments generated: 2

mnriem

Please address Copilot feedback

Copilot

Pull request overview

Adds a new preset composition option (strategy: wrap) for command/skill overrides, allowing presets to inject content around the existing core command by substituting a {CORE_TEMPLATE} placeholder, rather than fully replacing the command.

Changes:

Implement _substitute_core_template helper to resolve and substitute {CORE_TEMPLATE} from project core templates, bundled core pack, and installed extensions.
Wire wrap substitution into both preset skill generation (_register_skills) and agent command registration (register_commands).
Extend self-test preset + add comprehensive tests for wrap behavior, extension resolution, and script placeholder hygiene.

Show a summary per file

File	Description
`src/specify_cli/presets.py`	Introduces `{CORE_TEMPLATE}` substitution + merges core `scripts/agent_scripts` into wrapped skill frontmatter.
`src/specify_cli/agents.py`	Enables wrap substitution during agent command registration.
`tests/test_presets.py`	Adds `TestWrapStrategy` coverage and updates self-test manifest expectations.
`presets/self-test/preset.yml`	Adds a self-test wrap command entry.
`presets/self-test/commands/speckit.wrap-test.md`	New wrap-strategy command template used for end-to-end testing.
`presets/README.md`	Updates documentation to reflect wrap is implemented for commands/artifacts (scripts TBD).

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 6/6 changed files
Comments generated: 1

mnriem

Please address Copilot feedback

Copilot

Pull request overview

Adds support for a new preset composition mode (strategy: wrap) so preset command/skill overrides can inject content around an existing command body via a {CORE_TEMPLATE} placeholder, instead of fully replacing it.

Changes:

Introduces _substitute_core_template() and wires it into preset skill generation and agent command registration for strategy: wrap.
Extends command registration to merge missing scripts / agent_scripts from the wrapped “core” template and resolves placeholders for TOML command output.
Adds wrap-strategy tests plus a self-test preset command and updates preset documentation.

Show a summary per file

File	Description
`src/specify_cli/presets.py`	Adds core-template substitution helper and applies wrap behavior when generating SKILL.md overrides.
`src/specify_cli/agents.py`	Applies wrap substitution and frontmatter inheritance during command registration; resolves placeholders for TOML output.
`tests/test_presets.py`	Adds wrap-strategy test suite and adjusts self-test preset expectations; modifies bundled preset CLI error assertions.
`presets/self-test/preset.yml`	Registers a new self-test command entry for wrap strategy coverage.
`presets/self-test/commands/speckit.wrap-test.md`	Adds a wrap-strategy self-test command file using `{CORE_TEMPLATE}`.
`presets/README.md`	Updates documentation to reflect that command/template wrapping is implemented (scripts wrap not yet).

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (2)

tests/test_presets.py:3299

Same AGENT_CONFIGS leakage issue here: the test mutates registrar.AGENT_CONFIGS[...] (class-level) and then restores with registrar.AGENT_CONFIGS = original (instance-level assignment). This can leave test-agent in the class-level configs for subsequent tests. Restore on CommandRegistrar.AGENT_CONFIGS (class) or patch the dict in a context manager.

        registrar = CommandRegistrar()
        original = copy.deepcopy(registrar.AGENT_CONFIGS)
        registrar.AGENT_CONFIGS["test-agent"] = {
            "dir": str(agent_dir.relative_to(project_dir)),
            "format": "markdown",
            "args": "$ARGUMENTS",
            "extension": ".md",
            "strip_frontmatter_keys": [],
        }
        try:
            registrar.register_commands(
                "test-agent",
                [{"name": "speckit.specify", "file": "commands/speckit.specify.md"}],
                "test-preset",
                project_dir / "preset",
                project_dir,
            )
        finally:
            registrar.AGENT_CONFIGS = original

tests/test_presets.py:3347

Same AGENT_CONFIGS leakage issue in the TOML agent test: registrar.AGENT_CONFIGS is mutated (class-level) but restored via instance assignment (registrar.AGENT_CONFIGS = original). This can leak test-toml-agent into other tests. Restore via CommandRegistrar.AGENT_CONFIGS = original or patch the dict with patch.dict/monkeypatch.

        registrar = CommandRegistrar()
        original = copy.deepcopy(registrar.AGENT_CONFIGS)
        registrar.AGENT_CONFIGS["test-toml-agent"] = {
            "dir": str(toml_dir.relative_to(project_dir)),
            "format": "toml",
            "args": "{{args}}",
            "extension": ".toml",
            "strip_frontmatter_keys": [],
        }
        try:
            registrar.register_commands(
                "test-toml-agent",
                [{"name": "speckit.specify", "file": "commands/speckit.specify.md"}],
                "test-preset",
                project_dir / "preset",
                project_dir,
            )
        finally:
            registrar.AGENT_CONFIGS = original

Files reviewed: 6/6 changed files
Comments generated: 4

mnriem

Please address Copilot feedback

Copilot

Pull request overview

Implements a new preset command override strategy (strategy: wrap) that allows presets to inject content before/after an existing command template via a {CORE_TEMPLATE} placeholder, and wires this behavior into both skills generation and agent command registration paths.

Changes:

Added _substitute_core_template() and integrated wrap behavior into preset skill registration (_register_skills) and agent command registration (register_commands).
Updated TOML command generation to resolve script placeholders and convert argument placeholders appropriately.
Expanded preset test coverage (including self-test preset updates) for wrap strategy behavior and script metadata inheritance.

Show a summary per file

File	Description
`src/specify_cli/presets.py`	Adds `{CORE_TEMPLATE}` substitution helper and merges core script metadata into wrapped presets for skills generation.
`src/specify_cli/agents.py`	Applies wrap substitution for command registration and resolves placeholders for TOML outputs.
`tests/test_presets.py`	Adds a dedicated wrap strategy test suite and updates self-test preset assertions.
`presets/self-test/preset.yml`	Registers a new self-test wrap command entry.
`presets/self-test/commands/speckit.wrap-test.md`	Adds a wrap-strategy command fixture using `{CORE_TEMPLATE}`.
`presets/README.md`	Updates documentation to reflect wrap support status (commands/artifacts implemented; scripts not yet).

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (2)

tests/test_presets.py:3300

Same AGENT_CONFIGS restoration issue as above: reassigning CommandRegistrar.AGENT_CONFIGS = original won’t restore the dict object referenced by specify_cli.extensions.CommandRegistrar.AGENT_CONFIGS, so the temporary test-agent entry can leak into later tests. Prefer an in-place restore (clear/update) or patch.dict.

        registrar = CommandRegistrar()
        original = copy.deepcopy(registrar.AGENT_CONFIGS)
        registrar.AGENT_CONFIGS["test-agent"] = {
            "dir": str(agent_dir.relative_to(project_dir)),
            "format": "markdown",
            "args": "$ARGUMENTS",
            "extension": ".md",
            "strip_frontmatter_keys": [],
        }
        try:
            registrar.register_commands(
                "test-agent",
                [{"name": "speckit.specify", "file": "commands/speckit.specify.md"}],
                "test-preset",
                project_dir / "preset",
                project_dir,
            )
        finally:
            CommandRegistrar.AGENT_CONFIGS = original

tests/test_presets.py:3348

Same AGENT_CONFIGS restoration issue as above for the TOML agent config: reassigning the class attribute can leave specify_cli.extensions.CommandRegistrar.AGENT_CONFIGS pointing at the mutated dict (with test-toml-agent still present). Restore the dict contents in-place or use patch.dict to avoid cross-test leakage.

        registrar = CommandRegistrar()
        original = copy.deepcopy(registrar.AGENT_CONFIGS)
        registrar.AGENT_CONFIGS["test-toml-agent"] = {
            "dir": str(toml_dir.relative_to(project_dir)),
            "format": "toml",
            "args": "{{args}}",
            "extension": ".toml",
            "strip_frontmatter_keys": [],
        }
        try:
            registrar.register_commands(
                "test-toml-agent",
                [{"name": "speckit.specify", "file": "commands/speckit.specify.md"}],
                "test-preset",
                project_dir / "preset",
                project_dir,
            )
        finally:
            CommandRegistrar.AGENT_CONFIGS = original

Files reviewed: 6/6 changed files
Comments generated: 3

mnriem

Please address Copilot feedback

PresetResolver.resolve_extension_command_via_manifest() consults each installed extension.yml to find the actual file declared for a command name, rather than assuming the file is named <cmd_name>.md. This fixes _substitute_core_template for extensions like selftest where the manifest maps speckit.selftest.extension → commands/selftest.md. Resolution order in _substitute_core_template is now: 1. resolve_core(cmd_name) — project overrides win, then name-based lookup 2. resolve_extension_command_via_manifest(cmd_name) — manifest fallback 3. resolve_core(short_name) — core template short-name fallback Path traversal guard mirrors the containment check already present in ExtensionManager to reject absolute paths or paths escaping the extension root.

kennedy-whytech · 2026-04-18T17:01:19Z

Comment 1 — {SCRIPT} unresolved for Markdown/YAML:

Fixed. Script placeholder resolution and _convert_argument_placeholder() now run for the markdown and YAML branches in register_commands(), matching the existing TOML treatment.

Comment 2 — strategy: wrap leaks into markdown output:

Fixed. strategy is popped from frontmatter immediately after the wrap substitution block, so it no longer appears in installed .md / .agent.md files.

Comment 3 — Suggest test for {SCRIPT} in markdown output:

Added test_register_commands_markdown_resolves_inherited_scripts which asserts {SCRIPT} is absent from a markdown agent's installed output when the core template carries it.

Comment 4 — Multi-preset wrapping must respect priority ordering:

Addressed. _substitute_core_template now uses resolve_core() to skip the presets tier, and the new _replay_wraps_for_command() helper re-composes all wrap presets in priority order (lowest
precedence innermost) on both install and uninstall.

kennedy-whytech · 2026-04-18T17:06:38Z

@mnriem Out of scope:

There appears to be a core Spec Kit bug in how preset state is interpreted downstream.

Observed behavior:

A project initialized with .specify/init-options.json containing "preset": null

Then specify preset add --dev succeeds

Later downstream logic treats "preset": null in init-options.json as meaning no preset is active, and skips preset-specific post-
logic

Why this is a bug:

init-options.json["preset"] is init-time metadata, not authoritative active preset state

Presets can be added after init, removed later, and potentially stacked

A single nullable preset field is not a reliable representation of active preset state

Active preset state should come from the preset registry / resolver, not init-options.json

Impact:

Preset-specific command behavior can be skipped incorrectly even when the preset is installed and command resolution should apply

In our case this caused downstream logic to say the preset was null and skip Step 0 / post-logic

Suggested fix:

Stop using init-options.json["preset"] as the signal for whether a preset is active

Use the installed preset registry or command resolver as the source of truth

If preset remains in init-options.json, document it as init-only metadata

for this, I have removed init-options.json["preset"] for now. I'm not super sure about the intention of init-options.json["preset"] . Saving preset to init-options.json is redundant because the PresetRegistry already tracks all installed presets as the authoritative source. Whether the preset is maintainer-defined or user-specified, downstream operations should read the registry — not init-options.json? (We do have other options like template if it's maintainer-defined as well.) i guess it makes sense if the config is stored somewhere globally, so we can init a project with a set of presets?

resolve_core() was returning None for built-in commands (implement, specify, etc.) because PresetResolver only checked .specify/templates/ commands/ (Priority 4), which is never populated for commands in a normal project. strategy:wrap presets rely on resolve_core() to fetch the {CORE_TEMPLATE} body, so the wrap was silently skipped and SKILL.md was never updated. Priority 5 now checks core_pack/commands/ (wheel install) or repo_root/templates/commands/ (source checkout), mirroring the pattern used by _locate_core_pack() elsewhere. Updated two tests whose assertions assumed resolve_core() always returned None when .specify/templates/commands/ was absent.

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

src/specify_cli/agents.py:494

register_commands() resolves skill/script placeholders and re-converts injected $ARGUMENTS for markdown and toml outputs, but the yaml branch doesn’t. This means YAML recipe agents (e.g., Goose) can still receive unresolved {SCRIPT}/{ARGS}/AGENT placeholders and/or $ARGUMENTS that should be converted to the agent’s args token. Apply the same resolve_skill_placeholders + post-resolution argument conversion step for yaml format before render_yaml_command so YAML output matches the documented placeholder pipeline (see AGENTS.md and extensions/EXTENSION-DEVELOPMENT-GUIDE.md).

            elif agent_config["format"] == "toml":
                body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root)
                body = self._convert_argument_placeholder(body, "$ARGUMENTS", agent_config["args"])
                output = self.render_toml_command(frontmatter, body, source_id)
            elif agent_config["format"] == "yaml":
                output = self.render_yaml_command(
                    frontmatter, body, source_id, cmd_name
                )

Files reviewed: 7/7 changed files
Comments generated: 5

mnriem

Please address Copilot feedback

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (4)

src/specify_cli/agents.py:494

YAML-format agent outputs (e.g. Goose) still bypass resolve_skill_placeholders() and the second $ARGUMENTS conversion that Markdown/TOML now do. This means {SCRIPT}, {ARGS}, __AGENT__, and $ARGUMENTS injected during placeholder resolution can remain unresolved/incorrect in YAML recipes, especially for strategy: wrap commands that inherit scripts. Consider running resolve_skill_placeholders() and then re-running _convert_argument_placeholder() for the YAML branch as well (mirroring the Markdown/TOML flow).

            elif agent_config["format"] == "yaml":
                output = self.render_yaml_command(
                    frontmatter, body, source_id, cmd_name
                )

tests/test_presets.py:1032

These assertions compare Path values via str(result) against hardcoded POSIX-style substrings (e.g. "templates/commands/specify.md"). This is brittle on Windows where str(Path) uses backslashes and can cause false negatives/positives. Prefer comparing Path objects directly (e.g. result == core_cmd_dir / "specify.md") or use result.as_posix() for substring checks.

        resolver = PresetResolver(project_dir)
        result = resolver.resolve_core("specify", "command")
        assert result is not None
        assert "presets" not in str(result)
        assert "templates/commands/specify.md" in str(result)

tests/test_presets.py:1054

This assertion checks for a substring with forward slashes in str(result), which is OS-dependent. Use result.as_posix() or compare against the expected Path (e.g. result == ext_cmd_dir / "myext-cmd.md") to keep the test portable.

        resolver = PresetResolver(project_dir)
        result = resolver.resolve_core("myext-cmd", "command")
        assert result is not None
        assert "extensions/myext/commands" in str(result)

tests/test_presets.py:1043

This test asserts on a POSIX-style substring in str(result) ("overrides/specify.md"), which can fail on Windows due to path separator differences. Prefer result == override_dir / "specify.md" (or result.as_posix() for substring checks).

        resolver = PresetResolver(project_dir)
        result = resolver.resolve_core("specify", "command")
        assert result is not None
        assert "overrides/specify.md" in str(result)

Files reviewed: 8/8 changed files
Comments generated: 2

… in tests - outermost_pack_id now updates alongside outermost_frontmatter inside the wrap loop, so it reflects the actual last contributing preset rather than always taking wrap_presets[0] (which may have been skipped) - Replace str(path) substring checks in TestResolveCore with Path.parts tuple comparisons for correct behaviour on Windows (CI runs windows-latest)

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

src/specify_cli/agents.py:494

The YAML recipe output path doesn’t run resolve_skill_placeholders() (which expands {SCRIPT}, {ARGS}, __AGENT__, etc.) and therefore also can’t reliably convert $ARGUMENTS injected by placeholder resolution into the YAML agent’s args token. This leaves {SCRIPT} / {ARGS} literals in Goose recipes and breaks the documented placeholder pipeline for YAML agents (see AGENTS.md “YAML Format”). Apply the same preprocessing used for markdown/toml before calling render_yaml_command().

            elif agent_config["format"] == "yaml":
                output = self.render_yaml_command(
                    frontmatter, body, source_id, cmd_name
                )

Files reviewed: 8/8 changed files
Comments generated: 3

mnriem

Please address Copilot feedback. If not applicable, please explain why

…-processing in replay - ExtensionManifest._load raises ValidationError for non-dict YAML roots instead of TypeError - PresetManager._replay_wraps_for_command calls integration.post_process_skill_content, matching _register_skills behaviour - PresetResolver skips extensions that raise OSError/TypeError/AttributeError on manifest load - Tests: non-mapping YAML, OSError manifest skip, and replay integration post-processing

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

src/specify_cli/agents.py:494

The YAML agent branch doesn’t run resolve_skill_placeholders() (so {SCRIPT}/{ARGS}/AGENT/CONTEXT_FILE aren’t expanded) and also doesn’t re-run argument placeholder conversion after that expansion. This means Goose/YAML recipes can ship with literal {SCRIPT}/{ARGS} or unconverted $ARGUMENTS injected by placeholder resolution, unlike the markdown/toml branches. Consider applying the same two-step processing used for markdown/toml (resolve_skill_placeholders first, then _convert_argument_placeholder) before calling render_yaml_command().

            elif agent_config["format"] == "yaml":
                output = self.render_yaml_command(
                    frontmatter, body, source_id, cmd_name
                )

Files reviewed: 10/10 changed files
Comments generated: 0 new

mnriem · 2026-04-21T20:02:37Z

upstream/main merged PR #2189 (wrap-only strategy) which overlaps with our comprehensive composition strategies (prepend/append/wrap). Resolved conflicts keeping our implementation as source of truth: - README: keep our future considerations (composition is now fully implemented, not a future item) - presets.py: keep our composition architecture (_reconcile_composed_commands, collect_all_layers, resolve_content) while preserving #2189's _substitute_core_template which is used by agents.py for skill generation - tests: keep both test sets (our composition tests + #2189's wrap tests), removed TestReplayWrapsForCommand and TestInstallRemoveWrapLifecycle which test the superseded _replay_wraps_for_command API; our composition tests cover equivalent scenarios - Restored missing _unregister_commands call in remove() that was lost during #2189 merge

* feat: implement strategy: wrap * fix: resolve merge conflict for strategy wrap correctness * feat: multi-preset composable wrapping with priority ordering Implements comment github#4 from PR review: multiple installed wrap presets now compose in priority order rather than overwriting each other. Key changes: - PresetResolver.resolve() gains skip_presets flag; resolve_core() wraps it to skip tier 2, preventing accidental nesting during replay - _replay_wraps_for_command() recomposed all enabled wrap presets for a command in ascending priority order (innermost-first) after any install or remove - _replay_skill_override() keeps SKILL.md in sync with the recomposed command body for ai-skills-enabled projects - install_from_directory() detects strategy: wrap commands, stores wrap_commands in the registry entry, and calls replay after install - remove() reads wrap_commands before deletion, removes registry entry before rmtree so replay sees post-removal state, then replays remaining wraps or unregisters when none remain Tests: TestResolveCore (5), TestReplayWrapsForCommand (5), TestInstallRemoveWrapLifecycle (5), plus 2 skill/alias regression tests * fix: resolve extension commands via manifest file mapping PresetResolver.resolve_extension_command_via_manifest() consults each installed extension.yml to find the actual file declared for a command name, rather than assuming the file is named <cmd_name>.md. This fixes _substitute_core_template for extensions like selftest where the manifest maps speckit.selftest.extension → commands/selftest.md. Resolution order in _substitute_core_template is now: 1. resolve_core(cmd_name) — project overrides win, then name-based lookup 2. resolve_extension_command_via_manifest(cmd_name) — manifest fallback 3. resolve_core(short_name) — core template short-name fallback Path traversal guard mirrors the containment check already present in ExtensionManager to reject absolute paths or paths escaping the extension root. * fix: add bundled core_pack as Priority 5 in PresetResolver.resolve() resolve_core() was returning None for built-in commands (implement, specify, etc.) because PresetResolver only checked .specify/templates/ commands/ (Priority 4), which is never populated for commands in a normal project. strategy:wrap presets rely on resolve_core() to fetch the {CORE_TEMPLATE} body, so the wrap was silently skipped and SKILL.md was never updated. Priority 5 now checks core_pack/commands/ (wheel install) or repo_root/templates/commands/ (source checkout), mirroring the pattern used by _locate_core_pack() elsewhere. Updated two tests whose assertions assumed resolve_core() always returned None when .specify/templates/commands/ was absent. * fix: harden preset wrap replay removal * fix: stabilize existing directory error output * fix: track outermost_pack_id from contributing preset; use Path.parts in tests - outermost_pack_id now updates alongside outermost_frontmatter inside the wrap loop, so it reflects the actual last contributing preset rather than always taking wrap_presets[0] (which may have been skipped) - Replace str(path) substring checks in TestResolveCore with Path.parts tuple comparisons for correct behaviour on Windows (CI runs windows-latest) * fix: guard against non-mapping YAML manifests; apply integration post-processing in replay - ExtensionManifest._load raises ValidationError for non-dict YAML roots instead of TypeError - PresetManager._replay_wraps_for_command calls integration.post_process_skill_content, matching _register_skills behaviour - PresetResolver skips extensions that raise OSError/TypeError/AttributeError on manifest load - Tests: non-mapping YAML, OSError manifest skip, and replay integration post-processing

…plates, commands, and scripts (#2133) * fix: rebase onto upstream/main, resolve conflicts with PR #2189 upstream/main merged PR #2189 (wrap-only strategy) which overlaps with our comprehensive composition strategies (prepend/append/wrap). Resolved conflicts keeping our implementation as source of truth: - README: keep our future considerations (composition is now fully implemented, not a future item) - presets.py: keep our composition architecture (_reconcile_composed_commands, collect_all_layers, resolve_content) while preserving #2189's _substitute_core_template which is used by agents.py for skill generation - tests: keep both test sets (our composition tests + #2189's wrap tests), removed TestReplayWrapsForCommand and TestInstallRemoveWrapLifecycle which test the superseded _replay_wraps_for_command API; our composition tests cover equivalent scenarios - Restored missing _unregister_commands call in remove() that was lost during #2189 merge * fix: re-create skill directory in _reconcile_skills after removal After _unregister_skills removes a skill directory, _register_skills skips writing because the dir no longer passes the is_dir() check. Fix by ensuring the skill subdirectory exists before calling _register_skills so the next winning preset's content gets registered. Fixes the Claude E2E failure where removing a top-priority override preset left skill-based agents without any SKILL.md file. * fix: address twenty-third round of Copilot PR review feedback - Protect reconciliation in remove(): wrap _reconcile_composed_commands and _reconcile_skills in try/except so failures emit a warning instead of leaving the project in an inconsistent state - Protect reconciliation in install(): same pattern for post-install reconciliation so partial installs don't lack cleanup - Inherit scripts/agent_scripts from base frontmatter: when composing commands, merge scripts and agent_scripts keys from the base command's frontmatter into the top layer's frontmatter if missing, preventing composed commands from losing required script references - Add tier-5 bundled core fallback to collect_all_layers(): check the bundled core_pack (wheel) or repo-root templates (source checkout) when .specify/templates/ doesn't contain the core file, matching resolve()'s tier-5 fallback so composition can always find a base layer * fix: address twenty-fourth round of Copilot PR review feedback - Use yaml.safe_load for frontmatter parsing in resolve_content instead of CommandRegistrar.parse_frontmatter which uses naive find('---',3); strip strategy key from final frontmatter to prevent leaking internal composition directives into rendered agent command files - Filter _reconcile_skills to specific commands: use _FilteredManifest wrapper so only the commands being reconciled get their skills updated, preventing accidental overwrites of other commands' skills that may be owned by higher-priority presets * fix: address twenty-fifth round of Copilot PR review feedback - Support legacy command-frontmatter strategy: when preset.yml doesn't declare a strategy, check the command file's YAML frontmatter for strategy: wrap as a fallback so legacy wrap presets participate in composition and multi-preset chaining - Guard skill dir creation in _reconcile_skills: only re-create the skill directory if the skill was previously managed (listed in some preset's registered_skills), avoiding creation of new skill dirs that _register_skills would normally skip * fix: add explanatory comment to empty except in legacy frontmatter parsing * fix: address twenty-sixth round of Copilot PR review feedback - Unregister stale commands when composition fails: when resolve_content returns None during reconciliation (base layer removed), unregister the command from non-skill agents and emit a warning - Load extension aliases during reconciliation: _register_command_from_path now checks extension.yml for aliases when the winning layer is an extension, so alias files are restored after preset removal - Use line-based fence detection for legacy frontmatter strategy fallback: scan for --- on its own line instead of split('---',2) to avoid mis-parsing YAML values containing --- * fix: address twenty-seventh round of Copilot PR review feedback - Handle non-preset winners in _reconcile_skills: when the winning layer is core/extension/project-override, restore skills via _unregister_skills so skill-based agents stay consistent with the priority stack - Update base_frontmatter_text on replace layers: when a higher-priority replace layer occurs during composition, update both top and base frontmatter so scripts/agent_scripts inheritance reflects the effective base beneath the top composed layer * fix: address twenty-eighth round of Copilot PR review feedback - Parse only interior lines in _parse_fm_yaml: use lines[1:-1] instead of filtering all --- lines, preventing corruption when YAML values contain a line that is exactly --- - Omit empty frontmatter: skip re-rendering when top_fm is empty dict to avoid emitting ---/{}/--- for intentionally empty frontmatter - Update scaffold wrap comment: mention both {CORE_TEMPLATE} and $CORE_SCRIPT placeholders for templates/commands vs scripts - Clarify shell composition scope in ARCHITECTURE.md: note that bash/PS1 resolve_template_content only handles templates; command/script composition is handled by the Python resolver * fix: address twenty-ninth round of Copilot PR review feedback - Fix TestCollectAllLayers docstring: reference collect_all_layers() - Add default/unknown strategy handling in bash/PS1 composition: error on unrecognized strategy values instead of silently skipping - Fix comment: .composed/ is a persistent dir, not temporary - Fix comment: legacy fallback checks all valid strategies, not just wrap - Cache PresetRegistry in _reconcile_skills: build presets_by_priority once instead of constructing registry per-command * fix: address thirtieth round of Copilot PR review feedback - Guard legacy frontmatter fallback: only check command file frontmatter for strategy when the manifest entry doesn't explicitly include the strategy key, preventing override of manifest-declared strategies - Document rollback limitation: note that mid-registration failures may leave orphaned agent command files since partial progress isn't captured by the local vars * fix: handle project override skills and extension context in reconciliation * fix: add comment to empty except in extension registration fallback * fix: filter extension commands in reconciliation and fix type annotation * fix: filter extension commands from post-install reconciliation Apply the same extension-installed check used in _register_commands to the reconciliation command list, preventing reconciliation from registering commands for extensions that are not installed. * fix: skip convention fallback for explicit file paths and add stem fallback to tier-5 When a preset manifest provides an explicit file path that does not exist, skip the convention-based fallback to avoid masking typos. Also add speckit.<stem> to <stem>.md fallback in tier-5 bundled/source core lookup for consistency with tier-4. * fix: scan past non-replace layers to find base in resolve_content The base-finding scan now skips non-replace layers below a replace layer instead of stopping at the first non-replace. This fixes the case where a low-priority append/prepend layer sits below a replace that should serve as the base for composition. * fix: add context_note to non-skill agent registration for extensions Add context_note parameter to register_commands_for_non_skill_agents and pass extension name/id during reconciliation so rendered command files preserve the extension-specific context markers. * fix: Optional type, rollback safety, and override skill restoration - Fix context_note type to Optional[str] - Wrap shutil.rmtree in try/except during install rollback - Separate override-backed skills from core/extension in _reconcile_skills * fix: align bash/PS1 base-finding with Python resolver Rewrite bash and PowerShell composition loops to find the effective base replace layer first (scanning bottom-up, skipping non-replace layers below it), then compose only from the base upward. This prevents evaluation of irrelevant lower layers (e.g. a wrap with no placeholder below a replace) and matches resolve_content behavior. * fix: PS1 no-python warning, integration hook for override skills, alias cleanup - Warn when no Python 3 found in PS1 and presets use composition strategies - Apply post_process_skill_content integration hook when restoring override-backed skills so agent-specific flags are preserved - Unregister command aliases alongside primary name when composition fails to prevent orphaned alias files * fix: include aliases in removed_cmd_names during preset removal Read aliases from preset manifest before deleting pack_dir so alias command files are included in unregistration and reconciliation. * fix: add comment to empty except in alias extraction during removal * fix: scan top-down for effective base in all resolvers Change base-finding to scan from highest priority downward to find the nearest replace layer, then compose only layers above it. Prevents evaluation of irrelevant lower layers (e.g. a wrap without placeholder below a higher-priority replace) across Python, bash, and PowerShell. * fix: align CLI composition chain display with top-down base-finding Show only contributing layers (base and above) in preset resolve output, matching resolve_content top-down semantics. Layers below the effective base are omitted since they do not contribute. * fix: guard corrupted registry entries and make manifest authoritative - Add isinstance(meta, dict) guard in bash registry parsing so corrupted entries are skipped instead of breaking priority ordering - Only use convention-based file lookup when the manifest does not list the requested template, making preset.yml authoritative and preventing stray on-disk files from creating unintended layers * fix: align resolve() with manifest file paths and match extension context_note - Update resolve() preset tier to consult manifest file paths before convention-based lookup, matching collect_all_layers behavior - Use exact extension context_note format matching extensions.CommandRegistrar - Update test to declare template in manifest (authoritative manifest) * revert: restore resolve() convention-based behavior for backwards compatibility resolve() is the existing public API used by shell scripts and other callers. Changing it to manifest-authoritative breaks backward compat for presets that rely on convention-based file lookup. Only the new collect_all_layers/resolve_content path uses manifest-authoritative logic. * fix: only pre-compose when this preset is the top composing layer Skip composition in _register_commands when a higher-priority replace layer already wins for the command. Register the raw file instead and let reconciliation write the correct final content. * fix: deduplicate PyYAML warnings and use self.registry in reconciliation - Emit PyYAML-missing warning once per function call in bash/PS1 instead of per-preset to avoid spamming stderr - Use self.registry.list_by_priority() in reconciliation methods instead of constructing new PresetRegistry instances to avoid redundant I/O and potential consistency issues * fix: document strategy handling consistency between layers and registrar Composed output already strips strategy from frontmatter (resolve_content pops it). Raw file registration preserves legacy frontmatter strategy for backward compat; reconciliation corrects the final state. * fix: correct stale comments for alias tracking and base-finding algorithm * security: validate manifest file paths in bash/PowerShell resolvers Reject absolute paths and parent directory traversal (..) in the manifest-declared file field before joining with the preset directory. Matches the Python-side validation in PresetManifest._validate(). --------- Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com>

kennedy-whytech requested a review from mnriem as a code owner April 12, 2026 14:35

kennedy-whytech force-pushed the kennedy-whytech-feat-preset-wrap-strategy branch from 604b585 to 444daef Compare April 12, 2026 15:47

mnriem requested a review from Copilot April 13, 2026 12:28

mnriem self-assigned this Apr 13, 2026

Copilot started reviewing on behalf of mnriem April 13, 2026 12:28 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Comment thread src/specify_cli/presets.py Outdated

mnriem requested changes Apr 13, 2026

View reviewed changes

kennedy-whytech force-pushed the kennedy-whytech-feat-preset-wrap-strategy branch from 444daef to 53220f8 Compare April 13, 2026 15:55

kennedy-whytech requested a review from mnriem April 13, 2026 16:13

mnriem requested a review from Copilot April 13, 2026 16:20

Copilot started reviewing on behalf of mnriem April 13, 2026 16:21 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Comment thread tests/test_presets.py

Comment thread tests/test_presets.py

mnriem requested changes Apr 13, 2026

View reviewed changes

mnriem requested a review from Copilot April 13, 2026 17:08

Copilot started reviewing on behalf of mnriem April 13, 2026 17:09 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Comment thread src/specify_cli/agents.py Outdated

mnriem self-requested a review April 13, 2026 17:18

mnriem requested changes Apr 13, 2026

View reviewed changes

kennedy-whytech force-pushed the kennedy-whytech-feat-preset-wrap-strategy branch from 4fe7f81 to 49c83d2 Compare April 14, 2026 02:51

mnriem requested a review from Copilot April 14, 2026 11:45

Copilot started reviewing on behalf of mnriem April 14, 2026 11:46 View session

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Comment thread src/specify_cli/presets.py Outdated

Comment thread src/specify_cli/agents.py

Comment thread tests/test_presets.py

Comment thread tests/test_presets.py Outdated

mnriem self-requested a review April 14, 2026 12:30

mnriem requested changes Apr 14, 2026

View reviewed changes

mnriem requested a review from Copilot April 14, 2026 20:26

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Comment thread tests/test_presets.py

Comment thread src/specify_cli/presets.py Outdated

Comment thread tests/test_presets.py Outdated

mnriem self-requested a review April 14, 2026 20:35

mnriem requested changes Apr 14, 2026

View reviewed changes

mnriem requested a review from Copilot April 14, 2026 20:45

kennedy-whytech requested a review from mnriem April 18, 2026 17:07

mnriem requested a review from Copilot April 20, 2026 14:28

Copilot started reviewing on behalf of mnriem April 20, 2026 14:29 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

Comment thread src/specify_cli/presets.py Outdated

Comment thread src/specify_cli/presets.py Outdated

Comment thread src/specify_cli/presets.py Outdated

Comment thread src/specify_cli/presets.py Outdated

Comment thread src/specify_cli/presets.py

mnriem requested changes Apr 20, 2026

View reviewed changes

kennedy-whytech added 2 commits April 21, 2026 11:01

fix: harden preset wrap replay removal

881a8f0

fix: stabilize existing directory error output

02962ff

kennedy-whytech requested a review from mnriem April 21, 2026 15:23

mnriem requested a review from Copilot April 21, 2026 16:47

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Comment thread src/specify_cli/presets.py Outdated

Comment thread tests/test_presets.py

mnriem requested a review from Copilot April 21, 2026 17:20

Copilot started reviewing on behalf of mnriem April 21, 2026 17:21 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Comment thread src/specify_cli/presets.py

Comment thread src/specify_cli/presets.py Outdated

Comment thread src/specify_cli/presets.py Outdated

mnriem requested changes Apr 21, 2026

View reviewed changes

kennedy-whytech requested a review from mnriem April 21, 2026 19:39

mnriem requested a review from Copilot April 21, 2026 19:46

Copilot started reviewing on behalf of mnriem April 21, 2026 19:46 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

mnriem approved these changes Apr 21, 2026

View reviewed changes

mnriem merged commit 22e7699 into github:main Apr 21, 2026
15 checks passed

File	Description
tests/test_presets.py	Updates self-test manifest expectations; adds new `TestWrapStrategy` coverage.
src/specify_cli/presets.py	Adds `_substitute_core_template()` and wires it into preset skill generation.
src/specify_cli/agents.py	Wires wrap substitution into agent command registration.
presets/self-test/preset.yml	Adds a new wrap-strategy command entry to the self-test preset.
presets/self-test/commands/speckit.wrap-test.md	Adds a wrap command template using `{CORE_TEMPLATE}`.
presets/README.md	Updates roadmap/docs to reflect `wrap` implemented for commands/artifacts.

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

kennedy-whytech commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

AI Disclosure

AI assistance disclosure

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

kennedy-whytech commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

kennedy-whytech commented Apr 18, 2026

Uh oh!

kennedy-whytech commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

kennedy-whytech commented Apr 12, 2026 •

edited

Loading

kennedy-whytech commented Apr 13, 2026 •

edited

Loading

kennedy-whytech commented Apr 18, 2026 •

edited

Loading