Python: agent-hooks interception contract as a first-class experimental core feature - #7515
Conversation
Implement the AGENT-HOOKS-0.1 interception contract as a first-class experimental feature in agent_framework core. - Single public factory agent_hooks_middleware() returning a private agent/chat/function middleware trio (one object per middleware category); partial or stacked installs fail closed with loud errors. - All eight interception points: input/output at the agent seam, pre/post_model_call at the chat seam, pre/post_tool_call at the function seam, agent_startup/agent_shutdown bracketing each run. - Fail-closed enforcement throughout: transforms write back into the native contexts (messages, arguments, results) or raise; content is preserved as Content objects; MiddlewareTermination short-circuits are guarded at every seam; enforcement-layer failures halt the run; interceptor crashes surface as host_error denies. - Streaming is fully buffered per spec buffered_output semantics: no update egresses before the post_model_call/output verdicts; a deny at pull time releases zero updates; run state stays active across lazy pulls with cleanup on every exit path. - Session scoping: per-run by default (startup/shutdown bracket each run) or host-owned via emitter/builder parameters for one session spanning multiple runs. - agent-hooks-sdk is an opt-in agent-hooks extra (not in all), lazy-imported per the _mcp.py pattern; core imports cleanly without it and the factory raises a clear ModuleNotFoundError. - ExperimentalFeature.AGENT_HOOKS + @experimental decorator, lazy root export, typing surface, PACKAGE_STATUS.md entry. - 55 tests built on real Agent/mock-client flows covering deny-before- execution, transform write-back, rich-content preservation, complete streaming ordering, error cleanup, concurrency isolation, nested agents, and importability without the optional SDK. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds an experimental, first-class implementation of the AGENT-HOOKS-0.1 interception/enforcement contract to the Python core package (agent-framework-core), including an opt-in extra for the SDK dependency and a comprehensive test suite validating fail-closed behavior across agent/chat/tool seams (including buffered streaming).
Changes:
- Introduces
agent_framework/_agent_hooks.pywith the publicagent_hooks_middleware(...)factory that returns an agent/chat/function middleware trio implementing all eight interception points and fail-closed semantics. - Adds the opt-in
agent-hooksextra (agent-hooks-sdk>=0.1.0a4,<0.2) and updates exports + experimental feature registration/documentation. - Adds extensive unit tests covering deny/transform semantics, streaming buffering, short-circuit guarding, partial install detection, and optional-dependency importability.
Show a summary per file
| File | Description |
|---|---|
| python/uv.lock | Adds the agent-hooks extra lock entries and locks agent-hooks-sdk 0.1.0a4. |
| python/packages/core/tests/core/test_agent_hooks.py | New test suite for agent-hooks enforcement and semantics across seams (incl. streaming). |
| python/packages/core/pyrightconfig.dependency.json | Excludes the new module from dependency-bound pyright checking. |
| python/packages/core/pyproject.toml | Adds agent-hooks optional dependency extra (explicitly not part of all). |
| python/packages/core/agent_framework/_feature_stage.py | Registers ExperimentalFeature.AGENT_HOOKS. |
| python/packages/core/agent_framework/_agent_hooks.py | Implements the enforcement middleware trio + projections/write-back + buffering semantics. |
| python/packages/core/agent_framework/init.pyi | Adds typing export for agent_hooks_middleware. |
| python/packages/core/agent_framework/init.py | Adds lazy runtime export for agent_hooks_middleware. |
| python/PACKAGE_STATUS.md | Documents the new experimental feature and its opt-in extra. |
Review details
- Files reviewed: 8/9 changed files
- Comments generated: 2
- Review effort level: Lite
The pre-commit pyupgrade hook rewrites the quoted forward reference; ResponseStream is imported at runtime in this module, so the quotes were unnecessary. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
Reworks the agent-hooks feature per PR review: - Verdicts now precede durability: a run-scoped persistence gate (_sessions.py) defers per-service-call history persistence and after-run provider work until the covering post_model_call/output verdict permits; denied content never persists, transforms persist post-write-back. Unhooked runs are unchanged (verified against an instrumented baseline). - ResponseStream.buffered_and_gated: a buffered-gate combinator that applies the run's pending stream hooks before the gate, then seals the stream, so no middleware can rewrite egress after the output verdict. Replaces the hand-rolled replay iterator. - MiddlewareBundle (public, _middleware.py): the factory returns an indivisible bundle categorize_middleware splits, making partial installs impossible by construction; members are validated at construction. Bare (non-sequence) middleware at agent construction is now normalized instead of silently dropped, and unrecognized middleware logs a warning instead of vanishing. - Factory split and rename: create_agent_hooks_middleware (per-run sessions) and create_agent_hooks_middleware_from_emitter (host-owned); the sentinel parameter-diffing is gone. - Wire conversions live in per-point codec classes owning to_wire and write_back. Fixes in that code: tool-call name transforms apply or raise; non-object args transforms raise; argument write-back merges only changed keys (original values, including bytes, preserved by identity); message-list write-back matches by identity, not index. - function_approval_request objects on the normal return path pass through un-emitted, preserving the human approval pause. - Hosted (service-executed) tool calls surface in the post_model_call content projection; the tool-seam limitation is documented. - Import probe covers the full SDK surface and re-raises as missing-extra only for the agent_hooks module; module logger added; _json_safe replaced by make_json_safe (which gained bytes support); tools_registered uses normalize_tools; dependency-pyright analyzes the module again via the test dependency-group. - Tests: 75 in the feature suite (persistence gating, stream-hook sealing, approval passthrough, codec units, bundle validation, bare-bundle installs), full core suite green. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
moonbox3
left a comment
There was a problem hiding this comment.
Have a look at the failing CI/CD (code quality checks) too, please. Thanks.
| context variable before executing the collected callables, so re-entrant calls run | ||
| inline), and False when no gate is active and the caller must persist inline. | ||
| """ | ||
| pending = _DEFERRED_RUN_PERSISTENCE.get() |
There was a problem hiding this comment.
What happens when a hooked agent calls a sub-agent tool? The agent-level gate at _agent_hooks.py:979 spans the whole call_next(), including the tool loop, and this check has no notion of an owner, so an unhooked inner agent run (as_tool, session shared with the parent) has its _run_after_providers and per-service-call persists deferred into the outer run's pending list. If a later outer model call fails or is denied, that list is dropped and the inner run's fully-permitted history is silently never persisted; and a second call to the same sub-agent inside one outer run loads the store before the first call's deferred after_run executed, so it sees stale history. Could the gate carry an ownership token so only the gated run's own persistence defers and nested runs persist inline?
There was a problem hiding this comment.
You were right to push for ownership — implemented in c1cfa83, and your instinct held up better than our first divergence. RawAgent.run stamps a run identity over the run's whole dynamic extent (including streaming pulls and result hooks), and the gate accepts only its owner's persists; anything else persists inline, regardless of how the nested run started (tool call, middleware, custom run loop). One refinement over literal bind-at-first-arrival: a middleware-initiated run can start before the outer's own first persist, so first-arrival would bind the gate to the wrong run and invert your scenario into a fail-open — binding is instead an offer/adopt handshake keyed to the agent instance. The tool-seam suspension stays for one case identity can't see (a custom-run-loop sub-agent invoked as a tool never stamps); the single residual case (custom loop nested in a custom loop off the tool path) is fail-closed and documented. Proving the design also surfaced a latent pre-existing re-deferral (a nested hooked run's permitted after-run persistence re-deferred into an enclosing gate on flush) — fixed with a regression test. Your exact scenarios — sub-agent under outer deny, second sub-agent call reading fresh history, middleware-initiated runs before and after call_next — are all pinned in tests that fail with ownership disabled.
| hooked_updates: list[UpdateT] = [] | ||
| for update in updates: | ||
| hooked_update = update | ||
| for hook in transform_hooks: |
There was a problem hiding this comment.
It looks like the stream-hooks fix inverted the leak direction: hooks used to rewrite content after the verdict, and now they observe content before it. as_tool(stream_callback=...) registers a host-facing observer as a transform hook (_agents.py:668-669), and _materialize runs every hook over the raw buffered updates before gate fires at _types.py:3605, so on a deny the callback has already received the complete denied response, and on a redacting transform it saw the unredacted original (the re-derivation at _agent_hooks.py:1117 only rebuilds the released stream). That breaks the module's own no-egress-ahead-of-verdict contract, and the new regression test only covers the rewrite direction. Could hooks still apply pre-gate for verdict coverage but observers replay over the gated updates instead, or stream_callback attach to the released stream?
There was a problem hiding this comment.
Interesting one: the leak as described doesn't reproduce at a80b5f3 — as_tool's callback registers on the from_awaitable wrapper returned by Agent.run, and wrapper hooks run per update pulled out of the sealed gate, so deny fed the callback nothing and a transform fed it only the redacted form (verified empirically before changing anything). But that safety was an accident of the wrapper boundary, not a stated contract — so c1cfa83 makes it structural per your second option: as_tool now consumes the released stream and feeds stream_callback per released update, and buffered_and_gated's contract states that hooks are pre-gate rewriters while observers must consume the released stream. Both directions (deny → callback sees nothing; transform → transformed only; unhooked streams unaffected) are regression tests now.
|
|
||
| Some features (for example the agent-hooks enforcement middleware) consist of | ||
| several middleware objects that only uphold their contract when installed | ||
| together. A bundle carries those objects as one opaque unit: pass the bundle |
There was a problem hiding this comment.
Should this docstring promise less, or the chat seams enforce more? Passing a bundle to a ChatClient does expand it, but those paths only pick up the chat and function members and silently drop the agent one (_tools.py:2849-2855, _tools.py:3238-3244), and for the agent-hooks bundle the dropped member is the one holding the output gate and the state.halted re-raise.
Typing rejects it, but at runtime nothing complains, even though the docstring says the bundle can go anywhere middleware is accepted. Could we raise, or at least warn the way _add_middleware does at _middleware.py:918-927, when a bundle member lands in a category the call site throws away?
There was a problem hiding this comment.
Fixed in c1cfa83: categorize_middleware gained supported_categories; both chat-client sites pass their supported set, so a bundle member landing in a category the call site would throw away now raises MiddlewareException (bundle indivisibility — for this bundle the dropped member would have been the output gate), while bare middleware warns the way _add_middleware does and is skipped. SessionContext.extend_middleware and the provider seam are covered too, and the MiddlewareBundle docstring now matches reality.
| transformed = await self._emit_output(state, final) | ||
| await _run_pending_persistence(pending) | ||
| await self._emit_shutdown(state, "completed") | ||
| released = _agent_updates_from_response(final) if (transformed or hooks_applied) else updates |
There was a problem hiding this comment.
Would it make sense for buffered_and_gated to own the re-derivation rule instead of documenting it? The no-divergence half of the contract lives in the doc string at _types.py:3251-3254, and both gates hand-roll this same conditional (here and at _agent_hooks.py:1245). A third caller that forgets it silently egresses un-verdicted updates, which is fail-open. Passing a rederive callable to the combinator and letting it apply the rule whenever hooks ran or the gate reports a transform would be better as it shrinks the gate signature and deletes the duplication.
There was a problem hiding this comment.
Agreed — done in c1cfa83: buffered_and_gated(consume, gate, rederive) owns the rule; the gate now returns (final, transformed) and cannot choose the released updates, and both hand-rolled conditionals are deleted. A raising rederive is pinned fail-closed (zero egress, verdicted final still returned). Also marked the combinator @experimental to match MiddlewareBundle.
| try: | ||
| await self._emit_run_start(context, state) | ||
| termination: MiddlewareTermination | None = None | ||
| pending: list[_PersistCallback] = [] |
There was a problem hiding this comment.
Have we thought about wrapping the gate-owner protocol in a context manager owned by _sessions.py? The MUST in _defer_run_persistence's docstring (reset before draining) is executed by hand at four sites here (:978, :1087, :1161, :1227) over two privately-imported names, and getting the order wrong at any one site re-defers the re-entrant call into a list nobody drains, which is silent persistence loss. A run_persistence_gate() yielding a handle with flush() and drop() would make sure we get a reset-before-drain by construction, it would keep the ContextVar inside _sessions.py, and give the ownership fix for the nested-agent issue I mentioned.
There was a problem hiding this comment.
Done in c1cfa83: _RunPersistenceGate in _sessions.py — with gate: scope, flush()/drop(), flush() raises while the scope is active so reset-before-drain holds by construction, nested re-entry raises, and the ContextVar never leaves _sessions.py. The four hand-rolled sites collapsed to gate handles. One deviation from the sketch: sequential reuse of one gate object is allowed (the streaming chat seam needs two scopes per call), unit-tested. And as you predicted, this is exactly where the ownership fix for the nested-agent issue landed — flush() also now drains with the gate context suspended, which fixed a latent re-deferral bug that predates this PR.
| state.builder.post_tool_call(call_id=call_id, name=name, args=args, value=value) | ||
| ) | ||
| if outcome.target != value: | ||
| context.result = _ToolResultCodec.write_back(context.result, outcome.target) |
There was a problem hiding this comment.
Any reason not to give _ToolResultCodec.write_back the before wire value? The codec region promises every write_back applies the untouched-wire-means-untouched-native rule exactly once, but this codec cannot check that without before, so the comparison lives out here at the emit site and the function seam is the one middleware body still owning half its codec's contract. Moving it inside would make this body uniform with the other five.
There was a problem hiding this comment.
Done in c1cfa83: _ToolResultCodec.write_back(original, before, after) owns the untouched-wire rule and the function-seam body is an unconditional assignment like the other five. Working in that code surfaced a real bug in the untouched-check itself: Python == equates 1 == True, so a bool↔number transform looked untouched and was silently dropped — all six codecs now compare with bool-aware wire equality, with a dedicated test.
| cast(Sequence[MiddlewareTypes], base_middleware_attr) if isinstance(base_middleware_attr, Sequence) else [] | ||
| ) | ||
| base_middleware: Sequence[MiddlewareTypes] | ||
| if isinstance(base_middleware_attr, Sequence) and not isinstance(base_middleware_attr, (str, bytes)): |
There was a problem hiding this comment.
Curious why the bare-source rule now has three homes. categorize_middleware has always treated a non-sequence source as a one-element list (_middleware.py:1630-1633), and this commit adds hand-rolled copies here and at _agents.py:447-448. This one looks like we can delete it, since passing the raw attribute to categorize_middleware behaves identically. One owner for the rule means the next change can happen once. Seems worth a callout either way: a bare middleware object assigned to agent.middleware used to be silently ignored and now executes.
There was a problem hiding this comment.
Right on all counts — c1cfa83 deletes the AgentMiddlewareLayer.run copy (the raw attribute goes straight to categorize_middleware, verified behavior-identical) and makes categorize_middleware the rule's only owner. The BaseAgent.__init__ normalization stays, but re-justified as storage canonicalization: the constructor signatures are now widened to MiddlewareTypes | Sequence[MiddlewareTypes] | None (run overloads, as_agent, telemetry/harness layers, foundry too), so the typing matches what the runtime accepts and the normalization keeps the stored attribute's type honest. The behavior change you called out — bare middleware assigned to the attribute used to be silently ignored and now executes — is documented in both constructor docstrings, with a regression test. Also swept the adjacent if source: to if source is not None: so a falsy-__bool__ middleware can't be silently dropped.
| ) | ||
|
|
||
|
|
||
| class MiddlewareBundle: |
There was a problem hiding this comment.
I think MiddlewareBundle is missing @experimental, right? Both of its producers carry it the decorator.
There was a problem hiding this comment.
Yes — added in c1cfa83 (with the AGENT_HOOKS feature id both producers carry; per-feature dedup keeps it to one warning). Gave buffered_and_gated the same treatment for symmetry.
Addresses the second review round on the agent-hooks feature: - Nested-run persistence ownership: RawAgent.run stamps a run identity over the run's dynamic extent (including streaming pulls and result hooks); the persistence gate binds to its owning run via an offer/adopt handshake keyed to the agent instance and accepts only its owner's persists — nested runs persist inline regardless of how they were started (tool calls, middleware, custom run loops). The tool-seam suspension remains for custom-loop sub-agents invoked as tools; the one residual case (custom loop nested in a custom loop off the tool path) is fail-closed and documented. Fixes a latent pre-existing re-deferral: flush() now drains with the gate context suspended, so a nested hooked run's permitted after-run persistence no longer re-defers into an enclosing gate. - as_tool stream_callback consumes the released (verdicted) stream; observers cannot see denied or pre-transform content. Both directions are regression-tested. - categorize_middleware gained supported_categories: a bundle member landing in a category a call site cannot install raises; bare middleware warns like _add_middleware. Wired at the chat-client sites and the provider seam. - ResponseStream.buffered_and_gated owns the re-derivation rule via a rederive callable (gates cannot choose released updates) and is marked experimental. - Wire codecs compare with bool-aware equality (Python == equates 1 == True, which made bool/number transforms look untouched and get dropped) and _ToolResultCodec.write_back owns the untouched-wire rule via the before value. - middleware parameters accept a bare middleware or bundle everywhere the runtime does (constructors, run overloads, as_agent, telemetry and harness layers, foundry); the bare-source rule has a single owner in categorize_middleware; bare middleware assigned to the attribute now executes (documented behavior change). - MiddlewareBundle is experimental and validates members; approval passthrough, typing-check fixes (ty ignores mypy-coded ignore comments), logging, and documentation updates per review. Test count: 85 feature tests plus 12 new this round across sessions, middleware, agents; full core suite green; typing checked under mypy, pyrefly, ty, zuban, and pyright. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
Motivation & Context
Runtime controls for agents (policy engines, approval flows, information-flow checks, budget guards, audit pipelines) currently require one adapter per framework, and no framework defines what happens when a guardrail callback fails or lets a control author verify "supported" claims. AGENT-HOOKS-0.1 is a framework-neutral interception contract addressing this: eight interception points, a three-verdict model (allow / deny with liftable approval / transform), fail-closed host obligations, payload-free audit records, and a conformance test kit.
PR #7444 proposed this as an external adapter package. Maintainer feedback asked for a first-class experimental feature in core instead, with a single public factory, private middleware, an opt-in extra, and corrections to transform write-back, content preservation, and streaming semantics. This PR supersedes #7444 and implements exactly that design.
Description & Review Guide
agent_framework/_agent_hooks.py: one public factory,agent_hooks_middleware(...), returning a private agent/chat/function middleware trio (one object per middleware category, percategorize_middleware()). Partial installs and stacked trios fail closed with explicit errors, so a caller cannot accidentally install part of the control contract.input/outputat the agent seam,pre/post_model_callat the chat seam,pre/post_tool_callat the function seam,agent_startup/agent_shutdownbracketing each run. Transforms write back into the native contexts (messages,arguments,results) asContentobjects; an unappliable transform raises rather than proceeding untransformed.MiddlewareTerminationshort-circuits are guarded at every seam (a substituted result passes the relevant interception point before egress); enforcement-layer failures halt the run; interceptor crashes surface ashost_errordenies.buffered_outputsemantics): no update egresses before thepost_model_call/outputverdicts; a deny at pull time releases zero updates; run state stays active across lazy pulls (ResponseStream.from_awaitable+ result/cleanup hooks) with cleanup on every exit path.emitter/builderparameters for one audit session spanning multiple runs.agent-hooks-sdkis an opt-inagent-hooksextra (not inall), lazy-imported per the_mcp.pypattern; core imports cleanly without it.ExperimentalFeature.AGENT_HOOKS+@experimental, lazy root export, typing surface,PACKAGE_STATUS.mdentry.uv lock --checkpass locally.MiddlewareTerminationguarding at the four seams; the buffered-streaming trade-off (callers get the stream API but updates arrive only after theoutputverdict — the only fully fail-closed option); tool-seam deny semantics (policy deny returns a reason-only error payload and the loop continues;host_error:*halts the run); and the sibling/stacking verification approach.Known limitation to resolve before merge:
agent-hooks-sdkon PyPI currently ships a linux-x86_64 wheel only, souv sync --all-extrasbuilds it from sdist elsewhere (macOS/Windows wheels are being published; will update this PR when live).Related Issue
Supersedes #7444 (external-adapter draft, closed in favor of this first-class design per maintainer feedback).
Contribution Checklist