feat: wire remaining interception points and document the catalog#6518
feat: wire remaining interception points and document the catalog#6518lucasgomide wants to merge 2 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 5 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e2f88fc. Configure here.
| ) | ||
| dispatch(InterceptionPoint.TOOL_SELECTION, selection_ctx) | ||
|
|
||
| return selection_ctx.payload |
There was a problem hiding this comment.
Tool selection fires twice
High Severity
TOOL_SELECTION is dispatched in both crew._prepare_tools and agent.create_agent_executor. On a normal crew run, tools prepared in the crew are passed into the agent, which fires the same point again, so hooks run twice and additive changes can duplicate tools.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e2f88fc. Configure here.
| payload=dumped_params, | ||
| ) | ||
| dispatch(InterceptionPoint.PRE_STEP, pre_step_ctx) | ||
|
|
There was a problem hiding this comment.
Flow pre-step ignores payload
Medium Severity
Flow PRE_STEP builds a context with payload=dumped_params and dispatches, but never applies pre_step_ctx.payload back onto the args/kwargs used to invoke the method. Unlike task PRE_STEP and flow POST_STEP, replacements and non-shared mutations have no effect.
Reviewed by Cursor Bugbot for commit e2f88fc. Configure here.
| payload=self.definition.code, | ||
| ) | ||
| dispatch(InterceptionPoint.PRE_CODE_EXECUTION, code_ctx) | ||
|
|
There was a problem hiding this comment.
Code execution hook is inert
High Severity
PRE_CODE_EXECUTION dispatches with the script string as payload, but run always executes the handler compiled at init from self.definition.code. Payload replacements and ctx.code edits never change what runs; only abort works.
Reviewed by Cursor Bugbot for commit e2f88fc. Configure here.
| payload=self.transport, | ||
| ) | ||
| dispatch(InterceptionPoint.MCP_CONNECT, connect_ctx) | ||
|
|
There was a problem hiding this comment.
MCP connect ignores payload
Medium Severity
MCP_CONNECT puts self.transport on the context as payload/server_params, but after dispatch the client still connects with the original self.transport and never assigns connect_ctx.payload back. Returned connection-param replacements are ignored.
Reviewed by Cursor Bugbot for commit e2f88fc. Configure here.
| ) | ||
| dispatch(InterceptionPoint.PRE_DELEGATION, delegation_ctx) | ||
| task = delegation_ctx.payload | ||
|
|
There was a problem hiding this comment.
Delegation abort is swallowed
Medium Severity
PRE_DELEGATION dispatch sits inside a broad except Exception that turns failures into a tool error string. A HookAborted from the hook is caught instead of propagating, so abort telemetry and the public abort contract do not hold for this point.
Reviewed by Cursor Bugbot for commit e2f88fc. Configure here.
790bde5 to
ea20c1a
Compare
e2f88fc to
57618f0
Compare
Wires the step, agent, subsystem, and flow points onto the dispatcher: `pre_step`/`post_step`, `tool_selection`, `pre_delegation`, `retry_attempt`, `memory_write`/`memory_read`, `knowledge_retrieval`, `pre_code_execution`, `mcp_connect`, `flow_transition`, and `router_decision`, extending the typed- context module with their contexts. Each seam passes a typed context whose `payload` a hook may observe, mutate, or replace. Adds the conformance suite for these points and a new `interception-hooks` doc page with the full point/payload catalog and the interceptor contract.
Addresses review findings on the remaining interception points. `TOOL_SELECTION` is no longer dispatched in `Crew._prepare_tools`; the single dispatch in `Agent.create_agent_executor` covers both crew and standalone runs, so hooks no longer fire twice (and additive edits no longer duplicate tools). `PRE_DELEGATION` now dispatches outside the delegation `try/except`, so a `HookAborted` propagates instead of being swallowed into a tool-error string. The `PRE_STEP` (flow), `PRE_CODE_EXECUTION`, and `MCP_CONNECT` seams now apply the returned payload: flow step params are rebound onto the call, script code is recompiled from the edited source, and the MCP transport is replaced before connecting.
ea20c1a to
73bdfaa
Compare
57618f0 to
3444a2c
Compare


This completes the catalog by wiring the remaining step, agent, subsystem, and flow points —
pre_step/post_step,tool_selection,pre_delegation,retry_attempt,memory_write/memory_read,knowledge_retrieval,pre_code_execution,mcp_connect,flow_transition, androuter_decision— each passing a typed context whose payload a hook may observe, mutate, or replace. It also adds a documentation page describing the full point/payload catalog and the interceptor contract. Stacked on #6517; part of the OSS-86 interception-hook catalog.Note
Medium Risk
Hooks can alter tools, task context, memory content, flow routing, and retry behavior on hot paths; mistakes or abusive hooks could change execution semantics, though non-abort hook errors remain fail-open like existing LLM/tool hooks.
Overview
Completes the interception-hook catalog by calling
dispatch()at runtime seams that were previously undocumented-only, and adds docs that describe the full point list and shared contract.Runtime wiring adds typed contexts and
InterceptionPointdispatch for: task/flow pre_step / post_step (mutable step input/output); tool_selection when crews/agents assemble tools; pre_delegation before coworker handoff; retry_attempt on agent task retries, guardrail retries, and tool-parsing retries; memory_write / memory_read on unified memory; knowledge_retrieval on knowledge queries; pre_code_execution for flow script actions; mcp_connect before MCP connect; flow_transition and router_decision in the flow runtime (router hooks can reroute).Docs: new Interception Hooks page (catalog,
@on, mutate/replace/abort, telemetry); execution-hooks overview links to it; nav entry indocs.json.Tests: conformance coverage for flow pre/post step, transitions, and router rerouting.
Reviewed by Cursor Bugbot for commit 57618f0. Bugbot is set up for automated code reviews on this repo. Configure here.