feat(hooks): implement interceptor hooks system#1253
Merged
Conversation
Add before_exec, after_exec, before_tool, after_tool, on_error hooks alongside the existing on_exit hook. All hooks use the HookAction interceptor pattern (Continue/Cancel). Wired: before_exec (can rewrite/block scripts), after_exec (observe output), on_error (observe errors). Tool hooks defined but wired in follow-up (requires builtin pipeline changes). Builder API: .before_exec(), .after_exec(), .before_tool(), .after_tool(), .on_error() Closes #1235
chaliy
force-pushed
the
feat/issue-1235-hooks-system
branch
from
d7c80ab to
df54cc1
Compare
chaliy
added a commit
that referenced
this pull request
Apr 13, 2026
…ship skills PR #1253 auto-closed #1235 via `Closes #1235` despite explicitly noting that tool hooks and HTTP hooks were deferred as follow-up. This left dead code and missing features hidden behind a closed issue. Add a rule to both skills: only use closing keywords when every task/checkbox in the issue is complete. Use `Part of #N` for partial work.
9 tasks
chaliy
added a commit
that referenced
this pull request
Apr 13, 2026
…1255) ## Summary Completes the two main functional gaps from #1235 that PR #1253 deferred as follow-up: - **Wire `before_tool`/`after_tool`** into `execute_registered_builtin()` — hooks can now modify args, cancel tool invocations, and observe results - **Add `before_http`/`after_http`** — new `HttpRequestEvent`/`HttpResponseEvent` event types, interceptor hooks on `HttpClient` fired around every HTTP request (after allowlist check), builder API via `BashBuilder::before_http()`/`after_http()` - **Add "never close half-done issues" rule** to `process-issues` and `ship` skills to prevent premature issue closure - **Update `specs/009-implementation-status.md`** with hooks/interceptors section ### What changed | File | Change | |------|--------| | `hooks.rs` | Add `HttpRequestEvent`/`HttpResponseEvent` types; remove `#[allow(dead_code)]` | | `interpreter/mod.rs` | Fire `before_tool`/`after_tool` in builtin execution; add `fire_after_tool` helper | | `lib.rs` | Builder fields/methods for HTTP hooks; hook transfer during build; 6 new tests | | `network/client.rs` | HTTP hook storage, setters, firing in `request_with_headers` and `request_with_timeouts` | | Skills | "never close half-done issues" rule in `process-issues` and `ship` | | Spec 009 | Hooks/interceptors status table | ### Security - `before_http` fires **after** the allowlist check — security boundary stays in bashkit - Hooks are registered by the trusted embedder via `BashBuilder` (same trust model as `HttpHandler`) - Cancellation messages don't leak internal state ### What remains from #1235 Part of #1235 — the following items from the original issue are still open: - Runtime registration API (`bash.hooks().on_exit(...)`) - Dedicated hooks spec in `specs/` - Rustdoc guide in `crates/bashkit/docs/` - `examples/hooks.rs` ## Test plan - [x] `test_before_tool_hook_modifies_args` — hook rewrites args, output reflects change - [x] `test_before_tool_hook_cancels` — hook cancels echo, exit code 1 - [x] `test_after_tool_hook_observes_result` — hook captures tool name, stdout, exit code - [x] `test_before_tool_hook_does_not_fire_for_special_builtins` — declare (special) skipped, echo (registered) fires - [x] `test_before_http_hook_cancels_request` — hook blocks curl to blocked.example.com - [x] `test_after_http_hook_observes_response` — hook wires correctly, builder accepts it - [x] 2413 lib tests pass, 123 doctests pass - [x] `cargo fmt --check` clean - [x] `cargo clippy --all-targets --all-features -- -D warnings` clean
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
1 participant
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.
Summary
on_exitto a full interceptor pipelineExecInput,ExecOutput,ToolEvent,ToolResult,ErrorEventbefore_exec,after_exec,before_tool,after_tool,on_errorbefore_exec(can rewrite/block scripts),after_exec(observe output),on_error(observe errors) intoexec()pipelinefire_hooks()helper for DRY hook chain execution.before_exec(),.after_exec(),.before_tool(),.after_tool(),.on_error()Test plan
before_exechook modifies script textbefore_exechook cancels executionafter_exechook captures outputcargo clippycleanCloses #1235