Skip to content

fix(runner): run onEventCallback on beforeRunCallback early exits - #1148

Open
nniiovoo wants to merge 2 commits into
google:mainfrom
nniiovoo:nniiovoo/before-run-early-exit-on-event
Open

nniiovoo wants to merge 2 commits into
google:mainfrom
nniiovoo:nniiovoo/before-run-early-exit-on-event

Conversation

@nniiovoo

Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

Closes #1147

Problem:
When a plugin's beforeRunCallback returns content, the Runner stores and yields the early-exit event directly. No plugin's onEventCallback sees it, so a plugin can't log, redact or tag the blocked reply.

Solution:
Send the early-exit event through processAndAppendEvent in runAsync and runLive, as runAsync already does for the agent's events. The same before_run callback gap was fixed in adk-python by 63e918a (google/adk-python#7007), first released in v2.10.0.

A blocked reply now goes through onEventCallback before persistence and streaming. runAsync keeps its abort check after processing the event. runLive gets the same check, so an abort inside the callback prevents yielding the reply. The check comes after the helper's persistence step and does not roll back replies already persisted. This does not change afterRunCallback handling.

Testing Plan

Unit Tests:

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

Three regression tests:

  • One for runAsync in core/test/runner/runner_test.ts, using the existing MockPlugin, and one for runLive in core/test/runner/run_live_test.ts. Both check that rewritten text is yielded and stored under the same event id, and that the yielded event's author is model. The live test also checks the incoming blocked text and that no model connection opens.
  • A second live test aborts inside onEventCallback. It checks that nothing is yielded, the blocked reply remains stored, and no model connection opens.

All three regressions fail with upstream's runner.ts and pass with this change. The cancellation test also fails against the initial fix, before adding the abort check.

  • npx vitest run --project unit:core --project unit:dev --maxWorkers=4: 343 files passed, 1 skipped; 6104 tests passed, 2 skipped.
  • npm run ts:check, ts:check:samples, lint, format:check, docs:check and build pass.
  • bash scripts/check_license.sh and npx secretlint '**/*' pass.

Manual End-to-End (E2E) Tests:

The issue's stub-model script needs no credentials. The recorded results for the blocked run are:

  • Before (@google/adk 2.3.0 from npm): onEventCallback is never called; the only line is yielded: Blocked.
  • After (this branch): onEventCallback saw: Blocked., then yielded: Blocked.

The script was rerun on the revised working tree. Integration and e2e test projects and a live model are not covered by this script.

Checklist

  • The contribution guide and CONTRIBUTING.md have been reviewed.
  • The complete diff has been reviewed.
  • 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.
  • The stub-model reproduction has been run end to end (no live model).
  • Any dependent changes have been merged and published in downstream modules. (n/a, none)

When a plugin's beforeRunCallback returned content, the Runner stored and
yielded the early-exit event directly, so no plugin's onEventCallback saw
the blocked reply and a plugin could not log, redact or tag it.

Send the early-exit event through processAndAppendEvent in runAsync and
runLive, as runAsync already does for the agent's events and as adk-python
does since google/adk-python@63e918a.
Check the abort signal after processing a before-run live reply, before
yielding it. This matches the async early-exit path and keeps replies that
were already stored.

Add a regression that cancels inside onEventCallback and checks that no
reply is yielded or model connection opened while session history remains.
@Varun-S10 Varun-S10 self-assigned this Oct 26, 2026
@Varun-S10

Copy link
Copy Markdown
Contributor

Hi @nniiovoo, thank you for raising the PR to fix the issue. We have reviewed the code changes and it does resolve the issue. I will escalate the PR to the team to review, thank you for your patience and understanding.
Hi @kalenkevich, could you please take a look?

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

Labels

needs review [Status] The PR/issue is awaiting review from the maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

beforeRunCallback early exit skips every plugin's onEventCallback

2 participants