Skip to content

feat(integrations): add process_registered_command hook with source_id - #5215

Merged
mnriem merged 2 commits into
github:mainfrom
kevin9327:feat/registered-command-hook
Nov 2, 2026
Merged

mnriem merged 2 commits into
github:mainfrom
kevin9327:feat/registered-command-hook

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

Description

Prerequisite for #5168, split out as requested in this review and per the integration delivery boundary.

post_process_command_content() is the only per-integration hook that CommandRegistrar.register_commands() calls, and it only sees the rendered text. It cannot tell whether a command came from core or from an extension or preset, and it runs after $ARGUMENTS and __SPECKIT_COMMAND_*__ tokens are resolved. A rewrite that depends on the command's source therefore has nowhere to go except an agent-name branch inside the registrar (today, is_cline_ext in agents.py).

This PR adds IntegrationBase.process_registered_command(frontmatter, body, *, source_id) -> (frontmatter, body):

  • register_commands() calls it for every command it registers for the integration, after script-path adjustment and name injection, and before $ARGUMENTS conversion, __SPECKIT_COMMAND_*__ resolution and rendering, so aliases and every output format (Markdown, TOML, YAML, SKILL.md) use the result.
  • The default returns both values unchanged, so no integration's output changes in this PR.
  • The existing is_cline_ext branch is left as is here; fix(cline): keep .md extension on speckit.* file names in command bodies #5168 moves it into the Cline integration through this hook.

Testing

  • New tests/test_registered_command_hook.py (6 tests): the default is a no-op and default registration output is unchanged; an overriding integration receives the parsed frontmatter, the raw body (before $ARGUMENTS conversion) and source_id ("core" and an extension id), and its result is what gets written, including for a skills-layout agent and for aliases. 5 of the 6 fail on main (the hook does not exist); all pass with this change.

  • pytest -n 8 tests/integrations tests/test_post_process.py tests/test_extensions.py tests/test_registered_command_hook.py: 3708 passed, 8 skipped.

  • uvx ruff@0.15.0 check src tests: clean.

  • Tested locally with uv run specify --help

  • Ran existing tests with uv sync && uv run pytest (the subset above)

  • Tested with a sample project (if applicable)

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (fill in the disclosure below)

AI disclosure: Implemented by Grok Bot, an AI coding agent acting on behalf of @kevin9327, running autonomously (model unknown, not exposed to the agent). Extent: design, code, tests and this description. The commit carries an Assisted-by: trailer. The tests above were run by the agent; no personal human review is claimed.

post_process_command_content() only sees rendered text, so an integration
cannot tell a core command from an extension or preset command, and any
rewrite that depends on the source has to live as an agent-name branch in
CommandRegistrar.register_commands(). Add IntegrationBase
.process_registered_command(frontmatter, body, *, source_id), called for
every registered command after name injection and before placeholder
resolution and rendering. The default returns both values unchanged, so
no output changes.

Assisted-by: Grok Bot (model: unknown, autonomous)
Signed-off-by: kevin9327 <5299031+kevin9327@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The hook test cannot detect incorrect argument-conversion ordering because OpenCode leaves $ARGUMENTS unchanged and the fixture lacks a command-reference token.

1 open issue
What changed in this PR

Adds a source-aware integration hook to command registration, preparing for the Cline-specific rewrite changes in #5168 while preserving default output.

Changes:

  • Introduces process_registered_command() with parsed frontmatter, body, and source_id.
  • Calls the hook before placeholder resolution and rendering.
  • Adds tests for default behavior, source IDs, transformed output, skills layout, and aliases.
File Description
tests/​test_registered_command_hook.py Adds six tests covering the hook and registration output.
src/​specify_cli/​integrations/​base.py Defines and documents the default no-op hook.
src/​specify_cli/​agents.py Invokes the integration hook during command registration.

Review effort: Balanced (auto)

Comment thread tests/test_registered_command_hook.py Resolved
kevin9327 added a commit to kevin9327/spec-kit that referenced this pull request Nov 2, 2026
…lenames

Brings in the process_registered_command hook (github#5215) that the Cline rewrite moves onto.

Assisted-by: Grok Bot (model: unknown, autonomous)
Signed-off-by: kevin9327 <5299031+kevin9327@users.noreply.github.com>
@mnriem

mnriem commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

@mnriem mnriem added author-awaiting Waiting on author response triage-must-have Verdict: high-value, important work for Spec Kit — do first labels Nov 2, 2026
OpenCode keeps $ARGUMENTS unchanged and the old fixture had no
__SPECKIT_COMMAND_*__ token, so the existing test still passed if
process_registered_command() ran after argument conversion or command
reference resolution. Add a Gemini case whose source contains both
placeholders: the hook must receive them unresolved, and tokens the hook
adds must come out resolved ({{args}}, /speckit.tasks) in the written
TOML command.

Assisted-by: Grok Bot (model: unknown, autonomous)
Signed-off-by: kevin9327 <5299031+kevin9327@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The review identifies no active issues with the source-aware command registration hook.

No open issues 🎉

1 resolved since last review

Review effort: Balanced (auto)

@mnriem
mnriem merged commit 8e8b2f6 into github:main Nov 2, 2026
16 checks passed
@mnriem

mnriem commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

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

Labels

author-awaiting Waiting on author response triage-must-have Verdict: high-value, important work for Spec Kit — do first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants