fix: wire after_tasks and after_implement hook events into command templates#1702
Conversation
…mplates (github#1701) The HookExecutor backend in extensions.py was fully implemented but check_hooks_for_event() was never called by anything — the core command templates had no instructions to check .specify/extensions.yml. Add a final step to templates/commands/tasks.md (step 6, after_tasks) and templates/commands/implement.md (step 10, after_implement) that instructs the AI agent to: - Read .specify/extensions.yml if it exists - Filter hooks.{event} to enabled: true entries - Evaluate any condition fields and skip non-matching hooks - Output the RFC-specified hook message format, including EXECUTE_COMMAND: markers for mandatory (optional: false) hooks Bumps version to 0.1.7.
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #1701 by wiring extension hooks into command templates. The HookExecutor class in extensions.py was fully implemented but never invoked - core command templates had no instructions to check .specify/extensions.yml. This meant hooks silently did nothing after command completion. The fix adds step-by-step instructions to tasks.md and implement.md templates to read the extensions config, filter enabled hooks, evaluate conditions, and output the appropriate hook messages (including EXECUTE_COMMAND: markers for mandatory hooks).
Changes:
- Added hook checking instructions to
tasks.md(step 6) andimplement.md(step 10) templates - Version bump from 0.1.6 to 0.1.7 with corresponding changelog entry
- Templates now parse
.specify/extensions.ymland output formatted hook messages for AI agents to execute
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| templates/commands/tasks.md | Added step 6 to check for after_tasks hooks after task generation completes |
| templates/commands/implement.md | Added step 10 to check for after_implement hooks after implementation validation |
| pyproject.toml | Version bump from 0.1.6 to 0.1.7 |
| CHANGELOG.md | Added entry for version 0.1.7 documenting the hook wiring fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…emplates - Replace ambiguous "evaluate any condition value" instruction with explicit guidance to skip hooks with non-empty conditions, deferring evaluation to HookExecutor - Add instruction to skip hook checking silently if extensions.yml cannot be parsed or is invalid
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I have pushed two new commits directly addressing the review comments!
|
* feat(templates): implement before-hooks check as pre-execution phase * test(hooks): create scenario for LLMs/Agents on hooks
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (2)
templates/commands/implement.md:180
- This section also skips hooks with non-empty
condition, which will disable all conditionalafter_implementhooks unless condition evaluation is implemented somewhere else. Align behavior with HookExecutor’s condition support instead of skipping.
- For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions:
- If the hook has no `condition` field, or it is null/empty, treat the hook as executable
- If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation
templates/commands/tasks.md:35
- These instructions skip any hook with a non-empty
condition, but the templates never invokeHookExecutor.check_hooks_for_event(). That means conditional hooks will never execute. Either implement condition evaluation here (matching HookExecutor’s supported patterns) or include conditional hooks in the output so they can still be executed when conditions are met.
- For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions:
- If the hook has no `condition` field, or it is null/empty, treat the hook as executable
- If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation
- For each executable hook, output the following based on its `optional` flag:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@dhilipkumars Thanks for the help! Appreciate it bunches. |
Summary
Fixes #1701.
The HookExecutor class in extensions.py was fully implemented but check_hooks_for_event() was never invoked - the core command templates had no instructions to check .specify/extensions.yml. Hooks silently did nothing after every command run.
Changes
Testing