Skip to content

fix(workflow): stop delivering a workflow's output as two events - #669

Merged
kalenkevich merged 1 commit into
mainfrom
fix/workflow-duplicate-output-event
Aug 12, 2026
Merged

fix(workflow): stop delivering a workflow's output as two events#669
kalenkevich merged 1 commit into
mainfrom
fix/workflow-duplicate-output-event

Conversation

@kalenkevich

@kalenkevich kalenkevich commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Link to Issue or Description of Change

Problem:

WorkflowAgent announced the workflow's output as an extra event once the run settled. In the ordinary case that output is the terminal node's, and that node already emitted it on the way past — so a one-node workflow delivered its single result twice: once authored by the node, once by the agent.

Anything counting data events, or replaying them into a session, sees a duplicate. It also breaks the "at most one data event per node output" invariant that adk-python enforces with its _output_emitted / _output_delegated flags.

Solution:

The event cannot simply be dropped, because it carries a second, unstated contract: the workflow's output is the last output on the stream, which is how a consumer reads the result. Three cases depend on it:

  • a dynamicEntry returning something it computed rather than a child's value;
  • a node that sets ctx.output without ever emitting;
  • a later branch emitting after the node that produced the result, leaving the result buried mid-stream.

So: keep the contract, drop only the redundancy — announce the output unless it is already the last one delivered.

The decision also moves after the drain, which is what makes it exact. By then every node event has been seen, whereas root.output is only assigned once the whole workflow finishes, so a check made while events are still in flight would compare against a value that is not set yet.

Worth flagging for review: my first attempt skipped the event whenever the value had been emitted at all. The dynamic_nodes integration sample caught it — its dynamicEntry returns a headline that a node emitted earlier, then a scoring pass emits after it, so dropping the trailing event made the last output the score rather than the headline. That third bullet above is a real case, not a hypothetical, and there is now a unit test for it.

Longer term this is what nodeInfo.outputFor is for: adk-python marks the existing event as also serving as the ancestor's output instead of re-emitting. The field exists on the TypeScript Event but nothing writes it, so making it real is left to its own change.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Five tests in core/test/workflow/workflow_agent_test.ts. Two pin the fix (terminal-node output, and a dynamicEntry returning a child's value verbatim) and fail without it. Three guard against over-correcting — a computed dynamicEntry value, a node that assigns ctx.output without emitting, and the ordering case above — and pass either way, which is the point.

npm run ts:check          clean
npx vitest --project unit:core    209 files, 2865 passed
npm run test:integration           74 files, 206 passed | 8 skipped

Manual End-to-End (E2E) Tests:

Not run; covered by the workflow integration samples, which execute the real agents against recorded model responses.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

Independent of the other workflow PRs in flight. #654 and #641 both edit workflow_agent.ts but neither touches this block, so conflicts should be textual at worst.

`WorkflowAgent` announced the workflow's output as an extra event once the run
settled. In the ordinary case that output is the terminal node's, and that node
already emitted it on the way past -- so a one-node workflow produced its single
result twice: once authored by the node, once by the agent. Anything counting
data events, or replaying them into a session, saw a duplicate, and it breaks
the same "at most one data event per node output" invariant adk-python enforces
with its `_output_emitted` / `_output_delegated` flags.

The event cannot simply be dropped, because it also carries a contract: the
workflow's output is the last output on the stream, which is how a consumer
reads the result. Three cases rely on it. A `dynamicEntry` can return something
it computed rather than a child's value; a node can set `ctx.output` without
ever emitting; and a later branch can emit after the node that produced the
result, leaving the result buried mid-stream. The integration sample
`dynamic_nodes` is the third case, and it caught an earlier version of this
change that dropped the event whenever the value had been emitted at all.

So keep the contract and drop only the redundancy: announce the output unless
it is already the last one delivered. The decision moves after the drain, where
it can be exact -- by then every node event has been seen, whereas `root.output`
is only assigned once the whole workflow has finished, so a check made while
events are still flowing would compare against a value that is not set yet.

Longer term this is what `nodeInfo.outputFor` is for: adk-python marks the
existing event as also serving as the ancestor's output instead of re-emitting.
The field exists on the TypeScript event but nothing writes it, so that is left
for the change that makes it real.

@AmaadMartin AmaadMartin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve. The fix drops the redundant output event and keeps the "last output is the workflow output" contract. The !interrupted, root.output !== undefined, and Object.is guards are correct, and interrupted is read after await settle, so there is no race. I verified the source at head b2b9e66, the two-file diff, and the five tests. CI is green: run-tests passed on ubuntu, macOS, and Windows with real durations.

@kalenkevich
kalenkevich merged commit 7aa76e7 into main Aug 12, 2026
13 checks passed
@kalenkevich
kalenkevich deleted the fix/workflow-duplicate-output-event branch August 12, 2026 15:22
prasanna8585 pushed a commit to prasanna8585/adk-js that referenced this pull request Aug 21, 2026
…gle#669)

`WorkflowAgent` announced the workflow's output as an extra event once the run
settled. In the ordinary case that output is the terminal node's, and that node
already emitted it on the way past -- so a one-node workflow produced its single
result twice: once authored by the node, once by the agent. Anything counting
data events, or replaying them into a session, saw a duplicate, and it breaks
the same "at most one data event per node output" invariant adk-python enforces
with its `_output_emitted` / `_output_delegated` flags.

The event cannot simply be dropped, because it also carries a contract: the
workflow's output is the last output on the stream, which is how a consumer
reads the result. Three cases rely on it. A `dynamicEntry` can return something
it computed rather than a child's value; a node can set `ctx.output` without
ever emitting; and a later branch can emit after the node that produced the
result, leaving the result buried mid-stream. The integration sample
`dynamic_nodes` is the third case, and it caught an earlier version of this
change that dropped the event whenever the value had been emitted at all.

So keep the contract and drop only the redundancy: announce the output unless
it is already the last one delivered. The decision moves after the drain, where
it can be exact -- by then every node event has been seen, whereas `root.output`
is only assigned once the whole workflow has finished, so a check made while
events are still flowing would compare against a value that is not set yet.

Longer term this is what `nodeInfo.outputFor` is for: adk-python marks the
existing event as also serving as the ancestor's output instead of re-emitting.
The field exists on the TypeScript event but nothing writes it, so that is left
for the change that makes it real.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants