fix(agent): resume nested agent-as-tool interrupts across rehydration

[seanalbert]

Description

Resuming an interrupt raised inside a nested agent-as-tool only worked when the orchestrator and the sub-agent shared the same in-memory Interrupt object. The previous resume path (_is_sub_agent_interrupted / _build_interrupt_responses) read the response straight off the sub-agent's live _interrupt_state, which the executor and the orchestrator happened to share by reference. That assumption holds within a single long-lived process, but breaks the moment either agent is rebuilt from storage — e.g. a stateless Lambda that reconstructs every agent on each invocation. After rehydration the shared object is gone, the recomputed interrupt id no longer matches, and the resume silently fails to deliver the human's response to the sub-agent.

This change makes nested resume work purely as data, so it survives a full serialize/deserialize round-trip. When a sub-agent interrupt propagates upward, the agent-as-tool now attaches a minimal serializable snapshot of the interrupted turn (the sub-agent's messages, key/value state, and interrupt state — which carries the original tool_use_message and therefore the original toolUseId) to the ToolInterruptEvent. The event loop persists that snapshot inside the parent's own interrupt context, so it round-trips through the parent's session with no extra wiring and without giving the sub-agent its own session. On resume, the event loop routes the snapshot and the human's responses back down through invocation_state; the agent-as-tool overlays the snapshot onto a freshly built sub-agent, reproducing the exact pending tool call so the recomputed interrupt id matches and the response is applied.

The net effect: the sub-agent no longer needs to be the same in-memory object across the interrupt boundary, and it doesn't accumulate history across turns.

Related Issues

Fixes #3076

Documentation PR

No documentation changes required — all touched surfaces (_agent_as_tool.py, ToolInterruptEvent, _handle_tool_execution) are internal.

Type of Change

Bug fix

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce new warnings.

Added unit tests in test_agent_as_tool.py covering both halves of the round trip: that a propagated ToolInterruptEvent carries a snapshot preserving the original sub-agent toolUseId, and that a freshly built sub-agent (with no live interrupt state) resumes correctly from a JSON round-tripped snapshot routed via invocation_state.

• I ran hatch run prepare

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
• 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.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[opieter-aws]

@strandly-the-agent review the PR please

[strandly-the-agent]

🔴 Changes requested: the stateless path works for the covered case, but exact head f89a286 has three blocking safety/continuity issues.

✅ Verified: 243 focused/adjacent tests, git diff --check, exact-head CI unit matrix/lint/docs/CI Gate; four adversarial repros ran cold twice with byte-identical output.
🔴 Reproduced: one approval executes two colliding sub-agents; OpenAI Responses loses previous_response_id; restored conversation-manager state diverges.
⚠️ Integration and review-trigger jobs are still waiting.

This also needs api/needs-review → api/review-complete: the new field is emitted through the public callback/stream dictionary despite being described as internal.

Review breakdown

• Correctness/safety: three inline blockers below.
• Tests: current coverage proves the basic manual JSON round-trip, but not stateful provider/conversation-manager continuity or colliding nested interrupts.
• API/DevX: keep resume state private; exposing full messages/application state creates a new data and compatibility contract. No API meeting appears necessary.
• Suppressed: unproved middleware-ID concern, formatter noise not attributable to this PR, and snapshot aliasing as a separate comment because the versioned snapshot fix should resolve it.

Human review still owns the final API call.

[strandly-the-agent]

🔴 Scope this response to the outer agent-as-tool invocation. Interrupt IDs are local to the sub-agent: two concurrent sub-agents can generate the same ID, while the parent registers them with setdefault. This filter then forwards the single parent response to every snapshot containing that ID.

A cold repro ran twice identically: one response produced executed_calls={'A': 1, 'B': 1}. Please namespace the parent-visible ID by the outer toolUseId, retain its mapping to the sub-agent-local ID, and forward only the response owned by this snapshot.

[strandly-the-agent]

🔴 Persist the complete resumable session state. This custom snapshot ends at interrupt_state, so a fresh sub-agent loses both _model_state and conversation-manager state.

With the real OpenAI Responses request formatter, response_id='resp-before-interrupt' disappeared and the resumed tool result omitted previous_response_id, producing an error. With SlidingWindowConversationManager(per_turn=2), the fresh resume sent 3 messages at call count 1 instead of the restored control's 2 messages at count 2.

Would using the existing versioned session snapshot/load path be simpler here? Otherwise capture and defensively restore these fields too, with regressions for both cases.

[strandly-the-agent]

🔴 Keep this internal snapshot out of the emitted event dictionary. TypedEvent.as_dict() is delivered to callbacks and yielded by Agent.stream_async(), so nested interrupts now expose the sub-agent's complete messages, application state, and interrupt context; ordinary interrupts also gain an unconditional sub_agent_snapshot=None field.

Please carry this as private event metadata consumed by the event loop. If public exposure is intentional, it needs an API review plus explicit redaction, compatibility, and versioning contracts before merge.

[opieter-aws]

Hi @seanalbert thanks for contributing the fix! Can you please look into the review bot's comments? We'd like to get a the fix out, so I may take over the PR if you don't have time to finalize the PR (of course you'd be listed as co-author)

[opieter-aws]

@strandly-the-agent can you do another review pass on this PR?

[strandly-the-agent]

✅ All three requested changes are in and hold under adversarial re-test at f9949f2 — approving to clear my stale block. Two 🟡 non-blocking notes below; neither regresses what this PR set out to fix.

Prior blocker	Status at f9949f2
Colliding sub-agent interrupt IDs	✅ fixed — approving only A gives executed = {'A': 1}, B stays pending
Lost model_state / conversation-manager state	✅ fixed — real Responses formatter emits previous_response_id='resp_sub_0'; sliding-window resume now matches the state-restored control (model_call_count=2)
Snapshot in the public event payload	✅ fixed — off the dict, private attribute; verified no in-tree path drops it

✅ Also verified at this exact head: clean worktree, git diff --check clean, 344 focused/adjacent tests pass locally, full GitHub CI green (unit matrix, lint, docs, integration tests, CI Gate). Multi-level (3-deep) nesting, re-interrupt after resume, partial answers, a real FileSessionManager process restart with image bytes, a differently-built sub-agent, and mixed ordinary+nested interrupts in one turn all resume correctly; every repro ran cold twice with identical output.

Details: the two 🟡s, one question, and what I deliberately dropped

🟡 inline 1 — a failed snapshot restore silently eats the human's approval. Reproduced at head for schema-version skew, a missing data/created_at, the previous revision's snapshot shape, and a sub-agent rebuilt with a different ConversationManager type: all end stop_reason=end_turn, executed=[], interrupt deactivated, nothing raised. Worth narrowing the swallow, but it extends a pre-existing broad except and fails semi-visibly, so it's follow-up material rather than a gate.

🟡 inline 2 — two mutation-confirmed test gaps in exactly the code this PR fixes (mutants survive with the suite green): the collision fix is only proven by calling the private _namespace_interrupts by hand, and the restore side never asserts model_state / conversation-manager state.

❓ non-blocking question — types/_events.py:383-406: the resume-critical snapshot now lives only on a private attribute, so anything that rebuilds the event from its public tool_interrupt_event payload silently produces a non-resumable interrupt (I reproduced this with a wrap middleware). No in-tree path does that today and _middleware/ is still private per its own README — is the private-attribute shape the intended long-term contract before that surface opens up, or worth a payload-safe handle / comment?

Dropped after checking: the remaining _InterruptState.to_dict()/from_dict() context aliasing (interrupt.py:124,138 — untouched by this diff, latent, not reproducible as observable corruption); the stale-answered-id livelock (A/B'd — present at the merge base and strictly improved here); and the 9 formatter-only files as a scope objection (verified pre-existing drift, zero logic change — just worth keeping formatter sweeps in their own commit next time).

As always: solid work for a human to approve, and 2 Approvers for Bots still applies.

[strandly-the-agent]

🟡 Non-blocking: a failed restore here silently destroys the human's approval. Any exception from _restore_from_snapshot lands in the broad except Exception below, becomes an ordinary error ToolResult, and the parent then sees no interrupt and deactivates — so the approval and the pending protected operation are gone, with stop_reason=end_turn and nothing raised or retryable.

Reproduced at this head, all ending executed=[] interrupt_still_activated=False: schema_version="2.0" → "Unsupported snapshot schema version"; missing data; missing created_at; the previous revision's snapshot shape → "Agent error: 'session_snapshot'". Reachable without branch skew too — a sub-agent rebuilt with a different ConversationManager type raises ValueError("Invalid conversation manager state.") mid-load_snapshot, after messages/state are applied but before interrupt_state is, so the restore is also half-applied. A missing interrupt_id_map degrades even quieter: zero forwarded responses, no error at all.

Worth catching restore failures separately from sub-agent execution failures and failing loud (ERROR-level, alertable) — and ideally leaving the interrupt pending so a fixed deploy can retry, rather than converting it into a terminal tool error. The broad except predates this PR, so I'd be happy to see this as a follow-up issue rather than more churn here.

[seanalbert]

I can easily pull the restore into its own try/except with an ERROR log so it's loud and alertable rather than silently swallowed. Happy to add that here if others agree.. Re-emitting the interrupt so it stays retryable touches the contract between _AgentAsTool and the event loop in a way that feels like its own ticket.

[opieter-aws]

+1 on the separate try/except. Agree that re-emitting is out of scope

[strandly-the-agent]

🟡 Non-blocking: the restore half of the round trip isn't asserted. This fixture uses "model_state": {}, and nothing after tool.stream(...) checks the restored _model_state or conversation-manager state — so stripping either field inside _restore_from_snapshot leaves the whole file green (mutation-tested). A non-empty model_state (e.g. {"response_id": "resp_x"}) plus a non-default conversation-manager state here, with post-restore assertions, would lock in the continuity this PR just fixed.

Same shape for the collision fix: test_concurrent_sub_agents_with_same_local_interrupt_id_are_disambiguated calls the private _namespace_interrupts twice by hand, so no test ever has two nested interrupts land in one real orchestrator cycle — mutating the actual accumulation site (event_loop.py:806, e.g. rebinding sub_agent_snapshots instead of updating it) also stays green. One integration-style test with two concurrent nested interrupts through the event loop would cover it.

[Unshure]

I would like to revisit the design proposed in this pull request. We have an upcoming feature in the python sdk that will make serializing and deserializing agents much easier: #3283

I think we could take advantage of that, in addition to updating the AgentState of the parent agent to store this information across restart. Ill bring this idea to the team and see what they think.

[Unshure]

There are a number of these linting changes throughout this pull request, can you please revert them?

[mkmeral]

@seanalbert thanks for the contribution. I think we can simplify this PR with a couple of design principles. First, the session management is per agent. We should avoid cross-persisting snapshots. Any subagent or agent that requires rehydration should use its own session manager.
Then this simplifies the problem to data mapping, i.e. we should not pass around the object but map the data between subagent and orchestrator. That is essentially what we should solve.
I've asked strandly-the-agent to leave some more details

[mkmeral]

We should remove snapshots. The session management and rehydration of agents are responsibility of session managers, so for a subagent to be able to rehydrate, it should have it's own session manager

[strandly-the-agent]

Details on mkmeral's two principles, measured in a sandbox at main 4f869c4f6 vs this PR 7d30fee9.

The data mapping is ~50 lines in _agent_as_tool.py alone — 0 lines in event_loop.py / types/_events.py, because both inputs already exist: the answers are persisted at _interrupt_state.context["responses"] (interrupt.py:105) and the parent is reachable as invocation_state["agent"] (agent.py:1357).

# up:   Interrupt(id=f"{tool_use_id}:{i.id}", name=i.name, reason=i.reason)   # prefixed copies
# down: parent's context["responses"] filtered by that prefix, stripped back to sub-agent-local ids

Verified equivalent to this PR on: single process with no session managers, restart with the sub-agent's own session, partial answers, 3-deep nesting, and colliding local interrupt ids. 1539 passed, 2 failed — both failures encode the object-alias assumption and need rewriting either way (test_stream_interrupt_yields_tool_interrupt_event, test_stream_interrupt_resume_forwards_responses).

One trap: gate on the sub-agent's interrupt state, not on the responses

Keying "am I resuming?" off having responses breaks partial answers — when the human answers sub-agent A but not B, B must be re-invoked with an empty response list so it re-raises and stays pending; keying off the responses made it reset and re-run instead. Keep _is_sub_agent_interrupted() as the gate and use the mapped responses as the payload.

Additionally require that the parent holds an interrupt id with this tool_use_id prefix. Without that check, a sub-agent sharing a session with another caller can rehydrate carrying someone else's activated interrupt and silently pick up their pending turn instead of doing its own job (reproduced).

preserve_context=False + rehydration: pair it with SnapshotSessionManager

as_tool() defaults to preserve_context=False, which currently rejects a sub-agent that has a session manager (_agent_as_tool.py:89-95). Allowing it needs one more tweak: the session restore runs inside Agent.__init__, so _initial_messages — the baseline _reset_agent_state resets to — is captured from the restored history, and the sub-agent then accumulates history on every call (3rd call started with 4 messages). Resetting to an empty baseline when a session manager is present fixes the live agent (~8 lines).

What remains is the store shape. Measured over call → resume → call → resume, each turn in a fresh process:

sub-agent's session manager	resumed call starts with	store after 4 turns
FileSessionManager (message log)	6 messages — the previous call's history	8 messages, grows without bound
SnapshotSessionManager (#3283)	2 messages — only its own suspended turn	1 blob, ~1.2 KB, constant

The log-based managers grow because append_message keeps its own monotonic index (repository_session_manager.py:78-82), so resetting the live agent's messages doesn't rewind the log. Both honour every approval, but only the snapshot manager keeps a "stateless" sub-agent actually stateless — worth stating in the docs when this lands.

Happy to push the branch with this shape (seanalbert as co-author) if that's useful.

[seanalbert]

@seanalbert thanks for the contribution. I think we can simplify this PR with a couple of design principles. First, the session management is per agent. We should avoid cross-persisting snapshots. Any subagent or agent that requires rehydration should use its own session manager. Then this simplifies the problem to data mapping, i.e. we should not pass around the object but map the data between subagent and orchestrator. That is essentially what we should solve. I've asked strandly-the-agent to leave some more details

@mkmeral Thanks for the feedback. The use case driving this is a stateless Lambda where every agent (orchestrator and sub-agents) is rebuilt from scratch each invocation. The sub-agents use preserve_context=False and don't have session managers. They're intentionally ephemeral, starting fresh each call.

My concern with requiring each sub-agent to have its own session manager is that SessionManager persists messages along with everything else. There's no way to opt into just having interrupts without also getting full conversation persistence, which conflicts with the stateless sub-agent pattern.

The snapshot in this PR is essentially a continuation for a single interrupted turn, stored inside the orchestrator's own interrupt context and discarded once the resume completes. The sub-agent never owns persistent state. But I may be missing a simpler way to model this. Happy to rework if there's a pattern that fits better here.

[mkmeral]

@seanalbert so I had a bit of a back and forth here, but I think I agree with you. I do still think attaching session managers is the right technical solution (because you are still persisting invocation state and current messages), I don't think it's good devx to force customers to manage it for statless subagents, when it can be so trivially solved.

so yes, I am aligned with this PR. that said I still think we can simplify it much more. We don't need to update the tool stream event or anything, we only need to take snapshot and return it as part of interrupt context. That way strands will manage persisting and rehydrating, and you will get the context next time. so we can rehydrate from there.

One thing is, I do still think session manager is the right call for agents that have preserve_context=True, I would not want orchestrator to manage it right now. I think that's a new feature, in terms of how we look at session management in multiagent. I'd rather focus on the bug ahead for now.

I'll ask strandly to post a comment with some more details based on our conversations and findings, so that we can iterate on the PR

[strandly-the-agent]

Concrete shape from the thread, split by direction. Everything below is measured on main 70e91166f.

Answers travel as data — no event_loop.py or types/_events.py change needed. resume() already persists them at _interrupt_state.context["responses"] (interrupt.py:105), and the parent is reachable as invocation_state["agent"] (agent.py:1357), so this half is self-contained in _agent_as_tool.py:

# up:   Interrupt(id=f"{tool_use_id}:{i.id}", name=i.name, reason=i.reason)   # prefixed copies, unique per call
# down: parent's context["responses"] filtered by that prefix, stripped back to sub-agent-local ids

The continuation is one keyed entry in the orchestrator's interrupt context, written by _AgentAsTool itself — ToolInterruptEvent stays untouched and the private snapshot attribute disappears. One catch before you implement it: the loop rebuilds that record when it parks (event_loop.py:722, and :863 in the AfterToolsEvent rescue), so a key a tool stashed beforehand is silently discarded — verified, the ephemeral case does not resume without addressing it. Making the record preserve keys the loop didn't create is ~4 lines, and it brings the Agent loop in line with how Graph already treats interrupt context.

Net size: _agent_as_tool.py +75/−3, event_loop.py +6/−2, types/_events.py untouched — versus +125/−19, +22/−2, +29/−3 today.

Graph already implements both halves — the shape to copy

	Graph	here
continuation lives in	orchestrator's _interrupt_state.context[node_id] (graph.py:742)	orchestrator's context, keyed by tool_use_id
what it holds	node agent's messages, state, interrupt_state, model_state (:748-755)	same fields via take_snapshot(preset="session")
restore	assigns them back (:1189-1192)	load_snapshot
answer routing	filters the orchestrator's context["responses"] to that node's interrupt ids (:1180-1184)	same, filtered by the tool_use_id: prefix
cleanup	none explicit — deactivate() clears the context	same

And the relevant detail: Graph never replaces its context dict — it only ever writes keys (context[node_id] = …, context["completed_nodes"] = …), which is why one node's entry survives when another parks. Same in Swarm (swarm.py:722, read back at :108). The Agent event loop is the odd one out in rebuilding the dict wholesale.

Two caveats so this isn't over-read: Graph stores raw fields rather than a versioned Snapshot (using take_snapshot here is strictly better on cross-version resume), and Graph/Swarm forbid node agents from having session managers at all (graph.py:297-298, swarm.py:540-541) — which is also why this pattern is the only option inside an orchestrator.

Scope, and what I verified

Why preserve_context=False is the right line (not "any sub-agent without a session manager"): it bounds what the orchestrator ever carries to a single ephemeral turn. Measured continuation size at the interrupt, after N previously completed calls to the same sub-agent:

sub-agent	0 prior calls	3	6
preserve_context=False	1512 B	1512 B	1512 B
preserve_context=True, no session manager	1512 B	3168 B	4824 B

The first row is flat because the sub-agent resets each call; the second grows with history it keeps in memory. The whole entry is freed when the interrupt resolves either way — orchestrator record 4690 B while pending, 452 B after resume.

Behaviour with the scope applied:

config	outcome
preserve_context=False, no session manager, restart	resumes from the orchestrator's continuation
preserve_context=True + own session manager, restart	resumes via data mapping; load_snapshot never runs, sub-agent's own message log untouched
preserve_context=True, no session manager, same process	resumes in memory
preserve_context=True, no session manager, restart	does not resume — needs a loud error naming the fix; note this configuration is already broken on main today, so it is not a regression

Also unchanged: partial answers (answer A, B stays pending), 3-deep nesting, colliding sub-agent-local interrupt ids, and single-process with no session managers anywhere. 1147 passed, 2 failed across tests/strands/{agent,event_loop,session,multiagent} + test_interrupt.py; both failures assert the old shared-object behaviour and need rewriting either way (test_stream_interrupt_yields_tool_interrupt_event, test_stream_interrupt_resume_forwards_responses).

One implementation order note: gating the continuation to preserve_context=False requires landing the mapping path in the same change. This PR removed _is_sub_agent_interrupted and _build_interrupt_responses, so the snapshot is currently the only resume path — gate it without adding the mapping and preserve_context=True stops resuming entirely, which is worse than main.

[seanalbert]

@mkmeral I am going on vacation for 2 weeks from tomorrow so I will address those comments on my return. If someone else has capacity to move this forward while I am away, please feel free to do so.

[strandly-the-agent]

Picked this up while seanalbert is away, per his note above and mkmeral's request: #3675 implements the simplification agreed in this thread, with seanalbert co-authored on the commit.

Shape: interrupts propagate as copies with ids namespaced by the outer tool_use_id; on resume the orchestrator's persisted context["responses"] are mapped back to sub-agent-local ids; an ephemeral (preserve_context=False) sub-agent's interrupted turn is stored as one keyed entry in the orchestrator's interrupt context and consumed on resume; a preserve_context=True sub-agent keeps its turn in its own session manager and fails loudly if it has none. ToolInterruptEvent is untouched, and the only event_loop.py change is that parking an interrupt no longer discards keys it doesn't own — 4 files, +529/−58.

It is a draft until two independent review passes I dispatched come back; findings and the fixes will be posted there. Happy to close this PR in favour of it, or to keep iterating here instead if you'd rather — mkmeral's call.