github/github-spec-kit
1
0
Fork 0
mirror of https://github.com/github/spec-kit.git synced 2026-07-03 12:28:06 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: allow Claude to chain skills for hook execution (#2227)

Browse Source 
* fix: allow Claude to chain skills for hook execution (#2178)

- 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 #2178

* refactor: address PR review feedback

- 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.

* fix: match full hook instruction line in regex

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.

* refactor: use integration object directly for post_process_skill_content

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.
This commit is contained in:
Manfred Riem
2026-04-15 14:35:05 -05:00
committed by GitHub
parent b78a3cdd88
commit 8fc2bd3489
11 changed files with 257 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
extensions/git/scripts/bash/auto-commit.sh
View File

@@ -137,4 +137,4 @@ fi
_git_out=$(git add . 2>&1) || { echo "[specify] Error: git add failed: $_git_out" >&2; exit 1; }
_git_out=$(git commit -q -m "$_commit_msg" 2>&1) || { echo "[specify] Error: git commit failed: $_git_out" >&2; exit 1; }
echo "✓ Changes committed ${_phase} ${_command_name}" >&2
echo "[OK] Changes committed ${_phase} ${_command_name}" >&2

2
extensions/git/scripts/powershell/auto-commit.ps1
View File

@@ -146,4 +146,4 @@ try {
exit 1
}
Write-Host "✓ Changes committed $phase $commandName"
Write-Host "[OK] Changes committed $phase $commandName"

5
src/specify_cli/agents.py
View File

@@ -317,11 +317,6 @@ class CommandRegistrar:
"source": source,
},
}
if agent_name == "claude":
# Claude skills should be user-invocable (accessible via /command)
# and only run when explicitly invoked (not auto-triggered by the model).
skill_frontmatter["user-invocable"] = True
skill_frontmatter["disable-model-invocation"] = True
return skill_frontmatter
@staticmethod

6
src/specify_cli/extensions.py
View File

@@ -850,6 +850,7 @@ class ExtensionManager:
from . import load_init_options
from .agents import CommandRegistrar
from .integrations import get_integration
import yaml
written: List[str] = []
@@ -860,6 +861,7 @@ class ExtensionManager:
if not isinstance(selected_ai, str) or not selected_ai:
return []
registrar = CommandRegistrar()
integration = get_integration(selected_ai)
for cmd_info in manifest.commands:
cmd_name = cmd_info["name"]
@@ -939,6 +941,10 @@ class ExtensionManager:
f"# {title_name} Skill\n\n"
f"{body}\n"
)
if integration is not None and hasattr(integration, "post_process_skill_content"):
skill_content = integration.post_process_skill_content(
skill_content
)
skill_file.write_text(skill_content, encoding="utf-8")
written.append(skill_name)

10
src/specify_cli/integrations/base.py
View File

@@ -1102,6 +1102,16 @@ class SkillsIntegration(IntegrationBase):
invocation = f"{invocation} {args}"
return invocation
def post_process_skill_content(self, content: str) -> str:
"""Post-process a SKILL.md file's content after generation.
Called by external skill generators (presets, extensions) to let
the integration inject agent-specific frontmatter or body
transformations. The default implementation returns *content*
unchanged. Subclasses may override — see ``ClaudeIntegration``.
"""
return content
def setup(
self,
project_root: Path,

55
src/specify_cli/integrations/claude/__init__.py
View File

@@ -5,11 +5,21 @@ from __future__ import annotations
from pathlib import Path
from typing import Any
import re
import yaml
from ..base import SkillsIntegration
from ..manifest import IntegrationManifest
# Note injected into hook sections so Claude maps dot-notation command
# names (from extensions.yml) to the hyphenated skill names it uses.
_HOOK_COMMAND_NOTE = (
"- When constructing slash commands from hook command names, "
"replace dots (`.`) with hyphens (`-`). "
"For example, `speckit.git.commit` → `/speckit-git-commit`.\n"
)
# Mapping of command template stem → argument-hint text shown inline
# when a user invokes the slash command in Claude Code.
ARGUMENT_HINTS: dict[str, str] = {
@@ -148,6 +158,43 @@ class ClaudeIntegration(SkillsIntegration):
out.append(line)
return "".join(out)
@staticmethod
def _inject_hook_command_note(content: str) -> str:
"""Insert a dot-to-hyphen note before each hook output instruction.
Targets the line ``- For each executable hook, output the following``
and inserts the note on the line before it, matching its indentation.
Skips if the note is already present.
"""
if "replace dots" in content:
return content
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]*)(\r\n|\n|$)",
repl,
content,
)
def post_process_skill_content(self, content: str) -> str:
"""Inject Claude-specific frontmatter flags and hook notes."""
updated = self._inject_frontmatter_flag(content, "user-invocable")
updated = self._inject_frontmatter_flag(updated, "disable-model-invocation", "false")
updated = self._inject_hook_command_note(updated)
return updated
def setup(
self,
project_root: Path,
@@ -155,7 +202,7 @@ class ClaudeIntegration(SkillsIntegration):
parsed_options: dict[str, Any] | None = None,
**opts: Any,
) -> list[Path]:
"""Install Claude skills, then inject user-invocable, disable-model-invocation, and argument-hint."""
"""Install Claude skills, then inject Claude-specific flags and argument-hints."""
created = super().setup(project_root, manifest, parsed_options, **opts)
# Post-process generated skill files
@@ -173,11 +220,7 @@ class ClaudeIntegration(SkillsIntegration):
content_bytes = path.read_bytes()
content = content_bytes.decode("utf-8")
# Inject user-invocable: true (Claude skills are accessible via /command)
updated = self._inject_frontmatter_flag(content, "user-invocable")
# Inject disable-model-invocation: true (Claude skills run only when invoked)
updated = self._inject_frontmatter_flag(updated, "disable-model-invocation")
updated = self.post_process_skill_content(content)
# Inject argument-hint if available for this skill
skill_dir_name = path.parent.name # e.g. "speckit-plan"

