Skip to content

fix(remoteagent/v2): invoke AfterA2ARequestCallbacks on synthesized aggregated events#973

Open
nuthalapativarun wants to merge 1 commit into
google:mainfrom
nuthalapativarun:fix/948-after-a2a-callbacks-on-aggregated-events
Open

fix(remoteagent/v2): invoke AfterA2ARequestCallbacks on synthesized aggregated events#973
nuthalapativarun wants to merge 1 commit into
google:mainfrom
nuthalapativarun:fix/948-after-a2a-callbacks-on-aggregated-events

Conversation

@nuthalapativarun
Copy link
Copy Markdown

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

Link to Issue or Description of Change

Root Cause

aggregatePartial synthesizes new non-partial events via buildNonPartialAggregation when accumulated partial artifact chunks arrive before a terminal status update. These synthesized events were emitted directly via yield without going through runAfterA2ARequestCallbacks, so registered AfterA2ARequestCallback handlers never received them.

The original event (converted from the individual A2A chunk) correctly went through the callbacks, but the aggregated result—which is the event callers actually care about—did not.

Fix

In processEvent, after calling aggregatePartial, run runAfterA2ARequestCallbacks on each emitted event whose pointer differs from the original event (i.e., synthesized aggregated events). The original event already went through callbacks above the aggregatePartial call.

for _, toEmit := range processor.aggregatePartial(ctx, a2aEvent, event) {
    if toEmit != event {
        if cbResp, cbErr := processor.runAfterA2ARequestCallbacks(ctx, toEmit, nil); cbResp != nil || cbErr != nil {
            if cbErr != nil {
                return yieldErr(cbErr)
            }
            toEmit = cbResp
        }
    }
    if !yield(toEmit, nil) {
        return false
    }
}

Testing Plan

Unit Tests:

  • Added TestRemoteAgent_AfterCallbackInvokedOnAggregatedEvent: streams two partial artifact chunks ("Hel" + "lo") followed by a terminal status. Registers a callback that counts invocations and records non-partial content text. Asserts 4 callback invocations (partial×2 + aggregated + terminal) and ["Hello"] as the text seen by the callback for the aggregated non-partial event.
  • All existing tests in ./agent/remoteagent/v2/... continue to pass.

Manual End-to-End (E2E) Tests:
The fix is contained to the callback dispatch path in processEvent. The change is additive—existing behavior is preserved for the common case (single-event slice from aggregatePartial) and the new path only fires when a synthesized aggregated event is present.

Checklist

  • Read CONTRIBUTING.md
  • Self-reviewed code
  • Added tests proving fix
  • New and existing tests pass

…ggregated events

aggregatePartial can build new non-partial events from accumulated partial
artifact chunks (buildNonPartialAggregation). These synthesized events were
emitted directly without going through runAfterA2ARequestCallbacks, so
registered AfterA2ARequestCallback handlers never received them.

Fix: after aggregatePartial returns, run callbacks on any event whose pointer
differs from the original converted event (i.e., a synthesized aggregated event).
The original event already went through callbacks before aggregatePartial was called.

Closes google#948
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.

remoteagent/v2: AfterA2ARequestCallbacks are not invoked on the aggregated event synthesized from partial artifact chunks

1 participant