[opieter-aws]

Description

Swarm and Graph persist their run state via serialize_state() / deserialize_state() so a session can be resumed after a restart. Two round-trip defects meant that resuming silently discarded state.

Swarm shared context was erased on the next save. serialize_state reads shared context from the SwarmState-owned object while the restore path wrote it to the Swarm-owned object. During a live run these are the same object (stream_async binds them when it builds a fresh SwarmState), but the resume path never rebuilds that state — so after deserialize_state, the restored context lived on self.shared_context while serialize_state kept reading an empty self.state.shared_context, and the next save persisted {}. The fix re-establishes the object identity a live run relies on, so the serialize path and the node-input builder read the same context after resume.

Cumulative accounting reset to zero on both orchestrators. serialize_state never persisted execution_time / accumulated_usage / accumulated_metrics (plus execution_count on Graph), and the restore path never rehydrated them. After a resume these reset to their defaults, which corrupts the total-execution timeout budget (should_continue adds accumulated execution_time to fresh wall-clock) and under-reports the token/metric totals in the final result. These fields are now serialized and restored, so a resumed run continues accounting from where it left off rather than starting over.

Swarm restores three fields; Graph restores four — Graph tracks execution_count explicitly, whereas Swarm derives it from node_history, which already round-trips.

Related Issues

Requirement for #3283

Documentation PR

N/A

Type of Change

Bug fix

Testing

Added a serialize → deserialize → serialize round-trip regression test for each orchestrator, asserting the second serialize equals the first for shared context (Swarm) and the cumulative fields (both). Confirmed both tests fail on the pre-fix source and pass after.

Beyond the round-trip invariant, I drove the full restore → resume → run path end to end: restored a mid-run payload into a fresh instance, called invoke_async, and confirmed the resumed run accumulates on top of the restored totals (e.g. restored 33 tokens + 30 from the resumed node = 63; Graph execution_count 3 → 4) and that the Swarm node receives the restored shared context during execution. Ran the full multiagent unit suite and ruff.

Note: hatch run prepare could not run locally — the dev env fails to build an unrelated optional dependency (cedar-policy-mcp-schema-generator, needs rustc 1.89). The full gate will run in CI.

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

[github-actions]

Assessment: Comment (approve-leaning — no blocking issues)

Well-targeted fix for two real round-trip defects. I confirmed the swarm resume path (stream_async) does not rebuild SwarmState, so re-binding state.shared_context to the swarm-owned object is both necessary and correct, and the added cumulative fields are backward-compatible (deserialize defaults missing keys to zero/{}). Both new tests pass on the branch.

Review notes

• Testing conventions: Test docstrings narrate prior behavior ("Before the fix…"), which conflicts with the repo's evergreen-comment rule for regression tests — they should state the guaranteed invariant and link the guarded issue (feat: add snapshot session manager to python #3283).
• Test strength: The round-trip assertions are split per-field; a single full-dict equality would guard every serialized field, not just the ones changed here.
• Minor: serialize_state embeds live cumulative dicts by reference (mutated in place during a run); a shallow copy would be safer, though this mirrors the existing shared_context handling.

Nice job pinning down the object-identity subtlety and driving the full restore→resume→run path end to end.

[mkmeral]

@strandly-the-agent review pls

[strandly-the-agent]

🔴 Changes requested: persisted execution time still drops the current invocation at crash checkpoints, and Graph returns prior-only elapsed time after resume.

✅ Reviewed exact head ac77e2501b701fbfedbe9de3ab55504d3e53f542 · 112 focused tests passed · changed-file Ruff/format + diff check passed · exact-head CI green.

🔴 Two execution-time lifecycle blockers are inline; both were reproduced deterministically.

Review passes and verification

• Correctness/adversarial: repeated the FileSessionManager process-crash repro twice; both checkpoints stored execution_time=0 after a 250ms node, and both resumed past a 150ms total timeout. A separate mocked-clock probe restored 555ms, ran 600ms, returned 555ms, then left state at 1155ms.
• Surviving probes: restored usage/metrics/execution count accumulated correctly; Swarm restored shared context retained identity and reached resumed model input.
• Tests: hatch test -q tests/strands/multiagent/test_graph.py tests/strands/multiagent/test_swarm.py → 112 passed in 2.54s.
• Static checks: Ruff check/format passed on all four changed files; git diff --check passed. The full-tree local format check found 11 unrelated files outside this diff; GitHub Python Lint and CI Gate are green at this SHA.
• Suppressed: resolved threads about full-dict equality, shallow copies, and evergreen docstrings; weaker test/doc observations subsumed by the reproduced defects.
• Routing: correctness, adversarial, test-quality, and docs passes ran; API/LLM-context were not applicable because no public/model-facing surface changed.

[opieter-aws]

@strandly-the-agent re-review latest fixes

[github-actions]

Assessment: Approve (re-review of ef85b23)

The latest commit resolves both of @strandly-the-agent's execution-time lifecycle blockers. The fix centralizes the logic in two MultiAgentBase helpers — _execution_time_with_active_interval (adds the in-flight interval for any read: checkpoints and result building) and _commit_active_interval (folds it into the committed total exactly once at finalization, idempotently). Clean, well-documented, and symmetric across Swarm and Graph.

Verification

• Checkpoint (Blocker 1): serialize_state now persists prior-committed + elapsed, so a mid-run AfterNodeCall sync keeps the timeout budget. Guarded by test_{graph,swarm}_checkpoint_persists_in_flight_execution_time.
• Resumed result (Blocker 2): _build_result folds in the active interval; test_graph_execution_time_reflects_active_invocation asserts 555ms restored + 1200ms in-flight = 1755ms in both GraphResult and state, committed once (no double-count).
• Edge cases: exception path commits without building a result (accounting preserved); nested-graph propagation covered; _commit_active_interval idempotency prevents double-counting.
• Tests: test_graph.py + test_swarm.py → 115 passed locally on ef85b23.
• Earlier suggestion threads (full-dict equality, shallow copy) were discussed and reasonably resolved; evergreen docstrings on the new tests now state the guaranteed invariant.

Nice turnaround on the lifecycle subtlety — extracting the interval math into shared helpers made both the checkpoint and result paths correct in one place.

[strandly-the-agent]

🔴 Changes requested: both prior execution-time blockers are verified fixed at ef85b239, but tracing setup failures leave the new active timer running and corrupt later checkpoints.

✅ Exact head ef85b239f21c019ba16fdba1fb6720f25365bc85 · 115 focused tests passed · changed-file Ruff/format + diff check passed · exact-head CI green.

✅ Prior blockers resolved: the real FileSessionManager crash/restart repro now checkpoints 251ms and correctly rejects B under a 150ms total timeout (repeated twice); restored/nested timing now commits and propagates exactly once.

🔴 One new-delta lifecycle blocker is inline.

Review passes and verification

• Correctness/adversarial: exercised process restart, restored and nested Graph timing, repeated serialization, final-hook errors, execution errors, cancellation, early generator close, back-to-back invocations, and Swarm parity. No double-counting or marker leak occurred after entering the protected span.
• New failure repro: start_multiagent_span() and span context entry failures each leaked the marker for Graph and Swarm; repeated twice identically. At fake t=1000, serialization at t=1000.5 reported 500ms after the invocation had already raised. The prior head remained at 0.
• Tests: hatch test -q tests/strands/multiagent/test_graph.py tests/strands/multiagent/test_swarm.py → 115 passed in 2.74s.
• Static/CI: Ruff check and format passed on all five changed files; git diff --check passed; GitHub Python 3.10–3.14 matrices, integration, lint, docs, and CI Gate are green.
• Routing: correctness, adversarial, and test-quality passes ran. API/LLM-context/docs were not applicable. The advanced adversarial tier was unavailable under data-retention mode; its default-tier retry completed.
• Suppressed: resolved full-dict equality, shallow-copy, and evergreen-docstring threads; the weaker Swarm lifecycle test gap is subsumed by the reproduced tracing failure and requested regression test.

