feat: add snapshot to multi agent orchestrators#668
feat: add snapshot to multi agent orchestrators#668JackYPCOnline wants to merge 4 commits intostrands-agents:mainfrom
Conversation
src/multiagent/state.ts
Outdated
| nodeId: d.nodeId as string, | ||
| status: d.status as ResultStatus, | ||
| duration: d.duration as number, | ||
| content: (d.content as JSONValue[]).map((c) => contentBlockFromData(c as never)), |
There was a problem hiding this comment.
Issue: The as never cast suppresses type checking entirely.
Suggestion: contentBlockFromData accepts ContentBlockData type. Consider using that type explicitly:
content: (d.content as ContentBlockData[]).map(contentBlockFromData),This provides better type safety and documents the expected shape.
There was a problem hiding this comment.
Thanks for the update! Using ContentBlockData[] explicitly is much better for type safety. ✅
There was a problem hiding this comment.
Assessment: Request Changes
This PR introduces snapshot support for multi-agent orchestrators with well-designed state serialization and comprehensive test coverage. However, there are a few items that need attention before merging.
Required Actions
-
API Review Label: This PR introduces new public APIs (
takeSnapshot,loadSnapshot,MultiAgentSnapshotPreset,TakeMultiAgentSnapshotOptions) exported from the multiagent module. Per the API Bar Raising guidelines, please add theneeds-api-reviewlabel and include in the PR description:- Expected use cases for the snapshot feature
- Example code snippets demonstrating usage
- Complete API signatures with default parameter values
-
Documentation PR: This is a new feature that adds public API surface. Per the documentation review guidelines, a Documentation PR is required for new features with public APIs. Please add a "Documentation PR" section with a link to
https://github.com/strands-agents/docs/pull/...or provide justification if documentation is not needed.
Code Quality Suggestions
- See inline comments regarding type assertions in
state.ts(lines 81 and 261)
The implementation follows established patterns from the existing agent snapshot module and includes thorough test coverage with 54 tests.
|
Thanks for the update! I noticed the new commit that changes the default preset to Remaining items from previous review: The blocking issues from my previous review still need to be addressed:
Once these are addressed, I'm happy to re-review! |
There was a problem hiding this comment.
Assessment: Request Changes
Thanks for addressing the feedback! The code quality suggestions have been resolved and the needs-api-review label has been added.
Remaining Item
- Documentation PR: A link to the documentation PR in
https://github.com/strands-agents/docs/pull/...is still required for new features with public APIs. Please update the PR description with a "Documentation PR" section, or provide justification if documentation is not needed.
Once the documentation PR is linked, this is ready for approval from a code quality perspective.
|
Thanks for adding the example JSON structure to the PR description — this is very helpful for understanding the snapshot format and will be valuable for documentation! Current Status:
Remaining Item: From a code quality perspective, this PR is ready to go. Once the documentation PR is linked (or justification provided), I can approve. |
Description
Adds snapshot (state capture/restore) support for multi-agent orchestrators (Graph and Swarm), along with serialization (
toJSON/fromJSON) for all multi-agent state classes.State serialization (
state.ts)Adds
toJSON(): JSONValueandstatic fromJSON(data: JSONValue)methods to all four state classes:normalizeError), and optional structuredOutput. Omits absent optional fields from JSON.structuredOutputSchema(Zod schema is config, not state — must be re-provided by the caller).Multi-agent snapshot (
snapshot.ts)New module implementing
takeSnapshotandloadSnapshotfor Graph/Swarm orchestrators:Two presets:
session— lightweight: Sufficient for resume since agent nodes are isolated per-execution. THIS IS A PLACEHOLDER for now.full(default)— additionally captures per-node agent snapshots. NestedMultiAgentNodes are snapshotted recursively with properSnapshotenvelopes (orchestratorId + nodes, no state since nested execution state is ephemeral).takeSnapshot(orchestrator, state?, options?)— state parameter is optional (undefined for nested orchestrators). Nested recursive calls explicitly passappData: {}to prevent parent appData leaking into nested snapshots.loadSnapshot(orchestrator, snapshot)— validates scope (multiAgent), schemaVersion, and orchestratorId. Restores per-node agent snapshots when present. Nested orchestratorId validation is lenient (warn + skip, not throw) since stale nested snapshots shouldn't fail the entire load. ReturnsMultiAgentState | undefined.instanceof Agentguard — sinceAgentNode.agentreturnsAgentBase(notAgent), snapshot operations are guarded withnode.agent instanceof Agentto only snapshot concrete Agent instances that havemessages/stateproperties.Example
Exports (
index.ts)Exports
takeSnapshot,loadSnapshot,MultiAgentSnapshotPreset, andTakeMultiAgentSnapshotOptionsfrom the multiagent barrel.Test coverage
state.test.ts(24 tests) —toJSON/fromJSONround-trips for NodeResult (9), NodeState (3), MultiAgentResult (4), MultiAgentState (8). Covers completed/failed/cancelled results, structuredOutput with nested objects/null/primitives, multiple content block types, error serialization, schema exclusion, edge cases.snapshot.test.ts(30 tests) — takeSnapshot (14): session/full presets, appData, state omission, nested MultiAgentNode recursion, nested appData isolation, agentSnapshotOptions forwarding, Swarm support. loadSnapshot (11): state restoration, validation errors (scope/version/orchestratorId), unknown node warning, nested orchestratorId mismatch warning, null state handling. Round-trip (5): state fidelity, JSON.stringify/parse survival, full preset agent state preservation, nested graph through JSON serialization.Related Issues
Documentation PR
Type of Change
New feature
Testing
How have you tested the change?
All 54 new unit tests pass (
npx vitest run --project unit-node)0 type errors from new/modified files (
npx tsc --project src/tsconfig.json --noEmit)Pre-existing type errors unchanged (telemetry/config.ts)
I ran
npm run checkChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.