Skip to content

fix(runners): honor before_run_callback early-exit for Workflow runs#6032

Open
garyzava wants to merge 1 commit into
google:mainfrom
garyzava:fix/workflow-before-run-callback-halt
Open

fix(runners): honor before_run_callback early-exit for Workflow runs#6032
garyzava wants to merge 1 commit into
google:mainfrom
garyzava:fix/workflow-before-run-callback-halt

Conversation

@garyzava

@garyzava garyzava commented Jun 9, 2026

Copy link
Copy Markdown

Summary

In the Workflow run path (Runner._run_node_async), the before_run plugin
callback was awaited but its return value was discarded. Per the plugin contract,
returning a Content from before_run_callback should halt the run and end it
with that content — but for Workflow root agents the value was ignored and
execution continued into the workflow.

This captures the callback result and, when it is a Content, emits it as the
early-exit event and stops — matching the existing behavior of the non-workflow
run path (run_async).

Fixes #6013

What changed

  • src/google/adk/runners.py — in _run_node_async, capture
    run_before_run_callback(...)'s result; if it's a types.Content, build the
    early-exit Event, append it (subject to _should_append_event), yield it, and
    return — instead of unconditionally proceeding. Mirrors the non-workflow path.
  • tests/unittests/workflow/test_workflow_failures.py — regression test: a plugin
    whose before_run_callback returns Content halts the workflow and the node
    never executes.

Why this approach

The non-workflow path already implements the documented early-exit contract
(capture the result, emit an early-exit event, skip execution). This change makes
the Workflow path consistent with it rather than introducing new behavior, so the
fix is minimal and the intended semantics are unambiguous.

How I tested

$ pytest tests/unittests/workflow/test_workflow_failures.py -q
23 passed

The new test (test_workflow_halts_when_before_run_callback_returns_content)
fails before the change (the node executes; no early-exit event) and passes
after. Existing workflow-failure tests and the non-workflow plugin tests
(tests/unittests/test_runners.py -k plugin) still pass. pyink and isort
report no changes.

Backward compatibility

No behavior change when before_run_callback returns None (the common case) —
the workflow runs exactly as before. Only the documented halt-on-Content case is
affected, and it now matches the non-workflow path.

In the Workflow run path (_run_node_async), the before_run plugin callback
was awaited but its return value was discarded, so a callback returning a
Content (the documented signal to halt the run) was ignored and execution
continued into the workflow.

Capture the callback result and, when it is a Content, emit it as the
early-exit event and stop — matching the existing behavior of the
non-workflow run path.

Adds a regression test: a plugin whose before_run_callback returns Content
halts the workflow and the node never executes.

Fixes google#6013
@adk-bot adk-bot added the core [Component] This issue is related to the core interface and implementation label Jun 9, 2026
@adk-bot

adk-bot commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

🔍 ADK Pull Request Analysis: PR #6032

Title: fix(runners): honor before_run_callback early-exit for Workflow runs
Author: @garyzava
Status: open
Impact: 72 additions, 2 deletions across 2 files

Executive Summary

  1. Core Objective: Swallowing of before_run_callback returning values was occurring during root workflow executions (_run_node_async). This PR fixes the bug by capturing the callback's return value and gracefully halting/early-exiting with the returned Content, aligning the workflow run path with the non-workflow sequential execution path.
  2. Justification & Value: [Justified Fix] - Restores a core plugin capability of halting execution to workflow agents, preventing unwanted LLM requests, charge consumption, and downstream orchestration side effects.
  3. Alignment with Principles: [Pass with Nits] - Exceptional structural alignment with zero breaking changes, with very minor type-hinting/styling nits on the newly introduced unit-test callback.
  4. Recommendation: [Approve with Nits]
Detailed Findings & Analysis

1. Objectives & Impact ("What does it do?")

  • Context & Background: Issue #6013 reported that ADK 2.0 ignores before_run_callback returning a value for workflows. Because a Workflow is a BaseNode rather than a standard BaseAgent, root agent execution is routed through _run_node_async where the callback return value was awaited but discarded.
  • Implementation Mechanism: In runners.py:L531-L532, changed await ic.plugin_manager.run_before_run_callback(invocation_context=ic) to capture the return value as early_exit_result. If isinstance(early_exit_result, types.Content), it halts, constructs an early-exit Event (setting author='model'), applies custom run-config metadata, appends it to the sessions (if _should_append_event allows), yields the event, and returns. This accurately mirrors the standard sequential path implementation in runners.py:L1373-L1393.
  • Affected Surface: Zero breaking changes to public APIs. Changes are fully internal to the orchestration runner path.

2. Justification & Value ("Is it a valid and useful change?")

  • Workspace Verification:
    • Investigated baseline workspace file runners.py.
    • Confirmed that the baseline codebase strictly ignores the result of the before_run invocation:
      # Run before_run callbacks
      await ic.plugin_manager.run_before_run_callback(invocation_context=ic)
  • Value Assessment: Extremely high value addition. Bypassing execution is highly critical to preventing unnecessary downstream LLM network calls, helping developers minimize execution costs and side-effects.
  • Alternative Approaches: No cleaner alternative exists. Replicating the working sequential branch's early-exit semantics in the active node runtime branch creates unified execution parity.
  • Scope & Depth: [Systematic Fix] & [Root Cause] - This is a robust framework-level fix addressing the underlying architectural gap across execution path divisions, rather than adding a superficial or localized band-aid.

3. Principle & Style Alignment Checklist ("Does it follow rules?")

  • Public API & Visibility Boundaries:
    • Status: [Pass]
    • Analysis: No signature alterations or breaking changes to existing components.
  • Code Quality, Typing & Conventions:
    • Status: [Nits]
    • Analysis:
      • In test_workflow_failures.py, the mock callback before_run_callback lacks typing annotations for arguments and return types. For full styling compliance, it should be annotated:
        async def before_run_callback(
            self, *, invocation_context: InvocationContext
        ) -> types.Content:
      • While runners.py includes from __future__ import annotations, the test file test_workflow_failures.py does not contain it. Since it is an existing file and was only incrementally modified, this is not a violation (but worth clean-up if/where convenient).
  • Robustness & Edge Cases:
    • Status: [Pass]
    • Analysis: Uses isinstance(early_exit_result, types.Content) for clean type validation, preventing arbitrary issues or object mismatch bugs.
  • Test Integrity & Quality:
    • Status: [Pass]
    • Analysis: Adds a dedicated regression test test_workflow_halts_when_before_run_callback_returns_content. It verifies behavior from public interfaces, avoids mocking core engine structures, declares test helper instances (_RecordingNode, _HaltPlugin) inline/local to the test case, and cleanly follows the Arrange-Act-Assert pattern.

Summary of Triage & Findings

  • CLA Status: Verified and signed successfully (cla/google status check is SUCCESS).
  • PR Structure: Well-defined, localized, and cleanly designed.
  • Testing Quality: Excellent behavior-focused test.
  • Final Action Verdict: Approve with Nits. Request the contributor to add type hints for the newly introduced before_run_callback argument and return type in the test mock class.

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

Labels

core [Component] This issue is related to the core interface and implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ADK 2.0 ignores before_run callback for Workflow

2 participants