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

Share skills hook note post-processing (#2679)

Browse Source 
* fix(integrations): share skills hook note post-processing

* fix(integrations): tighten skill post-processing

Apply skill content post-processing before the initial write, use an exact hook-note sentinel for idempotence, and route Copilot skill post-processing through the shared helper before adding mode frontmatter.

* Make hook note injection per instruction

* Deduplicate Codex hook note processing

---------

Co-authored-by: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com>
Co-authored-by: Puneet Dixit <puneetdixit200@users.noreply.github.com>
This commit is contained in:
Puneet Dixit
2026-05-28 19:38:48 +05:30
committed by GitHub
parent 5a50b75adb
commit 5a678c552e
9 changed files with 184 additions and 179 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -25,6 +25,12 @@ import yaml
if TYPE_CHECKING:
from .manifest import IntegrationManifest
_HOOK_COMMAND_NOTE = (
"- When constructing slash commands from hook command names, "
"replace dots (`.`) with hyphens (`-`). "
"For example, `speckit.git.commit` → `/speckit-git-commit`.\n"
)
# ---------------------------------------------------------------------------
# IntegrationOption
@@ -1391,15 +1397,53 @@ class SkillsIntegration(IntegrationBase):
invocation = f"{invocation} {args}"
return invocation
@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 individual instructions that already have the note immediately
above them.
"""
note = _HOOK_COMMAND_NOTE.rstrip("\n")
def repl(m: re.Match[str]) -> str:
indent = m.group(1)
instruction = m.group(2)
previous_lines = content[:m.start()].splitlines()
if previous_lines and previous_lines[-1] == indent + note:
return m.group(0)
# ``eol`` is empty when the regex matched via ``$`` because the
# instruction was the final line of a file with no trailing
# newline. Default to ``\n`` so the note never collapses onto
# the same line as the instruction.
eol = m.group(3) or "\n"
return (
indent
+ note
+ eol
+ indent
+ instruction
+ eol
)
return re.sub(
r"(?m)^([ \t]*)(- For each executable hook, output the following[^\r\n]*)(\r\n|\n|$)",
repl,
content,
)
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``.
transformations. The base implementation injects shared skills
guidance for converting dotted hook command names to hyphenated
slash commands. Subclasses may override — see ``ClaudeIntegration``.
"""
return content
return self._inject_hook_command_note(content)
def setup(
self,
@@ -1502,6 +1546,8 @@ class SkillsIntegration(IntegrationBase):
f"{processed_body}"
)
skill_content = self.post_process_skill_content(skill_content)
# Write speckit-<name>/SKILL.md
skill_dir = skills_dir / skill_name
skill_file = skill_dir / "SKILL.md"

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

@@ -5,21 +5,11 @@ 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] = {
@@ -159,41 +149,11 @@ 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 = super().post_process_skill_content(content)
updated = self._inject_frontmatter_flag(updated, "user-invocable")
updated = self._inject_frontmatter_flag(updated, "disable-model-invocation", "false")
updated = self._inject_hook_command_note(updated)
return updated
def setup(
@@ -203,10 +163,9 @@ class ClaudeIntegration(SkillsIntegration):
parsed_options: dict[str, Any] | None = None,
**opts: Any,
) -> list[Path]:
"""Install Claude skills, then inject Claude-specific flags and argument-hints."""
"""Install Claude skills, then inject argument-hints."""
created = super().setup(project_root, manifest, parsed_options, **opts)
# Post-process generated skill files
skills_dir = self.skills_dest(project_root).resolve()
for path in created:
@@ -221,7 +180,7 @@ class ClaudeIntegration(SkillsIntegration):
content_bytes = path.read_bytes()
content = content_bytes.decode("utf-8")
updated = self.post_process_skill_content(content)
updated = content
# Inject argument-hint if available for this skill
skill_dir_name = path.parent.name # e.g. "speckit-plan"

80
src/specify_cli/integrations/codex/__init__.py
View File

@@ -6,22 +6,7 @@ Commands are deprecated; ``--skills`` defaults to ``True``.
from __future__ import annotations
import re
from pathlib import Path
from typing import Any
from ..base import IntegrationOption, SkillsIntegration
from ..manifest import IntegrationManifest
# Note injected into hook sections so Codex maps dot-notation command
# names (from extensions.yml) to the hyphenated skill names it uses.
# Without this, Codex emits ``/speckit.git.commit`` (which does not
# resolve) instead of ``/speckit-git-commit``.
_HOOK_COMMAND_NOTE = (
"- When constructing slash commands from hook command names, "
"replace dots (`.`) with hyphens (`-`). "
"For example, `speckit.git.commit` → `/speckit-git-commit`.\n"
)
class CodexIntegration(SkillsIntegration):
@@ -69,68 +54,3 @@ class CodexIntegration(SkillsIntegration):
help="Install as agent skills (default for Codex)",
),
]
@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`` is empty when the regex matched via ``$`` because the
# instruction was the final line of a file with no trailing
# newline. Default to ``\n`` so the note never collapses onto
# the same line as the instruction.
eol = m.group(3) or "\n"
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 the dot-to-hyphen hook command note."""
return self._inject_hook_command_note(content)
def setup(
self,
project_root: Path,
manifest: IntegrationManifest,
parsed_options: dict[str, Any] | None = None,
**opts: Any,
) -> list[Path]:
"""Install Codex skills, then inject the hook command note."""
created = super().setup(project_root, manifest, parsed_options, **opts)
skills_dir = self.skills_dest(project_root).resolve()
for path in created:
try:
path.resolve().relative_to(skills_dir)
except ValueError:
continue
if path.name != "SKILL.md":
continue
content = path.read_bytes().decode("utf-8")
updated = self.post_process_skill_content(content)
if updated != content:
path.write_bytes(updated.encode("utf-8"))
self.record_file_in_manifest(path, project_root, manifest)
return created

9
src/specify_cli/integrations/copilot/__init__.py
View File

@@ -265,12 +265,13 @@ class CopilotIntegration(IntegrationBase):
return f"speckit.{template_name}.agent.md"
def post_process_skill_content(self, content: str) -> str:
"""Inject Copilot-specific ``mode:`` field into SKILL.md frontmatter.
"""Inject shared hook guidance and Copilot ``mode:`` frontmatter.
Inserts ``mode: speckit.<stem>`` before the closing ``---`` so
Copilot can associate the skill with its agent mode.
"""
lines = content.splitlines(keepends=True)
updated = _CopilotSkillsHelper().post_process_skill_content(content)
lines = updated.splitlines(keepends=True)
# Extract skill name from frontmatter to derive the mode value
dash_count = 0
@@ -284,7 +285,7 @@ class CopilotIntegration(IntegrationBase):
continue
if dash_count == 1:
if stripped.startswith("mode:"):
return content # already present
return updated # already present
if stripped.startswith("name:"):
# Parse: name: "speckit-plan" → speckit.plan
val = stripped.split(":", 1)[1].strip().strip('"').strip("'")
@@ -295,7 +296,7 @@ class CopilotIntegration(IntegrationBase):
skill_name = val
if not skill_name:
return content
return updated
# Inject mode: before the closing --- of frontmatter
out: list[str] = []

31
src/specify_cli/integrations/vibe/__init__.py
View File

@@ -81,13 +81,13 @@ class VibeIntegration(SkillsIntegration):
out.append(line)
return "".join(out)
def post_process_skill_content(self, content: str) -> str:
"""
Inject Vibe-specific frontmatter flags:
Inject shared hook guidance and Vibe-specific frontmatter flags:
- user-invocable: allows the skill to be invoked by the user (not just other agents)
"""
updated = self._inject_frontmatter_flag(content, "user-invocable")
updated = super().post_process_skill_content(content)
updated = self._inject_frontmatter_flag(updated, "user-invocable")
return updated
def setup(
@@ -107,27 +107,4 @@ class VibeIntegration(SkillsIntegration):
err=True,
)
created = super().setup(project_root, manifest, parsed_options=parsed_options, **opts)
# Post-process generated skill files
skills_dir = self.skills_dest(project_root).resolve()
for path in created:
# Only touch SKILL.md files under the skills directory
try:
path.resolve().relative_to(skills_dir)
except ValueError:
continue
if path.name != "SKILL.md":
continue
content_bytes = path.read_bytes()
content = content_bytes.decode("utf-8")
updated = self.post_process_skill_content(content)
if updated != content:
path.write_bytes(updated.encode("utf-8"))
self.record_file_in_manifest(path, project_root, manifest)
return created
return super().setup(project_root, manifest, parsed_options=parsed_options, **opts)

