diff --git a/src/specify_cli/integrations/codex/__init__.py b/src/specify_cli/integrations/codex/__init__.py index 1c24a84bd2..5efdcf877a 100644 --- a/src/specify_cli/integrations/codex/__init__.py +++ b/src/specify_cli/integrations/codex/__init__.py @@ -6,7 +6,22 @@ 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): @@ -54,3 +69,68 @@ def options(cls) -> list[IntegrationOption]: 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 diff --git a/tests/integrations/test_integration_claude.py b/tests/integrations/test_integration_claude.py index 142db0dd92..1498dad44f 100644 --- a/tests/integrations/test_integration_claude.py +++ b/tests/integrations/test_integration_claude.py @@ -487,13 +487,15 @@ def test_non_claude_agents_lack_disable_model_invocation(self, tmp_path): 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 + def test_skills_default_post_process_is_identity(self, tmp_path): + """SkillsIntegration agents without an override leave content unchanged.""" + # ``agy`` is a plain SkillsIntegration with no post-process override, + # so it stands in for the base-class default behavior. + agy = get_integration("agy") + if agy is None: + return # agy not registered in this build content = "---\nname: test\n---\nBody" - assert codex.post_process_skill_content(content) == content + assert agy.post_process_skill_content(content) == content class TestClaudeHookCommandNote: diff --git a/tests/integrations/test_integration_codex.py b/tests/integrations/test_integration_codex.py index cc15d27cb5..415773da0d 100644 --- a/tests/integrations/test_integration_codex.py +++ b/tests/integrations/test_integration_codex.py @@ -1,5 +1,8 @@ """Tests for CodexIntegration.""" +from specify_cli.integrations import get_integration +from specify_cli.integrations.manifest import IntegrationManifest + from .test_integration_base_skills import SkillsIntegrationTests @@ -25,3 +28,89 @@ def test_ai_codex_without_ai_skills_auto_promotes(self, tmp_path): assert result.exit_code == 0, f"init --ai codex failed: {result.output}" assert (target / ".agents" / "skills" / "speckit-plan" / "SKILL.md").exists() + + +class TestCodexHookCommandNote: + """Verify dot-to-hyphen normalization note is injected in hook sections. + + Hook commands in ``extensions.yml`` use dotted ids like + ``speckit.git.commit`` but Codex skills are named with hyphens + (``speckit-git-commit``). Without this note, Codex emits + ``/speckit.git.commit``, which does not resolve. + """ + + 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("codex") + m = IntegrationManifest("codex", tmp_path) + i.setup(tmp_path, m, script_type="sh") + specify_skill = tmp_path / ".agents/skills/speckit-specify/SKILL.md" + assert specify_skill.exists() + content = specify_skill.read_text(encoding="utf-8") + assert "replace dots" in content, ( + "speckit-specify should have dot-to-hyphen hook note" + ) + + def test_hook_note_not_in_skills_without_hooks(self): + """Skills without hook sections should not get the note.""" + from specify_cli.integrations.codex import CodexIntegration + + content = "---\nname: test\ndescription: test\n---\n\nNo hooks here.\n" + result = CodexIntegration._inject_hook_command_note(content) + assert "replace dots" not in result + + def test_hook_note_idempotent(self): + """Injecting the note twice should not duplicate it.""" + from specify_cli.integrations.codex import CodexIntegration + + content = ( + "---\nname: test\n---\n\n" + "- For each executable hook, output the following based on its flag:\n" + ) + once = CodexIntegration._inject_hook_command_note(content) + twice = CodexIntegration._inject_hook_command_note(once) + assert once == twice, "Hook note injection should be idempotent" + + 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 + + content = ( + "---\nname: test\n---\n\n" + " - For each executable hook, output the following\n" + ) + result = CodexIntegration._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_hook_note_when_instruction_is_final_line_without_newline(self): + """Note must not collapse onto the instruction line when the file + ends without a trailing newline and the preceding line is not blank. + """ + from specify_cli.integrations.codex import CodexIntegration + + # No blank line before the instruction and no trailing newline: + # this is the case where the captured ``eol`` is empty and the + # captured indent is also empty, so a missing line separator would + # cause the note and instruction to collapse onto one line. + content = ( + "---\nname: test\n---\n" + "Body line\n" + "- For each executable hook, output the following" + ) + 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 + ) + instruction_line_idx = next( + i for i, l in enumerate(lines) + if l.lstrip().startswith("- For each executable hook") + ) + assert note_line_idx < instruction_line_idx, ( + "Note must appear before the instruction" + ) + assert "For each executable hook" not in lines[note_line_idx], ( + "Note and instruction must not be on the same line" + )