feat(plugins): run pre_tool/post_tool hooks on the Direct-API runtime#417
Conversation
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds Direct-API plugin tool hooks to generic-edit execution, including canonical action translation, capability-gated plugin loading, pre-execution blocking, post-execution annotations, documentation, and unit/integration coverage. ChangesDirect-API plugin tool hooks
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GenericEditRuntimeSession
participant PluginToolHook
participant NativeToolExecutor
GenericEditRuntimeSession->>PluginToolHook: pre_tool(canonical action)
PluginToolHook-->>GenericEditRuntimeSession: allow or block
GenericEditRuntimeSession->>NativeToolExecutor: execute allowed action
GenericEditRuntimeSession->>PluginToolHook: post_tool(result)
PluginToolHook-->>GenericEditRuntimeSession: annotation decision
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
OBenner
force-pushed
the
claude/hopeful-ishizaka-d23950
branch
from
a80f0c0 to
21e19a6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/agents/runtime/adapters/generic_edit.py`:
- Around line 73-100: Expose a public tool-hook predicate in plugins.runtime
that delegates to the existing private _can_use_tool_hooks behavior, then update
_load_tool_hook_plugins in generic_edit.py to import and call the public
predicate instead. Preserve the current plugin filtering and error handling.
- Around line 727-750: Update resume() to call
_prepare_plugin_hook_context(spec_dir, subtask_id) before invoking
_run_json_action_loop, ensuring resumed runs initialize _plugin_hook_context and
enable both plugin hook callbacks. Preserve the existing context preparation
behavior and argument values used by the resume flow.
In `@apps/backend/agents/runtime/plugin_tool_bridge.py`:
- Around line 189-212: The _git builder currently discards action parameters and
always emits a fixed command. Update _git and the git_status/git_diff entries in
_BUILDERS to preserve supported scope and output options from the action,
including path, include_untracked, cached, stat, and max_chars, by passing them
through or constructing the complete Bash command; keep unrelated local-action
builders unchanged.
In `@tests/test_generic_edit_plugin_hooks.py`:
- Around line 183-202: Add a parallel test in the hook test suite covering the
resume() path after it invokes _prepare_plugin_hook_context: configure a
blocking plugin through the normal resume setup, execute an action via
_execute_action, and assert the plugin blocks the action. Preserve the existing
run()-based tests and verify resume() provides equivalent hook behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ba3685ab-cf50-4b27-baf7-be4408d2ca63
📒 Files selected for processing (5)
apps/backend/agents/runtime/adapters/generic_edit.pyapps/backend/agents/runtime/plugin_tool_bridge.pydocs/guides/plugin-development.mdtests/test_generic_edit_plugin_hooks.pytests/test_plugin_tool_bridge.py
| async def test_execute_action_runs_post_tool_on_success(tmp_path): | ||
| session = _session(tmp_path) | ||
| _wire(session, tmp_path, post_predicate=lambda _n, _i, _r: True) | ||
| target = tmp_path / "post_written.py" | ||
|
|
||
| result = await session._execute_action( | ||
| {"tool": "write_file", "path": "post_written.py", "content": "x = 1"}, | ||
| loop="main", | ||
| iteration=0, | ||
| action_index=0, | ||
| spec_dir=tmp_path, | ||
| verbose=False, | ||
| phase=None, | ||
| subtask_id=None, | ||
| ) | ||
|
|
||
| # The write succeeded AND post_tool observed it (mutation not rolled back). | ||
| assert result.ok is True | ||
| assert target.exists() | ||
| assert "plugin_post_tool_block" in result.data |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add coverage for the resume() path once wired.
All hook tests here go through run()-style wiring (_wire() injects _tool_hook_plugins/_plugin_hook_context directly). Once resume() is updated to call _prepare_plugin_hook_context (see generic_edit.py comment), add a parallel test exercising resume() → _execute_action with a blocking plugin to lock in parity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_generic_edit_plugin_hooks.py` around lines 183 - 202, Add a
parallel test in the hook test suite covering the resume() path after it invokes
_prepare_plugin_hook_context: configure a blocking plugin through the normal
resume setup, execute an action via _execute_action, and assert the plugin
blocks the action. Preserve the existing run()-based tests and verify resume()
provides equivalent hook behavior.
Plugin tool hooks (pre_tool/post_tool) previously ran only on the Claude SDK backend, because the plugin runtime was built around the SDK's HookMatcher type and wired solely into create_client(). On the Direct-API in-process runtime (generic_edit / full_agent_runtime), tool calls were dispatched by LocalActionExecutor with no hook invocation — so a GENERIC_EDIT plugin's guard (e.g. skill-pack-runtime's skill-script permission guard) was silently bypassed on those backends. Wire the hooks into the Direct-API runtime: - plugin_tool_bridge: a pure, one-directional normalization layer mapping the native local-action vocabulary (run_command, write_file, ...) to the canonical Claude SDK tool vocabulary (Bash, Write, Edit, ...) so hooks written against SDK names match on both backends. Actions with no SDK equivalent get stable extension names (Delete/Move/ApplyPatch). Read-only actions are mapped but gated off by default. - generic_edit: load hook-capable plugins once per session, build the AgentContext per run, and invoke pre_tool (block before execution) and post_tool (observational) at the single _execute_action choke point, covering the executor and MCP-bridge paths. Documented limitations (by design): pre_tool is block-only (no input rewrite), post_tool is observational (no rollback), read-only hooking is opt-in, and Codex/CLI backends remain out of reach (opaque tool loop). augment_prompt cross-backend is intentionally not part of this change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
OBenner
force-pushed
the
claude/hopeful-ishizaka-d23950
branch
from
21e19a6 to
58de65b
Compare
|
…ocs (#424) Two follow-ups to the Direct-API augment_prompt work (#421): - The follow-up planner (planner.run_followup_planner) builds its own Direct-API session with a separate builder that #421 did not cover, so plugin prompt contributions never reached it. Apply the same augmentation to its message. - Centralize the guarded helper: move the coder-local _augment_direct_api_prompt into plugins.runtime as the public augment_direct_api_prompt, and have both coder.py and planner.py call it (planner no longer needs coder-private state). - Reconcile plugin-development.md: #417 landed a note stating augment_prompt is "Claude SDK only", which #421 made false. Flip the matrix row to cover Direct-API and Codex (delivered via the agent message) and rewrite the note. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Problem
Plugin tool hooks (
pre_tool/post_tool) ran only on the Claude SDK backend. The plugin runtime was built around the SDK'sHookMatchertype and wired solely intocreate_client(). On the Direct-API in-process runtime (generic_edit/full_agent_runtime), tool calls were dispatched byLocalActionExecutorwith no hook invocation.Consequence: a
GENERIC_EDITplugin's guard was silently bypassed on Direct-API. Concretely,skill-pack-runtime's skill-script permission guard matchestool_name.casefold() in {"bash","shell"}— the nativerun_commandname never matched, so skill scripts ran unguarded exactly in thegeneric_editmode where they execute.Change
Wire the hooks into the Direct-API runtime, mirroring the SDK path:
plugin_tool_bridge.py(new) — a pure, one-directional normalization layer mapping the native local-action vocabulary (run_command,write_file,replace_text, ...) to the canonical Claude SDK tool vocabulary (Bash,Write,Edit, ...), so hooks written against SDK names match on both backends. Actions with no SDK equivalent get stable extension names (Delete/Move/ApplyPatch). Read-only actions are mapped but gated off by default.generic_edit.py— load hook-capable plugins once per session, build theAgentContextper run, and invokepre_tool(block before execution) +post_tool(observational) at the single_execute_actionchoke point, covering both the executor and MCP-bridge paths.plugin-development.md— documents the backend-coverage matrix, the normalization contract, and the limitations below.Backend coverage after this change
pre_tool(block)post_tool(observe)before_session…)Limitations (by design, documented)
pre_toolis block-only on Direct-API — normalization is one-directional, so input rewrite is not honored.post_toolis observational — the action already ran; a block is logged + annotated (plugin_post_tool_block), not rolled back.augment_promptcross-backend is intentionally not part of this PR (separate follow-up).Tests
39 new tests + full regression pass — 310 passing (
test_plugin_tool_bridge,test_generic_edit_plugin_hooks,test_agent_runtime,test_plugin_runtime,test_skill_pack_runtime,test_plugin_integration,test_plugin_control_plane). The anchor test proves both the bug and the fix: the nativerun_commandname bypasses the guard, and the normalizedBashname blocks.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests