From 9b82d834870ea5f667984bcf0a707e9feb5a926e Mon Sep 17 00:00:00 2001 From: awschmeder Date: Wed, 15 Jul 2026 12:28:00 -0700 Subject: [PATCH] fix(runtime): stop content-only turns that end with a bare EOF A sub-agent reached via transfer_task returns control to its hub by finishing its turn: a final message with content and no tool call. When the provider closes that turn's SSE stream with a bare EOF and no per-choice finish_reason (common with OpenAI-compatible gateways such as litellm/CBORG), handleStream keyed its stop decision on empty content, so a content-bearing final turn with no tool calls reported Stopped=false. runTurn then re-entered the model with an unchanged message list and the model re-emitted the same completion forever, so the hand-off never completed. Key the bare-EOF stop decision on the absence of tool calls, matching the mid-stream path. A turn is terminal on bare EOF whenever no tool calls remain to execute, regardless of content. The empty-content cases the original #3145 guard covered (token limit, whitespace-only or reasoning-only reply) stay covered because they also have no tool calls. Add TestHandleStream_ContentOnlyBareEOFStops. Update TestCompactLiveSession_DuplicateSessionIDsCompactOnlyTargetEntry, whose older stream relied on a content-only bare-EOF turn continuing to a second model call; make that turn a tool call so it deterministically reaches its iteration boundary independent of the stop rule. Fixes #3668 Signed-off-by: awschmeder --- pkg/runtime/live_sessions_test.go | 18 +++++++++++++----- pkg/runtime/streaming.go | 19 ++++++++++++------- pkg/runtime/streaming_test.go | 27 +++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 12 deletions(-) diff --git a/pkg/runtime/live_sessions_test.go b/pkg/runtime/live_sessions_test.go index 2baf59e084..2d505df73b 100644 --- a/pkg/runtime/live_sessions_test.go +++ b/pkg/runtime/live_sessions_test.go @@ -446,14 +446,22 @@ func TestCompactLiveSession_DuplicateSessionIDsCompactOnlyTargetEntry(t *testing startedB := make(chan struct{}) releaseB := make(chan struct{}) prov := &stepProvider{id: "test/mock-model", steps: []providerStep{ - // Older stream turn 1: kept in flight while the newer stream - // registers under the same session ID. - {stream: newStreamBuilder().AddContent("older working").Build(), started: startedA, release: releaseA}, + // Older stream turn 1: a tool call keeps the stream live (the loop + // continues to execute the call) while the newer stream registers + // under the same session ID. A tool-call turn is used rather than a + // bare content turn so the older stream deterministically reaches a + // second model call regardless of the bare-EOF stop rule. + {stream: newStreamBuilder(). + AddToolCallName("call_older", "unknown_tool"). + AddToolCallArguments("call_older", "{}"). + AddToolCallStopWithUsage(1, 1). + Build(), started: startedA, release: releaseA}, // Newer stream turn 1, gated so it stays live throughout. {stream: newStreamBuilder().AddContent("newer working").Build(), started: startedB, release: releaseB}, // Older stream turn 2: natural stop. With the request left alone this - // is the older stream's next model call; stealing the request would - // consume this step as the compaction summary instead. + // is the older stream's next model call (after the tool result feeds + // back in); stealing the request would consume this step as the + // compaction summary instead. {stream: newStreamBuilder().AddStopWithUsage(1, 1).Build()}, // The compaction summary call, drained by the newer stream's own // iteration boundary. diff --git a/pkg/runtime/streaming.go b/pkg/runtime/streaming.go index 4c07ba4c0a..4f202c30a1 100644 --- a/pkg/runtime/streaming.go +++ b/pkg/runtime/streaming.go @@ -359,14 +359,19 @@ mainLoop: applyXMLFallback() - // If the stream completed without producing any usable content or tool - // calls, likely because of a token limit, stop to avoid breaking the request - // loop. Whitespace-only content counts as no output: it carries no answer, - // and runTurn's empty-turn detection uses the same trimmed-empty test, so - // treating it as stopped here guarantees that an empty-turn warning is always - // followed by a turn exit rather than an identical-message re-entry (#3145). + // The stream ended with a bare EOF: the provider closed the SSE + // connection without ever sending a per-choice finish_reason (common with + // OpenAI-compatible gateways such as litellm/CBORG, which emit only a + // terminal [DONE] sentinel). In that case a turn is terminal whenever there + // are no tool calls left to execute: there is nothing for the outer run + // loop to continue on, so it must stop regardless of whether the assistant + // produced content. Keying only on "empty content" (the original #3145 + // guard) leaves a non-empty final message with no tool calls reporting + // Stopped=false, which makes runTurn re-enter the loop and re-emit the same + // message forever. The empty-content case (token limit, whitespace-only + // reply) remains covered because it also has no tool calls. // NOTE(krissetto): this can likely be removed once compaction works properly with all providers (aka dmr) - stoppedDueToNoOutput := strings.TrimSpace(fullContent.String()) == "" && len(toolCalls) == 0 + stoppedDueToNoOutput := len(toolCalls) == 0 // Prefer the provider's explicit finish reason when available (e.g. // tool_calls). Only fall back to inference when no explicit reason was diff --git a/pkg/runtime/streaming_test.go b/pkg/runtime/streaming_test.go index f287faa3d0..129aa66e72 100644 --- a/pkg/runtime/streaming_test.go +++ b/pkg/runtime/streaming_test.go @@ -190,6 +190,33 @@ func TestHandleStream_WhitespaceOnlyContentStops(t *testing.T) { "a whitespace-only, bare-EOF turn must stop so the empty-turn warning is followed by a turn exit, not an identical re-entry (#3145)") } +// TestHandleStream_ContentOnlyBareEOFStops guards the loop that OpenAI-compatible +// gateways (litellm/CBORG) trigger: they close the SSE stream with a bare EOF and +// never send a per-choice finish_reason. A turn that produced real content but no +// tool calls has nothing left for the run loop to execute, so it must report +// Stopped=true. Keying the stop decision on empty content instead of "no tool +// calls" left such a final message with Stopped=false, so runTurn re-entered the +// model with identical messages and re-emitted the same completion text forever. +func TestHandleStream_ContentOnlyBareEOFStops(t *testing.T) { + stream := newStreamBuilder(). + AddContent("Frontmatter ingestion complete."). // real content, no tool calls + Build() // no terminal chunk: bare EOF, no finish reason + + a := agent.New("root", "test", agent.WithModel(&mockProvider{id: "test/mock-model", stream: stream})) + sess := session.New(session.WithUserMessage("go")) + + evCh := make(chan Event, 64) + res, err := handleStream( + t.Context(), nil, stream, a, nil, sess, nil, + defaultTelemetry{}, NewChannelSink(evCh), defaultStreamIdleTimeout, + ) + require.NoError(t, err) + + assert.Empty(t, res.Calls) + assert.True(t, res.Stopped, + "a content-bearing, bare-EOF turn with no tool calls must stop so the run loop exits instead of re-entering with identical messages") +} + // stalledStream is a chat.MessageStream that blocks in Recv() until // either unblocked or the stream is closed. It is used to simulate a // half-open TCP connection where the remote side stops sending data.