fix: allow Claude to chain skills for hook execution#2227
fix: allow Claude to chain skills for hook execution#2227mnriem wants to merge 4 commits intogithub:mainfrom
Conversation
- Set disable-model-invocation to false so Claude can invoke extension skills (e.g. speckit-git-feature) from within workflow skills - Inject dot-to-hyphen normalization note into Claude SKILL.md hook sections so the model maps extension.yml command names to skill names - Replace Unicode checkmark with ASCII [OK] in auto-commit scripts to fix PowerShell encoding errors on Windows - Move Claude-specific frontmatter injection to ClaudeIntegration via post_process_skill_content() hook on SkillsIntegration, wired through presets and extensions managers - Add positive and negative tests for all changes Fixes github#2178
There was a problem hiding this comment.
Pull request overview
Fixes Claude Code failing to execute chained speckit skills/hooks by ensuring generated skills are user-invocable, model-invocation is enabled (for skill chaining), and hook documentation clarifies dot-to-hyphen command normalization; also removes Unicode output that breaks PowerShell parsing on Windows.
Changes:
- Moves Claude-specific SKILL.md frontmatter mutation into
ClaudeIntegration.post_process_skill_content()and routes preset/extension skill generation through integration post-processing. - Injects a dot→hyphen normalization note into hook instruction sections in Claude-generated skills.
- Replaces Unicode
✓with ASCII[OK]in auto-commit scripts and adds/updates tests to lock in behavior.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/agents.py |
Removes Claude-only SKILL frontmatter from shared builder to avoid affecting other skill-generation paths. |
src/specify_cli/integrations/base.py |
Adds SkillsIntegration.post_process_skill_content() hook (default identity) for integration-specific skill post-processing. |
src/specify_cli/integrations/claude/__init__.py |
Implements Claude-specific post-processing: injects user-invocable: true, disable-model-invocation: false, and a hook command normalization note. |
src/specify_cli/presets.py |
Applies integration post-processing to preset-created/restored SKILL.md content. |
src/specify_cli/extensions.py |
Applies integration post-processing to extension-created SKILL.md content. |
extensions/git/scripts/bash/auto-commit.sh |
Replaces Unicode checkmark success output with [OK] for portability. |
extensions/git/scripts/powershell/auto-commit.ps1 |
Replaces Unicode checkmark success output with [OK] to avoid Windows PowerShell parser/encoding issues. |
tests/integrations/test_integration_claude.py |
Updates expectations for disable-model-invocation: false and adds coverage for Claude post-processing + hook note injection behavior. |
tests/test_extension_skills.py |
Updates extension skill YAML expectations to match new Claude post-processing (disable-model-invocation: false). |
tests/test_presets.py |
Updates preset skill assertions to expect disable-model-invocation: false. |
tests/extensions/git/test_git_extension.py |
Adds tests ensuring [OK] is used and Unicode ✓ is absent in script outputs. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 11/11 changed files
- Comments generated: 3
| return re.sub( | ||
| r"(?m)^(\s*)(- For each executable hook, output the following)", | ||
| lambda m: m.group(1) + _HOOK_COMMAND_NOTE.rstrip("\n") + "\n" + m.group(0), |
There was a problem hiding this comment.
_inject_hook_command_note() always inserts a \n newline. Elsewhere in this module the injection helpers preserve the existing EOL style (\n vs \r\n), so this can introduce mixed line endings if a SKILL.md uses CRLF. Consider preserving the existing line-ending style when inserting the hook note for consistency.
| return re.sub( | |
| r"(?m)^(\s*)(- For each executable hook, output the following)", | |
| lambda m: m.group(1) + _HOOK_COMMAND_NOTE.rstrip("\n") + "\n" + m.group(0), | |
| def repl(m: re.Match[str]) -> str: | |
| indent = m.group(1) | |
| instruction = m.group(2) | |
| eol = m.group(3) | |
| return ( | |
| indent | |
| + _HOOK_COMMAND_NOTE.rstrip("\n") | |
| + eol | |
| + indent | |
| + instruction | |
| + eol | |
| ) | |
| return re.sub( | |
| r"(?m)^(\s*)(- For each executable hook, output the following)(\r\n|\n|$)", | |
| repl, |
src/specify_cli/presets.py
Outdated
| @staticmethod | ||
| def _post_process_skill(agent_key: str, content: str) -> str: | ||
| """Delegate to the integration's post_process_skill_content if available.""" | ||
| if not isinstance(agent_key, str) or not agent_key: | ||
| return content | ||
| from specify_cli.integrations import get_integration | ||
|
|
||
| integration = get_integration(agent_key) | ||
| if integration is not None and hasattr(integration, "post_process_skill_content"): | ||
| return integration.post_process_skill_content(content) | ||
| return content |
There was a problem hiding this comment.
_post_process_skill() is duplicated here and in specify_cli/extensions.py. This duplication increases the risk of future drift (e.g., changing integration lookup or error handling in one place but not the other). Consider extracting a shared helper (module-level function) and reusing it in both call sites.
src/specify_cli/extensions.py
Outdated
| @staticmethod | ||
| def _post_process_skill(agent_key: str, content: str) -> str: | ||
| """Delegate to the integration's post_process_skill_content if available.""" | ||
| if not isinstance(agent_key, str) or not agent_key: | ||
| return content | ||
| from specify_cli.integrations import get_integration | ||
|
|
||
| integration = get_integration(agent_key) | ||
| if integration is not None and hasattr(integration, "post_process_skill_content"): | ||
| return integration.post_process_skill_content(content) | ||
| return content |
There was a problem hiding this comment.
_post_process_skill() is duplicated here and in specify_cli/presets.py. To reduce maintenance overhead and avoid inconsistencies, consider consolidating this into a single shared helper used by both extension and preset skill generators.
|
@copilot apply changes based on the comments in this thread |
- Preserve line-ending style (CRLF/LF) in _inject_hook_command_note instead of always inserting \n, matching the convention used by other injection helpers in the same module. - Extract duplicated _post_process_skill() from extensions.py and presets.py into a shared post_process_skill() function in agents.py. Both modules now import and call the shared helper.
The regex in _inject_hook_command_note only matched lines ending immediately after 'output the following', but the actual template lines continue with 'based on its `optional` flag:'. Use [^\r\n]* to capture the rest of the line before the EOL.
There was a problem hiding this comment.
Pull request overview
This PR fixes Claude Code hook execution for Speckit by ensuring Claude-generated skills are invocable/chained, normalizing hook command naming expectations, and removing a Unicode character that breaks PowerShell parsing on Windows.
Changes:
- Move Claude-specific SKILL.md frontmatter behavior out of the shared
build_skill_frontmatter()and into Claude’s integration post-processing (setsdisable-model-invocation: false,user-invocable: true). - Post-process SKILL.md content during preset/extension skill generation so Claude flags (and hook note) apply consistently across generation paths.
- Replace Unicode
✓with ASCII[OK]in auto-commit scripts and add tests to prevent regressions.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/agents.py |
Adds post_process_skill() helper to route skill content through the selected integration’s post-processor. |
src/specify_cli/integrations/base.py |
Introduces post_process_skill_content() hook (default no-op) for skills integrations. |
src/specify_cli/integrations/claude/__init__.py |
Implements Claude-specific post-processing: injects user-invocable: true, disable-model-invocation: false, and a dot→hyphen hook command note. |
src/specify_cli/presets.py |
Applies integration post-processing to generated/restored preset SKILL.md files. |
src/specify_cli/extensions.py |
Applies integration post-processing to generated extension SKILL.md files. |
extensions/git/scripts/bash/auto-commit.sh |
Replaces Unicode checkmark with [OK] in success output. |
extensions/git/scripts/powershell/auto-commit.ps1 |
Replaces Unicode checkmark with [OK] in success output (avoids Windows PowerShell parsing issues). |
tests/integrations/test_integration_claude.py |
Expands tests for Claude frontmatter flags and hook-note injection behavior. |
tests/test_extension_skills.py |
Updates expectation for Claude-generated extension SKILL.md to have disable-model-invocation: false. |
tests/test_presets.py |
Updates expectations for preset SKILL.md to have disable-model-invocation: false. |
tests/extensions/git/test_git_extension.py |
Adds tests asserting [OK] output and absence of Unicode checkmark for both bash and PowerShell scripts. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 11/11 changed files
- Comments generated: 0 new
Instead of a free function in agents.py that re-resolves the integration by key, callers in extensions.py and presets.py now resolve the integration once via get_integration() and call integration.post_process_skill_content() directly. The base identity method lives on SkillsIntegration.
Summary
Fixes #2178 — Claude Code fails to execute speckit commands because:
disable-model-invocation: trueprevents Claude from invoking extension skills (e.g.,speckit-git-feature) when chaining from workflow skillsspeckit.git.commit) but Claude skills use hyphens (speckit-git-commit)✓in PowerShellauto-commit.ps1causes parser errors on WindowsChanges
Core fix:
disable-model-invocation→falsebuild_skill_frontmatter()inagents.pypost_process_skill_content()hook toSkillsIntegration(identity default)ClaudeIntegrationoverrides it to injectuser-invocable: trueanddisable-model-invocation: falsepresets.pyandextensions.pyso all skill-generation paths go through the integrationHook command name normalization
ClaudeIntegration.post_process_skill_content()injects a dot-to-hyphen note into hook sections of generated SKILL.md files, so Claude mapsspeckit.git.commit→/speckit-git-commitPowerShell encoding fix
✓with ASCII[OK]in bothauto-commit.ps1andauto-commit.shTests
disable-model-invocation(positive: value isfalse, negative:truenever appears, non-Claude agents unaffected)[OK]in output, negative: no Unicode✓)All 1433 tests pass.