fix(codex): inject dot-to-hyphen hook command note in Codex skills (#2503)

* fix(codex): inject dot-to-hyphen hook command note in Codex skills

Hook commands in `.specify/extensions.yml` use dotted ids like
`speckit.git.commit`, but Codex skills are named with hyphens
(`speckit-git-commit`). The Claude integration handles this via an
explicit instruction injected into each generated SKILL.md by
`ClaudeIntegration.post_process_skill_content`, but the Codex
integration had no such override, so Codex would emit
`/speckit.git.commit` (which does not resolve) instead of
`/speckit-git-commit`.

This adds the same `_inject_hook_command_note` helper and a
`post_process_skill_content` override to `CodexIntegration`, plus a
small `setup()` override that applies the post-process to each
generated SKILL.md (mirroring the pattern in `ClaudeIntegration`).

Also widens the existing
`test_non_claude_post_process_is_identity` test to use `agy`
(another `SkillsIntegration` with no override), since asserting
identity behavior on Codex would now incorrectly fail.

Tests:
- New `TestCodexHookCommandNote` class mirrors
  `TestClaudeHookCommandNote`: setup-level injection, no-op when
  no hook block is present, idempotency, and indentation
  preservation.
- `pytest tests/` → 2866 passed, 34 skipped.

Signed-off-by: Chao Zhang <1175468+picklebento@users.noreply.github.com>

* fix(codex): handle empty eol when instruction is final line without newline

The hook-note injection regex allowed end-of-string matches via ``$``,
which left the captured ``eol`` empty. When the matched indent was also
empty, the substitution concatenated the note onto the same line as the
instruction. Default ``eol`` to ``\n`` when the capture is empty.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Signed-off-by: Chao Zhang <1175468+picklebento@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Chao Z
2026-05-20 07:04:47 -07:00
committed by GitHub
parent 68a031c768
commit 9735145289
3 changed files with 177 additions and 6 deletions

View File

@@ -6,7 +6,22 @@ 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):
@@ -54,3 +69,68 @@ 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

View File

@@ -487,13 +487,15 @@ class TestClaudeDisableModelInvocation:
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:

View File

@@ -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 @@ class TestCodexAutoPromote:
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"
)