Skip to content

Assert every command placeholder names a command that exists - #4249

Open
Hermann-web wants to merge 1 commit into
github:mainfrom
Hermann-web:chore/command-placeholder-target-coverage
Open

Assert every command placeholder names a command that exists#4249
Hermann-web wants to merge 1 commit into
github:mainfrom
Hermann-web:chore/command-placeholder-target-coverage

Conversation

@Hermann-web

Copy link
Copy Markdown

Problem

IntegrationBase.resolve_command_refs rewrites __SPECKIT_COMMAND_<NAME>__
with a bare regex and never consults a list of commands:

return re.sub(
    r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_]*)__",
    lambda m: prefix + "speckit" + separator
    + m.group(1).lower().replace("_", separator),
    content,
)

Any name that matches the pattern is rewritten. A typo such as
__SPECKIT_COMMAND_ASSES_SHAPE__ becomes /speckit.asses.shape, which is
written into a user's agent instructions as a plausible-looking command that no
agent can run. There is no error, and the failure surfaces only when someone
tries the command.

Existing coverage checks the mechanism, not the target:

  • tests/integrations/test_base.py asserts the separator variants are produced;
  • tests/test_agent_config_consistency.py asserts extension tokens resolve with
    the correct separator.

Neither asserts that the resolved command exists. This matters more now that
extensions/catalog.community.json carries 157 community extensions whose
authors are told to use these placeholders.

Change

One new test module, tests/test_command_placeholder_targets.py. No production
code changes.

The command set is derived rather than restated:

  • speckit.<stem> for every templates/commands/*.md;
  • every provides.commands[].name and aliases from each
    extensions/*/extension.yml and presets/*/preset.yml.

A command added or renamed is therefore covered without editing the test.

It asserts:

  1. Every placeholder in shipped markdown (templates/, extensions/,
    presets/) resolves to a command that exists.
  2. Core templates do not reference a command only an extension provides, since a
    core template has to work when that extension is not installed.
  3. Placeholders were actually found, so a change to the token spelling or to the
    shipped directories fails loudly instead of scanning nothing and passing.
  4. The mapping used by the test is the one resolve_command_refs implements,
    checked against the real function rather than reimplemented on faith.

Verification

Passes on current main (95efce4): the repository is clean today, with all 17
placeholders resolving and no core template leaking an extension reference.

Both assertions were confirmed to fail before committing, by appending
__SPECKIT_COMMAND_ASSES_SHAPE__ and __SPECKIT_COMMAND_BUG_FIX__ to
templates/commands/plan.md:

AssertionError: placeholders resolve to commands that do not exist:
  /speckit.asses.shape referenced by templates/commands/plan.md
AssertionError: core templates reference commands only an extension provides,
  so the instruction is dead unless that extension is installed:
  /speckit.bug.fix in templates/commands/plan.md

The injection was reverted; the diff is the new file only.

pytest tests/test_command_placeholder_targets.py is 6 passed.
pytest tests/test_agent_config_consistency.py tests/integrations/test_base.py tests/test_commands_package.py tests/test_command_placeholder_targets.py is
125 passed.

The full suite was also run: 6958 passed, 155 skipped, and 26 failures all in
test_ps_* PowerShell parity tests because pwsh is not installed on the
machine used. No non-PowerShell failures.

Scope

Deliberately small and additive, per CONTRIBUTING's note about large unagreed
changes. It adds coverage for an existing behaviour and changes nothing that
ships to users.

@Hermann-web
Hermann-web requested a review from mnriem as a code owner August 21, 2026 13:50
`resolve_command_refs` rewrites `__SPECKIT_COMMAND_<NAME>__` with a regex and
never consults a list of commands, so a misspelled placeholder does not fail.
`__SPECKIT_COMMAND_ASSES_SHAPE__` becomes `/speckit.asses.shape` and ships into
agent instructions as a plausible command no agent can run.

Existing coverage asserts the rewrite uses the right separator and prefix, which
is the mechanism rather than the target. Nothing checks that the target exists.

The command set is derived from `templates/commands/*.md` and from the
`provides.commands` entries of every extension and preset manifest, rather than
restated in the test, so a command added or renamed is covered without editing
this module.

A second assertion keeps core templates from referencing a command only an
extension provides, since a core template has to work when that extension is not
installed.

Both assertions were confirmed to fail on an injected typo and an injected
extension reference before being committed. No production code changes.
@Hermann-web
Hermann-web force-pushed the chore/command-placeholder-target-coverage branch from e8fd5f4 to 5468ad7 Compare August 21, 2026 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants