feat: implement intervention primitive in python with cancellation support#2693
Conversation
|
Assessment: Comment Clean, well-structured intervention primitive that integrates elegantly with the existing hook system. The API is intuitive, the test coverage is comprehensive (67 tests covering all action types and edge cases), and the TypeScript parity goal is well-served. Review Themes
The architecture (registry bridging handlers to hooks, Guide accumulation, Deny short-circuiting, Confirm interrupt integration) is well-thought-out. |
Re-review @
|
| # | Blocker | Status | Verification |
|---|---|---|---|
| 1 | ruff format on registry.py:216 |
✅ fixed | ruff format --check clean; CI Python / Lint passes |
| 2 | Cancel-path tests | ✅ added | new test_cancel_paths.py (7 tests) covers exactly the requested scenarios: Deny→end_turn + DENIED: text, model not invoked, plain-hook cancel=True→default text, cancel+retry→loop re-entry, before_invocation deny ×3. 74/74 intervention tests pass |
| 3 | Exports | ✅ resolved | Proceed/Deny/Guide/Confirm/Transform/InterventionAction/OnError all importable from strands.interventions (verified). Module docstring example updated to match |
| 4 | **kwargs: Any on lifecycle methods |
✅ added | all 5 methods |
Also noted: the factory functions and InterventionActions were removed entirely in favor of direct dataclass construction (Deny(reason="...")), per @mkmeral's open thread. The PR description was updated to match. Direct dataclasses are arguably more Pythonic and the namespacing concern is moot since Deny/Proceed are self-documenting names — but note this supersedes the resolution of @lizradway's namespacing thread (which was resolved via InterventionActions), so flagging for her re-confirmation.
🔴 New regression: removing confirm() reintroduced the exact hazard it guarded against
The factory's stated purpose (thread) was coalescing evaluate=None → default_evaluate. With the factory gone, that guard is gone:
Confirm(prompt="ok?", response=True, evaluate=None)
# → TypeError: 'NoneType' object is not callable (at dispatch time, deep in the registry)Verified on this head. Since evaluate: Callable[[Any], bool] = field(default=default_evaluate) only protects omission, not explicit None, please add a __post_init__ guard to the frozen dataclass:
def __post_init__(self) -> None:
if self.evaluate is None:
object.__setattr__(self, "evaluate", default_evaluate)(or type it Callable[[Any], bool] | None and resolve at use site — either way, explicit None shouldn't detonate at dispatch time in an approval primitive).
Still open from earlier reviews (maintainer calls, not blockers)
- Bare callable in
interventions=[...]still dies withAttributeError: 'function' object has no attribute 'name'at registration (verified) — a clearTypeErrorwould help, sincehooks=[...]does accept callables. - Instance-assigned handler methods still silently unenforced (
_is_overriddenchecks the class only — verified returnsFalse; TS detects this). Docs-only stance unchanged. - Phantom 0-token span —
model_invoke_spanstill starts before the cancel check, so cancelled calls emit a 0-usage span; TS emits nothing.
CI status
All code gates are green (Lint, unit tests on py3.10–3.14 × linux/windows). The remaining failures are process checks: check-api-review-label (PR carries api/needs-review without api/review-complete — awaiting API-review sign-off), label-size, and check-access-and-checkout.
Verdict: with the Confirm(evaluate=None) guard added, this is mergeable from my side — pending the API-review label sign-off.
|
Assessment: Comment (approve-leaning) Re-reviewed against the new HEAD Verification @ 726255b
Still open — minor, already have inline threads (replied)
Both are quick wins, neither blocks merge. Maintainer-call DevEx items (verified still present, not blocking)
From a code standpoint this is in good shape — the only merge gate I see is the external |
|
Assessment: Approve-leaning Re-reviewed against the new HEAD
Verification @ c150d11
The two remaining items are explicit maintainer calls, not blockers, and are unchanged: bare callable in Code-wise this is in good shape — the only outstanding merge gate is the external |
Description
Adds the intervention primitive — a composable control layer for agents that enables authorization, guardrails, steering, and operational controls to share a common interface with ordered evaluation, short-circuiting, and typed actions.
This implements Python parity with the TypeScript SDK's intervention primitive (strands-agents/sdk-typescript#883).
Resolves #2667
Public API Changes
Agent(interventions=...)Handlers are evaluated in registration order at each lifecycle event. Cheapest handlers (authorization, guardrails) should be listed first; expensive ones (LLM steering) last.
Public Exports
InterventionHandlerAll lifecycle methods default to
Proceed(). Override only the ones you need — the framework detects overrides at the class level and only registers hooks for those. Handlers can be sync (def) or async (async def).Action Types
Actions are frozen dataclasses constructed directly:
Action-to-Event Compatibility Matrix
OnErrorPolicy"throw""proceed""deny"Hook Ordering
INTERVENTION_INPUT(90) — after plugins (0)INTERVENTION_OUTPUT(-90) — before plugins (0)Flow:
plugins → intervention → [operation] → intervention → pluginsWhat's NOT exported (internal)
Infrastructure Changes (cancellation support)
This PR also adds general-purpose cancellation support to two hook events. These fields are usable by any hook or plugin, not just interventions — but are required for the intervention primitive's Deny action to work.
BeforeInvocationEvent.cancelWhen set by a hook callback, the agent loop:
MessageAddedEventEventLoopStopEventwithstop_reason="end_turn"AfterInvocationEventBeforeModelCallEvent.cancelWhen set by a hook callback, the event loop:
AfterModelCallEvent(allows retry viaevent.retry = True)ModelStopReasoneventHookOrderconstantsExample Usage
Related Issues
Resolves #2667
Documentation PR
N/A
Type of Change
New feature
Testing
How have you tested the change?
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.