33
tests/integrations/test_integration_base_skills.py
View File

@@ -176,6 +176,39 @@ class SkillsIntegrationTests:
f"skills agents must use /speckit-<name>"
)
def test_hook_sections_explain_dotted_command_conversion(self, tmp_path):
"""Generated skills with hook sections must explain dotted command conversion."""
i = get_integration(self.KEY)
m = IntegrationManifest(self.KEY, tmp_path)
i.setup(tmp_path, m)
specify_skill = i.skills_dest(tmp_path) / "speckit-specify" / "SKILL.md"
assert specify_skill.exists()
content = specify_skill.read_text(encoding="utf-8")
assert "replace dots" in content, (
"speckit-specify should explain dotted hook command conversion"
)
assert content.count("replace dots") == content.count(
"- For each executable hook, output the following"
)
def test_hook_note_injected_for_each_instruction_independently(self):
"""Existing hook notes should not suppress later missing notes."""
content = (
"---\n"
"name: test\n"
"---\n\n"
"- When constructing slash commands from hook command names, "
"replace dots (`.`) with hyphens (`-`). "
"For example, `speckit.git.commit` → `/speckit-git-commit`.\n"
"- For each executable hook, output the following first block:\n"
"\n"
"- For each executable hook, output the following second block:\n"
)
result = SkillsIntegration._inject_hook_command_note(content)
assert result.count("replace dots (`.`) with hyphens") == 2
def test_skill_body_has_content(self, tmp_path):
"""Each SKILL.md body should contain template content after the frontmatter."""
i = get_integration(self.KEY)

49
tests/integrations/test_integration_claude.py
View File

