feat(hooks): tool hook phases — mandatory guards, transforms, dedup and strict output - #4211
Open
dgageot wants to merge 4 commits into
Open
feat(hooks): tool hook phases — mandatory guards, transforms, dedup and strict output#4211dgageot wants to merge 4 commits into
dgageot wants to merge 4 commits into
Conversation
Add pkg/hooks/events with a stdlib-only catalog of all hook event names, their capabilities (blocking, tool-phase, etc.) and allowed on_error values. pkg/hooks/contracts.go formalizes the Phase type, verdict constants and error sentinels used across config, executor and runtime. pkg/hooks/protocol.go captures the JSON wire-format shared between executor and dispatcher.
Deduplicate hook definitions by identity (all fields, including strict_output; nil/empty args and env treated as equivalent), retaining first occurrence. Migrate hook type definitions out of types.go into pkg/config/latest/hooks.go with config-level iteration replacing repeated event lists. Invalid regex patterns, unknown policies and misplaced preempt_yolo are now caught at config-load time rather than at runtime.
… output Introduce a fixed execution order for tool hooks: tool_input_transform runs unconditionally first (rewrites input), tool_guard runs mandatory approval (guards and rules are rechecked after any input rewrite), then normal on_error/approval hooks proceed. Security guards and pre_tool_use hooks fail closed on unexpected outcomes. Parent-context cancellation is now reported as canceled rather than denied. Consistent errors are returned for unexpected exit codes, malformed JSON and invalid verdicts. opt-in strict_output validates that hook scripts only emit recognized JSON event types; stop and post_tool_use hooks are detected as non-consuming contexts and strict mode flags that. Experimental WASM does not yet implement native mandatory tool phases (documented).
Update agent-schema.json with new hook fields (strict_output, guard phase). Revise hooks documentation with phase ordering, compatibility notes (no-op scripts must exit 0, on_error:block rejected on non-blocking events, customized built-ins are distinct from automatic defaults). Add tool_hook_phases.yaml example demonstrating the full phase pipeline.
docker-agent
reviewed
Sep 9, 2026
docker-agent
left a comment
Contributor
There was a problem hiding this comment.
⚠️ Review incomplete
chunk 1: analyzing
chunk 2: Drafter did not complete
Findings so far:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hook execution for tool calls had no guaranteed ordering between transform, guard and approval hooks, deduplication was shallow (missing
strict_outputand ignoring nil-vs-empty equivalence), and error semantics were inconsistent across cancellation, bad exit codes and malformed JSON.This PR introduces a fixed, mandatory execution order:
tool_input_transformalways runs first and may rewrite the tool's input;tool_guardthen runs mandatory approval with guards and rules rechecked against any rewritten input; normal approval hooks follow. Security guards andpre_tool_usehooks fail closed on unexpected outcomes. Parent-context cancellations are now reported as canceled rather than denied. Unexpected exit codes, malformed JSON and invalid verdicts all produce consistent, structured errors.strict_outputis an opt-in per-hook flag that validates hook scripts to only emit recognized JSON event types;stopandpost_tool_usecontexts are non-consuming and strict mode detects that. A shared, stdlib-onlypkg/hooks/eventscatalog drives config validation, execution-strategy selection and result aggregation, replacing scattered repeated event lists. Hook deduplication now covers all fields and treats nil and emptyargs/envas equivalent, retaining the first occurrence. Invalid regex patterns, unknown policies and misplacedpreempt_yoloare caught at config-load time.A few compatibility notes: successful no-op scripts must exit 0 rather than 1;
on_error:blockis rejected at config load for events that cannot block; customized or named built-ins are treated as distinct from the automatic defaults; experimental WASM does not yet implement the native mandatory tool phases (documented). Depends on #4207 (sequential pipeline), already merged.