Repository navigation
Description
Checks
- Checked against
mainat1b671d6a1a3609ae3f46bc20c599e4c9ad0db4cf(source checkout) - Checked the documentation — this contradicts the contract stated in the repo's own test docstring (
test_graph.py:3436-3443: "resume runs the fan-in exactly once with all inputs present") - Searched issues — closest is fix(graph): keep fan-in node out of resume while a parallel sibling is in-flight #3390, which added the very signal that turns out to be incomplete here; no duplicate
SDK Language: Python
Strands Version: main @ 1b671d6a (source checkout)
Language Runtime Version: Python 3.13.14
Operating System: Linux (aarch64)
Installation Method: from source
Summary
_compute_pending_sources (graph.py:1415) decides which not-yet-completed nodes are still "live" from a single signal:
executing_nodes = {node for node in self.nodes.values() if node.execution_status == Status.EXECUTING}A node that has been dispatched but has not yet reached node.execution_status = Status.EXECUTING (set at graph.py:1023, the first line of _execute_node's generator body) is therefore indistinguishable from a node that was never scheduled — a "bypassed dead branch". When a mid-batch session checkpoint lands in that window, the persisted next_nodes_to_execute wrongly includes a downstream fan-in node, and on resume that node executes a second time.
This is the same class of bug #3390 fixed. The EXECUTING seed it added closes the window for nodes that have started; it does not cover nodes that are dispatched-but-not-yet-started.
Preconditions
All three are needed at once:
- A fan-in whose parents share no completed ancestor — e.g. two independent entry points feeding one join. (In a
root -> {left,right} -> joindiamond the hole never opens: forward reachability from the completedrootstill seesright. Verified.) - A
SessionManager, or anyAfterNodeCallEventhook that persists, so state is written mid-batch —session_manager.py:55→repository_session_manager.py:321→serialize_state(). - A sibling that hasn't reached
EXECUTINGwhen that write happens.
On main today, (3) requires an executor whose body reaches its first await without suspending — e.g. an Agent whose Model.stream() is an async generator with no awaits (a cached, replayed, or local in-process model; Model is a public extension point). A network-backed Bedrock/OpenAI agent always suspends, so it does not trigger this. That makes the bug narrow on main — but not hypothetical, and it stops being narrow as soon as anything else delays a node's start.
PR #3448 (max_concurrency) is exactly such a delay: a node queued on the new per-batch concurrency semaphore stays PENDING for an unbounded time, so with that PR the precondition holds for any executor, including a normal network-backed agent. I'm filing this separately rather than against that PR because the defect is in _compute_pending_sources, which #3448 leaves byte-identical to main — it widens reachability, it doesn't introduce the bug.
Reproduction
Verified end-to-end through a real FileSessionManager with a hard os._exit the instant the session write lands (no cleanup, no later overwrite), then a fresh process to resume:
[write] completed=['left'] next_nodes=['right', 'join'] status=executing | right=pending
[write] ^^ crashing here (hard os._exit)
--- restart ---
[resume] snapshot on disk : next_nodes= ['right', 'join'] status= executing
[resume] restored frontier : ['right', 'join']
[resume] status : completed
[resume] execution_order : ['left', 'right', 'join', 'join']
[resume] join.stream_async : 2 call(s) <-- DOUBLE EXECUTION
Control — same topology and agents, sibling reaching EXECUTING normally:
[write] completed=['left'] next_nodes=['right'] status=executing | right=executing
[resume] join ran : 1 time(s) (correct)
Reproduced both with mock agents and with real strands.Agent instances (join agent messages : 4).
Two methodology notes that cost me time and are worth recording for whoever writes the regression test:
AfterNodeCallEvent.should_reverse_callbacksisTrue(events.py:385-387), so a hook added viagraph.add_hook()runs before theSessionManager's sync — reading the session file from such a hook gets a stale snapshot. The reliable interception point isupdate_multi_agent.- Cancelling the invocation instead of hard-exiting lets
AfterMultiAgentInvocationEventoverwrite the bad snapshot with a correct one, which masks the bug entirely.
Expected vs actual
Expected: a dispatched-but-not-yet-started node counts as in-flight, so the fan-in stays out of next_nodes_to_execute and runs exactly once on resume — the contract in test_graph.py:3436-3443.
Actual: the fan-in is persisted as ready, runs once off one parent, then _find_newly_ready_nodes re-fires it when the other parent lands. completed_nodes is a set, so the second run is absorbed silently: GraphResult.status == COMPLETED, no error, no warning. In my repro both runs saw both parents' outputs, so the final answer was still correct — the damage is doubled work: doubled cost and doubled side effects for a non-idempotent node.
Suggested direction
Make "dispatched" observable rather than inferring it from EXECUTING: either set the status (or a separate in-flight marker) at dispatch time in _execute_nodes_parallel, or seed _compute_pending_sources from current batch membership in addition to executing_nodes.
Worth a regression test in TestResumeInFlightSibling (test_graph.py:3285-3516) covering a dispatched-but-not-started sibling — the existing cases all seed EXECUTING directly (test_graph.py:3400), which is why none of them catch this.
Metadata
Metadata
Assignees
Labels
Type
Fields
Language
Activity
strandly-the-agent commentedon Aug 26, 2026
Update from the PR #3448 review cycle — two things worth capturing here.
1. A concrete, validated fix approach.
gnanirahulnutakkiproposed (and locally validated, 3/3) marking every node in the dispatch batchEXECUTINGsynchronously in_execute_nodes_parallelbeforecreate_task— rather than at the top of the task coroutine, which only runs at the task's first event-loop slice. That placement closes this issue's batch-dispatch window entirely: no created-but-not-yet-started task can be mistaken for a bypassed dead branch, because the marking no longer depends on the scheduler at all. Implementation notes from the review: place it after the interrupt re-entry filter and after the runtimemax_concurrencyvalidation in_execute_nodes_parallel. Origin thread: #3448 (comment)Independently validated (PR head
14f0b335+ the 5-line placement change): the non-suspending-executor repro goes from 12/12 bad-frontier + double-execution to 0/12, capped and uncapped (the queued sibling isEXECUTINGat every snapshot), and the full graph module still passes 100/100 with the change applied. Fail-fast/cancellation semantics are unchanged relative tomain: a dispatched-but-cancelled node ends upEXECUTINGin the failed snapshot and correctly re-runs on resume, same as an in-flight node does onmaintoday.2. Scope confirmation. PR #3448 at head
14f0b335no longer aggravates this bug: with suspending (i.e. realistic) executors, capped and uncapped both pass 15/15; the failure now needs the non-suspending-executor precondition described in this issue, and fires identically at 15/15 on the PR head (capped or not) and onmain(5f357aef). Differential matrix: #3448 (comment)The regression test for the fix should serialize from
AfterNodeCallEventwith a non-suspending first entry point (the sleep-based assertions in the existing suite pass even while the window is open), and assert both the frontier excludes the join and the restored join runs exactly once.fix(multiagent): add DISPATCHED status to prevent fan-in double-execu…
fix(multiagent): add DISPATCHED status to prevent fan-in double-execu…
fix(multiagent): add DISPATCHED status to prevent fan-in double-execu…