@@ -8,7 +8,7 @@ from unittest.mock import patch
import yaml
from specify_cli.integrations import INTEGRATION_REGISTRY, get_integration
from specify_cli.integrations.base import IntegrationBase
from specify_cli.integrations.base import IntegrationBase, SkillsIntegration
from specify_cli.integrations.claude import ARGUMENT_HINTS
from specify_cli.integrations.manifest import IntegrationManifest
@@ -487,8 +487,8 @@ class TestClaudeDisableModelInvocation:
assert "disable-model-invocation" not in fm
assert "user-invocable" not in fm
def test_skills_default_post_process_is_identity(self, tmp_path):
"""SkillsIntegration agents without an override leave content unchanged."""
def test_skills_default_post_process_preserves_content_without_hooks(self, tmp_path):
"""SkillsIntegration agents without an override preserve non-hook content."""
# ``agy`` is a plain SkillsIntegration with no post-process override,
# so it stands in for the base-class default behavior.
agy = get_integration("agy")
@@ -505,7 +505,7 @@ class TestClaudeHookCommandNote:
"""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")
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")
@@ -516,35 +516,54 @@ class TestClaudeHookCommandNote:
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)
result = SkillsIntegration._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)
once = SkillsIntegration._inject_hook_command_note(content)
twice = SkillsIntegration._inject_hook_command_note(once)
assert once == twice, "Hook note injection should be idempotent"
def test_hook_note_fills_missing_repeated_instructions(self, tmp_path):
"""Already-noted hook sections should not suppress later sections."""
from specify_cli.integrations.base import _HOOK_COMMAND_NOTE
content = (
"---\nname: test\n---\n\n"
f"{_HOOK_COMMAND_NOTE}"
"- For each executable hook, output the following based on its flag:\n"
"\n"
" - For each executable hook, output the following based on its flag:\n"
)
result = SkillsIntegration._inject_hook_command_note(content)
assert result.count("replace dots (`.`) with hyphens") == 2
def test_hook_note_not_suppressed_by_unrelated_phrase(self, tmp_path):
"""Unrelated text should not trip the hook-note idempotence guard."""
content = (
"---\nname: test\n---\n\n"
"This paragraph says replace dots in a different context.\n"
"- For each executable hook, output the following based on its flag:\n"
)
result = SkillsIntegration._inject_hook_command_note(content)
assert "This paragraph says replace dots in a different context." in result
assert result.count("replace dots (`.`) with hyphens") == 1
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)
result = SkillsIntegration._inject_hook_command_note(content)
lines = result.splitlines()
note_line = [l for l in lines if "replace dots" in l][0]
note_line = [line for line in lines if "replace dots" in line][0]
assert note_line.startswith(" "), "Note should preserve indentation"
def test_post_process_injects_all_claude_flags(self):

36
tests/integrations/test_integration_codex.py
View File

@@ -71,6 +71,34 @@ class TestCodexHookCommandNote:
twice = CodexIntegration._inject_hook_command_note(once)
assert once == twice, "Hook note injection should be idempotent"
def test_hook_note_fills_missing_repeated_instructions(self):
"""Already-noted hook sections should not suppress later sections."""
from specify_cli.integrations.base import _HOOK_COMMAND_NOTE
from specify_cli.integrations.codex import CodexIntegration
content = (
"---\nname: test\n---\n\n"
f"{_HOOK_COMMAND_NOTE}"
"- For each executable hook, output the following based on its flag:\n"
"\n"
" - For each executable hook, output the following based on its flag:\n"
)
result = CodexIntegration._inject_hook_command_note(content)
assert result.count("replace dots (`.`) with hyphens") == 2
def test_hook_note_not_suppressed_by_unrelated_phrase(self):
"""Unrelated text should not trip the hook-note idempotence guard."""
from specify_cli.integrations.codex import CodexIntegration
content = (
"---\nname: test\n---\n\n"
"This paragraph says replace dots in a different context.\n"
"- For each executable hook, output the following based on its flag:\n"
)
result = CodexIntegration._inject_hook_command_note(content)
assert "This paragraph says replace dots in a different context." in result
assert result.count("replace dots (`.`) with hyphens") == 1
def test_hook_note_preserves_indentation(self):
"""The injected note should match the indentation of the target line."""
from specify_cli.integrations.codex import CodexIntegration
@@ -81,7 +109,7 @@ class TestCodexHookCommandNote:
)
result = CodexIntegration._inject_hook_command_note(content)
lines = result.splitlines()
note_line = [l for l in lines if "replace dots" in l][0]
note_line = [line for line in lines if "replace dots" in line][0]
assert note_line.startswith(" "), "Note should preserve indentation"
def test_hook_note_when_instruction_is_final_line_without_newline(self):
@@ -102,11 +130,11 @@ class TestCodexHookCommandNote:
result = CodexIntegration._inject_hook_command_note(content)
lines = result.splitlines()
note_line_idx = next(
i for i, l in enumerate(lines) if "replace dots" in l
i for i, line in enumerate(lines) if "replace dots" in line
)
instruction_line_idx = next(
i for i, l in enumerate(lines)
if l.lstrip().startswith("- For each executable hook")
i for i, line in enumerate(lines)
if line.lstrip().startswith("- For each executable hook")
)
assert note_line_idx < instruction_line_idx, (
"Note must appear before the instruction"

24
tests/integrations/test_integration_copilot.py
View File

@@ -404,6 +404,20 @@ class TestCopilotSkillsMode:
updated = copilot.post_process_skill_content(content)
assert "mode: speckit.plan" in updated
def test_post_process_skill_content_injects_hook_note(self):
"""post_process_skill_content() should inject shared hook guidance."""
copilot = self._make_copilot()
content = (
"---\n"
'name: "speckit-specify"\n'
'description: "Specify workflow"\n'
"---\n"
"\n- For each executable hook, output the following\n"
)
updated = copilot.post_process_skill_content(content)
assert "replace dots" in updated
assert "mode: speckit.specify" in updated
def test_post_process_idempotent(self):
"""post_process_skill_content() must be idempotent."""
copilot = self._make_copilot()
@@ -434,6 +448,14 @@ class TestCopilotSkillsMode:
stem = skill_dir_name.removeprefix("speckit-")
assert fm["mode"] == f"speckit.{stem}"
def test_skills_hook_sections_explain_dotted_command_conversion(self, tmp_path):
"""Generated skills with hook sections should include shared hook guidance."""
copilot = self._make_copilot()
self._setup_skills(copilot, tmp_path)
specify_skill = tmp_path / ".github" / "skills" / "speckit-specify" / "SKILL.md"
content = specify_skill.read_text(encoding="utf-8")
assert "replace dots" in content
# -- Template processing ----------------------------------------------
def test_skills_templates_are_processed(self, tmp_path):
@@ -724,4 +746,4 @@ class TestCopilotSkillsMode:
# Must NOT show the dotted /speckit.plan form
assert "/speckit.plan" not in result.output, (
f"Should not show /speckit.plan in skills mode:\n{result.output}"
)
)