16
src/specify_cli/presets.py
View File

@@ -707,6 +707,7 @@ class PresetManager:
from . import SKILL_DESCRIPTIONS, load_init_options
from .agents import CommandRegistrar
from .integrations import get_integration
init_opts = load_init_options(self.project_root)
if not isinstance(init_opts, dict):
@@ -716,6 +717,7 @@ class PresetManager:
return []
ai_skills_enabled = bool(init_opts.get("ai_skills"))
registrar = CommandRegistrar()
integration = get_integration(selected_ai)
agent_config = registrar.AGENT_CONFIGS.get(selected_ai, {})
# Native skill agents (e.g. codex/kimi/agy/trae) materialize brand-new
# preset skills in _register_commands() because their detected agent
@@ -789,6 +791,10 @@ class PresetManager:
f"# Speckit {skill_title} Skill\n\n"
f"{body}\n"
)
if integration is not None and hasattr(integration, "post_process_skill_content"):
skill_content = integration.post_process_skill_content(
skill_content
)
skill_file = skill_subdir / "SKILL.md"
skill_file.write_text(skill_content, encoding="utf-8")
@@ -816,6 +822,7 @@ class PresetManager:
from . import SKILL_DESCRIPTIONS, load_init_options
from .agents import CommandRegistrar
from .integrations import get_integration
# Locate core command templates from the project's installed templates
core_templates_dir = self.project_root / ".specify" / "templates" / "commands"
@@ -824,6 +831,7 @@ class PresetManager:
init_opts = {}
selected_ai = init_opts.get("ai")
registrar = CommandRegistrar()
integration = get_integration(selected_ai) if isinstance(selected_ai, str) else None
extension_restore_index = self._build_extension_skill_restore_index()
for skill_name in skill_names:
@@ -877,6 +885,10 @@ class PresetManager:
f"# Speckit {skill_title} Skill\n\n"
f"{body}\n"
)
if integration is not None and hasattr(integration, "post_process_skill_content"):
skill_content = integration.post_process_skill_content(
skill_content
)
skill_file.write_text(skill_content, encoding="utf-8")
continue
@@ -906,6 +918,10 @@ class PresetManager:
f"# {title_name} Skill\n\n"
f"{body}\n"
)
if integration is not None and hasattr(integration, "post_process_skill_content"):
skill_content = integration.post_process_skill_content(
skill_content
)
skill_file.write_text(skill_content, encoding="utf-8")
else:
# No core or extension template — remove the skill entirely

56
tests/extensions/git/test_git_extension.py
View File

