Skip to content

fix(graph): keep fan-in node out of resume while a parallel sibling is in-flight - #3390

Merged
opieter-aws merged 3 commits into
strands-agents:mainfrom
opieter-aws:opieter-aws/graph-fanin-inflight-sibling
Jul 22, 2026
Merged

opieter-aws merged 3 commits into
strands-agents:mainfrom
opieter-aws:opieter-aws/graph-fanin-inflight-sibling

Conversation

@opieter-aws

@opieter-aws opieter-aws commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Description

When a session-backed graph is serialized mid-execution, serialize_state runs from the per-node AfterNodeCallEvent hook — so it fires while sibling nodes in the same parallel batch are still executing. An in-flight sibling is in none of the terminal sets (completed / interrupted / failed), and _is_node_ready_for_resume treated any such untouched edge source as a bypassed dead branch. In a fan-in (AND-join), that dropped the in-flight parent's edge and marked the join node ready off its already-completed parent alone. On restore, the persisted frontier contained both the still-pending parent and the join node, so the join ran twice — its first run seeing incomplete inputs — duplicating side effects.

The fix distinguishes a live pending parent from a genuinely bypassed branch. _compute_pending_sources seeds from the nodes actually executing at serialize time (execution_status == EXECUTING) plus completed nodes, then walks traversable edges forward to collect every not-yet-completed node that will run on resume. A fan-in node stays out of the resume frontier while any such parent is outstanding. The executing-node seed is the authoritative in-flight signal and covers siblings with no completed ancestor (e.g. two concurrent entry points feeding one join), which forward reachability from completed nodes alone would miss.

Related Issues

Requirement for #3283

Documentation PR

N/A

Type of Change

Bug fix

Testing

Ran the multiagent unit suite (tests/strands/multiagent/), which covers the graph resume/serialize frontier logic, the interrupt-resume path, and the conditional-skip/bypass cases that must remain unaffected. Added regression coverage for both in-flight-sibling shapes (diamond with a shared root, and two independent entry points), each as a full serialize → deserialize → resume round-trip that captures a real mid-flight snapshot; both fail against the pre-fix code (frontier ['right', 'join'], join runs twice) and pass with the fix.

Ran the suite, ruff check, ruff format --check, and mypy on the changed files directly rather than via hatch run prepare, which fails in this environment building an unrelated optional cedar Rust dependency.

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

@github-actions github-actions Bot added size/m area-multiagent Multi-agent related bug Something isn't working labels Jul 21, 2026
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@opieter-aws
opieter-aws marked this pull request as ready for review July 21, 2026 18:29
@opieter-aws
opieter-aws requested a review from a team as a code owner July 21, 2026 18:29
@opieter-aws
opieter-aws requested review from arielnabavian and removed request for arielnabavian July 21, 2026 18:29
@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Approve

Well-reasoned fix for a subtle mid-flight-serialize concurrency bug. The distinction between a live pending parent and a bypassed dead branch is correct, and the change is strictly more conservative — adding pending_sources to live_sources can only add edges to the AND-join check, so it can never mark a fan-in ready incorrectly. Verified locally: the 4 new tests fail against main (frontier ['right', 'join']) and pass with the fix; full test_graph.py suite (77) passes; ruff and mypy are clean.

Review Categories
  • Correctness: Sound. The EXECUTING-status seed is valid because _compute_ready_nodes_for_resume runs only at serialize time (in-memory state), and it correctly covers entry-point siblings with no completed ancestor.
  • Code quality: Extracting _is_edge_traversable removes the prior duplicated short-circuit logic; docstrings thoroughly capture the reasoning and tradeoffs.
  • Testing: Strong — both fan-in shapes covered as full serialize → deserialize → resume round-trips with genuine mid-flight snapshots and call-count assertions; complements existing bypass/deadlock resume tests without duplication.
  • API: Private methods only — no public surface change, so API bar-raising does not apply.

Nicely scoped change with regression coverage that provably guards against the double-run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-multiagent Multi-agent related bug Something isn't working size/m

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants