Repository navigation
feat(session-py): support snapshot session management for Graph and Swarm - #4240
Conversation
Extend the Snapshot Scope literal and VALID_SCOPES with "multiAgent" so orchestrator snapshots carry a distinct scope from single-agent ones and load_snapshot can reject a cross-scope load.
Replace the multi-agent rejection in SnapshotSessionManager with orchestrator persistence, mirroring the TypeScript SessionManager. On MultiAgentInitializedEvent the manager registers a lazy restore on BeforeMultiAgentInvocationEvent (guarded by a restored-ids set) and saves on AfterNodeCallEvent (under multi_agent_save_latest_on "node") and AfterMultiAgentInvocationEvent. Orchestrator state is wrapped in the versioned Snapshot envelope via a new multiagent/snapshot.py, latest-only under the multiAgent storage scope, with an orchestrator-id check on load.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Derive the expected scope set from the Scope Literal via get_args so it
tracks the type instead of hardcoding {"agent"}, which went stale when the
multiAgent scope was added.
|
@strandly-the-agent review |
1 similar comment
|
@strandly-the-agent review |
There was a problem hiding this comment.
Changes requested — the crash-between-nodes resume genuinely works (verified end to end), but two reachable defects in the restore path don't: an interrupted Graph/Swarm can never resume in a new process, and a failed restore silently overwrites the good checkpoint. The headline resume path also has no test, and the docs page still says this feature doesn't exist.
- 🔴 Lazy restore lands after the orchestrator has consumed the interrupt response → interrupted runs can't resume after a restart.
- 🔴 Orchestrator id is marked restored before the restore runs → a transient read error or corrupt/newer snapshot means the retry silently re-runs from scratch and overwrites the checkpoint.
- 🟡 Storage-required check is construction-order dependent; a per-node save failure aborts the run and masks the node's error;
multiagent/snapshot.pyreads public by accident; resume-through-the-manager is untested (3 delete-restore mutations survive 121 tests). - 🟡 Docs:
site/.../session-management.mdx:92,215still saySnapshotSessionManagerdoes not support Graph/Swarm; the PR template asks for docs in-PR. - Public surface changed (new kwarg, widened
Scope, new module, removedNotImplementedError) and there's noneeds-api-reviewlabel — it needs one. Not an API-meeting case.
Verified (head 4408ed0)
| ✅ Tests | pytest tests/strands/session tests/strands/multiagent tests/strands/agent/test_snapshot.py --ignore=…/a2a → 450 passed; the two touched files → 121 passed |
| ✅ Lint | ruff + mypy clean on changed files |
| ✅ Repros | both 🔴s reproduced with real Graph/Swarm + mocked models (no Bedrock), independently re-run; fix for 🔴#2 applied locally → 121 pass, C1 scenario flips to resuming ['b','c'] with the snapshot intact |
| ✅ Works | linear Graph crash-after-node resume (runs only the remaining nodes); Swarm crash after 1st/2nd node resume; completed-then-reinvoked Graph and Swarm start clean with the new task |
| ✅ Cleared | register_hooks override leaks none of the base's raising sync_multi_agent/initialize_multi_agent stubs (registry dump); delete_session covers multiAgent keys; no dangling refs to _reject_multi_agent |
| adversarial pass produced scripts/output but timed out before reporting (results folded in where corroborated); API/DevX pass ran on the default model tier | |
| 📎 Artifacts | repro scripts + captured output, mutation-test results, and 9 verified proposed tests are uploaded to the Strandly portal for this PR |
Issue alignment vs #3662 — partially aligned
Every letter item is implemented at TS parity (key layout byte-identical; lazy restore + restored-ids guard; node/invocation cadence registering AfterNodeCallEvent only under node; versioned envelope + id check). The gap is proof and prose.
In-contract: types/_snapshot.py:20,31 · multiagent/snapshot.py:22-59 · snapshot_session_manager.py:71-83, :159-168, :240+:275-279, :318-368, removal of _reject_multi_agent.
Supporting: the new tests, test_snapshot.py:304, docstrings :162, :218-221, :253-254.
Out-of-contract: multiagent/snapshot.py as a non-underscore module with public-looking functions (see inline). Nothing else — the issue's follow-ups (migration, deprecation, #3633 fix, rewind) are correctly absent.
| Acceptance bar | Status |
|---|---|
| (a) crash/restart stopping between nodes | ❌ no test drives restore through the manager (behaviour verified working by probe) |
| (b) completed-then-reinvoked | :205 exists but the re-invoke assertion result.status is not None can't fail; bypasses the manager |
| (c) serialize→deserialize→serialize invariant | main (test_swarm.py:1288, test_graph.py:3211); envelope round-trip verified by probe |
| (d) four landmines | 2/3/4 fixed on main and correctly relied on; #3633 still open, its precondition is this PR's default cadence, and the diff doesn't mention it |
Questions
Blocking
- Ownership. #3662 is agent-filed with zero maintainer comments, and you raise the layer question yourself. Closest signal on record is
zastrowmon #1230 endorsing a snapshotting session manager as the TS design. Is a maintainer ack on #3662 the real gate here? - Storage contract. Agents get a
LocalFileStoragefallback; orchestrators hard-fail — but only if no Agent resolved storage first. Is the intent "storage required for orchestrators", or "never guess a backend"? (If the latter, is the agent fallback the thing that's wrong?) - #3633. The default
"node"cadence is precondition #2 of the open fan-in double-execute, and we reproduced it through this manager (checkpoint frontier['a','c']with siblingastill PENDING →cran twice on resume). This PR adds no new exposure — the repository managers already persist per node with no opt-out — so "caveat + ship" is defensible. Land #3633's fix first, or document the caveat in theMultiAgentSaveLatestStrategydocstring and link it from the PR? ResolvesvsPart of. Latest-only is in-contract, butResolvesauto-closes the issue with its rewind use case undelivered. KeepResolvesand re-file rewind onto #2777, or downgrade?
Non-blocking
5. Is cross-SDK reading of snapshot payloads a goal, or only path compatibility? data uses orchestrator_id vs TS orchestratorId, so the byte-identical key holds payloads the other SDK can't read. That answer also settles the camelCase "multiAgent" literal vs AGENTS.md:52, and deserves a team/DECISIONS.md line either way.
6. sync_multi_agent/initialize_multi_agent still raise "does not support multi-agent persistence" on a manager that now does. Intentional, or worth overriding for API honesty?
Reading order
src/strands/types/_snapshot.py:20,31— theScope/VALID_SCOPESwidening; smallest hunk, highest criticality (persisted cross-SDK format).src/strands/multiagent/snapshot.py— the envelope contract:take_snapshot:22,load_snapshot:40with its three rejection checks.src/strands/session/snapshot_session_manager.py:318-368— the whole feature:_init_multi_agentwiring → lazy restore + guard (:336-342) → save handlers → restore/save internals. All inline findings live here.- Same file, supporting surface:
MultiAgentSaveLatestStrategy:71-83, key builder:159-168, class docstring:218-221, ctor kwarg:240/:275-279. tests/strands/session/test_snapshot_session_manager.py— write path:89-125, envelope + restore:128-223, wiring:226-265.tests/strands/agent/test_snapshot.py:304— the only change outside the feature.
Appendix — non-blocking (19) + pre-existing
Robustness
- 🟡
snapshot_session_manager.py:363-368— sibling nodes in a parallel Graph batch each save the whole snapshot to one key with no ordering/CAS; on S3 (put_objectviato_thread)snapshot_latestcan go backwards (simulated:[['p1','p2'], ['p1'], …]).LocalFileStorageis safe (tmp +os.replace). Fix:asyncio.Lockper orchestrator across serialize+write. - ⚪
:159-166—_multi_agent_latest_keylacks the non-empty/./..guard the ctor applies tosession_id;id=""collapses the path segment. Build the key once in_init_multi_agentso a bad id fails at construction, not mid-run. - ⚪
:339— restored-ids keyed onorchestrator.id; two unnamed orchestrators (default_graph) on one manager share a key and the second never restores. Warn when an id is wired twice. - ⚪
multiagent/snapshot.py:40— a Graph and Swarm sharing an id:KeyError: 'context'on restore;load_snapshotchecks scope and id but notstate["type"].
Tests (mutation-tested: 18 applied, 8 survived; 9 verified replacement tests are in the uploaded artifacts)
5. 🟡 :332-333, :344-350 — neither cadence is proven, only hook registration; both persisted-after-run tests use one-node orchestrators. A write-count test (node→3, invocation→1) kills three surviving mutations; pattern exists at :481-501.
6. 🟡 :226-250 — registration tests assert event types only; swapping save/restore handlers keeps everything green. Assert (event_type, callback) pairs.
7. 🟡 multiagent/snapshot.py:51 — deleting snapshot.validate() breaks no test; schema-version rejection unproven.
8. ⚪ :223 — assert result.status is not None is vacuous (Status is non-optional). Pin COMPLETED, one fresh turn of history, and the new task.
9. ⚪ multiagent/snapshot.py:36 — app_data untested and stored by reference, while Agent.take_snapshot deep-copies (agent/agent.py:2024). A behavioural decision, not just a test gap.
10. ⚪ No tests/strands/multiagent/test_snapshot.py mirroring the new module (strands-py/AGENTS.md:267).
11. ⚪ test_snapshot.py:304 — set(VALID_SCOPES) == set(get_args(Scope)) is self-consistency, not value-pinning; == {"agent", "multiAgent"} costs nothing.
12. ⚪ No integ coverage for orchestrators (tests_integ/test_snapshot_session.py); "node" cadence against real S3Storage is the case a unit test can't reach.
API / docs
13. 🟡 session/__init__.py:19-28 exports SaveLatestStrategy but not MultiAgentSaveLatestStrategy, which the ctor docstring points users at (:254). Package docstring :5-9 still says "persists a single agent".
14. ⚪ Ctor Raises: (:260-263) omits the new multi_agent_save_latest_on ValueError (strands-py/AGENTS.md:118); _restore_multi_agent's bool return is unused.
15. ⚪ multiagent/snapshot.py:5 — "mirrors TS"/"byte-identical" is over-broad given orchestrator_id vs orchestratorId; narrow the claim to the key layout.
16. ⚪ types/_snapshot.py:20,31 — first camelCase literal value in a public Python type vs AGENTS.md:52; decide explicitly and leave a one-line comment citing the rule.
17. ⚪ :71-79 — a caveat naming #3633 and the fan-in condition under which "node" can double-execute on resume.
18. ⚪ Resolves #3662 over-claims the rewind use case (Q4).
19. ⚪ Docs layout block session-management.mdx:558-571 lacks multiAgent/, unlike the TS block at :593-607.
Pre-existing in graph.py resume (reproduce identically with FileSessionManager; not this PR's problem, will file as issues)
- A resumed Graph reports
FAILEDeven when every node succeeds:_from_dictrestoresfailed_nodes(graph.py:1476-1478) and never clears an entry that later succeeds. Lands on this PR's headline flow — a clean crash-restart resume returnsFAILED. - A completed Graph with a never-taken conditional edge serializes a non-empty frontier (
_compute_ready_nodes_for_resumehas noCOMPLETEDshort-circuit,graph.py:1327-1329), so the next process resumes that branch with the old task. - A Graph whose topology changed between deploys runs zero nodes and reports
COMPLETED:_from_dictfilters unknown frontier ids (graph.py:1509) but keeps_resume_from_session = True; Swarm guards this (swarm.py:1129-1134). - Fan-in double-execute through this manager's default cadence → already open as #3633; confirmation belongs there.
Restore Graph and Swarm snapshot state before applying interrupt responses so a fresh process can resume persisted human-in-the-loop interrupts. Mark orchestrators restored only after a successful read so transient failures remain retryable. Add manager-level regression tests for Graph and Swarm interrupt restart and for retrying a failed restore.
Match the current TypeScript SessionManager ordering by marking an orchestrator restored before loading its snapshot. Remove the Python-only retry regression test; track that cross-SDK recovery defect separately. Keep the Graph and Swarm interrupt-resume ordering and tests because those mirror the existing TypeScript lifecycle behavior.
# Conflicts: # strands-py/src/strands/session/snapshot_session_manager.py # strands-py/tests/strands/session/test_snapshot_session_manager.py
|
Assessment: Comment (mostly ready — a few loose ends) This revision cleanly resolves the two headline defects from the prior review: the module is now private ( Still open after this revision
The mark-restored-before-restore ordering (🔴 in the open thread) I'm leaving to that discussion — the TS-parity rationale and follow-up plan are reasonable. Overall this is a well-structured, low-complexity port that matches the TS layout closely. |
# Conflicts: # strands-py/src/strands/session/snapshot_session_manager.py
Keep the persisted multi-agent scope aligned with issue #3662 and the TypeScript on-disk layout. Strengthen completed Swarm restoration coverage to prove the fresh task executes.
|
Assessment: Comment (re-review of latest revision) Good iteration — most open threads are now resolved. Resolved this round
Still open (minor)
Core feature and the interrupt-ordering fix look solid; only a docstring line and a docs follow-up remain. |
|
@strandly-the-agent Could you give this PR another review. |
strandly-the-agent
left a comment
There was a problem hiding this comment.
Approving at 53e1862. Of my six findings, one 🔴 and two 🟡 are fixed and verified; the other 🔴 and two 🟡 were deliberately kept for TS parity with tracking issues promised. Nothing new rises above ⚪. Three things to land before merge, none of which need another review from me:
- Link the two tracking issues you promised (restore-guard ordering; parallel-node write race) in their threads — neither exists yet as far as I can find, and the restore-ordering one ships a known silent-rerun-on-retry path.
- Ctor
Raises:(snapshot_session_manager.py:251-254) still omits themulti_agent_save_latest_onValueError. - Public surface changed and there's still no
needs-api-reviewlabel.
Status of previous findings
| # | Finding | Status |
|---|---|---|
| 🔴 1 | Lazy restore after interrupt response consumed | Fixed (933a5fe): _interrupt_state.resume(task) now runs after BeforeMultiAgentInvocationEvent in graph.py:649-650 and swarm.py:424-425; new cross-process interrupt tests for Graph and Swarm drive restore through the manager. Re-verified: crash-after-n1 on a 3-node chain resumes in a fresh manager+graph with n1 not re-executed. |
| 🔴 2 | Id marked restored before restore runs (:176-179) |
Open by choice — kept for TS parity, TS-first fix + port promised. Still reproducible; I'm not re-blocking on a decision you've explained, but please link the tracking issue. |
| 🟡 3 | Storage check order-dependent | Withdrawn for parity — accepted. |
| 🟡 4 | Per-node save failure aborts run | Withdrawn for parity — accepted. |
| 🟡 5 | multiagent/snapshot.py public by accident |
Fixed (af67b17): _snapshot.py. |
| 🟡 6 | Restore through the manager untested | Fixed (933a5fe): the interrupt tests construct a fresh manager and restore via _on_before_multi_agent_invocation. A non-interrupt crash-between-nodes test would still be a nice addition but isn't needed. |
| 🟡 docs | Site docs say unsupported | Still true (session-management.mdx:92,215), deferred to a follow-up by agreement — fine, but please open it so it isn't lost. |
Verified (head 53e1862)
pytest tests/strands/session tests/strands/multiagent tests/strands/agent/test_snapshot.py tests/strands/test_interrupt.py --ignore=…/a2a→ 492 passed; ruff clean on changed files (mypy errored internally in my sandbox — environmental).- Scope back to
multiAgentwith the one-line comment;MultiAgentSaveLatestStrategyexported; package docstring fixed;test_completed_orchestrator_reinvokes_from_scratchnow assertsCOMPLETED+ new task. - The reorder in
graph.py/swarm.pyis the only core change; all existing interrupt/resume tests pass.
Appendix — non-blocking
- ⚪
graph.py:649-650/swarm.py:424-425— side effect of the reorder: ifresume(task)raisesTypeError(plain-string task while an interrupt is active),BeforeMultiAgentInvocationEventhas fired butAfterMultiAgentInvocationEventnever does (previously neither fired). Verified. Only matters to hooks that pair before/after; if you care, validate the task before dispatching the event. - ⚪ Pre-existing
graph.pyresume bugs from my first review (resumed run reportsFAILEDafter a successful retry; completed Graph with a never-taken conditional edge resumes the old task; topology change runs zero nodes) are still unfiled — the first one shows up in the resume path above (run2 status: FAILEDwith every node completed). Happy to file them if you'd like.
|
Assessment: Comment (re-review at The docs commit resolves the last of my non-parity items — Remaining open — two deferred parity items (for maintainer merge decision)Both are being deferred to separate cross-SDK issues on TS-parity grounds, which is a defensible stance — but they are real defects, not cosmetics, so they should be tracked+linked before merge:
Minor: one inline note on the constructor |
Documentation Preview ReadyYour documentation preview has been successfully deployed! Changed pages: Updated at: 2026-09-18T18:31:52.989Z |
Description
SnapshotSessionManager(added in #3283) is single-agent only — attaching it to a Graph or Swarm raisesNotImplementedError, leaving orchestrators on the message-log managers (FileSessionManager/S3SessionManager), which have no checkpointing and cannot resume from a snapshot after a crash or restart. Applications that run both single agents and orchestrators are forced to mix two session systems with two on-disk formats, and the legacy managers cannot be deprecated while they remain the only option for orchestrators.This brings the Python SDK to parity with the TypeScript
SessionManager, which already persists Graph and Swarm throughinitMultiAgentwith a node/invocation save strategy. Python uses the spec-definedmultiAgentstorage scope for a consistent cross-SDK on-disk format. A long-running Graph that crashes on node 40 of 60 now resumes from the last completed node instead of re-running from the start, and a singleStoragebackend can hold both an app's agents and its orchestrators.Two decisions worth surfacing for review. State is restored lazily on the orchestrator's first invocation (guarded so it runs once), rather than eagerly at construction — matching TS and deferring the storage read until it is actually needed. Orchestrator state is wrapped in the versioned
Snapshotenvelope rather than persisted as a bare dict, so the format can evolve, and the load path rejects a snapshot whose orchestrator id does not match the target.One open question for maintainers: the issue notes overlap with durable execution / checkpointing (#2777, #1230) and asks which layer should own orchestrator resume. This implements it at the session-management layer to match TS; if that ownership belongs elsewhere, it is worth settling before merge.
Related Issues
Resolves #3662
Documentation PR
Updated the session management guide with Python Graph configuration and save-strategy behavior.
Type of Change
New feature
Testing
hatch run prepareAdditionally exercised end to end: a Graph and a Swarm run with the manager attached persist their state to the
multiAgentscope, a fresh orchestrator restores from that snapshot on its first invocation, and a completed orchestrator re-runs from the start rather than resuming a terminal frontier.Checklist
Public API Changes
SnapshotSessionManagergains amulti_agent_save_latest_onoption and no longer rejects orchestrators:multi_agent_save_latest_on: Literal["node", "invocation"] = "node"chooses whether the orchestrator snapshot is saved after every node (a mid-run crash resumes at the last completed node) or only at invocation boundaries (lower I/O for a large graph on remote storage). Orchestrators are persisted latest-only under a distinct scope,session/<session_id>/scopes/multiAgent/<orchestrator_id>/snapshots/, so single-agent snapshots (scopes/agent/...) are unchanged and the two never collide. TheSnapshotscopeliteral additionally accepts"multiAgent". Storage must be supplied to the constructor when the manager backs an orchestrator, since an orchestrator has no agent-level storage to resolve from.