Skip to content

fix(multiagent): clear pending interrupts when a graph or swarm invocation fails - #5633

Merged
mkmeral merged 2 commits into
strands-agents:mainfrom
strandly-the-agent:agent-tasks/5340
Oct 28, 2026
Merged

mkmeral merged 2 commits into
strands-agents:mainfrom
strandly-the-agent:agent-tasks/5340

Conversation

@strandly-the-agent

@strandly-the-agent strandly-the-agent commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Requested by: @mkmeral — investigate #5340 and fix it with the fail-fast approach (drop pending interrupts when an invocation fails), kept private, Swarm included.

Human Overview

When parallel node executions in graph fail and interrupt at the same batch. the interrupt is keeping the graph from working. Continuing is meaningless because the graph already failed, so it forces unnecessary back and forth.

This PR adds a private method to clear interrupts. The agents can recover from dangling tool use, so everything continues properly in the next invocation.

Description

When one node in a parallel Graph batch interrupts and a sibling in the same batch raises, _activate_interrupt has already set _interrupt_state.activated, but the exception skips the batch-exit bookkeeping in _execute_graph (writing context["completed_nodes"] / deactivate()). The graph is then locked for good: a new task raises TypeError … must resume from interrupt, the interrupt responses raise KeyError: 'completed_nodes', and a session reload carries the broken state forward. Swarm has the same lockout when a node fails while resuming an interrupt (it returns FAILED without raising, so the top-level handler never ran either).

A FAILED invocation now drops its pending interrupts so the next call starts fresh: MultiAgentBase._clear_interrupts() (private) deactivates the orchestrator's _interrupt_state; Graph overrides it to also clear every node executor (nested MultiAgentBase recursively, anything else via _interrupt_state.deactivate()), Swarm to clear every node agent. It runs in stream_async's finally when the status is FAILED, before AfterMultiAgentInvocationEvent, so session managers persist the cleared state. Interrupted agents' messages are left alone — the agent already heals a dangling toolUse on its next invocation.

Deliberately out of scope: a public "abandon this interrupt" API (Agent has the same gap; needs its own API review — happy to file the follow-up), turning the fail-fast raise into a FAILED result, and asyncio.CancelledError escaping a batch (#4419).

Related Issues

Fixes #5340

Documentation PR

N/A — no public API change.

Type of Change

Bug fix

Testing

Five regression tests (tests/strands/multiagent/test_graph.py, test_swarm.py), each fails on main without the src change: hook interrupt + failing sibling, agent-level interrupt + failing sibling, failure on the resume path (node agent's own _interrupt_state must be cleared), nested-MultiAgentBase recursion, and Swarm interrupt → failed resume → new task. The parallel-batch tests order the failure after the interrupt with an asyncio.Event, not a sleep.

== ruff format --check      377 files already formatted
== ruff check               All checks passed!
== mypy (base/graph/swarm)  Success: no issues found in 3 source files
== pytest tests/strands/multiagent (a2a skipped, extra not installed)
                            262 passed
== new tests x20            20 × "5 passed" (no flakes)

Offline repro scripts (stub Model, no network) for the four scenarios — issue repro, tool-level interrupt landing first, nested Graph node, Swarm — all recover on this branch and lock out on main. hatch run prepare not run as such (no hatch in the sandbox); the equivalent ruff/mypy/pytest steps above were run from the project venv.

  • I ran the quality gates (ruff format/check, mypy, pytest) — see above

Additional Details

Independent review ledger (two rounds + adversarial pass)

Round 1 — 🔴 fixed in f7ce961. Graph._clear_interrupts iterated state.interrupted_nodes, which _execute_graph empties at the start of a resume, so a failure during a resume (exception, or max_node_executions exhausted → should_continue returns FAILED without raising) left the node agent activated and the next string task still raised TypeError. Now iterates self.nodes.values() (Swarm's shape). New test test_graph_node_failure_on_resume_clears_interrupted_agent_state fails on the first commit, passes on the second.

Round 2 — approved, no new findings. Verified with a real Agent + stub model that the no-raise resume-limit path is cleared; confirmed iterating all nodes doesn't over-clear (nodes are fixed after build(), every node task is gathered before the finally, agents already drop answered invocation-scoped responses at the end of their own invocation).

⚪ nits — accepted: asyncio.sleep ordering in tests → asyncio.Event; one-line comment on why the clear sits before AfterMultiAgentInvocationEvent; #5340 docstring on the nested test.

⚪ nits — not taken, with reasons:

  • Annotate _interrupt_state on MultiAgentBase and call it directly instead of getattr. Kept getattr: Graph recurses into nested MultiAgentBase executors, which can be third-party subclasses without that attribute. An AttributeError there would run inside a finally and mask the original exception, which is worse than skipping the clear.
  • Also reset state.interrupted_nodes / node execution_status on clear. Not needed: nothing reads them while activated is False, and the next invocation builds a fresh GraphState. Matches what a plain failure leaves behind today.
  • TS parity port. strands-ts very likely has the same lockout (its status resolution ranks FAILED over INTERRUPTED in a mixed batch). Flagging rather than bundling — a [PORT] follow-up is the right vehicle.

Adversarial pass (a first run timed out at 15 min and was re-run with a 10-min budget; it completed the high-priority probes). Probed and fine: original repro, tool-level interrupt + sibling raise, nested Graph, Swarm, max_node_executions exhausted on first resume iteration, interrupt responses submitted after a clear (loud ValueError, no lockout), legitimate interrupt with no failure still resumes normally and is never cleared (status is INTERRUPTED, not FAILED). Pre-existing, out of scope: a consumer that breaks out of stream_async right after the interrupt event and then resumes hits KeyError: 'completed_nodes' — identical on main, same family as this bug (completed_nodes is only written on the clean interrupt return), worth its own issue. Not probed for lack of time: nested Swarm as a Graph node, cancel_node from a hook on resume, node_timeout on an interrupted node's sibling, reset_on_revisit cycles, and FileSessionManager restore into a fresh builder after the failure (the before-the-hook placement is what should make that work; verified by code reading only).

Checklist

  • I have read the CONTRIBUTING document
  • I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
  • My change is focused and reasonably small; I have split unrelated work into separate PRs
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly (none needed — private API)
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…ation fails

A node failing alongside an interrupted sibling (or while resuming one) left
_interrupt_state activated with no completed_nodes recorded, so every later
call raised TypeError or KeyError. A FAILED invocation now drops its pending
interrupts, including those of interrupted node executors, so the next call
starts fresh.

Fixes strands-agents#5340
_execute_graph empties state.interrupted_nodes at the start of a resume, so
a failure during a resume left the node agent activated. Iterate every node
executor instead, matching Swarm.
@strandly-the-agent
strandly-the-agent requested a review from a team as a code owner October 28, 2026 17:52
@github-actions github-actions Bot added area-multiagent Multi-agent related area-hil Human in the loop and suspend/resume python Pull requests that update python code bug Something isn't working strands-running complexity/medium Touched functions have moderate cognitive complexity (11-25) size/s labels Oct 28, 2026
@codecov

codecov Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
strands-py/src/strands/multiagent/base.py 75.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Comment (approve-leaning)

Clean, well-scoped fix. The private _clear_interrupts() hook on MultiAgentBase with Graph/Swarm overrides is a sound way to drop pending interrupts on a FAILED invocation, and placing the call in finally before AfterMultiAgentInvocationEvent correctly lets session managers persist the cleared state. No public API change, so no API bar-raising needed. I ran the five new regression tests locally — all pass.

Review notes
  • Correctness: The fail-fast "drop interrupts when FAILED" semantics (including the mixed interrupt+failure batch and the resume path) look right; the isinstance(MultiAgentBase) recursion vs. hasattr(_interrupt_state) fallback in Graph is reasonable, and the getattr/hasattr guards are justified for third-party executors in a finally. The thorough self-review ledger covers the over-clear and no-raise-FAILED concerns I'd otherwise have raised.
  • Testing (inline): the new tests assert interrupt state field-by-field; since _InterruptState is an eq dataclass, a single == _InterruptState() check is stronger and matches the existing convention in these files. Minor: test_graph_node_failure_clears_interrupted_agent_state verifies cleared state but, unlike its siblings, doesn't re-run a fresh task to prove the user-facing recovery — worth adding for parity.
  • Cross-SDK parity: the strands-ts lockout you flagged is best handled as a separate [PORT] follow-up, agreed.

Nicely documented reasoning throughout the PR.

@mkmeral
mkmeral merged commit 3ee280f into strands-agents:main Oct 28, 2026
42 of 47 checks passed
gautamsirdeshmukh pushed a commit to gautamsirdeshmukh/harness-sdk that referenced this pull request Nov 2, 2026
…ation fails (strands-agents#5633)

Co-authored-by: strandly-the-agent <strandly-the-agent@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-hil Human in the loop and suspend/resume area-multiagent Multi-agent related bug Something isn't working complexity/medium Touched functions have moderate cognitive complexity (11-25) python Pull requests that update python code size/s

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Graph: a sibling failure in the same parallel batch as an interrupt leaves _interrupt_state permanently stuck

3 participants