[bug-fix] Fix command-token-hyphen-gap: add verbatim token form for hyphenated command names - #4204
Conversation
…command names Apply the remediation from the bug assessment on issue #4198. Extends resolve_command_refs() with a new verbatim token form __SPECKIT_COMMAND(speckit.agent-context.update)__ that carries the command ID verbatim. Dots are replaced by the invoke_separator; hyphens within segments are preserved (and collapse naturally to the separator when separator is '-'). The existing uppercase __SPECKIT_COMMAND_NAME__ form is unchanged. Refs #4198 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR #4204 added the verbatim `__SPECKIT_COMMAND(...)__` token form to the central `resolve_command_refs()` (used for command-file agents and presets), but the parallel resolver in `_register_extension_skills()` — the sole writer of extension SKILL.md content when the active integration is in skills mode — still matched only the uppercase form. A verbatim token therefore leaked into SKILL.md as a raw literal. Teach that resolver the verbatim form as well, carrying the command id verbatim so hyphenated names (e.g. `speckit.agent-context.update`) survive intact. Adds regression tests exercising `_register_extension_skills` directly for both dollar- and slash-skills agents. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e7f0535c-26ca-4f4a-af95-d6b240dfd0e4
|
Pushed This PR wires the new verbatim The commit teaches Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously. |
There was a problem hiding this comment.
Pull request overview
Adds a verbatim command-reference token to correctly support hyphenated command IDs across integration invocation styles.
Changes:
- Supports
__SPECKIT_COMMAND(...)__tokens. - Handles extension skill registration paths.
- Adds unit and integration regression tests.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/integrations/base.py |
Implements verbatim token resolution. |
src/specify_cli/extensions/__init__.py |
Resolves verbatim tokens in extension skills. |
tests/integrations/test_base.py |
Tests resolver behavior and compatibility. |
tests/test_extensions.py |
Tests Codex and Claude skill rendering. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
| + "speckit" | ||
| + separator | ||
| + m.group(1).lower().replace("_", separator), | ||
| r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_]*)__|__SPECKIT_COMMAND\(([^)]+)\)__", |
| r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_]*)__" | ||
| r"|__SPECKIT_COMMAND\(([^)]+)\)__", |
Bug fix — command-token-hyphen-gap
Proposed fix for issue #4198, applying the remediation from the bug assessment.
Verdict: Valid · Severity: medium
Summary
Extends
resolve_command_refs()with a new verbatim token form__SPECKIT_COMMAND(speckit.agent-context.update)__that carries the command ID verbatim, resolving correctly for both.and-invoke separators. The existing uppercase form is unchanged.Changes
src/specify_cli/integrations/base.pyresolve_command_refs()now handles both the existing uppercase form and the new verbatim form via a combined regex; updated docstringtests/integrations/test_base.py./-separators, custom prefix, mixed usage, and an explicit regression guard for the unchanged uppercase formTests Added or Updated
TestResolveCommandRefs::test_verbatim_dot_separator_with_hyphen— verbatim form +.separator →/speckit.agent-context.updateTestResolveCommandRefs::test_verbatim_hyphen_separator_with_hyphen— verbatim form +-separator →/speckit-agent-context-updateTestResolveCommandRefs::test_verbatim_dollar_prefix— verbatim form with$prefixTestResolveCommandRefs::test_verbatim_simple_command_no_hyphens— verbatim form also works for non-hyphenated commandsTestResolveCommandRefs::test_verbatim_and_uppercase_mixed— both forms coexist in the same stringTestResolveCommandRefs::test_uppercase_form_unaffected_by_verbatim_support— regression guard: uppercase form behavior is unchangedLocal Verification
python3 -c "..."(inline test ofresolve_command_refslogic) → all 7 cases passedDeviations from Assessment
The preferred remediation called for the verbatim form to have the
invoke_separatorrewrite applied "only to the existing uppercase form". Implemented as: for the verbatim form,.separators in the command ID are replaced with the separator, and hyphens are kept as-is (they naturally collapse to the separator when separator is-). This gives the correct result for both separator styles and is consistent with what the assessment described.Callers (
extensions/__init__.py,presets/__init__.py,agents.py,shared_infra.py) were not modified — they pass content throughresolve_command_refs()unchanged, so the new token form will work anywhere the existing one does.Risks & Review Notes
__SPECKIT_COMMAND_<NAME>__form is unaffected; the two regex alternatives are disjoint.agent-contextextension:speckit.agent-context.updateis the only hyphenated bundled command identified; no existing token incore_packcurrently references it via__SPECKIT_COMMAND__syntax, so there is no migration needed for bundled content.Refs #4198 · cc
@minzzang144