[bug-fix] Fix command-token-hyphen-gap: add verbatim token form for hyphenated command names

[github-actions]

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

File	Change	Notes
src/specify_cli/integrations/base.py	modified	resolve_command_refs() now handles both the existing uppercase form and the new verbatim form via a combined regex; updated docstring
tests/integrations/test_base.py	added tests	7 new test cases covering verbatim form with ./- separators, custom prefix, mixed usage, and an explicit regression guard for the unchanged uppercase form

Tests Added or Updated

• TestResolveCommandRefs::test_verbatim_dot_separator_with_hyphen — verbatim form + . separator → /speckit.agent-context.update
• TestResolveCommandRefs::test_verbatim_hyphen_separator_with_hyphen — verbatim form + - separator → /speckit-agent-context-update
• TestResolveCommandRefs::test_verbatim_dollar_prefix — verbatim form with $ prefix
• TestResolveCommandRefs::test_verbatim_simple_command_no_hyphens — verbatim form also works for non-hyphenated commands
• TestResolveCommandRefs::test_verbatim_and_uppercase_mixed — both forms coexist in the same string
• TestResolveCommandRefs::test_uppercase_form_unaffected_by_verbatim_support — regression guard: uppercase form behavior is unchanged

Local Verification

• Commands run: python3 -c "..." (inline test of resolve_command_refs logic) → all 7 cases passed
• No project venv/pytest available in this runner environment; logic was verified by extracting and running the function directly.

Deviations from Assessment

The preferred remediation called for the verbatim form to have the invoke_separator rewrite 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 through resolve_command_refs() unchanged, so the new token form will work anywhere the existing one does.

Risks & Review Notes

• Backward compatibility: the existing __SPECKIT_COMMAND_<NAME>__ form is unaffected; the two regex alternatives are disjoint.
• Bundled agent-context extension: speckit.agent-context.update is the only hyphenated bundled command identified; no existing token in core_pack currently references it via __SPECKIT_COMMAND__ syntax, so there is no migration needed for bundled content.
• Documentation: authoring guides that document the uppercase token form should eventually be updated to mention the verbatim form as the correct choice when hyphens are present (tracked in the assessment as a separate improvement).

Refs #4198 · cc @minzzang144

• Fixes [Bug]: __SPECKIT_COMMAND_*__ tokens can't reference commands whose names contain hyphens #4198

Generated by 🛠️ Fix Bug from Labeled Issue for issue #4198 · 388.7 AIC · ⌖ 17.1 AIC · ⊞ 36.4K · ◷

[mnriem]

Pushed dcb5f36 to close a gap this PR leaves open.

This PR wires the new verbatim __SPECKIT_COMMAND(...)__ token into the central resolve_command_refs(), which covers command-file agents and presets. But extension command bodies rendered as skills go through a separate resolver in ExtensionManager._register_extension_skills() — and that resolver is the sole writer of extension SKILL.md content when the active integration is in skills mode (_active_command_registration_scope() returns an empty set for command-backed integrations in skills mode, so the registrar writes nothing). That resolver still matched only the uppercase form, so a verbatim token leaked into SKILL.md as a raw literal.

The commit teaches _register_extension_skills() the verbatim form too (same combined regex as resolve_command_refs), carrying the command id verbatim so hyphenated names like speckit.agent-context.update survive. Added two regression tests that drive _register_extension_skills directly for a dollar-skills agent (codex) and a slash-skills agent (claude); both fail without the source change. Full test_extensions.py, test_presets.py, and integrations/test_base.py suites pass (1225).

Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously.

[Copilot]

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

[mnriem]

Please address Copilot feedback

[minzzang144]

@mnriem

Thanks for picking this up so fast, and for catching the skills-mode gap, @mnriem — I hadn't realized that labeling the issue would kick off an implementation + PR automatically. The ideas in the original issue were meant as loose sketches for someone with the actual context to sanity-check, not concrete proposals, so now that I see where they landed, I'd like to revisit one more direction before this settles.

Option 1 — the currently implemented direction: the verbatim form (__SPECKIT_COMMAND(speckit.agent-context.update)__)

• 👍 You can copy-paste the command id verbatim, no encoding rule to remember
• 👍 The code change is localized (one new branch added alongside the existing one)
• 👎 It introduces a second grammar with a completely different notation from the uppercase form (uppercase snake_case vs. a lowercase dotted-hyphenated id wrapped in parens) — now there are two rules to learn instead of one
• 👎 The capture is [^)]+, which is too permissive — malformed input (spaces, newlines, arbitrary text) passes through silently and gets turned into an invalid invocation with no warning. That's exactly the "silent failure" mode the original issue was trying to prevent, just relocated to the new form (needs a separate validation step)
• 👎 The speckit. prefix has to be repeated inside the parens, which is redundant since the token name already says COMMAND

Option 2 — a direction I'd like to propose: widen the uppercase form's character class to allow the literal hyphen

Command names are constrained to speckit.<segment>(.<segment>)*, where each segment is [a-z0-9-]+ (see EXTENSION_COMMAND_NAME_PATTERN) — a segment can never contain an underscore. So instead of introducing a new grammar, the existing regex's character class can simply be widened:

# original, pre-PR
r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_]*)__"
# proposed
r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_-]*)__"

The decode logic doesn't even need to change — .replace("_", separator) already leaves - untouched. This works regardless of how many hyphens appear per segment, or where they fall (start, middle, end):

__SPECKIT_COMMAND_AGENT-CONTEXT_UPDATE__        → speckit.agent-context.update
__SPECKIT_COMMAND_CODE-REVIEW_REQUEST-CHANGES__ → speckit.code-review.request-changes

• 👍 No new grammar — the notation stays singular
• 👍 The narrow character class filters out malformed input for free, solving Option 1's silent-failure problem without any separate validation logic
• 👍 No redundant prefix; the parallel branches in resolve_command_refs/_register_extension_skills collapse back into one, simplifying the implementation
• 👍 It's consistent with the actual command-name grammar, guaranteeing lossless representation (a segment never contains _, so there's no dot/hyphen collision possible)
• 👎 The mixed uppercase-and-hyphen shape (AGENT-CONTEXT) may look aesthetically odd, less like a "valid identifier"
• 🤔 This extension only allows the hyphen. How far to widen the character class is really a policy call — for now I'd propose extending it only as far as the actual command-name grammar requires ([a-z0-9-]), but if other special characters become necessary down the line, that would need its own discussion.
• 👎 Some of what's already implemented and tested on this branch would need to be walked back

My take: I lean toward Option 2 — it reaches the same safety guarantees as Option 1 without a second grammar or extra validation logic, more simply. That said, there's a real cost to reverting work already on this branch, so I'll leave the final call to the maintainers.

On docs: either direction, extensions/EXTENSION-DEVELOPMENT-GUIDE.md's "Referencing other commands" section (roughly lines 297–325) — which still only documents the uppercase form, states the token "does not carry hyphens within a segment," and has a stale skills-mode limitation callout — no longer matches reality and should be updated.

[mnriem]

Please go for option 2