[opieter-aws]

@strandly-the-agent re-review

[github-actions]

Assessment: Approve (re-review of aca610f)

The only change since my last pass (ef85b23) is a clean merge of latest main into the branch — no new source changes to the multiagent code and no conflict markers. Re-verified that the execution-time lifecycle fix survives the merge intact:

• _execution_time_with_active_interval / _commit_active_interval helpers unchanged in base.py; wiring intact in graph.py (finally-commit + checkpoint/result reads) and swarm.py (finally-commit + checkpoint read, result built post-commit).
• tests/strands/multiagent/ → 129 passed locally against main.
• ruff check clean on base.py, graph.py, swarm.py.

No new issues. Both previously-blocking findings remain resolved; PR stays approvable pending a maintainer formally clearing @strandly-the-agent's "Changes requested" status.

[github-actions]

Assessment: Approve (re-review of aca610f)

All three of @strandly-the-agent's execution-time lifecycle blockers are now resolved and each is locked down by a targeted regression test. The latest change fixes the third blocker (tracing-setup failure leaking the timer) by moving the _invocation_start_time marker inside the span context so its clearing finally is guaranteed to run.

Verification

• Blocker 3 (timer leak on tracing failure): marker set at graph.py:667 / swarm.py:417 — after start_multiagent_span() + use_span().__enter__(), immediately guarded by try/finally. If tracing setup raises, the marker is never set, so a later serialize_state() doesn't accrue wall time against an abandoned invocation. Guarded by test_{graph,swarm}_tracing_setup_failure_does_not_leak_timer.
• Blocker 1 (crash checkpoint) & Blocker 2 (resumed result): remain fixed via the _execution_time_with_active_interval / _commit_active_interval helpers; verified previously.
• Merge: aca610f merges main cleanly; no regressions in the changed modules.
• Gate: test_graph.py + test_swarm.py → 121 passed locally; ruff check clean on all changed files.

Solid handling of a genuinely subtle lifecycle seam — placing the timer start inside the span context keeps the start/commit symmetric and closes the failure-path leak.

Note: @strandly-the-agent's review is still in "Changes requested" state at the review level; only that reviewer or a maintainer can formally dismiss it before merge, though all its threads are now resolved.

[strandly-the-agent]

✅ Approved: the timer-marker leak is fixed for both Graph and Swarm; no findings remain at aca610f4b29118db4c9e20ef3a65729292ac469c.

✅ 121 focused tests passed · Ruff/format/diff-check clean · exact-head CI green · tracing failure matrix and real crash/restart repros passed repeatedly.

Re-review evidence

• Prior blocker: verified resolved. Graph and Swarm now publish _invocation_start_time only after successful span entry; span creation and use_span.__enter__ failures both leave the marker unset and serialization stable.
• Cold adversarial checks: Graph/Swarm × both tracing failure phases passed in two local runs (8/8 cases). FileSessionManager crash/restart passed twice: a 250ms node persisted 251ms, and resume correctly rejected B under a 150ms total timeout.
• Lifecycle checks: restored/nested totals, repeated serialization, cancellation, early close, node/final-hook errors, and back-to-back invocations showed no leaks or double-counting. Main #3390 resume-frontier interactions also passed.
• Merge provenance: aca610f4 matches the synthetic merge of fix commit 6c9ca250 with main 7b0717c4; no conflict-resolution hunk altered the timer fix.
• Tests/static: hatch test -q tests/strands/multiagent/test_graph.py tests/strands/multiagent/test_swarm.py → 121 passed in 2.91s. Ruff check/format and git diff --check passed; worktree stayed clean.
• CI: Python 3.10–3.14 Linux/Windows/macOS, integration, lint, docs, dependency review, and CI Gate all succeeded at this SHA.
• Routing: correctness, adversarial, and test-quality passes ran; API/LLM-context/docs were not applicable. The advanced adversarial tier was unavailable under its data-retention restriction; the default-tier retry completed.