@@ -491,6 +491,34 @@ class TestAutoCommitBash:
result = _run_bash("auto-commit.sh", project)
assert result.returncode != 0
def test_success_message_uses_ok_prefix(self, tmp_path: Path):
"""auto-commit.sh success message uses [OK] (not Unicode)."""
project = _setup_project(tmp_path)
_write_config(project, (
"auto_commit:\n"
" default: false\n"
" after_specify:\n"
" enabled: true\n"
))
(project / "new-file.txt").write_text("content")
result = _run_bash("auto-commit.sh", project, "after_specify")
assert result.returncode == 0
assert "[OK] Changes committed" in result.stderr
def test_success_message_no_unicode_checkmark(self, tmp_path: Path):
"""auto-commit.sh must not use Unicode checkmark in output."""
project = _setup_project(tmp_path)
_write_config(project, (
"auto_commit:\n"
" default: false\n"
" after_plan:\n"
" enabled: true\n"
))
(project / "new-file.txt").write_text("content")
result = _run_bash("auto-commit.sh", project, "after_plan")
assert result.returncode == 0
assert "\u2713" not in result.stderr, "Must not use Unicode checkmark"
@pytest.mark.skipif(not HAS_PWSH, reason="pwsh not available")
class TestAutoCommitPowerShell:
@@ -523,6 +551,34 @@ class TestAutoCommitPowerShell:
)
assert "ps commit" in log.stdout
def test_success_message_uses_ok_prefix(self, tmp_path: Path):
"""auto-commit.ps1 success message uses [OK] (not Unicode)."""
project = _setup_project(tmp_path)
_write_config(project, (
"auto_commit:\n"
" default: false\n"
" after_specify:\n"
" enabled: true\n"
))
(project / "new-file.txt").write_text("content")
result = _run_pwsh("auto-commit.ps1", project, "after_specify")
assert result.returncode == 0
assert "[OK] Changes committed" in result.stdout
def test_success_message_no_unicode_checkmark(self, tmp_path: Path):
"""auto-commit.ps1 must not use Unicode checkmark in output."""
project = _setup_project(tmp_path)
_write_config(project, (
"auto_commit:\n"
" default: false\n"
" after_plan:\n"
" enabled: true\n"
))
(project / "new-file.txt").write_text("content")
result = _run_pwsh("auto-commit.ps1", project, "after_plan")
assert result.returncode == 0
assert "\u2713" not in result.stdout, "Must not use Unicode checkmark"
# ── git-common.sh Tests ──────────────────────────────────────────────────────

118
tests/integrations/test_integration_claude.py
View File

