From 58c64c518ac4855ee1cc95876856af9a0e89fbf6 Mon Sep 17 00:00:00 2001 From: lipandeng Date: Wed, 1 Jul 2026 21:44:40 +0800 Subject: [PATCH 1/3] fix(adk): stop forwarding backgrounded sub-agent events to the parent turn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A managed sub-agent run that is backgrounded (explicit run_in_background or auto-moved at the foreground deadline) runs detached and outlives the turn that launched it. It kept forwarding the inner agent's events to the parent turn's event generator, which the turn closes on end — injecting events into a stream the user has stopped watching, and racing a send against that close. Introduce a lifecycle signal (backgroundtask TaskInfo.Backgrounded) that closes when a task moves to the background, and an AgentTool option (WithAgentToolParentForwardUntil) that bounds parent forwarding by it. The subagent middleware wires the two together, so forwarding stops the moment a run detaches; foreground runs are unaffected. The gated path uses a non-panicking send by definition (it may outlive its consumer); the plain foreground path keeps a panicking send so an unexpected closed-generator send still surfaces. Co-Authored-By: Claude Opus 4 --- adk/agent_tool.go | 67 +++++++++++++++++- adk/agent_tool_test.go | 94 ++++++++++++++++++++++++++ adk/backgroundtask/manager.go | 66 ++++++++++++++++-- adk/backgroundtask/manager_test.go | 77 +++++++++++++++++++++ adk/middlewares/subagent/agent_tool.go | 11 ++- 5 files changed, 309 insertions(+), 6 deletions(-) diff --git a/adk/agent_tool.go b/adk/agent_tool.go index 0d2c40565..07a0a9f2c 100644 --- a/adk/agent_tool.go +++ b/adk/agent_tool.go @@ -168,6 +168,14 @@ func (at *typedAgentTool[M]) InvokableRun(ctx context.Context, argumentsInJSON s } gen, enableStreaming := getEmitGeneratorAndEnableStreaming[M](opts) + // A background-capable invocation (WithAgentToolParentForwardUntil) bounds parent + // forwarding by a "backgrounded" signal and uses a non-panicking send: once the + // run detaches it outlives the parent turn, whose generator is then closed. A + // plain foreground invocation sets neither, so it forwards with a panicking Send + // — a send on an unexpectedly-closed generator is a bug there and must surface. + toolOpts := tool.GetImplSpecificOptions[agentToolOptions](nil, opts...) + gatedForward := toolOpts.gatedForward + forwardParentUntil := toolOpts.forwardParentUntil var ms *bridgeStore var iter *AsyncIterator[*TypedAgentEvent[M]] var err error @@ -274,7 +282,16 @@ func (at *typedAgentTool[M]) InvokableRun(ctx context.Context, argumentsInJSON s // loop can skip them. stampAgentToolSessionEvent(event, childSessionID) tmp := copyTypedAgentEvent(event) - gen.Send(event) + if gatedForward { + // Background-capable: forward only until the run is backgrounded, + // then drop (the run outlives the parent turn). trySend, not Send: + // after detach the parent generator may be closed. + if !backgrounded(forwardParentUntil) { + gen.trySend(event) + } + } else { + gen.Send(event) + } event = tmp } } @@ -329,6 +346,14 @@ type agentToolOptions struct { agentName string opts []AgentRunOption enableStreaming bool + + // gatedForward is set by WithAgentToolParentForwardUntil to mark this as a + // background-capable invocation: parent forwarding is bounded by + // forwardParentUntil and uses a non-panicking send (see the option's doc). + gatedForward bool + // forwardParentUntil, when gatedForward is set, stops parent forwarding once + // closed (the run has been backgrounded). Nil means "never backgrounded". + forwardParentUntil <-chan struct{} } // typedAgentToolEventOptions carries the parent runner's event generator for a @@ -355,6 +380,46 @@ func withTypedAgentToolEventGenerator[M MessageType](gen *AsyncGenerator[*TypedA }) } +// WithAgentToolParentForwardUntil marks an AgentTool invocation as +// background-capable and bounds how long the inner agent's events are forwarded to +// the parent's event stream: forwarding continues until done is closed, then +// stops. +// +// It is meant for a middleware that runs an AgentTool as a managed +// (backgroundable) task: pass the task's "backgrounded" signal as done. A run that +// is backgrounded outlives the parent turn, which closes its event generator on +// turn end; forwarding to the parent after that point is both wrong (the events +// belong to a stream the user has stopped watching) and unsafe (a send on the +// closed generator would race). Gating on done stops forwarding at the moment the +// run detaches. +// +// Presence of this option also selects the non-panicking send used on this +// background-capable path (a plain foreground AgentTool, which never sets it, +// forwards with a panicking send so a send on an unexpectedly-closed generator +// surfaces as the bug it is). A nil done is treated as "never backgrounded": +// forwarding runs for the whole invocation. +func WithAgentToolParentForwardUntil(done <-chan struct{}) tool.Option { + return tool.WrapImplSpecificOptFn(func(o *agentToolOptions) { + o.forwardParentUntil = done + o.gatedForward = true + }) +} + +// backgrounded reports whether the done signal has fired (the run has been moved +// to the background). A nil done never fires — the run stays foreground for its +// whole lifetime. +func backgrounded(done <-chan struct{}) bool { + if done == nil { + return false + } + select { + case <-done: + return true + default: + return false + } +} + func getOptionsByAgentName(agentName string, opts []tool.Option) []AgentRunOption { var ret []AgentRunOption for _, opt := range opts { diff --git a/adk/agent_tool_test.go b/adk/agent_tool_test.go index 16e768811..bc37494e4 100644 --- a/adk/agent_tool_test.go +++ b/adk/agent_tool_test.go @@ -1277,3 +1277,97 @@ func TestCrossTypeAgentToolGracefulError(t *testing.T) { "Error should mention cross-message-type incompatibility") } } + +// --- Parent-forward gate (WithAgentToolParentForwardUntil) --- + +// collectForwarded drains a parent generator's iterator and returns the content of +// every non-streaming message event forwarded to it. Runs in a goroutine; the +// caller closes the generator to end the drain. +func collectForwarded(iter *AsyncIterator[*AgentEvent], out *[]string, done chan<- struct{}) { + for { + e, ok := iter.Next() + if !ok { + close(done) + return + } + if e.Output != nil && e.Output.MessageOutput != nil && !e.Output.MessageOutput.IsStreaming && + e.Output.MessageOutput.Message != nil { + *out = append(*out, e.Output.MessageOutput.Message.Content) + } + } +} + +// With the gate open (done never fires), a background-capable invocation forwards +// the inner agent's events to the parent generator — same as a plain foreground +// agent-as-tool call. +func TestAgentToolParentForwardUntil_OpenGateForwards(t *testing.T) { + ctx := context.Background() + sub := &emitEventsAgent{events: []*AgentEvent{ + EventFromMessage(schema.AssistantMessage("child output", nil), nil, schema.Assistant, ""), + }} + at := NewAgentTool(ctx, sub).(tool.InvokableTool) + + iter, gen := NewAsyncIteratorPair[*AgentEvent]() + var got []string + done := make(chan struct{}) + go collectForwarded(iter, &got, done) + + // nil done => never backgrounded => gate stays open the whole run. + _, err := at.InvokableRun(ctx, `{"request":"q"}`, + withAgentToolEventGenerator(gen), + WithAgentToolParentForwardUntil(nil)) + require.NoError(t, err) + + gen.Close() + <-done + assert.Contains(t, got, "child output") +} + +// With the gate already closed (the run is backgrounded before it starts), a +// background-capable invocation forwards nothing to the parent generator. +func TestAgentToolParentForwardUntil_ClosedGateDropsForwarding(t *testing.T) { + ctx := context.Background() + sub := &emitEventsAgent{events: []*AgentEvent{ + EventFromMessage(schema.AssistantMessage("child output", nil), nil, schema.Assistant, ""), + }} + at := NewAgentTool(ctx, sub).(tool.InvokableTool) + + iter, gen := NewAsyncIteratorPair[*AgentEvent]() + var got []string + done := make(chan struct{}) + go collectForwarded(iter, &got, done) + + backgrounded := make(chan struct{}) + close(backgrounded) // already backgrounded + + _, err := at.InvokableRun(ctx, `{"request":"q"}`, + withAgentToolEventGenerator(gen), + WithAgentToolParentForwardUntil(backgrounded)) + require.NoError(t, err) + + gen.Close() + <-done + assert.Empty(t, got) +} + +// On the gated (background-capable) path, forwarding to an already-closed parent +// generator must not panic — a backgrounded run outlives the turn that closes it. +func TestAgentToolParentForwardUntil_ClosedParentGeneratorNoPanic(t *testing.T) { + ctx := context.Background() + sub := &emitEventsAgent{events: []*AgentEvent{ + EventFromMessage(schema.AssistantMessage("child output", nil), nil, schema.Assistant, ""), + }} + at := NewAgentTool(ctx, sub).(tool.InvokableTool) + + // A parent generator that is already closed, with the gate still open: the + // gated path uses a non-panicking send, so this must not panic. + _, gen := NewAsyncIteratorPair[*AgentEvent]() + gen.Close() + + require.NotPanics(t, func() { + _, err := at.InvokableRun(ctx, `{"request":"q"}`, + withAgentToolEventGenerator(gen), + WithAgentToolParentForwardUntil(nil)) + require.NoError(t, err) + }) +} diff --git a/adk/backgroundtask/manager.go b/adk/backgroundtask/manager.go index edf301ae4..7540d7880 100644 --- a/adk/backgroundtask/manager.go +++ b/adk/backgroundtask/manager.go @@ -288,6 +288,13 @@ type taskRecord struct { // state. Wait selects on it so waiting for one task neither holds m.mu nor is // woken by unrelated tasks finishing. doneCh chan struct{} + // backgrounded is closed exactly once (guarded by bgClosed) when the task moves + // to background execution: before the work starts for an explicit + // RunInBackground launch, or at the auto-background transition (detach). It stays + // open for a run that completes in the foreground. Handed to the work via + // TaskInfo.Backgrounded so foreground-only side effects can stop when it fires. + backgrounded chan struct{} + bgClosed bool } // New creates a new Manager. @@ -545,13 +552,45 @@ func (m *Manager) registerTaskLocked(input *RunInput, id string) (string, error) OutputFile: input.OutputFile, Metadata: cloneMetadata(input.Metadata), }, - doneCh: make(chan struct{}), + doneCh: make(chan struct{}), + backgrounded: make(chan struct{}), } m.sendEventLocked(m.tasks[id], TaskEventCreated) return id, nil } +// markBackgroundedLocked closes a task's backgrounded signal, idempotently. Must be +// called with m.mu held. +func (m *Manager) markBackgroundedLocked(rec *taskRecord) { + if !rec.bgClosed { + rec.bgClosed = true + close(rec.backgrounded) + } +} + +// backgroundedCh returns the task's backgrounded signal channel. Returns a nil +// channel if the task is absent (should not happen for a just-created task); a nil +// channel never fires, which is the correct "stayed foreground" reading. +func (m *Manager) backgroundedCh(id string) <-chan struct{} { + m.mu.Lock() + defer m.mu.Unlock() + if rec, ok := m.tasks[id]; ok { + return rec.backgrounded + } + return nil +} + +// markBackgrounded closes a task's backgrounded signal, idempotently. Used for an +// explicit RunInBackground launch, where the run is background from the start. +func (m *Manager) markBackgrounded(id string) { + m.mu.Lock() + defer m.mu.Unlock() + if rec, ok := m.tasks[id]; ok { + m.markBackgroundedLocked(rec) + } +} + func (m *Manager) closedError() error { return fmt.Errorf("the background task manager has shut down and is no longer accepting new tasks. " + "Do not retry this; finish using any results you already have") @@ -701,6 +740,7 @@ func (m *Manager) detach(id string) bool { return false } rec.task.RunInBackground = true + m.markBackgroundedLocked(rec) m.sendEventLocked(rec, TaskEventBackgrounded) return true } @@ -789,6 +829,14 @@ type TaskInfo struct { // closure is already built. The launcher passes it to MarkOutputFileUnreliable // to report an output-file write failure against this task. ID string + // Backgrounded is closed when this task moves to background execution: before + // the work starts for an explicit RunInBackground launch, or at the + // auto-background transition (foreground deadline reached and permitted). It + // stays open for a run that completes in the foreground. Work uses it to stop + // side effects that are only valid while the run is foreground — most notably + // forwarding events to the launching turn's live stream, which the turn closes + // once it ends. It is never nil. + Backgrounded <-chan struct{} } // WorkFunc performs a single managed execution. It is supplied by the caller @@ -852,6 +900,8 @@ func (m *Manager) Run(ctx context.Context, input *RunInput, work WorkFunc) (*Tas runCtx, cancel := context.WithCancel(detachedCtx{parent: ctx}) m.storeCancelFunc(id, cancel) + bgCh := m.backgroundedCh(id) + // run executes the work and finalizes the task. The terminal outcome lives on // the task record (set by completeTask/failTask), which Run reads back via // taskSnapshot — so run signals completion rather than returning a value. @@ -863,7 +913,7 @@ func (m *Manager) Run(ctx context.Context, input *RunInput, work WorkFunc) (*Tas m.failTask(id, safe.NewPanicErr(p, debug.Stack())) } }() - r, runErr := work(runCtx, TaskInfo{ID: id}) + r, runErr := work(runCtx, TaskInfo{ID: id, Backgrounded: bgCh}) if runErr != nil { m.failTask(id, runErr) } else { @@ -872,8 +922,10 @@ func (m *Manager) Run(ctx context.Context, input *RunInput, work WorkFunc) (*Tas } // Explicit background: run in goroutine, return immediately. createTask already - // marked the task RunInBackground. + // marked the task RunInBackground; signal backgrounded before the work starts so + // the work never treats itself as foreground. if input.RunInBackground { + m.markBackgrounded(id) task := m.taskSnapshot(id) go run() return task, nil @@ -1027,7 +1079,13 @@ func (m *Manager) forwardStream(r *streamRun) { } }() - ws, err := r.work(r.runCtx, TaskInfo{ID: r.id}) + // Explicit background: the run is background from the start, so signal it before + // the work starts (RunStream forces budgetMs=0 for this case). + if r.input.RunInBackground { + m.markBackgrounded(r.id) + } + + ws, err := r.work(r.runCtx, TaskInfo{ID: r.id, Backgrounded: m.backgroundedCh(r.id)}) if err != nil { m.failTask(r.id, err) r.sw.Send("", err) diff --git a/adk/backgroundtask/manager_test.go b/adk/backgroundtask/manager_test.go index c39f7b944..bf60a719f 100644 --- a/adk/backgroundtask/manager_test.go +++ b/adk/backgroundtask/manager_test.go @@ -789,3 +789,80 @@ func TestManager_ContextCancelStopsWork(t *testing.T) { require.True(t, ok) assert.Equal(t, StatusCanceled, task.Status) } + +// --- TaskInfo.Backgrounded signal Tests --- + +// isClosed reports whether a done channel has fired without blocking. +func isClosed(ch <-chan struct{}) bool { + select { + case <-ch: + return true + default: + return false + } +} + +// An explicit RunInBackground launch is background from the start, so the work +// must observe Backgrounded already closed on entry. +func TestManager_Backgrounded_ExplicitClosedBeforeWork(t *testing.T) { + m := New(context.Background(), &Config{}) + defer closeWithTimeout(m) + + seen := make(chan bool, 1) + release := make(chan struct{}) + result, err := m.Run(context.Background(), &RunInput{Description: "bg", RunInBackground: true}, + func(_ context.Context, task TaskInfo) (string, error) { + seen <- isClosed(task.Backgrounded) + <-release + return "ok", nil + }) + require.NoError(t, err) + assert.Equal(t, StatusRunning, result.Status) + + assert.True(t, <-seen, "explicit background work should see Backgrounded already closed") + close(release) + waitTask(t, m, result.ID) +} + +// A run that completes in the foreground is never backgrounded: its Backgrounded +// signal stays open for the whole run. +func TestManager_Backgrounded_ForegroundStaysOpen(t *testing.T) { + m := New(context.Background(), &Config{}) + defer closeWithTimeout(m) + + var duringRun bool + result, err := run(m, "fg", false, func(_ context.Context, task TaskInfo) (string, error) { + duringRun = isClosed(task.Backgrounded) + return "ok", nil + }) + require.NoError(t, err) + assert.Equal(t, StatusCompleted, result.Status) + assert.False(t, duringRun, "foreground work must not see Backgrounded closed") +} + +// A foreground run that is auto-moved to the background at its deadline must have +// its Backgrounded signal close at the transition, so still-running work learns it +// detached. +func TestManager_Backgrounded_AutoBackgroundCloses(t *testing.T) { + m := New(context.Background(), &Config{ForegroundTimeoutMs: intPtr(50), ShouldAutoBackground: allowBackground}) + defer closeWithTimeout(m) + + closedCh := make(chan struct{}) + result, err := m.Run(context.Background(), &RunInput{Description: "slow"}, + func(_ context.Context, task TaskInfo) (string, error) { + // Block until the deadline detaches the run, then confirm the signal fired. + <-task.Backgrounded + close(closedCh) + return "slow result", nil + }) + require.NoError(t, err) + assert.Equal(t, StatusRunning, result.Status) + + select { + case <-closedCh: + case <-time.After(time.Second): + t.Fatal("Backgrounded did not close at the auto-background transition") + } + waitTask(t, m, result.ID) +} + diff --git a/adk/middlewares/subagent/agent_tool.go b/adk/middlewares/subagent/agent_tool.go index 6b3a392c5..4a534045d 100644 --- a/adk/middlewares/subagent/agent_tool.go +++ b/adk/middlewares/subagent/agent_tool.go @@ -118,7 +118,16 @@ func newManagedAgentTool(mgr *backgroundtask.Manager, subAgents map[string]tool. Metadata: map[string]any{MetadataKeySubagentType: in.SubagentType}, OutputFile: outputFile, }, func(workCtx context.Context, task backgroundtask.TaskInfo) (string, error) { - out, runErr := a.InvokableRun(workCtx, params, opts...) + // Bound the inner agent's forwarding to the launching turn's event + // stream by the task's backgrounded signal: a backgrounded run outlives + // the turn (which closes its event generator on turn end), so forwarding + // to it past that point is wrong and unsafe. The AgentTool stops + // forwarding when task.Backgrounded fires — for an explicit + // run_in_background it is already closed here, for an auto-backgrounded + // run it closes at the deadline. Foreground runs never fire it and + // forward for their whole lifetime. + runOpts := append(opts, adk.WithAgentToolParentForwardUntil(task.Backgrounded)) + out, runErr := a.InvokableRun(workCtx, params, runOpts...) if runErr != nil { return "", runErr } From 24c9fe0e57cad02797fb2d0419637e86d1548752 Mon Sep 17 00:00:00 2001 From: lipandeng Date: Wed, 1 Jul 2026 22:15:54 +0800 Subject: [PATCH 2/3] refactor(adk): rename AgentTool event-forward options for self-documentation Make the two halves of AgentTool event forwarding read as a pair: the target (where inner events go) and the bound (how long they go there). - withAgentToolEventGenerator -> withAgentToolEventForwardTarget - withTypedAgentToolEventGenerator -> withTypedAgentToolEventForwardTarget - WithAgentToolParentForwardUntil -> WithAgentToolEventForwardUntil - agentToolOptions.gatedForward/forwardParentUntil -> eventForwardGated/eventForwardUntil Behavior is unchanged; this is a naming-only refactor. Co-Authored-By: Claude Opus 4 --- adk/agent_tool.go | 69 +++++++++++++------------- adk/agent_tool_test.go | 20 ++++---- adk/chatmodel.go | 4 +- adk/middlewares/subagent/agent_tool.go | 2 +- 4 files changed, 48 insertions(+), 47 deletions(-) diff --git a/adk/agent_tool.go b/adk/agent_tool.go index 07a0a9f2c..d5a9450b0 100644 --- a/adk/agent_tool.go +++ b/adk/agent_tool.go @@ -168,14 +168,15 @@ func (at *typedAgentTool[M]) InvokableRun(ctx context.Context, argumentsInJSON s } gen, enableStreaming := getEmitGeneratorAndEnableStreaming[M](opts) - // A background-capable invocation (WithAgentToolParentForwardUntil) bounds parent - // forwarding by a "backgrounded" signal and uses a non-panicking send: once the - // run detaches it outlives the parent turn, whose generator is then closed. A - // plain foreground invocation sets neither, so it forwards with a panicking Send - // — a send on an unexpectedly-closed generator is a bug there and must surface. + // A background-capable invocation (WithAgentToolEventForwardUntil) bounds event + // forwarding to the caller by a "backgrounded" signal and uses a non-panicking + // send: once the run detaches it outlives the parent turn, whose generator is + // then closed. A plain foreground invocation sets neither, so it forwards with a + // panicking Send — a send on an unexpectedly-closed generator is a bug there and + // must surface. toolOpts := tool.GetImplSpecificOptions[agentToolOptions](nil, opts...) - gatedForward := toolOpts.gatedForward - forwardParentUntil := toolOpts.forwardParentUntil + eventForwardGated := toolOpts.eventForwardGated + eventForwardUntil := toolOpts.eventForwardUntil var ms *bridgeStore var iter *AsyncIterator[*TypedAgentEvent[M]] var err error @@ -282,11 +283,11 @@ func (at *typedAgentTool[M]) InvokableRun(ctx context.Context, argumentsInJSON s // loop can skip them. stampAgentToolSessionEvent(event, childSessionID) tmp := copyTypedAgentEvent(event) - if gatedForward { + if eventForwardGated { // Background-capable: forward only until the run is backgrounded, // then drop (the run outlives the parent turn). trySend, not Send: // after detach the parent generator may be closed. - if !backgrounded(forwardParentUntil) { + if !backgrounded(eventForwardUntil) { gen.trySend(event) } } else { @@ -347,18 +348,19 @@ type agentToolOptions struct { opts []AgentRunOption enableStreaming bool - // gatedForward is set by WithAgentToolParentForwardUntil to mark this as a - // background-capable invocation: parent forwarding is bounded by - // forwardParentUntil and uses a non-panicking send (see the option's doc). - gatedForward bool - // forwardParentUntil, when gatedForward is set, stops parent forwarding once - // closed (the run has been backgrounded). Nil means "never backgrounded". - forwardParentUntil <-chan struct{} + // eventForwardGated is set by WithAgentToolEventForwardUntil to mark this as a + // background-capable invocation: forwarding to the caller is bounded by + // eventForwardUntil and uses a non-panicking send (see the option's doc). + eventForwardGated bool + // eventForwardUntil, when eventForwardGated is set, stops forwarding to the + // caller once closed (the run has been backgrounded). Nil means "never + // backgrounded". + eventForwardUntil <-chan struct{} } -// typedAgentToolEventOptions carries the parent runner's event generator for a -// specific message type. This keeps forwarded internal events type-compatible -// with the parent event stream. +// typedAgentToolEventOptions carries the event-forward target (the caller's event +// generator) for a specific message type. This keeps forwarded internal events +// type-compatible with the caller's event stream. type typedAgentToolEventOptions[M MessageType] struct { generator *AsyncGenerator[*TypedAgentEvent[M]] } @@ -370,38 +372,37 @@ func withAgentToolOptions(agentName string, opts []AgentRunOption) tool.Option { }) } -func withAgentToolEventGenerator(gen *AsyncGenerator[*AgentEvent]) tool.Option { - return withTypedAgentToolEventGenerator(gen) +func withAgentToolEventForwardTarget(gen *AsyncGenerator[*AgentEvent]) tool.Option { + return withTypedAgentToolEventForwardTarget(gen) } -func withTypedAgentToolEventGenerator[M MessageType](gen *AsyncGenerator[*TypedAgentEvent[M]]) tool.Option { +func withTypedAgentToolEventForwardTarget[M MessageType](gen *AsyncGenerator[*TypedAgentEvent[M]]) tool.Option { return tool.WrapImplSpecificOptFn(func(o *typedAgentToolEventOptions[M]) { o.generator = gen }) } -// WithAgentToolParentForwardUntil marks an AgentTool invocation as -// background-capable and bounds how long the inner agent's events are forwarded to -// the parent's event stream: forwarding continues until done is closed, then -// stops. +// WithAgentToolEventForwardUntil pairs with the event-forward target +// (withAgentToolEventForwardTarget): the target says where the inner agent's +// events are forwarded, this says how long. It marks an AgentTool invocation as +// background-capable and forwards to the caller until done is closed, then stops. // // It is meant for a middleware that runs an AgentTool as a managed // (backgroundable) task: pass the task's "backgrounded" signal as done. A run that -// is backgrounded outlives the parent turn, which closes its event generator on -// turn end; forwarding to the parent after that point is both wrong (the events -// belong to a stream the user has stopped watching) and unsafe (a send on the -// closed generator would race). Gating on done stops forwarding at the moment the -// run detaches. +// is backgrounded outlives the caller's turn, which closes its event generator on +// turn end; forwarding after that point is both wrong (the events belong to a +// stream the user has stopped watching) and unsafe (a send on the closed generator +// would race). Gating on done stops forwarding at the moment the run detaches. // // Presence of this option also selects the non-panicking send used on this // background-capable path (a plain foreground AgentTool, which never sets it, // forwards with a panicking send so a send on an unexpectedly-closed generator // surfaces as the bug it is). A nil done is treated as "never backgrounded": // forwarding runs for the whole invocation. -func WithAgentToolParentForwardUntil(done <-chan struct{}) tool.Option { +func WithAgentToolEventForwardUntil(done <-chan struct{}) tool.Option { return tool.WrapImplSpecificOptFn(func(o *agentToolOptions) { - o.forwardParentUntil = done - o.gatedForward = true + o.eventForwardUntil = done + o.eventForwardGated = true }) } diff --git a/adk/agent_tool_test.go b/adk/agent_tool_test.go index bc37494e4..c321071b5 100644 --- a/adk/agent_tool_test.go +++ b/adk/agent_tool_test.go @@ -1278,7 +1278,7 @@ func TestCrossTypeAgentToolGracefulError(t *testing.T) { } } -// --- Parent-forward gate (WithAgentToolParentForwardUntil) --- +// --- Event-forward gate (WithAgentToolEventForwardUntil) --- // collectForwarded drains a parent generator's iterator and returns the content of // every non-streaming message event forwarded to it. Runs in a goroutine; the @@ -1300,7 +1300,7 @@ func collectForwarded(iter *AsyncIterator[*AgentEvent], out *[]string, done chan // With the gate open (done never fires), a background-capable invocation forwards // the inner agent's events to the parent generator — same as a plain foreground // agent-as-tool call. -func TestAgentToolParentForwardUntil_OpenGateForwards(t *testing.T) { +func TestAgentToolEventForwardUntil_OpenGateForwards(t *testing.T) { ctx := context.Background() sub := &emitEventsAgent{events: []*AgentEvent{ EventFromMessage(schema.AssistantMessage("child output", nil), nil, schema.Assistant, ""), @@ -1314,8 +1314,8 @@ func TestAgentToolParentForwardUntil_OpenGateForwards(t *testing.T) { // nil done => never backgrounded => gate stays open the whole run. _, err := at.InvokableRun(ctx, `{"request":"q"}`, - withAgentToolEventGenerator(gen), - WithAgentToolParentForwardUntil(nil)) + withAgentToolEventForwardTarget(gen), + WithAgentToolEventForwardUntil(nil)) require.NoError(t, err) gen.Close() @@ -1325,7 +1325,7 @@ func TestAgentToolParentForwardUntil_OpenGateForwards(t *testing.T) { // With the gate already closed (the run is backgrounded before it starts), a // background-capable invocation forwards nothing to the parent generator. -func TestAgentToolParentForwardUntil_ClosedGateDropsForwarding(t *testing.T) { +func TestAgentToolEventForwardUntil_ClosedGateDropsForwarding(t *testing.T) { ctx := context.Background() sub := &emitEventsAgent{events: []*AgentEvent{ EventFromMessage(schema.AssistantMessage("child output", nil), nil, schema.Assistant, ""), @@ -1341,8 +1341,8 @@ func TestAgentToolParentForwardUntil_ClosedGateDropsForwarding(t *testing.T) { close(backgrounded) // already backgrounded _, err := at.InvokableRun(ctx, `{"request":"q"}`, - withAgentToolEventGenerator(gen), - WithAgentToolParentForwardUntil(backgrounded)) + withAgentToolEventForwardTarget(gen), + WithAgentToolEventForwardUntil(backgrounded)) require.NoError(t, err) gen.Close() @@ -1352,7 +1352,7 @@ func TestAgentToolParentForwardUntil_ClosedGateDropsForwarding(t *testing.T) { // On the gated (background-capable) path, forwarding to an already-closed parent // generator must not panic — a backgrounded run outlives the turn that closes it. -func TestAgentToolParentForwardUntil_ClosedParentGeneratorNoPanic(t *testing.T) { +func TestAgentToolEventForwardUntil_ClosedParentGeneratorNoPanic(t *testing.T) { ctx := context.Background() sub := &emitEventsAgent{events: []*AgentEvent{ EventFromMessage(schema.AssistantMessage("child output", nil), nil, schema.Assistant, ""), @@ -1366,8 +1366,8 @@ func TestAgentToolParentForwardUntil_ClosedParentGeneratorNoPanic(t *testing.T) require.NotPanics(t, func() { _, err := at.InvokableRun(ctx, `{"request":"q"}`, - withAgentToolEventGenerator(gen), - WithAgentToolParentForwardUntil(nil)) + withAgentToolEventForwardTarget(gen), + WithAgentToolEventForwardUntil(nil)) require.NoError(t, err) }) } diff --git a/adk/chatmodel.go b/adk/chatmodel.go index f5309eda5..04a6c9e4d 100644 --- a/adk/chatmodel.go +++ b/adk/chatmodel.go @@ -1461,7 +1461,7 @@ func (a *TypedChatModelAgent[M]) buildMessageReActRunFunc(_ context.Context, bc var runOpts []compose.Option runOpts = append(runOpts, mp.composeOpts...) if a.toolsConfig.EmitInternalEvents { - runOpts = append(runOpts, compose.WithToolsNodeOption(compose.WithToolOption(withAgentToolEventGenerator(mp.generator)))) + runOpts = append(runOpts, compose.WithToolsNodeOption(compose.WithToolOption(withAgentToolEventForwardTarget(mp.generator)))) } if mp.input.EnableStreaming { runOpts = append(runOpts, compose.WithToolsNodeOption(compose.WithToolOption(withAgentToolEnableStreaming(true)))) @@ -1602,7 +1602,7 @@ func (a *TypedChatModelAgent[M]) buildAgenticReActRunFunc(_ context.Context, bc var runOpts []compose.Option runOpts = append(runOpts, ap.composeOpts...) if a.toolsConfig.EmitInternalEvents { - runOpts = append(runOpts, compose.WithToolsNodeOption(compose.WithToolOption(withTypedAgentToolEventGenerator[*schema.AgenticMessage](ap.generator)))) + runOpts = append(runOpts, compose.WithToolsNodeOption(compose.WithToolOption(withTypedAgentToolEventForwardTarget[*schema.AgenticMessage](ap.generator)))) } if ap.input.EnableStreaming { runOpts = append(runOpts, compose.WithToolsNodeOption(compose.WithToolOption(withAgentToolEnableStreaming(true)))) diff --git a/adk/middlewares/subagent/agent_tool.go b/adk/middlewares/subagent/agent_tool.go index 4a534045d..a9d3f9ed2 100644 --- a/adk/middlewares/subagent/agent_tool.go +++ b/adk/middlewares/subagent/agent_tool.go @@ -126,7 +126,7 @@ func newManagedAgentTool(mgr *backgroundtask.Manager, subAgents map[string]tool. // run_in_background it is already closed here, for an auto-backgrounded // run it closes at the deadline. Foreground runs never fire it and // forward for their whole lifetime. - runOpts := append(opts, adk.WithAgentToolParentForwardUntil(task.Backgrounded)) + runOpts := append(opts, adk.WithAgentToolEventForwardUntil(task.Backgrounded)) out, runErr := a.InvokableRun(workCtx, params, runOpts...) if runErr != nil { return "", runErr From b065dd3e663d22ca1e7520284dfa74bdeb6de8dc Mon Sep 17 00:00:00 2001 From: lipandeng Date: Mon, 6 Jul 2026 11:45:09 +0800 Subject: [PATCH 3/3] refactor(adk): consolidate event-forward gate into internal/agenttool.ForwardGate Move the event-forwarding gate option to a self-documenting ForwardGate struct in internal/agenttool, replacing the split across agentToolOptions and the exported WithAgentToolEventForwardUntil. Also remove the AgentEvent type-specialization wrapper (withAgentToolEventForwardTarget) in favor of the generic withTypedAgentToolEventForwardTarget. Co-Authored-By: Claude --- adk/agent_tool.go | 57 ++++---------------------- adk/agent_tool_test.go | 21 +++++----- adk/chatmodel.go | 2 +- adk/internal/agenttool/options.go | 40 ++++++++++++++++++ adk/middlewares/subagent/agent_tool.go | 3 +- 5 files changed, 63 insertions(+), 60 deletions(-) create mode 100644 adk/internal/agenttool/options.go diff --git a/adk/agent_tool.go b/adk/agent_tool.go index d5a9450b0..2122ea048 100644 --- a/adk/agent_tool.go +++ b/adk/agent_tool.go @@ -26,6 +26,7 @@ import ( "github.com/bytedance/sonic" "github.com/google/uuid" + "github.com/cloudwego/eino/adk/internal/agenttool" "github.com/cloudwego/eino/components/tool" "github.com/cloudwego/eino/compose" "github.com/cloudwego/eino/schema" @@ -168,15 +169,12 @@ func (at *typedAgentTool[M]) InvokableRun(ctx context.Context, argumentsInJSON s } gen, enableStreaming := getEmitGeneratorAndEnableStreaming[M](opts) - // A background-capable invocation (WithAgentToolEventForwardUntil) bounds event - // forwarding to the caller by a "backgrounded" signal and uses a non-panicking - // send: once the run detaches it outlives the parent turn, whose generator is - // then closed. A plain foreground invocation sets neither, so it forwards with a - // panicking Send — a send on an unexpectedly-closed generator is a bug there and - // must surface. - toolOpts := tool.GetImplSpecificOptions[agentToolOptions](nil, opts...) - eventForwardGated := toolOpts.eventForwardGated - eventForwardUntil := toolOpts.eventForwardUntil + // A background-capable invocation bounds event forwarding to the caller by a + // "backgrounded" signal and uses a non-panicking send: once the run detaches it + // outlives the parent turn, whose generator is then closed. A plain foreground + // invocation sets neither, so it forwards with a panicking Send — a send on an + // unexpectedly-closed generator is a bug there and must surface. + forwardGate := tool.GetImplSpecificOptions[agenttool.ForwardGate](nil, opts...) var ms *bridgeStore var iter *AsyncIterator[*TypedAgentEvent[M]] var err error @@ -283,11 +281,11 @@ func (at *typedAgentTool[M]) InvokableRun(ctx context.Context, argumentsInJSON s // loop can skip them. stampAgentToolSessionEvent(event, childSessionID) tmp := copyTypedAgentEvent(event) - if eventForwardGated { + if forwardGate.Enabled { // Background-capable: forward only until the run is backgrounded, // then drop (the run outlives the parent turn). trySend, not Send: // after detach the parent generator may be closed. - if !backgrounded(eventForwardUntil) { + if !backgrounded(forwardGate.Until) { gen.trySend(event) } } else { @@ -347,15 +345,6 @@ type agentToolOptions struct { agentName string opts []AgentRunOption enableStreaming bool - - // eventForwardGated is set by WithAgentToolEventForwardUntil to mark this as a - // background-capable invocation: forwarding to the caller is bounded by - // eventForwardUntil and uses a non-panicking send (see the option's doc). - eventForwardGated bool - // eventForwardUntil, when eventForwardGated is set, stops forwarding to the - // caller once closed (the run has been backgrounded). Nil means "never - // backgrounded". - eventForwardUntil <-chan struct{} } // typedAgentToolEventOptions carries the event-forward target (the caller's event @@ -372,40 +361,12 @@ func withAgentToolOptions(agentName string, opts []AgentRunOption) tool.Option { }) } -func withAgentToolEventForwardTarget(gen *AsyncGenerator[*AgentEvent]) tool.Option { - return withTypedAgentToolEventForwardTarget(gen) -} - func withTypedAgentToolEventForwardTarget[M MessageType](gen *AsyncGenerator[*TypedAgentEvent[M]]) tool.Option { return tool.WrapImplSpecificOptFn(func(o *typedAgentToolEventOptions[M]) { o.generator = gen }) } -// WithAgentToolEventForwardUntil pairs with the event-forward target -// (withAgentToolEventForwardTarget): the target says where the inner agent's -// events are forwarded, this says how long. It marks an AgentTool invocation as -// background-capable and forwards to the caller until done is closed, then stops. -// -// It is meant for a middleware that runs an AgentTool as a managed -// (backgroundable) task: pass the task's "backgrounded" signal as done. A run that -// is backgrounded outlives the caller's turn, which closes its event generator on -// turn end; forwarding after that point is both wrong (the events belong to a -// stream the user has stopped watching) and unsafe (a send on the closed generator -// would race). Gating on done stops forwarding at the moment the run detaches. -// -// Presence of this option also selects the non-panicking send used on this -// background-capable path (a plain foreground AgentTool, which never sets it, -// forwards with a panicking send so a send on an unexpectedly-closed generator -// surfaces as the bug it is). A nil done is treated as "never backgrounded": -// forwarding runs for the whole invocation. -func WithAgentToolEventForwardUntil(done <-chan struct{}) tool.Option { - return tool.WrapImplSpecificOptFn(func(o *agentToolOptions) { - o.eventForwardUntil = done - o.eventForwardGated = true - }) -} - // backgrounded reports whether the done signal has fired (the run has been moved // to the background). A nil done never fires — the run stays foreground for its // whole lifetime. diff --git a/adk/agent_tool_test.go b/adk/agent_tool_test.go index c321071b5..738152cfd 100644 --- a/adk/agent_tool_test.go +++ b/adk/agent_tool_test.go @@ -27,6 +27,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/cloudwego/eino/adk/internal/agenttool" "github.com/cloudwego/eino/components/model" "github.com/cloudwego/eino/components/tool" "github.com/cloudwego/eino/compose" @@ -1278,7 +1279,7 @@ func TestCrossTypeAgentToolGracefulError(t *testing.T) { } } -// --- Event-forward gate (WithAgentToolEventForwardUntil) --- +// --- Event-forward gate --- // collectForwarded drains a parent generator's iterator and returns the content of // every non-streaming message event forwarded to it. Runs in a goroutine; the @@ -1300,7 +1301,7 @@ func collectForwarded(iter *AsyncIterator[*AgentEvent], out *[]string, done chan // With the gate open (done never fires), a background-capable invocation forwards // the inner agent's events to the parent generator — same as a plain foreground // agent-as-tool call. -func TestAgentToolEventForwardUntil_OpenGateForwards(t *testing.T) { +func TestAgentToolEventForwardGate_OpenGateForwards(t *testing.T) { ctx := context.Background() sub := &emitEventsAgent{events: []*AgentEvent{ EventFromMessage(schema.AssistantMessage("child output", nil), nil, schema.Assistant, ""), @@ -1314,8 +1315,8 @@ func TestAgentToolEventForwardUntil_OpenGateForwards(t *testing.T) { // nil done => never backgrounded => gate stays open the whole run. _, err := at.InvokableRun(ctx, `{"request":"q"}`, - withAgentToolEventForwardTarget(gen), - WithAgentToolEventForwardUntil(nil)) + withTypedAgentToolEventForwardTarget(gen), + agenttool.WithForwardGate(nil)) require.NoError(t, err) gen.Close() @@ -1325,7 +1326,7 @@ func TestAgentToolEventForwardUntil_OpenGateForwards(t *testing.T) { // With the gate already closed (the run is backgrounded before it starts), a // background-capable invocation forwards nothing to the parent generator. -func TestAgentToolEventForwardUntil_ClosedGateDropsForwarding(t *testing.T) { +func TestAgentToolEventForwardGate_ClosedGateDropsForwarding(t *testing.T) { ctx := context.Background() sub := &emitEventsAgent{events: []*AgentEvent{ EventFromMessage(schema.AssistantMessage("child output", nil), nil, schema.Assistant, ""), @@ -1341,8 +1342,8 @@ func TestAgentToolEventForwardUntil_ClosedGateDropsForwarding(t *testing.T) { close(backgrounded) // already backgrounded _, err := at.InvokableRun(ctx, `{"request":"q"}`, - withAgentToolEventForwardTarget(gen), - WithAgentToolEventForwardUntil(backgrounded)) + withTypedAgentToolEventForwardTarget(gen), + agenttool.WithForwardGate(backgrounded)) require.NoError(t, err) gen.Close() @@ -1352,7 +1353,7 @@ func TestAgentToolEventForwardUntil_ClosedGateDropsForwarding(t *testing.T) { // On the gated (background-capable) path, forwarding to an already-closed parent // generator must not panic — a backgrounded run outlives the turn that closes it. -func TestAgentToolEventForwardUntil_ClosedParentGeneratorNoPanic(t *testing.T) { +func TestAgentToolEventForwardGate_ClosedParentGeneratorNoPanic(t *testing.T) { ctx := context.Background() sub := &emitEventsAgent{events: []*AgentEvent{ EventFromMessage(schema.AssistantMessage("child output", nil), nil, schema.Assistant, ""), @@ -1366,8 +1367,8 @@ func TestAgentToolEventForwardUntil_ClosedParentGeneratorNoPanic(t *testing.T) { require.NotPanics(t, func() { _, err := at.InvokableRun(ctx, `{"request":"q"}`, - withAgentToolEventForwardTarget(gen), - WithAgentToolEventForwardUntil(nil)) + withTypedAgentToolEventForwardTarget(gen), + agenttool.WithForwardGate(nil)) require.NoError(t, err) }) } diff --git a/adk/chatmodel.go b/adk/chatmodel.go index 04a6c9e4d..302a6f241 100644 --- a/adk/chatmodel.go +++ b/adk/chatmodel.go @@ -1461,7 +1461,7 @@ func (a *TypedChatModelAgent[M]) buildMessageReActRunFunc(_ context.Context, bc var runOpts []compose.Option runOpts = append(runOpts, mp.composeOpts...) if a.toolsConfig.EmitInternalEvents { - runOpts = append(runOpts, compose.WithToolsNodeOption(compose.WithToolOption(withAgentToolEventForwardTarget(mp.generator)))) + runOpts = append(runOpts, compose.WithToolsNodeOption(compose.WithToolOption(withTypedAgentToolEventForwardTarget(mp.generator)))) } if mp.input.EnableStreaming { runOpts = append(runOpts, compose.WithToolsNodeOption(compose.WithToolOption(withAgentToolEnableStreaming(true)))) diff --git a/adk/internal/agenttool/options.go b/adk/internal/agenttool/options.go new file mode 100644 index 000000000..440c6c85a --- /dev/null +++ b/adk/internal/agenttool/options.go @@ -0,0 +1,40 @@ +/* + * Copyright 2026 CloudWeGo Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Package agenttool carries ADK-internal AgentTool event-forwarding options +// shared between AgentTool and middleware plumbing. +package agenttool + +import "github.com/cloudwego/eino/components/tool" + +// ForwardGate controls the internal AgentTool event-forwarding gate. +type ForwardGate struct { + // Enabled marks the AgentTool invocation as background-capable: forwarding to + // the caller is bounded by Until and uses a non-panicking send. + Enabled bool + // Until stops forwarding to the caller once closed. Nil means "never + // backgrounded". + Until <-chan struct{} +} + +// WithForwardGate marks an AgentTool invocation as background-capable and +// forwards inner-agent events to the caller until done is closed. +func WithForwardGate(done <-chan struct{}) tool.Option { + return tool.WrapImplSpecificOptFn(func(o *ForwardGate) { + o.Enabled = true + o.Until = done + }) +} diff --git a/adk/middlewares/subagent/agent_tool.go b/adk/middlewares/subagent/agent_tool.go index a9d3f9ed2..ac3e2c02d 100644 --- a/adk/middlewares/subagent/agent_tool.go +++ b/adk/middlewares/subagent/agent_tool.go @@ -30,6 +30,7 @@ import ( "github.com/cloudwego/eino/adk/backgroundtask" "github.com/cloudwego/eino/adk/filesystem" "github.com/cloudwego/eino/adk/internal" + "github.com/cloudwego/eino/adk/internal/agenttool" "github.com/cloudwego/eino/components/tool" "github.com/cloudwego/eino/components/tool/utils" "github.com/cloudwego/eino/compose" @@ -126,7 +127,7 @@ func newManagedAgentTool(mgr *backgroundtask.Manager, subAgents map[string]tool. // run_in_background it is already closed here, for an auto-backgrounded // run it closes at the deadline. Foreground runs never fire it and // forward for their whole lifetime. - runOpts := append(opts, adk.WithAgentToolEventForwardUntil(task.Backgrounded)) + runOpts := append(opts, agenttool.WithForwardGate(task.Backgrounded)) out, runErr := a.InvokableRun(workCtx, params, runOpts...) if runErr != nil { return "", runErr