Repository navigation
Workflow: a node that returns None runs again when the workflow resumes after RequestInput #7666
Description
🔴 Required Information
Describe the Bug:
A workflow node that finishes without an output, route or state change runs a second time when the workflow resumes after a RequestInput. If that node has a side effect (sending an email, charging a card, writing a record), the side effect happens twice.
docs/guides/workflow/workflow/index.md says a resumed workflow "rebuilds what already ran from the session history, so completed nodes do not run a second time", and dynamic_nodes/index.md recommends moving side effects into ctx.run_node children because completed children are replayed rather than re-executed. A node that returns None and changes no state is not covered:
_NodeRunner._flush_output_and_deltas(src/google/adk/workflow/_node_runner.py) returns without emitting an event when there is no output, route or delta. So the session has no record that the node ran.- On resume,
Workflowlooks up the node's recorded execution (loop_state.recovered_executions), finds none, and starts the node fresh. The "static nodes with no outcome (e.g. return None) should be fast-forwarded" branch incheck_interception(workflow/utils/_replay_interceptor.py, Case 6) is never reached, because there is no recovered entry to pass to it.
The same happens for a dynamic child started with ctx.run_node that returns None: it runs again on resume, while a sibling that returns a value is replayed. It also happens with ResumabilityConfig(is_resumable=True).
Steps to Reproduce:
pip install google-adk==2.11.0- Run the snippet below.
send_emailreturns nothing, thenaskpauses for input, then the snippet answers it.
Expected Behavior:
after the first turn: ['email']
after answering: ['email']
Observed Behavior:
after the first turn: ['email']
after answering: ['email', 'email']
If send_email returns any value (e.g. return "sent"), it runs once, as expected.
Environment Details:
- ADK Library Version: 2.11.0 (also reproduced on
mainat d324744) - Desktop OS: macOS
- Python Version: 3.12.13
Model Information:
- Are you using LiteLLM: No
- Which model is being used: N/A (function nodes only)
🟡 Optional Information
Minimal Reproduction Code:
import asyncio
from google.adk.events._interrupts import extract_event_interrupt_ids
from google.adk.events.request_input import RequestInput
from google.adk.runners import Runner
from google.adk.sessions import InMemorySessionService
from google.adk.workflow import START, Workflow
from google.adk.workflow.utils._workflow_hitl_utils import create_request_input_response
from google.genai import types
emails_sent = []
def send_email(node_input):
emails_sent.append("email") # side effect, no return value
def ask(node_input):
return RequestInput(message="Approve?")
def finish(node_input):
return node_input
wf = Workflow(name="wf", edges=[(START, send_email, ask, finish)])
async def main():
runner = Runner(app_name="app", node=wf, session_service=InMemorySessionService())
session = await runner.session_service.create_session(app_name="app", user_id="u")
async def run(message):
events = []
async for event in runner.run_async(
user_id="u", session_id=session.id, new_message=message
):
events.append(event)
return events
events = await run(types.UserContent("go"))
request = next(e for e in events if extract_event_interrupt_ids(e))
interrupt_id = next(iter(extract_event_interrupt_ids(request)))
print("after the first turn:", emails_sent)
await run(
types.Content(
role="user",
parts=[create_request_input_response(interrupt_id, {"result": "yes"})],
)
)
print("after answering: ", emails_sent)
asyncio.run(main())How often has this issue occurred?:
- Always (100%)
Proposed fix (looking for guidance before I open a PR)
This changes what resume records, so I'd like to check the approach first.
- Record every completed node run (my preference). When a node finishes with nothing to flush, still emit a minimal completion event for it (no content, carrying its
node_infopath). Replay then finds a recovered entry with no output, and Case 6 fast-forwards it as the code already intends, for both static nodes andctx.run_nodechildren. The cost is one extra, empty event per such node in the session history, which UIs and event consumers would see. It could carry a marker so consumers can skip it. - Infer completion from what came after. If a later node in the same run has recorded events, its static predecessors must have completed, so fast-forward them. This adds no events, but it only covers static nodes on a path to a recorded node. It doesn't cover parallel branches that finished without output or
ctx.run_nodechildren. - Document it.
retry_config/index.mdalready says this for retries ("A child that produced neither leaves nothing to replay and runs again on every attempt, so give a side-effecting node an output or a state change"). The same note could go in the workflow and dynamic-nodes guides for resume. This needs no code change, but the failure stays silent, and it's easy to hit with a node that only performs an action.
Questions:
- Is an extra completion event per no-output node acceptable in the session history? If so, is there an existing event shape or marker you'd want it to use?
- Or would you rather keep the history as it is and go with option 2 or 3?
Once a direction is agreed, I'll open a PR with regression tests for the static, parallel and ctx.run_node cases, resumable and non-resumable.
Activity
xxwtiancai commentedon Oct 22, 2026
AI-assisted investigation for xxwtiancai. @AkshayShah03, I see your plan to implement the fix and that @sanketpatil06 is assigned. Would collaboration on offline regression tests and completion-replay coverage be useful? I won't duplicate the implementation.
Static inspection supports the observation that
_flush_output_and_deltasemits no event when there is no output, route, or delta; I haven't run the reproduction or tests yet. Dynamic replay and rehydration need checking together, so I wouldn't assume an empty event alone resolves every case.For the proposed minimal completion marker, how should replay distinguish successful completion with no output from an interrupted or failed execution? Once you agree on the event contract, I'd be happy to help test static nodes, parallel branches, and
ctx.run_nodechildren acrossRequestInputresume, with resumability both enabled and disabled.I'll wait for your preferred division of work before starting, since your proposed regression coverage already includes those cases.
xxwtiancai commentedon Oct 23, 2026
@AkshayShah03 @sanketpatil06, with AI assistance I prepared a narrow local test case at
37b1ab3cfor completion-event ownership. Six existing-behavior boundary checks passed, and a separate agent independently reran all six successfully.After Event JSON round-tripping, a descendant's empty event leaves an ancestor's failure intact and keeps it runnable through rehydration and replay. The checks cover full-path and direct-child grouping, including empty events with
output_formetadata, with a direct ancestor event as the control. They do not reproduce or fix the missing-completion-event bug, and I have not run the Runner/RequestInput matrix.With you retaining the implementation and core static/parallel/
ctx.run_nodetest matrix, would this boundary be useful as a test-only contribution, and where would you prefer it covered? I'll wait for agreement on that division before preparing an upstream change.AkshayShah03 commentedon Oct 23, 2026
Thanks @xxwtiancai. On distinguishing the cases: replay already tells them apart by what was recorded. A failed run carries an
error_code(Case 3 incheck_interceptionreruns it), and an interrupted one carries interrupt ids (Cases 2 and 5). A completion marker would carry neither, so it would land in Case 6 as a finished run. That said, the approach isn't settled yet. I'd like to wait for @sanketpatil06 / the maintainers to choose between the options before splitting up work or adding tests.sanketpatil06 commentedon Oct 27, 2026
Thanks @AkshayShah03, I reproduced this on
google-adk==2.13.0:send_emailruns twice after resume, and so does actx.run_nodechild that returnsNone.Option 1 sounds good, with a typed completion marker rather than an untyped empty event. Note that
check_interceptionalways re-runs non-static dynamic nodes that have no outcome (should_run = current_run is not None and not current_run.is_static). Soctx.run_nodechildren also need an explicit completion flag from the scan, which could work like the existingfinished_after_resume. Without it, they'll still run twice.Please go ahead with the PR and tests. @xxwtiancai's boundary tests are welcome too.
Workaround for now: return a value or set a state key from side-effecting nodes.
AkshayShah03 commentedon Oct 27, 2026
Thanks @sanketpatil06, I'm starting the PR with a typed completion marker for runs that finish without output, plus an explicit completion flag from the scan so
ctx.run_nodechildren are fast-forwarded too. Tests will cover sequential, parallel andctx.run_nodecases, with resumability on and off.@xxwtiancai your boundary tests would fit well as a follow-up on top of it. I'll link the PR here once it's up.
xxwtiancai commentedon Oct 27, 2026
Thank you @sanketpatil06 for the decision and the
should_run = current_run is not None and not current_run.is_staticdetail, and @AkshayShah03 for taking the implementation — I'll stay out of your way there.Happy to pick up the boundary tests as the follow-up once your PR is linked. The six checks I had prepared covered completion-event ownership under replay: a marker-only/empty descendant event leaving an ancestor's
error_codefailure intact and still runnable through rehydration and replay, full-path vs direct-child grouping, empty events carryingoutput_formetadata, and a direct-ancestor control. Since the typed completion marker changes what those events look like, I'll adapt them to the marker's actual shape rather than port them verbatim, and keep them compatible with your sequential/parallel/ctx.run_nodematrix. Will link the follow-up PR here once yours is up.AkshayShah03 commentedon Oct 27, 2026
PR is up: #7771. It adds a typed
NodeInfo.completedmarker for runs that finish without leaving any other trace, and acompletedflag on the scan state socheck_interceptionreplays suchctx.run_nodechildren instead of rerunning them. One case is left out, noted in the PR: actx.run_nodechild that emits content but no output still reruns. @xxwtiancai your boundary tests can go on top of it.xxwtiancai commentedon Oct 27, 2026
Thanks @AkshayShah03 — reviewed #7771 and rebased the boundary tests on top of
604d0cc3: all invariants hold against the marker implementation, nothing to report back as a problem.The follow-up now carries twelve cases: the six ownership boundaries (a descendant's empty event leaves an ancestor's
error_codeintact and rerunnable throughcheck_interceptionafter a JSON round-trip, both scan groupings,output_forempty events, direct-ancestor control) plus six marker-specific ones: a descendant's marker does not complete an ancestor (mirroring the empty-event rule) in either grouping, a direct marker records completion after a round-trip, a later interrupt un-completes the run, a recorded completion is replayed instead of rerunning, and your documented gap — content without output still reruns — is pinned as a labeled characterization test that flips when the gap closes. Fulltests/unittests/workflow/on top of your branch: 1277 passed.Holding the branch locally until #7771 lands so the follow-up PR applies cleanly to main — will link it here right after.
fix(workflow): record runs that finish without a result so resume rep…