@@ -59,7 +59,7 @@ class TestClaudeIntegration:
parsed = yaml.safe_load(parts[1])
assert parsed["name"] == "speckit-plan"
assert parsed["user-invocable"] is True
assert parsed["disable-model-invocation"] is True
assert parsed["disable-model-invocation"] is False
assert parsed["metadata"]["source"] == "templates/commands/plan.md"
def test_setup_installs_update_context_scripts(self, tmp_path):
@@ -179,7 +179,7 @@ class TestClaudeIntegration:
assert skill_file.exists()
skill_content = skill_file.read_text(encoding="utf-8")
assert "user-invocable: true" in skill_content
assert "disable-model-invocation: true" in skill_content
assert "disable-model-invocation: false" in skill_content
init_options = json.loads(
(project / ".specify" / "init-options.json").read_text(encoding="utf-8")
@@ -280,7 +280,7 @@ class TestClaudeIntegration:
assert "preset:claude-skill-command" in content
assert "name: speckit-research" in content
assert "user-invocable: true" in content
assert "disable-model-invocation: true" in content
assert "disable-model-invocation: false" in content
metadata = manager.registry.get("claude-skill-command")
assert "speckit-research" in metadata.get("registered_skills", [])
@@ -400,3 +400,115 @@ class TestClaudeArgumentHints:
lines = result.splitlines()
hint_count = sum(1 for ln in lines if ln.startswith("argument-hint:"))
assert hint_count == 1
class TestClaudeDisableModelInvocation:
"""Verify disable-model-invocation is false for Claude skills."""
def test_setup_sets_disable_model_invocation_false(self, tmp_path):
"""Generated SKILL.md files must have disable-model-invocation: false."""
i = get_integration("claude")
m = IntegrationManifest("claude", tmp_path)
created = i.setup(tmp_path, m, script_type="sh")
skill_files = [f for f in created if f.name == "SKILL.md"]
assert len(skill_files) > 0
for f in skill_files:
content = f.read_text(encoding="utf-8")
parts = content.split("---", 2)
parsed = yaml.safe_load(parts[1])
assert parsed["disable-model-invocation"] is False, (
f"{f.parent.name}: expected disable-model-invocation: false"
)
def test_disable_model_invocation_not_true(self, tmp_path):
"""No Claude skill should have disable-model-invocation: true."""
i = get_integration("claude")
m = IntegrationManifest("claude", tmp_path)
created = i.setup(tmp_path, m, script_type="sh")
for f in created:
if f.name != "SKILL.md":
continue
content = f.read_text(encoding="utf-8")
assert "disable-model-invocation: true" not in content, (
f"{f.parent.name}: must not have disable-model-invocation: true"
)
def test_non_claude_agents_lack_disable_model_invocation(self, tmp_path):
"""Non-Claude skill agents should not get disable-model-invocation."""
from specify_cli.agents import CommandRegistrar
fm = CommandRegistrar.build_skill_frontmatter(
"codex", "speckit-plan", "desc", "templates/commands/plan.md"
)
assert "disable-model-invocation" not in fm
assert "user-invocable" not in fm
def test_non_claude_post_process_is_identity(self, tmp_path):
"""Non-Claude integrations should not modify skill content."""
codex = get_integration("codex")
if codex is None:
return # codex not registered in this build
content = "---\nname: test\n---\nBody"
assert codex.post_process_skill_content(content) == content
class TestClaudeHookCommandNote:
"""Verify dot-to-hyphen normalization note is injected in hook sections."""
def test_hook_note_injected_in_skills_with_hooks(self, tmp_path):
"""Skills that have hook sections should get the normalization note."""
i = get_integration("claude")
m = IntegrationManifest("claude", tmp_path)
created = i.setup(tmp_path, m, script_type="sh")
specify_skill = tmp_path / ".claude/skills/speckit-specify/SKILL.md"
assert specify_skill.exists()
content = specify_skill.read_text(encoding="utf-8")
# specify.md has hook sections
assert "replace dots" in content, (
"speckit-specify should have dot-to-hyphen hook note"
)
def test_hook_note_not_in_skills_without_hooks(self, tmp_path):
"""Skills without hook sections should not get the note."""
from specify_cli.integrations.claude import ClaudeIntegration
content = "---\nname: test\ndescription: test\n---\n\nNo hooks here.\n"
result = ClaudeIntegration._inject_hook_command_note(content)
assert "replace dots" not in result
def test_hook_note_idempotent(self, tmp_path):
"""Injecting the note twice should not duplicate it."""
from specify_cli.integrations.claude import ClaudeIntegration
content = (
"---\nname: test\n---\n\n"
"- For each executable hook, output the following based on its flag:\n"
)
once = ClaudeIntegration._inject_hook_command_note(content)
twice = ClaudeIntegration._inject_hook_command_note(once)
assert once == twice, "Hook note injection should be idempotent"
def test_hook_note_preserves_indentation(self, tmp_path):
"""The injected note should match the indentation of the target line."""
from specify_cli.integrations.claude import ClaudeIntegration
content = (
"---\nname: test\n---\n\n"
" - For each executable hook, output the following\n"
)
result = ClaudeIntegration._inject_hook_command_note(content)
lines = result.splitlines()
note_line = [l for l in lines if "replace dots" in l][0]
assert note_line.startswith(" "), "Note should preserve indentation"
def test_post_process_injects_all_claude_flags(self):
"""post_process_skill_content should inject all Claude-specific fields."""
i = get_integration("claude")
content = (
"---\nname: test\ndescription: test\n---\n\n"
"- For each executable hook, output the following\n"
)
result = i.post_process_skill_content(content)
assert "user-invocable: true" in result
assert "disable-model-invocation: false" in result
assert "replace dots" in result

2
tests/test_extension_skills.py
View File

@@ -269,7 +269,7 @@ class TestExtensionSkillRegistration:
assert isinstance(parsed, dict)
assert parsed["name"] == "speckit-test-ext-hello"
assert "description" in parsed
assert parsed["disable-model-invocation"] is True
assert parsed["disable-model-invocation"] is False
def test_no_skills_when_ai_skills_disabled(self, no_skills_project, extension_dir):
"""No skills should be created when ai_skills is false."""

4
tests/test_presets.py
View File

@@ -1975,7 +1975,7 @@ class TestPresetSkills:
assert skill_file.exists()
content = skill_file.read_text()
assert "preset:self-test" in content, "Skill should reference preset source"
assert "disable-model-invocation: true" in content
assert "disable-model-invocation: false" in content
# Verify it was recorded in registry
metadata = manager.registry.get("self-test")
@@ -2057,7 +2057,7 @@ class TestPresetSkills:
content = skill_file.read_text()
assert "preset:self-test" not in content, "Preset content should be gone"
assert "templates/commands/specify.md" in content, "Should reference core template"
assert "disable-model-invocation: true" in content
assert "disable-model-invocation: false" in content
def test_skill_restored_on_remove_resolves_script_placeholders(self, project_dir):
"""Core restore should resolve {SCRIPT}/{ARGS} placeholders like other skill paths."""