Share skills hook note post-processing#2679
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR centralizes “hook command dot-to-hyphen conversion” guidance by moving hook-note injection into the shared skills integration base, then updates integrations and tests to rely on the shared behavior.
Changes:
- Add shared hook-note injection to the base skills post-processing pipeline.
- Update Claude/Vibe/Copilot integrations to use base post-processing and remove duplicated logic.
- Add/adjust integration tests to verify hook guidance appears only where expected and remains idempotent.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integrations/test_integration_copilot.py | Adds coverage that Copilot skills include shared hook guidance and still inject mode frontmatter. |
| tests/integrations/test_integration_claude.py | Refactors tests to validate shared base behavior and removes Claude-specific injection calls. |
| tests/integrations/test_integration_base_skills.py | Adds base-level test ensuring hook sections include dotted-command conversion guidance. |
| src/specify_cli/integrations/vibe/init.py | Switches to base post-processing instead of bespoke post-generation rewriting. |
| src/specify_cli/integrations/copilot/init.py | Injects hook guidance for Copilot skills (but not via super() yet). |
| src/specify_cli/integrations/claude/init.py | Removes duplicated hook-note injector and delegates shared behavior to base post-processing. |
| src/specify_cli/integrations/base.py | Introduces shared hook-note injection and applies post-processing during skill generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
| if _HOOK_COMMAND_NOTE.rstrip("\n") 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 | ||
| ) |
There was a problem hiding this comment.
Fixed in 8a1f747. The hook-note injection now checks idempotence per matched instruction instead of using a global content guard, so already-noted hook sections do not suppress later missing notes. I also applied the same behavior to Codex’s override and added regression coverage for the shared helper, Claude path, and Codex path. Verification: targeted Codex/Claude repeated-instruction tests passed, the touched integration subset passed (145 tests), full tests/integrations passed (1999 tests), uv run specify init --help ran, and git diff --check passed.
There was a problem hiding this comment.
AI-assisted follow-up pushed in 8a1f747.
The injection guard is now per matched hook instruction, so an existing note only skips that specific instruction when it immediately precedes it. I also narrowed the indentation regex to spaces/tabs so blank lines are not consumed as indentation, and applied the same behavior to the Codex-specific copy.
Verification:
- RED first: focused regression failed before implementation (
1 failed) - Focused review cases:
4 passed - Skills integration slice:
340 passed - Full integration suite:
1999 passed git diff --checkuv run specify init --help
| @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: | ||
| if _HOOK_COMMAND_NOTE.rstrip("\n") in content: | ||
| return content | ||
|
|
Summary
Fixes #2523
Tests
git diff --checkuvx ruff check src/uv run python -m pytest tests/integrations/test_integration_base_skills.py tests/integrations/test_integration_claude.py tests/integrations/test_integration_copilot.py tests/integrations/test_integration_vibe.py tests/integrations/test_integration_codex.py tests/integrations/test_integration_agy.py tests/integrations/test_integration_devin.py tests/integrations/test_integration_cursor_agent.py tests/integrations/test_integration_kimi.py tests/integrations/test_integration_lingma.py tests/integrations/test_integration_trae.py -quv run pytest -quv run specify --helpAI Disclosure