feat(commands): wire before/after hook events into specify and plan templates#1886
feat(commands): wire before/after hook events into specify and plan templates#1886mvanhorn wants to merge 5 commits intogithub:mainfrom
Conversation
…emplates Replicates the hook evaluation pattern from tasks.md and implement.md (introduced in PR github#1702) into the specify and plan command templates. This completes the hook lifecycle across all SDD phases. Changes: - specify.md: Add before_specify/after_specify hook blocks - plan.md: Add before_plan/after_plan hook blocks - EXTENSION-API-REFERENCE.md: Document new hook events - EXTENSION-USER-GUIDE.md: List all available hook events Fixes github#1788 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR completes the hook lifecycle across the SDD workflow by adding before_*/after_* hook evaluation instructions to the specify and plan command templates, and updating extension documentation to include the newly-supported events.
Changes:
- Add
before_specify/after_specifyhook evaluation blocks totemplates/commands/specify.md. - Add
before_plan/after_planhook evaluation blocks totemplates/commands/plan.md. - Update extension docs to list the expanded set of hook events.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| templates/commands/specify.md | Adds pre/post hook-check instructions for the specification phase. |
| templates/commands/plan.md | Adds pre/post hook-check instructions for the planning phase. |
| extensions/EXTENSION-API-REFERENCE.md | Documents the new hook events in the API reference and examples. |
| extensions/EXTENSION-USER-GUIDE.md | Updates the configuration example comments to list available hook events. |
Comments suppressed due to low confidence (2)
templates/commands/specify.md:220
- This block currently instructs skipping hooks with non-empty
condition. The repository’sHookExecutor.check_hooks_for_event()is explicitly designed to return hooks “with condition evaluation applied”, so skipping conditional hooks here meansafter_specifyhooks can never be conditionally enabled. Recommend aligning the template logic withHookExecutor: defaultenabledto true when missing and evaluate (or otherwise handle)conditioninstead of unconditionally skipping.
- Filter to only hooks where `enabled: true`
- 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:
templates/commands/plan.md:85
- This
after_planhook-check block instructs skipping hooks wheneverconditionis non-empty. SinceHookExecutor.check_hooks_for_event()supports condition evaluation, this prevents usingconditionforafter_planhooks in practice. Recommend updating the template to evaluate supported condition expressions (or otherwise not exclude hooks purely because a condition is present), and to treat missingenabledas enabled by default.
- Filter to only hooks where `enabled: true`
- 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.
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback. If not applicable please explain why.
These hook events are defined in the API reference but not yet wired into any core command template. Marking them as planned rather than removing them, since the infrastructure supports them. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Addressed the Copilot feedback: Hook condition skipping (specify.md and plan.md): This pattern is consistent with every other command template in the project - before_commit/after_commit events: Fixed in a8693c7 - marked these as "planned" in the extension docs since they're defined in the API reference but not wired into any core command template yet. |
There was a problem hiding this comment.
Pull request overview
Adds missing hook evaluation points to the early SDD phases so extensions can participate in the full lifecycle (specify → plan → tasks → implement) using consistent “before_” and “after_” hook events.
Changes:
- Added
before_specify/after_specifyhook evaluation blocks to the/speckit.specifycommand template. - Added
before_plan/after_planhook evaluation blocks to the/speckit.plancommand template. - Updated extension documentation to list and reference the newly supported hook events.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| templates/commands/specify.md | Adds pre- and post- execution hook evaluation instructions for specify phase. |
| templates/commands/plan.md | Adds pre- and post- execution hook evaluation instructions for plan phase. |
| extensions/EXTENSION-API-REFERENCE.md | Documents the expanded hook event list and clarifies commit hooks are planned/not wired. |
| extensions/EXTENSION-USER-GUIDE.md | Updates the .specify/extensions.yml example comments to include the new events. |
Comments suppressed due to low confidence (2)
templates/commands/specify.md:220
- Same enablement issue as in the pre-exec hook block: core HookExecutor treats missing
enabledas True (viahook.get('enabled', True)), but these instructions implyenabled: truemust be present. Please adjust the guidance to treat missingenabledas enabled and only filter out hooks withenabled: false.
- If it exists, read it and look for entries under the `hooks.after_specify` key
- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally
- Filter to only hooks where `enabled: true`
- 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:
templates/commands/plan.md:85
- Same enablement mismatch in the after_plan hook block: HookExecutor treats missing
enabledas True, but the template guidance impliesenabled: truemust be present. Please update the instruction to only skip hooks whereenabledis explicitly false (and treat missingenabledas enabled).
- If it exists, read it and look for entries under the `hooks.after_plan` key
- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally
- Filter to only hooks where `enabled: true`
- 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.
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback. Specifically it should be assume it is true by default, and false only when specified.
Matches HookExecutor.get_hooks_for_event() semantics where hooks without an explicit enabled field are treated as enabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Fixed in 01d1bda - hook templates now filter out only hooks with |
|
Addressed in 01d1bda: updated all four hook blocks (before/after specify, before/after plan) to filter out only hooks with Also marked |
There was a problem hiding this comment.
Pull request overview
This PR completes the extension hook lifecycle for the early SDD phases by wiring before_specify/after_specify and before_plan/after_plan hook checks into the /speckit.specify and /speckit.plan command templates, and updates extension documentation to reflect the newly supported events.
Changes:
- Added pre-execution hook blocks for
before_specifyandbefore_plan. - Added post-completion hook blocks for
after_specifyandafter_plan. - Updated extension docs to list the expanded set of hook events.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| templates/commands/specify.md | Adds before/after hook evaluation instructions to the specify command template. |
| templates/commands/plan.md | Adds before/after hook evaluation instructions to the plan command template. |
| extensions/EXTENSION-USER-GUIDE.md | Updates the user guide example to list hook event names. |
| extensions/EXTENSION-API-REFERENCE.md | Expands the documented hook event list and references. |
Comments suppressed due to low confidence (2)
templates/commands/plan.md:85
- Same as the pre-hook block: skipping hooks with a non-empty
conditionmeans conditionalafter_planhooks will never execute, even though HookExecutor supports condition evaluation. Align the template behavior with HookExecutor’s condition handling (or clearly state conditions aren’t supported here).
- 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:
templates/commands/specify.md:33
- The instructions say to skip any hook with a non-empty
condition, but hook conditions are supported byHookExecutor.should_execute_hook()(it evaluates a limited grammar). As written, conditionalbefore_specifyhooks will never run. Consider evaluating conditions using the same rules as HookExecutor (or explicitly documenting that conditions are unsupported here instead of silently 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
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - Check if `.specify/extensions.yml` exists in the project root. | ||
| - If it exists, read it and look for entries under the `hooks.before_specify` key | ||
| - If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally | ||
| - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. |
| - 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 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 |
| - Check if `.specify/extensions.yml` exists in the project root. | ||
| - If it exists, read it and look for entries under the `hooks.before_plan` key | ||
| - If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally | ||
| - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. |
extensions/EXTENSION-USER-GUIDE.md
Outdated
| # before_tasks, after_tasks, before_implement, after_implement, | ||
| # before_commit, after_commit |
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback and align all of the hooks to behave similarly and update the documentation to match. Hooks are enabled by default, unless specified otherwise. Appreciate the hard work!
The yaml config comment listed before_commit/after_commit as "Available events" but they are not yet wired into core templates. Moved them to a separate "Planned" line, consistent with the API reference. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds missing hook lifecycle integration for the specify and plan SDD phases by extending the command templates to check .specify/extensions.yml for before_*/after_* hook events, and updates extension documentation to include the new events.
Changes:
- Add
before_specify/after_specifyhook-evaluation blocks totemplates/commands/specify.md - Add
before_plan/after_planhook-evaluation blocks totemplates/commands/plan.md - Document the new hook events in
extensions/EXTENSION-API-REFERENCE.mdand list them in the.specify/extensions.ymlexample inextensions/EXTENSION-USER-GUIDE.md
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| templates/commands/specify.md | Adds pre- and post-specify hook evaluation instructions in the command template |
| templates/commands/plan.md | Adds pre- and post-plan hook evaluation instructions in the command template |
| extensions/EXTENSION-USER-GUIDE.md | Updates the example config comment to include new hook events |
| extensions/EXTENSION-API-REFERENCE.md | Expands documented hook events and updates examples to reference new events |
Comments suppressed due to low confidence (2)
templates/commands/specify.md:220
- Same as the pre-hook block: skipping hooks with a non-empty
conditionmeans condition-basedafter_specifyhooks will never run, even though HookExecutor supports condition evaluation. Consider evaluating conditions (or delegating to HookExecutor) rather than skipping these hooks outright.
- 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:
templates/commands/plan.md:36
- The template instructs skipping hooks when
conditionis non-empty, which prevents condition-basedbefore_planhooks from ever being surfaced/executed. Since HookExecutor has first-class condition evaluation, consider using that evaluation here instead of skipping conditional hooks.
- 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
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - 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 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 |
| - Check if `.specify/extensions.yml` exists in the project root. | ||
| - If it exists, read it and look for entries under the `hooks.before_specify` key | ||
| - If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally | ||
| - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. |
| - Check if `.specify/extensions.yml` exists in the project root. | ||
| - If it exists, read it and look for entries under the `hooks.before_plan` key | ||
| - If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally | ||
| - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. |
…lates
tasks.md and implement.md previously said "Filter to only hooks where
enabled: true", which would skip hooks that omit the enabled field.
Updated to match specify.md/plan.md and HookExecutor's h.get('enabled', True)
behavior: filter out only hooks where enabled is explicitly false.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Aligned enabled-filtering across all four command templates in 3063fdc. tasks.md and implement.md now use the same wording as specify.md/plan.md: hooks without an On the condition-skipping feedback: the "skip hooks with non-empty condition" pattern is pre-existing in tasks.md and implement.md (predates this PR). Changing condition evaluation semantics across all templates is a larger design decision - happy to open a follow-up issue for that if you'd like. |
There was a problem hiding this comment.
Pull request overview
This PR completes the extension hook lifecycle coverage for the full Spec-Driven Development flow by adding before_* / after_* hook evaluation blocks to the specify and plan command templates, aligning them with the existing hooks support already present in tasks and implement templates.
Changes:
- Added
before_specify/after_specifyhook evaluation blocks totemplates/commands/specify.md. - Added
before_plan/after_planhook evaluation blocks totemplates/commands/plan.md. - Updated extension documentation to list and reference the newly supported hook events, plus clarified “planned” commit hooks.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/commands/specify.md | Adds pre- and post- specification hook discovery/output blocks (before_specify, after_specify). |
| templates/commands/plan.md | Adds pre- and post- planning hook discovery/output blocks (before_plan, after_plan). |
| templates/commands/tasks.md | Aligns enabled filtering text with default-enabled behavior (treat missing enabled as enabled). |
| templates/commands/implement.md | Aligns enabled filtering text with default-enabled behavior (treat missing enabled as enabled). |
| extensions/EXTENSION-API-REFERENCE.md | Documents the expanded standard hook event set and updates hook event examples. |
| extensions/EXTENSION-USER-GUIDE.md | Updates the configuration example comments to include the expanded hook event list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| hooks: # Optional, event hooks | ||
| event_name: # e.g., "after_tasks", "after_implement" | ||
| event_name: # e.g., "after_specify", "after_plan", "after_tasks", "after_implement" |
| # Available events: before_specify, after_specify, before_plan, after_plan, | ||
| # before_tasks, after_tasks, before_implement, after_implement | ||
| # Planned (not yet wired into core templates): before_commit, after_commit | ||
| hooks: | ||
| after_tasks: |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
All hook enablement feedback addressed in 01d1bda and 3063fdc - hooks default to enabled across all four templates (specify, plan, tasks, implement), matching HookExecutor semantics. On condition evaluation: the "skip hooks with non-empty condition" pattern pre-dates this PR and exists in tasks.md/implement.md. The templates delegate condition evaluation to HookExecutor at runtime. Changing this across all templates is a separate design decision - happy to tackle it in a follow-up PR if you'd prefer to keep this PR focused on the hook lifecycle wiring. |
Summary
Completes the hook lifecycle across all SDD phases by adding
before_specify/after_specifyandbefore_plan/after_planhook evaluation to the specify and plan command templates. This follows the pattern established in PR #1702 for tasks and implement templates.Why this matters
Extensions can register hooks for
after_tasksandafter_implement(since PR #1702), but cannot hook into the specify or plan phases (#1788). This gap means extensions that need to run automation after specification or planning (e.g., creating tracking tickets, running validation, triggering notifications) have no supported hook point.Per @dhilipkumars's feedback on #1788: both
before_andafter_hooks are included, and extension documentation is updated.Changes
templates/commands/specify.md: Addedbefore_specify(Pre-Execution Checks) andafter_specify(step 8) hook evaluation blockstemplates/commands/plan.md: Addedbefore_plan(Pre-Execution Checks) andafter_plan(step 5) hook evaluation blocksextensions/EXTENSION-API-REFERENCE.md: Added all new events to the documented hook events list and updated example referencesextensions/EXTENSION-USER-GUIDE.md: Listed all available hook events in the configuration exampleThe hook blocks replicate the exact format from
tasks.mdandimplement.md- same optional/mandatory handling, same condition-skipping logic, same output format.Testing
uv run pytest)ruff check src/passesAI Disclosure
This PR was authored with AI assistance (Claude Code). The hook blocks were replicated from the existing tasks.md/implement.md pattern and verified for consistency.
Fixes #1788