diff --git a/packages/agent/.changes/empty-turn-retry.md b/packages/agent/.changes/empty-turn-retry.md new file mode 100644 index 0000000000..c3d0d9eeec --- /dev/null +++ b/packages/agent/.changes/empty-turn-retry.md @@ -0,0 +1 @@ +- Fixed the agent loop treating an empty final assistant turn (no output content and no tool calls, e.g. a provider ending the stream thinking-only with a normal stop reason) as successful completion: the request is silently resent up to 3 attempts without the empty turns polluting the retry context or transcript, and a turn error is surfaced after the third consecutive empty response. diff --git a/packages/agent/src/agent-loop.ts b/packages/agent/src/agent-loop.ts index 31b3a15ccc..d10b5f4530 100644 --- a/packages/agent/src/agent-loop.ts +++ b/packages/agent/src/agent-loop.ts @@ -8,6 +8,7 @@ import { type AssistantMessageEvent, type Context, EventStream, + isContextOverflow, streamSimple, type ToolResultMessage, validateToolArguments, @@ -448,12 +449,55 @@ async function runLoop( await emit({ type: "agent_end", messages: newMessages }); } +const MAX_EMPTY_TURN_ATTEMPTS = 3; + +/** + * A final turn with no tool calls and no non-thinking content. Providers occasionally + * end a stream like this with a normal stop reason; treating it as completion would + * silently abandon the task, so it is retried instead. Error, abort, and length turns + * are excluded: they are signals of their own, and an identical resend cannot help. + */ +function isEmptyAssistantTurn(message: AssistantMessage): boolean { + if (message.stopReason === "error" || message.stopReason === "aborted" || message.stopReason === "length") { + return false; + } + return !message.content.some( + (part) => part.type === "toolCall" || (part.type === "text" && part.text.trim().length > 0), + ); +} + async function streamAssistantResponse( context: AgentContext, config: AgentLoopConfig, signal: AbortSignal | undefined, emit: AgentEventSink, streamFn?: StreamFn, +): Promise { + for (let attempt = 1; ; attempt++) { + const message = await streamAssistantResponseAttempt(context, config, signal, emit, streamFn); + // Overflow turns must pass through untouched so compaction recovery can see them. + if (isEmptyAssistantTurn(message) && !isContextOverflow(message, config.model.contextWindow)) { + if (attempt < MAX_EMPTY_TURN_ATTEMPTS) { + // Drop the empty attempt so it is neither resent to the provider nor + // finalized as a transcript turn (message_end is what makes it durable). + context.messages.pop(); + continue; + } + message.stopReason = "error"; + message.errorMessage = `Model returned an empty response (no output content or tool calls) ${MAX_EMPTY_TURN_ATTEMPTS} times in a row`; + } + await emit({ type: "message_end", message }); + return message; + } +} + +/** Runs one assistant stream and places the final message in context, without emitting message_end. */ +async function streamAssistantResponseAttempt( + context: AgentContext, + config: AgentLoopConfig, + signal: AbortSignal | undefined, + emit: AgentEventSink, + streamFn?: StreamFn, ): Promise { let partialMessage: AssistantMessage | null = null; let addedPartial = false; @@ -465,7 +509,6 @@ async function streamAssistantResponse( context.messages.push(finalMessage); await emit({ type: "message_start", message: { ...finalMessage } }); } - await emit({ type: "message_end", message: finalMessage }); return finalMessage; }; @@ -559,7 +602,6 @@ async function streamAssistantResponse( if (!addedPartial) { await emit({ type: "message_start", message: { ...finalMessage } }); } - await emit({ type: "message_end", message: finalMessage }); return finalMessage; } } @@ -572,7 +614,6 @@ async function streamAssistantResponse( context.messages.push(finalMessage); await emit({ type: "message_start", message: { ...finalMessage } }); } - await emit({ type: "message_end", message: finalMessage }); return finalMessage; } catch (error) { if (signal?.aborted && isAbortError(error)) { diff --git a/packages/agent/test/agent-loop.test.ts b/packages/agent/test/agent-loop.test.ts index d8167c0c13..7602405e43 100644 --- a/packages/agent/test/agent-loop.test.ts +++ b/packages/agent/test/agent-loop.test.ts @@ -1964,3 +1964,112 @@ describe("agentLoopContinue with AgentMessage", () => { expect(messages[0].role).toBe("assistant"); }); }); + +describe("empty assistant turn retry", () => { + function emptyTurnConfig(): AgentLoopConfig { + return { + model: createModel(), + convertToLlm: identityConverter, + }; + } + + function streamFnReturning(messages: AssistantMessage[]) { + const requests: Message[][] = []; + let call = 0; + const streamFn = ((_model: unknown, llmContext: { messages: Message[] }) => { + requests.push(llmContext.messages.slice()); + const message = messages[Math.min(call, messages.length - 1)]; + call += 1; + const stream = new MockAssistantStream(); + queueMicrotask(() => { + stream.push({ type: "done", reason: "stop", message }); + }); + return stream; + }) as unknown as Parameters[5]; + return { streamFn, requests }; + } + + async function runOnce(message: AssistantMessage) { + const context: AgentContext = { systemPrompt: "sys", messages: [], tools: [] }; + const { streamFn, requests } = streamFnReturning([message]); + const events: AgentEvent[] = []; + const messages = await runAgentLoop( + [createUserMessage("Hello")], + context, + emptyTurnConfig(), + (event) => { + events.push(event); + }, + undefined, + streamFn, + ); + return { requests, events, assistant: messages.find((m) => m.role === "assistant") as AssistantMessage }; + } + + it("silently retries empty turns and keeps the transcript clean", async () => { + const context: AgentContext = { systemPrompt: "sys", messages: [], tools: [] }; + const thinkingOnly = createAssistantMessage([{ type: "thinking", thinking: "pondering..." }]); + const whitespaceOnly = createAssistantMessage([{ type: "text", text: " \n" }]); + const goodMessage = createAssistantMessage([{ type: "text", text: "done" }]); + const { streamFn, requests } = streamFnReturning([thinkingOnly, whitespaceOnly, goodMessage]); + const events: AgentEvent[] = []; + + const messages = await runAgentLoop( + [createUserMessage("Hello")], + context, + emptyTurnConfig(), + (event) => { + events.push(event); + }, + undefined, + streamFn, + ); + + expect(requests.length).toBe(3); + // Discarded attempts are neither resent to the provider nor kept as transcript turns. + expect(requests[1].filter((message) => message.role === "assistant")).toEqual([]); + expect(requests[2].filter((message) => message.role === "assistant")).toEqual([]); + expect(messages.filter((message) => message.role === "assistant")).toEqual([goodMessage]); + const assistantEnds = events.filter( + (event) => event.type === "message_end" && event.message.role === "assistant", + ); + expect(assistantEnds.length).toBe(1); + }); + + it("surfaces an error after three consecutive empty turns", async () => { + const { requests, assistant } = await runOnce(createAssistantMessage([{ type: "thinking", thinking: "..." }])); + + expect(requests.length).toBe(3); + expect(assistant.stopReason).toBe("error"); + expect(assistant.errorMessage).toMatch(/empty response/i); + }); + + it("does not retry turns with visible content, a length stop, or a silent overflow", async () => { + const visible = await runOnce( + createAssistantMessage([ + { type: "thinking", thinking: "..." }, + { type: "text", text: "partial but visible" }, + ]), + ); + expect(visible.requests.length).toBe(1); + expect(visible.assistant.stopReason).toBe("stop"); + + const lengthStop = await runOnce(createAssistantMessage([{ type: "thinking", thinking: "..." }], "length")); + expect(lengthStop.requests.length).toBe(1); + expect(lengthStop.assistant.stopReason).toBe("length"); + expect(lengthStop.assistant.errorMessage).toBeUndefined(); + + // z.ai-style silent overflow: normal stop, empty content, input past the window. + // It must reach message_end untouched so compaction recovery can see it. + const overflowMessage = createAssistantMessage([{ type: "text", text: "" }]); + overflowMessage.usage.input = createModel().contextWindow + 1; + const overflow = await runOnce(overflowMessage); + expect(overflow.requests.length).toBe(1); + expect(overflow.assistant).toBe(overflowMessage); + expect(overflow.assistant.stopReason).toBe("stop"); + expect(overflow.assistant.errorMessage).toBeUndefined(); + expect(overflow.events.some((event) => event.type === "message_end" && event.message === overflowMessage)).toBe( + true, + ); + }); +}); diff --git a/packages/coding-agent/.changes/empty-turn-retry.md b/packages/coding-agent/.changes/empty-turn-retry.md new file mode 100644 index 0000000000..5a3bcdbc55 --- /dev/null +++ b/packages/coding-agent/.changes/empty-turn-retry.md @@ -0,0 +1 @@ +- Surfaced an RLM child whose final turn ended in an error (e.g. exhausted empty-response retries) to the parent as a child failure message instead of a bare completed-without-reply notice. diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index c0e036cf1d..035738cae1 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -10636,15 +10636,28 @@ export class AgentSession { !run.suppressTerminalNotice && child._parentReplyCount === parentReplyCountBeforeRun ) { - const lastAssistantText = child.getLastAssistantText(); - await deliverTerminalMessageToParent( - createRlmChildTerminalNoticeMessage({ - kind: "completed_without_reply", - childId: run.id, - sessionName, - lastAssistantTextPreview: lastAssistantText ? compactRlmText(lastAssistantText) : undefined, - }), - ); + // A turn that ends with a graceful error message resolves promptAndWait, + // so it must be surfaced here or the parent never learns the task failed. + const lastAssistant = this._findLastAssistantInMessages(child.messages); + if (lastAssistant?.stopReason === "error") { + await deliverTerminalMessageToParent( + createRlmChildFailureMessage({ + childId: run.id, + sessionName, + error: lastAssistant.errorMessage ?? "Assistant turn failed", + }), + ); + } else { + const lastAssistantText = child.getLastAssistantText(); + await deliverTerminalMessageToParent( + createRlmChildTerminalNoticeMessage({ + kind: "completed_without_reply", + childId: run.id, + sessionName, + lastAssistantTextPreview: lastAssistantText ? compactRlmText(lastAssistantText) : undefined, + }), + ); + } } if (!this.registerRlmChildSession(run.id, child) && !run.detachedDeletion) { if (childRuntime && this._subagentRuntimeHost?.releaseRlmSubagentRuntime) { diff --git a/packages/coding-agent/test/agent-session-recursion.test.ts b/packages/coding-agent/test/agent-session-recursion.test.ts index d085d010a2..b52d29835d 100644 --- a/packages/coding-agent/test/agent-session-recursion.test.ts +++ b/packages/coding-agent/test/agent-session-recursion.test.ts @@ -1332,6 +1332,49 @@ describe("AgentSession rlm recursion", () => { }); }); + it("surfaces a child turn that exhausts empty-response retries as a failure to the parent", async () => { + const emptyAssistantMessage = (): AssistantMessage => ({ + role: "assistant", + content: [{ type: "thinking", thinking: "pondering" }], + api: model.api, + provider: model.provider, + model: model.id, + usage: usage(), + stopReason: "stop", + timestamp: Date.now(), + }); + const root = createSession({ + // The child task prompt streams empty turns; the parent's reaction to the failure notice answers normally. + streamFn: (_model, context) => { + if (!userText(context).includes("empty child")) { + return streamAnswer("acknowledged"); + } + const stream = createAssistantMessageEventStream(); + queueMicrotask(() => { + stream.push({ type: "done", reason: "stop", message: emptyAssistantMessage() }); + }); + return stream; + }, + }); + // Session-level auto-retry also retries error turns; disable it so the + // child's turn error surfaces immediately instead of after backoff. + root.settingsManager.setRetryEnabled(false); + + await root.runRlmChild("empty child", { name: "empty-worker" }); + await vi.waitFor(() => { + const failures = root.messages.filter( + (message) => message.role === "custom" && message.customType === "rlm_child_failure", + ); + expect(failures).toHaveLength(1); + expect((failures[0] as { content: string }).content).toMatch(/empty response/i); + }); + expect( + root.messages.filter( + (message) => message.role === "custom" && message.customType === "rlm_child_terminal_notice", + ), + ).toHaveLength(0); + }); + it("suppresses a done child's unsettled fallback notice at the cancellation cut", async () => { const root = createSession(); let suppressed = false;