fix: support hyphenated command names in __SPECKIT_COMMAND_<NAME>__ tokens - #4361
fix: support hyphenated command names in __SPECKIT_COMMAND_<NAME>__ tokens#4361minzzang144 wants to merge 4 commits into
Conversation
Widens the uppercase command-ref token's character class from
[A-Z0-9_] to [A-Z0-9_-] in both resolve_command_refs() and the
extension-skills resolver, so a hyphenated command name (e.g.
speckit.agent-context.update) round-trips without a second token
grammar. Decode logic is unchanged since replace("_", separator)
already leaves literal hyphens untouched.
Implements the "Option 2" direction agreed with @mnriem in the
review discussion on github#4204, as an alternative to that PR's verbatim
__SPECKIT_COMMAND(...)__ form.
Fixes github#4198
This change was implemented with AI assistance (Claude Code); I
reviewed the diff and ran the full test suite myself before opening
this PR.
Adds cases for a hyphen adjacent to a digit, multiple hyphens within one segment, and more than two dotted segments, confirming the widened character class has no arbitrary limit on segment count.
STEP-2/RUN and MULTI-WORD-SEGMENT read unambiguously as synthetic test data, instead of awkwardly extending the real agent-context extension's command name with made-up suffixes.
There was a problem hiding this comment.
Pull request overview
Adds support for literal hyphens in command-reference tokens across command and extension-skill rendering.
Changes:
- Expands both token resolvers to accept hyphens.
- Updates extension-author documentation.
- Adds unit and integration-style regression tests.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/integrations/base.py |
Expands the shared token grammar. |
src/specify_cli/extensions/__init__.py |
Expands the extension-skills token grammar. |
tests/integrations/test_base.py |
Tests hyphenated token variants. |
tests/test_extensions.py |
Tests skill-format command registration. |
extensions/EXTENSION-DEVELOPMENT-GUIDE.md |
Documents hyphen encoding and skills behavior. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
|
|
||
| return re.sub( | ||
| r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_]*)__", _replacement, body | ||
| r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_-]*)__", _replacement, body |
There was a problem hiding this comment.
Good catch, and genuinely useful. This is actually the same distinction @mnriem already pointed out on #4204 — resolve_command_refs() covers command-file agents and presets, but _register_extension_skills() is a separate resolver that's the sole writer of extension SKILL.md content in skills mode.
Cross-checking this comment against that context made clear our test coverage had the same kind of gap flagged there — the test we added goes through CommandRegistrar.register_commands_for_agent(), which only exercises resolve_command_refs() (the shared resolver) and never touches _register_extension_skills(). One of the two places we actually fixed was left without a regression test.
Added a test (test_skill_registration_resolves_hyphenated_command_ref_tokens) in tests/test_extension_skills.py::TestExtensionSkillRegistration that exercises _register_extension_skills() directly via install_from_directory(..., register_commands=False), parametrized across all 6 agents the same way the sibling test right above it (test_skill_registration_resolves_command_ref_tokens) already is. Confirmed it's a real regression test by temporarily reverting the character-class widening in extensions/__init__.py — all 6 fail with the raw token leaking into the SKILL.md — then restoring it, where all 6 pass.
…upport The existing hyphen tests in tests/test_extensions.py go through CommandRegistrar.register_commands_for_agent(), which only exercises the shared resolve_command_refs() resolver. _register_extension_skills() in extensions/__init__.py is a separate resolver (the sole writer of extension SKILL.md content in skills mode, per @mnriem's note on github#4204) that was left without direct coverage even though its own character class was widened in the same commit. Adds test_skill_registration_resolves_hyphenated_command_ref_tokens, parametrized across all 6 agents like its sibling test_skill_registration_resolves_command_ref_tokens, exercising _register_extension_skills() directly via install_from_directory(..., register_commands=False). Verified this is a real regression test: reverting the character class in extensions/__init__.py makes all 6 cases fail with the raw token leaking into SKILL.md; restoring it makes all 6 pass.
Description
__SPECKIT_COMMAND_<NAME>__tokens can't represent a hyphen in a command name, so referencing a hyphenated command (e.g. the bundledspeckit.agent-context.update) silently resolves to the wrong path. This widens the character class in bothresolve_command_refs()(src/specify_cli/integrations/base.py) and the extension-skills resolver (src/specify_cli/extensions/__init__.py) from[A-Z][A-Z0-9_]*to[A-Z][A-Z0-9_-]*. No decode-logic change is needed —.replace("_", separator)already leaves a literal hyphen untouched. Also updatesextensions/EXTENSION-DEVELOPMENT-GUIDE.md's "Referencing other commands" section, which still stated hyphens couldn't be represented and had a stale skills-mode limitation note.Fixes #4198, Fixes #4328
Testing
uv run specify --helpuv sync && uv run pytest(ran via.venv/bin/python -m pytest testsper the guidance in CONTRIBUTING.md/AGENTS.md to avoid resolvingspecify_clito another checkout): 7145 passed. The 9 failures present are Python-parity template tests that also fail onmainwithout this change, unrelated to this fix.specify init --integration claude --script sh --non-interactiveagainst this branch and confirmed the scaffolded project has no unresolved__SPECKIT_COMMANDtokens left in it.Added 9 new unit test cases covering hyphen + each separator style, multiple hyphens within one segment, a hyphen adjacent to a digit, and an unlimited number of dotted segments.
AI Disclosure
This change was written with AI assistance (Claude Code). I reviewed the diff and ran the test suite myself before opening this PR.