diff --git a/CHANGELOG.md b/CHANGELOG.md index 7da3b2eef..cc88c90d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Changed +- `AgentLoopConfig.toolCallLeakRetries` is removed. The low-level loop no longer guesses a tool intent from markup in text; it records the structural classification (`classifyIncompleteAssistantTurn()`, now in `@step-harness/providers`) on every finished assistant message as `incomplete`, and hosts decide what to do about it. - Plan mode is now entered only by the user, with `/plan` or `--plan`; the `enter_plan_mode` tool is removed. The default system prompt no longer carries a `# Planning` section. Entering plan mode appends one hidden `` block with the plan method and plan path, and leaving it appends a short Default block, so the system prompt and tool block stay byte-identical across a plan cycle. Sessions recorded with an agent-initiated plan still restore. See [plan approval](docs/plan-approval.md#mode-announcements). - Subagents are no longer an automatic escalation. The `subagent` tool description now carries an explicit-request gate and a delegation method (critical path first, self-contained briefs, disjoint write sets, no redoing delegated work), replacing the system prompt's `# Delegation` section; `# Coordination primitives` only routes between authorized primitives. Session-standing `/ultracode on` also authorizes proactive delegation. See [orchestration lifecycle](docs/orchestration-lifecycle.md#subagent-delegation-authorization). - `--mode json` and `--mode rpc` now write only session **facts**: `message_update` and `tool_execution_update` are live frames — each superseded by `message_end` / `tool_execution_end` — and no longer appear in that stream. A harness that redirects one of them to a file therefore gets a record proportional to the work done, not to the tokens streamed; a subagent call that wrote 4,992 MiB of repeated child transcripts now writes a few KiB. Pass the new `--include-live-events` flag to get the frames back: a live viewer wants them, a log does not. See [JSON output](packages/coding-agent/docs/json.md) and [RPC](packages/coding-agent/docs/rpc.md). @@ -11,6 +12,8 @@ ### Fixed +- Recover a normal assistant response that contains neither visible text nor an executable tool call: keep the original message in history unchanged, append one hidden user-role recovery prompt, and continue. Recovery is bounded by a streak of consecutive empty turns. Only a turn with visible text or an executed tool call resets it, and so does any user input, so a long autonomous run is not ended by one transient empty response and a provider error in between does not refill the budget. Automatic compaction runs first when the response also exceeded the context window. +- A run that ends without a usable answer reports it on `agent_settled` as `failure` (reason and message), instead of rewriting the assistant message's provider stop reason, so a settled run no longer means success by itself. Subagent lanes and goal continuation act on it, and print/JSON pick their exit code from the same classification, so no consumer decides success from `stopReason` alone. `assistantTerminalFailure()` in `@step-harness/providers` applies the same rule to a message that has not settled yet. The hidden recovery message records the entry it answers in `details.recoversEntryId`. - Summary requests now use the provider's model output limit and remaining-context sizing instead of fixed compaction/branch caps or reserve-derived budgets. Existing reserve arguments remain accepted for call compatibility; automatic trigger and branch source-selection settings are unchanged. - Compaction and branch-summary inputs omit provider thinking blocks while retaining visible replies, tool calls, diagnostic output, and existing skill-retention rules. Original session history is unchanged. These changes adapt Pi commits `15e7bdc` and `6fcb486` (upstream PR #11036). diff --git a/apps/cli/src/ui/runtime/session-events.ts b/apps/cli/src/ui/runtime/session-events.ts index 009fc658c..c83c05561 100644 --- a/apps/cli/src/ui/runtime/session-events.ts +++ b/apps/cli/src/ui/runtime/session-events.ts @@ -28,6 +28,8 @@ import { type AgentSessionEvent, createCompactionSummaryMessage, getStepGoalStatus, + INCOMPLETE_TURN_RECOVERY_ATTEMPTS, + INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE, theme, } from "@step-harness/coding-agent"; import { Spacer, Text } from "@step-harness/pi-tui"; @@ -135,6 +137,26 @@ export async function handleSessionEvent(ctx: RuntimeContext, event: AgentSessio case "message_start": if (event.message.role === "custom") { + // This event starts the actual recovery input, unlike the earlier turn + // classification. Use past tense because showStatus adds a history row. + if (event.message.customType === INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE) { + const details = event.message.details; + const recordedAttempt = + typeof details === "object" && details !== null && "attempt" in details ? details.attempt : undefined; + const recordedMax = + typeof details === "object" && details !== null && "maxAttempts" in details + ? details.maxAttempts + : undefined; + const attempt = + typeof recordedAttempt === "number" && Number.isInteger(recordedAttempt) && recordedAttempt > 0 + ? recordedAttempt + : 1; + const maxAttempts = + typeof recordedMax === "number" && Number.isInteger(recordedMax) && recordedMax > 0 + ? recordedMax + : INCOMPLETE_TURN_RECOVERY_ATTEMPTS; + ctx.showStatus(`Retry attempt ${attempt}/${maxAttempts} started after an empty model turn.`); + } ctx.addMessageToChat(event.message); ctx.redraw.requestRender(); } else if (event.message.role === "user") { @@ -346,6 +368,9 @@ export async function handleSessionEvent(ctx: RuntimeContext, event: AgentSessio break; case "agent_settled": + // A settled run is not automatically a successful one, and the transcript line + // alone reads as a remark about one turn. Say the run ended, and why. + if (event.failure) ctx.showError(event.failure.message); await ctx.checkShutdownRequested(); break; diff --git a/apps/cli/src/ui/view/transcript/assistant-message.ts b/apps/cli/src/ui/view/transcript/assistant-message.ts index 9525a482c..ab1a7722c 100644 --- a/apps/cli/src/ui/view/transcript/assistant-message.ts +++ b/apps/cli/src/ui/view/transcript/assistant-message.ts @@ -9,6 +9,7 @@ import { Text, } from "@step-harness/pi-tui"; import type { AssistantMessage } from "@step-harness/providers"; +import { INCOMPLETE_TURN_MESSAGES } from "@step-harness/providers"; import { createLiveMarkdownTransform } from "./markdown-transform.ts"; const OSC133_ZONE_START = "\x1b]133;A\x07"; @@ -219,6 +220,13 @@ export class AssistantMessageComponent extends Container { const errorMsg = message.errorMessage || "Unknown error"; this.contentContainer.addChild(new Spacer(1)); this.contentContainer.addChild(new Text(theme.fg("error", `Error: ${errorMsg}`), this.outputPad, 0)); + } else if (message.incomplete && INCOMPLETE_TURN_MESSAGES[message.incomplete]) { + // A turn's classification cannot prove that recovery started or the run + // ended. The live host presents those actions from their own events. + this.contentContainer.addChild(new Spacer(1)); + this.contentContainer.addChild( + new Text(theme.fg("dim", INCOMPLETE_TURN_MESSAGES[message.incomplete]), this.outputPad, 0), + ); } } } diff --git a/apps/cli/src/ui/view/transcript/step-message.ts b/apps/cli/src/ui/view/transcript/step-message.ts index dbba2ff95..df967347c 100644 --- a/apps/cli/src/ui/view/transcript/step-message.ts +++ b/apps/cli/src/ui/view/transcript/step-message.ts @@ -141,6 +141,7 @@ function makeNativeRunMessage(message: AssistantMessage, content: VisibleAssista // an extra error/length row of its own. stopReason: "stop", errorMessage: undefined, + incomplete: undefined, }; } @@ -707,7 +708,8 @@ export class StepAssistantMessageComponent extends AssistantMessageComponent { const hasToolCalls = message.content.some((content) => content.type === "toolCall"); const needsStatus = message.stopReason === "length" || - (!hasToolCalls && (message.stopReason === "aborted" || message.stopReason === "error")); + (!hasToolCalls && + (message.stopReason === "aborted" || message.stopReason === "error" || !!message.incomplete)); if (!needsStatus) { this.nativeStatusComponent = undefined; return; @@ -716,6 +718,7 @@ export class StepAssistantMessageComponent extends AssistantMessageComponent { const statusMessage = makeNativeRunMessage(message, []); statusMessage.stopReason = message.stopReason; statusMessage.errorMessage = message.errorMessage; + statusMessage.incomplete = message.incomplete; if (!this.nativeStatusComponent) { this.nativeStatusComponent = new AssistantMessageComponent( statusMessage, diff --git a/apps/cli/test/assistant-message.test.ts b/apps/cli/test/assistant-message.test.ts index 161fe360f..2f8ddecca 100644 --- a/apps/cli/test/assistant-message.test.ts +++ b/apps/cli/test/assistant-message.test.ts @@ -11,7 +11,7 @@ const OSC133_ZONE_FINAL = "\x1b]133;C\x07"; function createAssistantMessage( content: AssistantMessage["content"], - overrides: Partial> = {}, + overrides: Partial> = {}, ): AssistantMessage { return { role: "assistant", @@ -28,6 +28,7 @@ function createAssistantMessage( cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, }, stopReason: overrides.stopReason ?? "stop", + incomplete: overrides.incomplete, timestamp: Date.now(), }; } @@ -73,6 +74,48 @@ describe("AssistantMessageComponent", () => { expect(rendered).toContain("Response was truncated before completion."); }); + test("renders an incomplete turn without assuming a run outcome", () => { + initTheme("dark"); + + const component = new AssistantMessageComponent( + createAssistantMessage([{ type: "thinking", thinking: "private reasoning" }], { + stopReason: "stop", + incomplete: "no_visible_output", + }), + true, + ); + const rendered = stripAnsi(component.render(80).join("\n")); + + expect(rendered).toContain("The model returned no visible answer or executable tool call."); + expect(rendered).not.toMatch(/retried|retry limit|run ended/); + }); + + test.each([1, 2])("classification streak %s does not prove recovery or termination", (consecutiveTurns) => { + initTheme("dark"); + + const message = createAssistantMessage([{ type: "thinking", thinking: "private reasoning" }], { + stopReason: "stop", + incomplete: "no_visible_output", + }); + message.diagnostics = [ + { type: "incomplete_turn", timestamp: 1, details: { consecutiveTurns, maxAttempts: 1 } }, + ]; + const rendered = stripAnsi(new AssistantMessageComponent(message, true).render(80).join("\n")); + + expect(rendered).toContain("The model returned no visible answer or executable tool call."); + expect(rendered).not.toMatch(/retried|retry limit|run ended/); + }); + + test("renders a completed turn without a status line", () => { + initTheme("dark"); + + const component = new AssistantMessageComponent(createAssistantMessage([{ type: "text", text: "answer" }]), true); + const rendered = stripAnsi(component.render(80).join("\n")); + + expect(rendered).toContain("answer"); + expect(rendered).not.toContain("no visible answer"); + }); + test("coalesces adjacent thinking blocks into one hidden thinking label", () => { initTheme("dark"); diff --git a/apps/cli/test/session-events-working-tracker.test.ts b/apps/cli/test/session-events-working-tracker.test.ts index f8f2287e9..55ff2ccdc 100644 --- a/apps/cli/test/session-events-working-tracker.test.ts +++ b/apps/cli/test/session-events-working-tracker.test.ts @@ -3,6 +3,56 @@ import type { AssistantMessage } from "@step-harness/providers"; import { WorkingOutputTracker } from "../src/ui/view/chrome/status-indicator.ts"; import type { RuntimeContext } from "../src/ui/runtime/context.ts"; import { handleSessionEvent } from "../src/ui/runtime/session-events.ts"; +import { INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE } from "@step-harness/coding-agent"; + +test("announces only a recovery that actually starts and uses its attempt metadata", async () => { + const showStatus = vi.fn(); + const ctx = { + isInitialized: true, + footer: { invalidate: vi.fn() }, + showStatus, + addMessageToChat: vi.fn(), + redraw: { requestRender: vi.fn() }, + } as unknown as RuntimeContext; + await handleSessionEvent(ctx, { + type: "message_start", + message: { role: "custom", customType: "other", content: "", display: false, timestamp: 1 }, + }); + expect(showStatus).not.toHaveBeenCalled(); + await handleSessionEvent(ctx, { + type: "message_start", + message: { + role: "custom", + customType: INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE, + content: "", + display: false, + timestamp: 2, + details: { attempt: 2, maxAttempts: 3 }, + }, + }); + expect(showStatus).toHaveBeenCalledExactlyOnceWith("Retry attempt 2/3 started after an empty model turn."); + expect(showStatus.mock.calls[0][0]).not.toContain("retrying"); +}); + +test("settlement displays a final failure even when it equals the turn explanation", async () => { + const showError = vi.fn(); + const ctx = { + isInitialized: true, + footer: { invalidate: vi.fn() }, + showError, + checkShutdownRequested: vi.fn(), + } as unknown as RuntimeContext; + const failure = { + kind: "incomplete_turn" as const, + reason: "no_visible_output" as const, + message: "The model returned no visible answer or executable tool call.", + }; + await handleSessionEvent(ctx, { type: "agent_settled", failure }); + expect(showError).toHaveBeenCalledExactlyOnceWith(failure.message); + showError.mockClear(); + await handleSessionEvent(ctx, { type: "agent_settled" }); + expect(showError).not.toHaveBeenCalled(); +}); test.each(["agent_end", "auto_clm_end", "compaction_end"] as const)( "clears terminal progress on %s even after the setting is disabled", diff --git a/docs/completion-check.md b/docs/completion-check.md index e6d4f4e42..9a32bf7ec 100644 --- a/docs/completion-check.md +++ b/docs/completion-check.md @@ -108,10 +108,14 @@ review is not sent. Budget exhaustion and final-output exit codes are unchanged. After the follow-up budget is exhausted, a valid final answer still returns exit code **0** even if Git conditions remain unsatisfied. The canonical task verifier owns the score; an ordinary failed task must not become an infrastructure error -that resamples the attempt. Missing/thinking-only final output returns **2** with -an explicit incomplete diagnostic. Existing terminal denials and final assistant -errors keep exit code **1**. A successful external-approval handoff produced by the -latest prompt keeps exit code **0** even when the last message is its tool result. +that reruns the attempt. Missing/thinking-only final output returns **2** with +an explicit incomplete diagnostic. When either completion feature is enabled, +print mode disables the generic incomplete-turn recovery for the whole +invocation so the checker's bounded follow-up budget remains the only recovery +path, including a mid-run empty turn. Existing terminal +denials and final assistant errors keep exit code **1**. A successful +external-approval handoff produced by the latest prompt keeps exit code **0** +even when the last message is its tool result. An earlier handoff does not excuse missing output from a later explicit prompt: that prompt must provide final text or a fresh handoff, otherwise the exit code is **2**. Automatic follow-ups stay blocked for the rest of the invocation. diff --git a/docs/orchestration-lifecycle.md b/docs/orchestration-lifecycle.md index c9cb86ac2..adb6ba63b 100644 --- a/docs/orchestration-lifecycle.md +++ b/docs/orchestration-lifecycle.md @@ -146,7 +146,7 @@ subagent runner passes no ACL: its children read as top-level and each started a further wave of workflow agents, multiplying concurrent provider streams well past the account limit and producing cascading rate-limit failures. -A turn settles on `agent_settled`, a failed prompt ack, or child exit. Because a +A turn settles on `agent_settled`, a failed prompt ack, or child exit. Settlement is not success: a run that produced no usable answer reports it on `agent_settled.failure`, and the lane's exit code follows that rather than the absence of an error in the transcript. Because a child that stays alive without emitting any of those would strand its parent indefinitely, and `executeSubagent` awaits every lane, each turn also carries an idle watchdog. The budget is measured against child output rather than wall clock: diff --git a/docs/research/2026-10-27-subagent-live-update-amplification/README.md b/docs/research/2026-10-27-subagent-live-update-amplification/README.md index 0c729fe54..4bca9afe0 100644 --- a/docs/research/2026-10-27-subagent-live-update-amplification/README.md +++ b/docs/research/2026-10-27-subagent-live-update-amplification/README.md @@ -185,6 +185,8 @@ a reader reconstructs the transcript from `message_end` exactly as before. and the loop can emit more `message_end` events than it lists (a resampled tool-call leak, a throwing `shouldStopAfterTurn`). A reader detects a truncated run by a message that never got its `message_end`, not by comparing counts. +(The leak resampling has since been removed; an incomplete turn is now reported +on the message and the settled run, so only the throwing hook can still do this.) This is boundary-payload deduplication, **not** an exactly-once record. What still repeats, and why it stays: diff --git a/docs/tui-rendering-pipeline.md b/docs/tui-rendering-pipeline.md index 1a67faba6..b7ae687fb 100644 --- a/docs/tui-rendering-pipeline.md +++ b/docs/tui-rendering-pipeline.md @@ -43,6 +43,12 @@ 产品侧接线在 `apps/cli/src/ui/`：`interactive-mode.ts`（组合根）、`runtime/input-dispatch.ts`（键位动作注册与提交路由）、`runtime/session-events.ts`（会话事件 → 组件树）、`runtime/redraw.ts`（渲染门面）。 +### 空回合与运行失败 + +`AssistantMessage.incomplete` 仅说明回合没有可见回答或可执行调用，transcript 用中性说明展示；分类诊断不证明重试已开始，也不证明运行已结束。 + +`runtime/session-events.ts` 在隐藏恢复消息的 `message_start` 时展示开始重试的历史行，次数来自消息的 `details.attempt/maxAttempts`。`agent_settled.failure` 则通过 `showError` 展示红色终态错误，即使其文案与回合说明相同也不省略。历史重载保留回合分类，不从连续空回合数推断运行终态。 + ## 1. 终端协商与输出原语 `ProcessTerminal.start`（`terminal.ts:161`）按顺序做： diff --git a/packages/agent-core/README.md b/packages/agent-core/README.md index 5b87d7dfd..297fe17db 100644 --- a/packages/agent-core/README.md +++ b/packages/agent-core/README.md @@ -159,7 +159,9 @@ const stream = agentLoop( `shouldStopAfterTurn` runs after `turn_end` is emitted and after the assistant response and any tool executions have completed normally. If it returns `true`, the loop emits `agent_end` and exits before polling steering or follow-up queues, and before starting another LLM call. It does not abort the provider stream, does not cancel running tools, and does not alter the assistant message stop reason. The `AgentOptions` callback also receives the active run's `AbortSignal` as its second argument. -To change the context right before a provider request instead of stopping, set `beforeModelRequest` (on `AgentLoopConfig`, `AgentOptions`, or the `Agent` instance). It runs before every provider request: `phase: "initial"` for the first request of a prompt or `continue()`, and `phase: "follow-up"` for each later request. The context it receives already contains the prompts, completed tool results, and steering injected for that request. Return `{ context, model, thinkingLevel }` replacements, or `undefined` to send the context unchanged. Tool-call markup resampling of the same request does not call it again. On follow-up requests that have not received steering yet, the loop polls steering once more after the hook, so input queued during a long hook such as compaction reaches the same request. `prepareNextTurn` keeps its existing timing: after `turn_end`, before steering is polled. +To change the context right before a provider request instead of stopping, set `beforeModelRequest` (on `AgentLoopConfig`, `AgentOptions`, or the `Agent` instance). It runs before every provider request: `phase: "initial"` for the first request of a prompt or `continue()`, and `phase: "follow-up"` for each later request. The context it receives already contains the prompts, completed tool results, and steering injected for that request. Return `{ context, model, thinkingLevel }` replacements, or `undefined` to send the context unchanged. On follow-up requests that have not received steering yet, the loop polls steering once more after the hook, so input queued during a long hook such as compaction reaches the same request. `prepareNextTurn` keeps its existing timing: after `turn_end`, before steering is polled. + +An assistant turn is incomplete when it ends normally without visible text or an executable tool call, or when it reports `toolUse` without a tool call. The loop records that on the finished message as `AssistantMessage.incomplete`, using `classifyIncompleteAssistantTurn()` from `@step-harness/providers`; it reads text only to decide whether any is visible, never to interpret it, and it does not retry the turn or change the stop reason. Product sessions decide whether to recover them, and report a terminal one as their own run outcome. The former `toolCallLeakRetries` loop option was removed because text matching cannot prove that a turn meant to call a tool. When you use the `Agent` class, assistant `message_end` processing is treated as a barrier before tool preflight begins. That means `beforeToolCall` sees agent state that already includes the assistant message that requested the tool call. diff --git a/packages/agent-core/docs/harness.md b/packages/agent-core/docs/harness.md index 988658cbf..82c1626fa 100644 --- a/packages/agent-core/docs/harness.md +++ b/packages/agent-core/docs/harness.md @@ -1305,6 +1305,10 @@ Overflow is checked before retryable error, so an oversized request compacts rat An overflow classification never produces a tool plan. A *genuine* `length` that carries tool calls does produce the full plan, executes nothing, and appends one `isError: true` result per call explaining that truncation may have corrupted the arguments — those results then require another assistant turn. +**Incomplete responses are a product decision.** A response that stops normally while carrying neither visible text nor a structured tool call, or that reports `toolUse` without one, is structurally incomplete. The loop classifies it with `classifyIncompleteAssistantTurn()` from `stopReason` and content-block types only and records the result on the finished message as `AssistantMessage.incomplete`. It still ends the turn normally: it reads thinking not at all and text only to decide whether any is visible, and it does not discard the response, retry it, or rewrite the provider's stop reason. + +A product session owns any bounded recovery, such as appending a hidden recovery prompt and continuing once, and must bound how often a repeated incomplete response can happen. Because the stop reason stays truthful, a session must report a terminal incomplete turn as its own outcome rather than encoding it as a provider error: a product can carry it on its settled event, and consumers that must decide "did the run succeed" read `assistantTerminalFailure()` from `@step-harness/providers`, never `stopReason` alone. + ## 3.8 Tools | From | Trigger | Transaction | To | diff --git a/packages/agent-core/src/agent-loop.ts b/packages/agent-core/src/agent-loop.ts index 9c159a35c..eae8f31f1 100644 --- a/packages/agent-core/src/agent-loop.ts +++ b/packages/agent-core/src/agent-loop.ts @@ -7,6 +7,7 @@ import { type AssistantMessage, type AssistantMessageEvent, type Context, + classifyIncompleteAssistantTurn, classifyProviderError, EventStream, type ToolResultMessage, @@ -261,20 +262,7 @@ async function runLoop( } // Stream assistant response - let message = await streamAssistantResponse(currentContext, config, signal, emit, streamFunction); - - // Serving-side tool parsers can fail and leak the model's tool-call - // markup into plain text: the turn then carries no executable call - // and the loop would end even though the model meant to act. - // Resample the identical context a bounded number of times; the - // leaked attempt is dropped from the request context while its - // message events above remain for observability. - const leakRetryLimit = config.toolCallLeakRetries ?? DEFAULT_TOOL_CALL_LEAK_RETRIES; - for (let attempt = 0; attempt < leakRetryLimit && isToolCallMarkupLeak(message); attempt++) { - if (currentContext.messages[currentContext.messages.length - 1] !== message) break; - currentContext.messages.pop(); - message = await streamAssistantResponse(currentContext, config, signal, emit, streamFunction); - } + const message = await streamAssistantResponse(currentContext, config, signal, emit, streamFunction); newMessages.push(message); if (message.stopReason === "error" || message.stopReason === "aborted") { @@ -348,32 +336,6 @@ async function runLoop( * Stream an assistant response from the LLM. * This is where AgentMessage[] gets transformed to Message[] for the LLM. */ -/** Bounded default for resampling turns whose tool call leaked into text. */ -const DEFAULT_TOOL_CALL_LEAK_RETRIES = 2; - -const TOOL_CALL_MARKUP_RE = /|; - /** - * Bounded resampling for turns whose tool call leaked into plain text. - * - * Serving-side tool parsers can fail on the model's raw output, leaving - * ``/` { expect(messages[0].role).toBe("assistant"); }); }); - -describe("tool-call markup leak retry", () => { - const LEAK_TEXT = " ls "; - - function scriptedStream(script: AssistantMessage[]) { - let call = 0; - const requestMessages: Message[][] = []; - const streamFn = (_model: unknown, llmContext: { messages: Message[] }) => { - requestMessages.push([...llmContext.messages]); - const stream = new MockAssistantStream(); - const message = script[Math.min(call, script.length - 1)]; - call++; - queueMicrotask(() => { - stream.push({ type: "done", reason: message.stopReason as "stop", message }); - }); - return stream; - }; - return { streamFn, calls: () => call, requestMessages }; - } - - it("resamples a leaked turn and keeps only the good message in context", async () => { - const leaked = createAssistantMessage([{ type: "text", text: LEAK_TEXT }]); - const good = createAssistantMessage([{ type: "text", text: "done cleanly" }]); - const { streamFn, calls, requestMessages } = scriptedStream([leaked, good]); - - const context: AgentContext = { systemPrompt: "", messages: [], tools: [] }; - const config: AgentLoopConfig = { model: createModel(), convertToLlm: identityConverter }; - - const events: AgentEvent[] = []; - const stream = agentLoop([createUserMessage("go")], context, config, undefined, streamFn); - for await (const event of stream) events.push(event); - const messages = await stream.result(); - - expect(calls()).toBe(2); - // Leaked attempt is not committed anywhere. - expect(messages.map((m) => m.role)).toEqual(["user", "assistant"]); - expect((messages[1] as AssistantMessage).content).toEqual([{ type: "text", text: "done cleanly" }]); - // The retry request must not contain the leaked attempt: identical - // context to the first call. - expect(requestMessages[1]).toEqual(requestMessages[0]); - // Both attempts remain observable in the event stream. - expect(events.filter((e) => e.type === "message_end" && (e as any).message.role === "assistant")).toHaveLength(2); - // Only one turn ends. - expect(events.filter((e) => e.type === "turn_end")).toHaveLength(1); - }); - - it("stops after the bounded retries and commits the last attempt", async () => { - const leaked = createAssistantMessage([{ type: "text", text: LEAK_TEXT }]); - const { streamFn, calls, requestMessages } = scriptedStream([leaked, leaked, leaked]); - - const context: AgentContext = { systemPrompt: "", messages: [], tools: [] }; - const config: AgentLoopConfig = { model: createModel(), convertToLlm: identityConverter }; - - const stream = agentLoop([createUserMessage("go")], context, config, undefined, streamFn); - for await (const _event of stream) { - // drain - } - const messages = await stream.result(); - - // 1 initial + 2 default retries. - expect(calls()).toBe(3); - expect(requestMessages[1]).toEqual(requestMessages[0]); - expect(requestMessages[2]).toEqual(requestMessages[0]); - expect((messages.at(-1) as AssistantMessage).content).toEqual([{ type: "text", text: LEAK_TEXT }]); - }); - - it("can be disabled with toolCallLeakRetries: 0", async () => { - const leaked = createAssistantMessage([{ type: "text", text: LEAK_TEXT }]); - const { streamFn, calls } = scriptedStream([leaked]); - - const context: AgentContext = { systemPrompt: "", messages: [], tools: [] }; - const config: AgentLoopConfig = { - model: createModel(), - convertToLlm: identityConverter, - toolCallLeakRetries: 0, - }; - - const stream = agentLoop([createUserMessage("go")], context, config, undefined, streamFn); - for await (const _event of stream) { - // drain - } - - expect(calls()).toBe(1); - }); - - it("does not retry when a structured tool call is present alongside markup text", async () => { - const executed: string[] = []; - const toolSchema = Type.Object({ value: Type.String() }); - const tool: AgentTool = { - name: "echo", - label: "Echo", - description: "Echo tool", - parameters: toolSchema, - async execute(_toolCallId, params) { - executed.push(params.value); - return { content: [{ type: "text", text: `echoed: ${params.value}` }], details: params }; - }, - }; - - const mixed = createAssistantMessage( - [ - { type: "text", text: LEAK_TEXT }, - { type: "toolCall", id: "tool-1", name: "echo", arguments: { value: "hi" } }, - ], - "toolUse", - ); - const final = createAssistantMessage([{ type: "text", text: "done" }]); - const { streamFn, calls } = scriptedStream([mixed, final]); - - const context: AgentContext = { systemPrompt: "", messages: [], tools: [tool] }; - const config: AgentLoopConfig = { model: createModel(), convertToLlm: identityConverter }; - - const stream = agentLoop([createUserMessage("go")], context, config, undefined, streamFn); - for await (const _event of stream) { - // drain - } - - expect(executed).toEqual(["hi"]); - // Two model calls: tool turn + final turn, no leak retry in between. - expect(calls()).toBe(2); - }); -}); diff --git a/packages/coding-agent/docs/context-management.md b/packages/coding-agent/docs/context-management.md index 49b7b9757..37ffeaad2 100644 --- a/packages/coding-agent/docs/context-management.md +++ b/packages/coding-agent/docs/context-management.md @@ -93,8 +93,8 @@ The default maximum is two maintenance requests (one edit plus one correction), a 90-second wait including authentication, and 8,192 output tokens including thinking. An automatic edit must save at least 1,024 tokens and 5% of the mirrored context. Attempts consume a three-completed-turn cooldown, reconstructed from the -active session branch on resume. Failed, aborted, truncated, and parser-resampled -responses do not count as completed turns. +active session branch on resume. Failed, aborted, truncated, and recovered +incomplete responses do not count as completed turns. A correction keeps the original task context and includes the rejected edit draft and error as data. It does not replay the failed maintenance response's @@ -273,9 +273,9 @@ use a revision whose source history matches that branch. The known distinction between persisted failed provider responses and the live retry context is reconciled without skipping user messages, tool results, or successful responses. Custom-message persistence timestamps are normalized for source matching while -their content and control metadata remain part of the identity. When the native -loop resamples a response, its omitted messages are tracked explicitly so later -edits and resume still use the same source history. +their content and control metadata remain part of the identity. An incomplete +turn stays in canonical history when it is recovered in place, so later edits +and resume still use the same source history. Malformed saved message bodies, source hashes, or source-index lists are ignored before activation, allowing recovery from a previous valid revision or canonical context. diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index 22339c888..f0242d217 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -618,7 +618,7 @@ Fired for message lifecycle updates. - `message_start` and `message_end` fire for user, assistant, and toolResult messages. - `message_update` fires for assistant streaming updates. -- `message_end` handlers can return `{ message }` to replace the finalized message. The replacement must keep the same `role`. +- `message_end` handlers can return `{ message }` to replace the finalized message. The replacement must keep the same `role`. The replacement runs before the session classifies the turn, so an extension that adds visible text or a tool call prevents an incomplete-turn recovery, and one that removes both triggers it. The stop reason is never rewritten: handlers, session listeners, `turn_end`, and persistence all observe the provider's own value, and an incomplete turn is described by `message.incomplete` plus an `incomplete_turn` diagnostic. ```typescript pi.on("message_start", async (event, ctx) => { diff --git a/packages/coding-agent/docs/json.md b/packages/coding-agent/docs/json.md index 0e0d03597..2ce20f282 100644 --- a/packages/coding-agent/docs/json.md +++ b/packages/coding-agent/docs/json.md @@ -76,10 +76,17 @@ over messages reports the boundary instead of repeating them. `{ messageRole, toolResultCount }` and `{ willRetry, messageCount }`: what only the boundary says is where it fell, not what it contained. -`messageCount` is the length of that run's `messages` array, not a checksum: the -loop can emit more `message_end` events than it lists (a resampled tool-call -leak, a throwing `shouldStopAfterTurn`), so do not use it to detect a truncated -run. A message that never got its `message_end` is the reliable signal. +`messageCount` is the length of that run's `messages` array, not a checksum: a +throwing `shouldStopAfterTurn` can make the loop emit more `message_end` events +than it lists, so do not use it to detect a truncated run. A message that never +got its `message_end` is the reliable signal. + +An assistant turn that carries nothing a client can act on is described twice, +for two different readers. Its `message_end` keeps the provider's `stopReason` +and adds `incomplete`; the settled run adds `failure` to `agent_settled`, which +is the record's answer to "did this run produce anything". That covers a repeated +empty turn, and an empty turn that also overflowed the context window when +compaction could not complete or is disabled. What the record still repeats, deliberately: diff --git a/packages/coding-agent/docs/rpc.md b/packages/coding-agent/docs/rpc.md index f15a3f9dc..a614557ab 100644 --- a/packages/coding-agent/docs/rpc.md +++ b/packages/coding-agent/docs/rpc.md @@ -900,7 +900,7 @@ Events are streamed to stdout as JSON lines during agent operation. Events do no |-------|-------------| | `agent_start` | Agent begins processing | | `agent_end` | One low-level agent run completes (may still be followed by retry, compaction, or queued continuations); carries `willRetry` and a message count, not the messages | -| `agent_settled` | Agent run is fully settled; no automatic retry, compaction retry, or queued continuation remains | +| `agent_settled` | Agent run is fully settled; no automatic retry, compaction retry, or queued continuation remains. Carries `failure` when the run produced no usable answer | | `turn_start` | New turn begins | | `turn_end` | Turn completes; carries the message role and a tool-result count, not the messages | | `message_start` | Message begins | @@ -938,9 +938,14 @@ What only this event says is that the run ended and whether it will be retried, so the record carries `messageCount` instead of the array. `messageCount` is the length of the run's `messages` array, **not** a checksum: a -resampled tool-call leak or a throwing `shouldStopAfterTurn` can make the loop -emit more `message_end` events than it lists. Detect a truncated run by a message -that never got its `message_end`, not by comparing counts. +throwing `shouldStopAfterTurn` can make the loop emit more `message_end` events +than it lists. Detect a truncated run by a message that never got its +`message_end`, not by comparing counts. + +One `agent_end` does not bound one prompt: an incomplete-turn recovery, a +compaction retry, and queued continuations all emit their own. `willRetry` +reports the provider retry that follows in this run, not those continuations; +`agent_settled` is the boundary that says the run is over. ```json { @@ -954,10 +959,23 @@ that never got its `message_end`, not by comparing counts. Emitted after the full session-level run settles. At this point Step will not continue automatically through retry, compaction retry, or queued follow-up messages. +A run that produced no usable answer carries `failure` here. It is not reported by rewriting the assistant message: an incomplete turn keeps the provider's own `stopReason`, and a client that inferred success from the transcript would be wrong. Provider errors and cancellations keep using `stopReason`. + ```json {"type": "agent_settled"} ``` +```json +{ + "type": "agent_settled", + "failure": { + "kind": "incomplete_turn", + "reason": "no_visible_output", + "message": "The model returned no visible answer or executable tool call." + } +} +``` + ### turn_start / turn_end A turn consists of one assistant response plus any resulting tool calls and results. diff --git a/packages/coding-agent/docs/sdk.md b/packages/coding-agent/docs/sdk.md index c74ed179a..78e805cf6 100644 --- a/packages/coding-agent/docs/sdk.md +++ b/packages/coding-agent/docs/sdk.md @@ -112,6 +112,9 @@ interface AgentSession { sessionFile: string | undefined; sessionId: string; + // Suppress the generic incomplete-turn recovery when the host owns completion. + setIncompleteTurnRecoveryEnabled(enabled: boolean): void; + // Model control setModel(model: Model): Promise; setThinkingLevel(level: ThinkingLevel): void; @@ -140,6 +143,20 @@ interface AgentSession { } ``` +`setIncompleteTurnRecoveryEnabled()` defaults to `true`. Setting it to `false` suppresses the generic recovery and the run-level `failure` it would report; the `incomplete` classification is still recorded on every assistant message, so the host that owns completion decides the outcome itself. Print mode sets it to `false` when either completion feature (`--completion-check` or `--completion-review`) owns the bounded follow-up for missing output, so the two paths do not each make a model call. For the whole invocation, not just its end: a mid-run empty turn also waits for the checker. Reapply the setting after a runtime replaces the active session. + +A run that ends without a usable answer reports it on `agent_settled`, not by rewriting the assistant message: + +```typescript +session.subscribe((event) => { + if (event.type !== "agent_settled") return; + // { kind: "incomplete_turn", reason: "no_visible_output", message: "…" } + if (event.failure) reportRunFailure(event.failure); +}); +``` + +Provider errors and cancellations are still visible on the assistant message's `stopReason`. For an incomplete turn the stop reason stays the provider's own value, so a host must not infer success from it: read `event.failure`, or `assistantTerminalFailure(message)` from `@step-harness/providers` for a message that has not settled yet. + Session replacement APIs such as new-session, resume, fork, and import live on `AgentSessionRuntime`, not on `AgentSession`. ### createAgentSessionRuntime() and AgentSessionRuntime @@ -333,6 +350,11 @@ session.subscribe((event) => { case "agent_end": // Agent finished (event.messages contains new messages) break; + case "agent_settled": + // Fully settled: no retry, compaction retry, or queued continuation left. + // `failure` is set when the run produced no usable answer. + if (event.failure) reportRunFailure(event.failure); + break; // Turn lifecycle (one LLM response + tool calls) case "turn_start": diff --git a/packages/coding-agent/docs/session-format.md b/packages/coding-agent/docs/session-format.md index 3c1603a99..f8ac2c6b9 100644 --- a/packages/coding-agent/docs/session-format.md +++ b/packages/coding-agent/docs/session-format.md @@ -89,6 +89,10 @@ interface AssistantMessage { usage: Usage; stopReason: "stop" | "length" | "toolUse" | "error" | "aborted"; errorMessage?: string; + // Present when the turn carried nothing a client could act on; the stop reason + // keeps the provider's own value. See "Incomplete turns" below. + incomplete?: "no_visible_output" | "missing_tool_call"; + diagnostics?: AssistantMessageDiagnostic[]; timestamp: number; } @@ -353,6 +357,12 @@ Fields: - `display`: `true` = show in TUI with distinct styling, `false` = hidden - `details`: Optional extension-specific metadata (not sent to LLM) +Step uses a hidden `step-incomplete-turn-recovery` custom message when a normal assistant response contains neither visible text nor an executable tool call, or reports `toolUse` without a tool call. The incomplete assistant message remains in the transcript unchanged — its `stopReason` stays the provider's own value and the structural fact is recorded as `incomplete` on the message — and the recovery message becomes a user-role message for the next model request. Recovery is bounded by a streak of consecutive empty turns: any turn with visible text or an executed tool call resets it, any user input resets it, and the second consecutive empty turn ends the run with `failure` on `agent_settled`. A response that also exceeded the context window is compacted before recovery; if compaction cannot complete, the run fails without rewriting the assistant message that `message_end` already published, and the failure travels on `agent_settled`. If automatic compaction is disabled for that overflow, the run fails immediately instead. Every incomplete turn carries an `incomplete_turn` diagnostic recorded before the message is persisted, whose details include `reason`, `consecutiveTurns`, `overflowKind` when known, and the provider's `rawStopReason` when present. The classifier reads only stop reasons and content-block types; it never parses thinking, and reads text only to decide whether any is visible. + +The recovery message carries `details.recoversEntryId`, the session entry of the turn it answers. A reader pairs the two by identity: a session also writes usage provenance, compaction, and session-state entries in between, so their positions never imply a pairing. A recovery message without it was written by an older build and should be read as an unpaired follow-up rather than guessed at. + +The run-level outcome is not a session entry. `agent_settled.failure` exists only in the live event stream, because a message the transcript already holds must not change after it is written: reload a session and the per-turn `incomplete` classification and its diagnostic are still there, but "did this run end without an answer" is reconstructed from the last turn. A recorder that needs the run's own verdict has to capture the event. + ### LabelEntry User-defined bookmark/marker on an entry. diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index b00abc9e6..0d9a6d490 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -25,7 +25,14 @@ import type { PrepareNextTurnContext, ThinkingLevel, } from "@step-harness/agent-core"; -import { contentText } from "@step-harness/providers"; +import { + classifyIncompleteAssistantTurn, + contentBlocks, + contentText, + INCOMPLETE_TURN_MESSAGES, + type IncompleteTurnReason, + isIncompleteAssistantTurn, +} from "@step-harness/providers"; import type { AssistantMessage, AuthResult, @@ -114,6 +121,12 @@ import { wrapRegisteredTools, } from "./extensions/index.ts"; import { emitSessionShutdownEvent } from "./extensions/runner.ts"; +import { + createIncompleteTurnRecoveryMessage, + INCOMPLETE_TURN_RECOVERY_ATTEMPTS, + incompleteTurnFailureMessage, + recordIncompleteTurn, +} from "./incomplete-turn-recovery.ts"; import { type BashExecutionMessage, type CustomMessage, convertToLlm } from "./messages.ts"; import { ModelRegistry } from "./model-registry.ts"; import type { ModelRuntime } from "./model-runtime.ts"; @@ -121,6 +134,7 @@ import { expandPromptTemplate, type PromptTemplate } from "./prompt-templates.ts import { isRateLimitError, rateLimitDelay } from "./rate-limit.ts"; import { readRequestUsage } from "./request-provenance.ts"; import type { ResourceExtensionPaths, ResourceLoader } from "./resource-loader.ts"; +import type { AgentRunFailure } from "./run-failure.ts"; import { exportSessionToJsonl } from "./session-export.ts"; import type { BranchSummaryEntry, CompactionEntry, SessionEntry, SessionManager } from "./session-manager.ts"; import { getLatestCompactionEntry } from "./session-manager.ts"; @@ -135,6 +149,7 @@ import { boundToolResultContent } from "./tools/tool-output.ts"; import { addUsageToTotals, createUsageTotals, entryUsage } from "./usage-totals.ts"; export { type ParsedSkillBlock, parseSkillBlock } from "../utils/skill-block.ts"; +export type { AgentRunFailure } from "./run-failure.ts"; /** Session-specific events that extend the core AgentEvent */ export type AgentSessionEvent = @@ -144,7 +159,7 @@ export type AgentSessionEvent = messages: AgentMessage[]; willRetry: boolean; } - | { type: "agent_settled" } + | { type: "agent_settled"; failure?: AgentRunFailure } | { type: "live_context"; outcome: LiveContextOutcome } | { type: "auto_clm_start"; reason: "native-threshold" | "soft-threshold" } | { type: "auto_clm_end"; result: AutoClmResult } @@ -208,6 +223,45 @@ export type AgentSessionEventListener = (event: AgentSessionEvent) => void; // Types // ============================================================================ +/** Whether an assistant message carries a tool call the loop can execute. */ +function hasToolCall(message: AssistantMessage): boolean { + return contentBlocks(message).some((block) => block.type === "toolCall"); +} + +/** + * Whether a finished turn actually produced something. + * + * Visible text or a structured tool call is progress: the model answered, or it + * asked for work it can have executed. Everything else — an empty turn, a + * provider error, an abort, a truncated reply — left the task where it was, so it + * must not refill a recovery budget. + */ +function turnMadeProgress(message: AssistantMessage): boolean { + if (hasToolCall(message)) return true; + return contentBlocks(message).some( + (block) => block.type === "text" && typeof block.text === "string" && block.text.trim().length > 0, + ); +} + +/** + * A view of the message that silent-overflow detection can read. + * + * Silent overflow is defined for a completed `stop`, because that is the response + * a provider accepted while the request no longer fit. A tool-use turn that carries + * no call never delivered the work it promised, so it belongs in the same bucket: + * without this, an oversized empty tool-use turn would spend the incomplete-turn + * recovery on a request that cannot fit. + * + * A tool-use turn that does carry calls is deliberately excluded. Overflow recovery + * removes the assistant message and replays the turn, which would separate those + * calls from the results already appended after them. + */ +function silentOverflowCandidate(message: AssistantMessage): AssistantMessage { + if (message.stopReason !== "toolUse") return message; + if (hasToolCall(message)) return message; + return { ...message, stopReason: "stop" }; +} + function withoutDeletedHeaders(headers: ProviderHeaders | undefined): Record | undefined { return headers ? Object.fromEntries(Object.entries(headers).filter((entry): entry is [string, string] => entry[1] !== null)) @@ -744,8 +798,8 @@ export class AgentSession { // request that does not fit. _runAutoCompaction reports its own failures. if (!signal?.aborted) this._recordMaintenanceFault(error); } - // Only native compaction rebuilds canonical state. Otherwise keep the loop context, which may - // exclude parser-resampled responses, while CLM overlays its existing prefix. + // Only native compaction rebuilds canonical state. Otherwise keep the loop context + // while CLM overlays its existing prefix. if (getLatestCompactionEntry(this.sessionManager.getBranch())?.id === previousCompaction) return context; return { ...context, messages: this.agent.state.messages.slice() }; } @@ -825,8 +879,8 @@ export class AgentSession { if (!automatic?.attempted) { context = await this._compactBeforeNextAssistantResponse(turn.context); } else if (getLatestCompactionEntry(this.sessionManager.getBranch())?.id !== previousCompaction) { - // Only native compaction rebuilds canonical state. Keep parser-resampling - // exclusions in the loop context while CLM overlays its existing prefix. + // Only native compaction rebuilds canonical state. Keep the loop context + // while CLM overlays its existing prefix. context = { ...turn.context, messages: this.agent.state.messages.slice() }; } } @@ -1327,9 +1381,13 @@ export class AgentSession { private async _emitAgentSettled(): Promise { this._isAgentRunActive = false; + // Capture and clear before notifying: a listener may start the next run, and + // that run must not inherit this one's failure. + const failure = this._runFailure; + this._runFailure = undefined; try { - await this._extensionRunner.emit({ type: "agent_settled" }); - this._emit({ type: "agent_settled" }); + await this._extensionRunner.emit(failure ? { type: "agent_settled", failure } : { type: "agent_settled" }); + this._emit(failure ? { type: "agent_settled", failure } : { type: "agent_settled" }); } finally { this._resolveIdleWaitIfIdle(); } @@ -1337,6 +1395,22 @@ export class AgentSession { // Track last assistant message for auto-compaction check private _lastAssistantMessage: AssistantMessage | undefined = undefined; + /** Whether a thinking-only or missing-response turn may start one recovery continuation. */ + private _incompleteTurnRecoveryEnabled = true; + /** + * Consecutive incomplete turns. Any actionable turn, or any user input, resets + * it, so one transient empty response in a long autonomous run recovers while a + * model that keeps producing nothing cannot loop. + */ + private _incompleteTurnStreak = 0; + /** Run-level failure the transcript cannot express; reported on `agent_settled`. */ + private _runFailure: AgentRunFailure | undefined = undefined; + /** Last automatic-compaction failure detail, used to explain a failed incomplete-turn recovery. */ + private _lastAutoCompactionFailure: string | undefined = undefined; + /** Session entry the last assistant message was persisted as, for pairing a recovery. */ + private _lastAssistantEntryId: string | undefined = undefined; + /** Recovery prompt selected at the end of a turn and waiting for its continuation to start. */ + private _pendingIncompleteTurnRecovery: CustomMessage | undefined = undefined; /** Internal handler for agent events - shared by subscribe and reconnect */ private _handleAgentEvent = async (event: AgentEvent, signal?: AbortSignal): Promise => { @@ -1344,6 +1418,11 @@ export class AgentSession { // This ensures the UI sees the updated queue state if (event.type === "message_start" && event.message.role === "user") { this._overflowRecoveryAttempted = false; + // User input is a new attempt and the run's outcome is theirs to decide: a + // run that recovered once must not turn the next empty response into a + // terminal failure, and an earlier empty turn is no longer the outcome. + this._incompleteTurnStreak = 0; + this._runFailure = undefined; const messageText = contentText(event.message.content, ""); if (messageText) { // Check steering queue first @@ -1362,9 +1441,14 @@ export class AgentSession { } } - // Emit to extensions first + // Let message_end handlers replace content first. Classification and every + // later consumer must describe the message that is actually persisted. await this._emitExtensionEvent(event, signal); + if (event.type === "message_end" && event.message.role === "assistant") { + this._classifyAssistantTurn(event.message as AssistantMessage); + } + // Notify all listeners this._emit(event.type === "agent_end" ? { ...event, willRetry: this._willRetryAfterAgentEnd(event) } : event); @@ -1386,6 +1470,8 @@ export class AgentSession { ) { // Regular LLM message - persist as SessionMessageEntry const entryId = this.sessionManager.appendMessage(event.message); + // The entry id lets a later recovery message name the turn it answers. + if (event.message.role === "assistant") this._lastAssistantEntryId = entryId; if (event.message.role === "assistant" && this._extensionRunner?.hasHandlers("usage_recorded")) { const provenance = readRequestUsage(event.message); if (provenance) await this._extensionRunner.emit({ type: "usage_recorded", entryId, provenance }); @@ -1433,6 +1519,79 @@ export class AgentSession { } }; + /** + * Record the turn's structural classification on the message. + * + * The classification is re-derived after extension `message_end` replacement, + * so it always describes the message that is persisted. The provider's stop + * reason is left untouched: only the diagnostic and `incomplete` describe the + * outcome, and consumers that map a run to success read + * `assistantTerminalFailure()`. + */ + private _classifyAssistantTurn(message: AssistantMessage): void { + const reason = classifyIncompleteAssistantTurn(message); + // Always describe the message that is persisted, even when this host owns + // completion: `incomplete` is a property of the turn, not of our policy. + message.incomplete = reason; + if (!reason) { + // Only real progress refills the budget and resolves the run. An error, an + // abort, or a truncated turn also fails to classify as incomplete, but + // counting it as progress is what would let "empty, error, empty, error" + // recover forever. + if (turnMadeProgress(message)) { + this._incompleteTurnStreak = 0; + this._runFailure = undefined; + } + return; + } + const overflowKind = this._getOverflowKind(message); + // Record the classification on the message here, before it is persisted, so + // the diagnostic survives a reload. A later decision must not edit a message + // the transcript already holds. + recordIncompleteTurn(message, reason, { + streak: this._incompleteTurnStreak + 1, + maxAttempts: INCOMPLETE_TURN_RECOVERY_ATTEMPTS, + details: { ...(overflowKind ? { overflowKind } : {}) }, + }); + if (!this._incompleteTurnRecoveryEnabled) { + // The host owns completion: it decides the run's outcome from `incomplete`. + // The streak still counts, because the diagnostic records what happened + // rather than what this host chose to do about it. + this._incompleteTurnStreak += 1; + this._runFailure = undefined; + return; + } + this._incompleteTurnStreak += 1; + if (this._incompleteTurnStreak > INCOMPLETE_TURN_RECOVERY_ATTEMPTS) { + this._recordIncompleteRunFailure( + reason, + incompleteTurnFailureMessage(reason, { + maxAttempts: INCOMPLETE_TURN_RECOVERY_ATTEMPTS, + }), + ); + return; + } + if (overflowKind && !this.settingsManager.getCompactionSettings().enabled) { + this._recordIncompleteRunFailure( + reason, + `${INCOMPLETE_TURN_MESSAGES[reason]} The response also exceeded the model context window, and automatic compaction is disabled.`, + ); + return; + } + // The recovery continuation may still resolve the run, so this attempt is not + // the run's outcome yet. + this._runFailure = undefined; + } + + private _recordIncompleteRunFailure(reason: IncompleteTurnReason, message_: string): void { + this._runFailure ??= { kind: "incomplete_turn", reason, message: message_ }; + } + + /** Let a host that already owns turn completion suppress the generic recovery. */ + setIncompleteTurnRecoveryEnabled(enabled: boolean): void { + this._incompleteTurnRecoveryEnabled = enabled; + } + private _willRetryAfterAgentEnd(event: Extract): boolean { const settings = this.settingsManager.getRetrySettings(); if (!settings.enabled) { @@ -1949,6 +2108,11 @@ export class AgentSession { const runAbortController = new AbortController(); this._agentRunAbortController = runAbortController; this._isAgentRunActive = true; + // A run starts with a clean recovery budget. The streak itself survives only + // as the count of consecutive empty turns, which user input resets. + this._incompleteTurnStreak = 0; + this._runFailure = undefined; + this._lastAutoCompactionFailure = undefined; try { const incoming = Array.isArray(messages) ? messages : [messages]; const context = { @@ -2013,12 +2177,28 @@ export class AgentSession { while (!runAbortController.signal.aborted) { const next = await this._handlePostAgentRun(); if (next === "stop" || runAbortController.signal.aborted) break; + // The recovery is a prompt of its own: the empty turn is the last message, so + // `continue()` would have nothing to continue from, and the follow-up has to + // reach the model as new input rather than as a queued steering line. + const recovery = this._pendingIncompleteTurnRecovery; + this._pendingIncompleteTurnRecovery = undefined; + if (recovery) { + await this.agent.prompt(recovery); + continue; + } // Only the queued-work decision turns a waiting message into this turn's // input. A retry or a context repair resumes the same work, and a message // the person queued behind it is meant to arrive after it finishes. await this.agent.continue({ deliverQueued: next === "deliver" }); } } finally { + // The user's cancellation outranks our diagnosis: a terminal empty turn can + // be classified before they stop the run, and reporting that as the outcome + // of a run they ended themselves would turn their pause into a failure. + if (runAbortController.signal.aborted) this._runFailure = undefined; + this._pendingIncompleteTurnRecovery = undefined; + this._lastAssistantEntryId = undefined; + this._incompleteTurnStreak = 0; this._checkedInitialRequest = undefined; this._systemPromptOverride = undefined; this._flushPendingBashMessages(); @@ -2035,7 +2215,7 @@ export class AgentSession { } } - private async _handlePostAgentRun(): Promise<"stop" | "retry" | "recover" | "deliver"> { + private async _handlePostAgentRun(): Promise<"stop" | "retry" | "recover" | "deliver" | "recover-incomplete"> { const msg = this._lastAssistantMessage; this._lastAssistantMessage = undefined; if (!msg) { @@ -2071,15 +2251,23 @@ export class AgentSession { // Overflow and truncation recovery repair the transcript so the next // request fits. They are maintenance, not work on the user's behalf, so a // decision still holds them off from *starting* anything but lets them run: - // threshold compaction stays deferred to the next request either way. + // threshold compaction stays deferred to the next request either way. An + // empty turn's recovery is the opposite — it is work on the user's behalf — + // so it waits for the decision to be answered. return (await this._checkCompaction(msg, true, true)) ? "recover" : "stop"; } + const incompleteReason = this._incompleteTurnRecoveryEnabled ? msg.incomplete : undefined; + const incomplete = incompleteReason !== undefined; + const overflowKind = incomplete ? this._getOverflowKind(msg) : undefined; + const compactionBefore = getLatestCompactionEntry(this.sessionManager.getBranch())?.id; + // CLM maintains context for a future request. A successful settled answer // needs no extra model call; the next prompt/continuation performs the // same threshold check with its new instructions. Explicit maintenance // commands, recovery and extension-queued continuations retain their path. if ( + !incomplete && this.settingsManager.getContextProjectionMode() === "clm-v1" && !this._manualClm && msg.stopReason === "stop" && @@ -2095,6 +2283,60 @@ export class AgentSession { return "recover"; } + if (incomplete) { + // A cancellation during compaction must settle quietly: the user already + // ended the run, and their cancellation outranks our diagnosis. + if (this._agentRunAbortController?.signal.aborted) { + this._runFailure = undefined; + return "stop"; + } + // Silent overflow must compact before another request. If compaction did + // not commit, do not spend the recovery on a context that cannot fit. + // An incomplete turn can only carry a context overflow here: the classifier + // excludes length stops, which are the only other overflow kind. This is the + // one place where even the person's own queued words do not run first: the + // request they would join cannot fit, so it would fail as well. + if ( + overflowKind && + // Compaction being disabled was already terminal at classification time. + this.settingsManager.getCompactionSettings().enabled && + getLatestCompactionEntry(this.sessionManager.getBranch())?.id === compactionBefore + ) { + // The assistant message already reached listeners and the transcript at + // message_end, and its stop reason is the provider's own value. Report the + // run-level failure instead of rewriting the message: a message the + // transcript already holds must not change under it. + const detail = this._lastAutoCompactionFailure; + const message = detail + ? `Cannot recover an incomplete assistant turn because automatic compaction did not complete: ${detail}` + : "Cannot recover an incomplete assistant turn because automatic compaction did not complete."; + this._recordIncompleteRunFailure(incompleteReason, message); + return "stop"; + } + // Let the user's own queued input run first. If it resolves the turn, the old + // incomplete response stays in context without forcing a stale retry. This runs + // after the overflow guard because queued input cannot make a request that still + // exceeds the context window succeed, and before the decision gate because the + // person's own words are how they answer it. + if (this.agent.hasDeliverableQueuedMessage(this._queueDeliveryAllowed)) return "deliver"; + // Compaction awaits provider auth and extension hooks, so a decision may have + // become pending while it ran; re-check it here, after that window. The recovery + // itself is work on the user's behalf and waits for their answer — the queue + // above already released the words they wrote. + if (this.hasPendingUserDecision) return "stop"; + // The turn is already terminal — a repeated empty turn, or an overflow that + // could not be compacted because compaction is disabled. Nothing left to try + // here; a compaction may still have run above, which only helps the next + // prompt fit. + if (this._runFailure) return "stop"; + this._pendingIncompleteTurnRecovery = createIncompleteTurnRecoveryMessage(incompleteReason, { + recoversEntryId: this._lastAssistantEntryId, + attempt: this._incompleteTurnStreak, + maxAttempts: INCOMPLETE_TURN_RECOVERY_ATTEMPTS, + }); + return "recover-incomplete"; + } + // The agent loop drains both queues before emitting agent_end, except when a // tool result ended the turn first. Messages here were queued by `agent_end` // handlers, or were still pending when a control barrier ended the run. @@ -3174,6 +3416,26 @@ export class AgentSession { this._branchSummaryAbortController?.abort(); } + /** + * Classify a response as a context overflow or a recoverable truncated response. + */ + private _getOverflowKind(assistantMessage: AssistantMessage): "context" | "length" | undefined { + const contextWindow = this.model?.contextWindow ?? 0; + + // Do not apply overflow recovery from a different model's response after a + // model switch, or from a response older than the latest compaction. + const sameModel = + this.model && assistantMessage.provider === this.model.provider && assistantMessage.model === this.model.id; + const compactionEntry = getLatestCompactionEntry(this.sessionManager.getBranch()); + if (compactionEntry !== null && assistantMessage.timestamp <= new Date(compactionEntry.timestamp).getTime()) { + return undefined; + } + if (!sameModel) return undefined; + if (isContextOverflow(silentOverflowCandidate(assistantMessage), contextWindow)) return "context"; + if (isRecoverableLength(assistantMessage, this.model?.maxTokens ?? 0)) return "length"; + return undefined; + } + /** * Dispatch automatic compaction after `agent_end` or before prompt submission. * Manual compaction does not call this method; it enters through `compact()`. @@ -3208,31 +3470,26 @@ export class AgentSession { if (skipAbortedCheck && assistantMessage.stopReason === "aborted") return false; const contextWindow = this.model?.contextWindow ?? 0; - - // Skip overflow check if the message came from a different model. - // This handles the case where user switched from a smaller-context model (e.g. opus) - // to a larger-context model (e.g. codex) - the overflow error from the old model - // shouldn't trigger compaction for the new model. - const sameModel = - this.model && assistantMessage.provider === this.model.provider && assistantMessage.model === this.model.id; - - // Skip compaction checks if this assistant message is older than the latest - // compaction boundary. This prevents a stale pre-compaction usage/error - // from retriggering compaction on the first prompt after compaction. const compactionEntry = getLatestCompactionEntry(this.sessionManager.getBranch()); - const assistantIsFromBeforeCompaction = - compactionEntry !== null && assistantMessage.timestamp <= new Date(compactionEntry.timestamp).getTime(); - if (assistantIsFromBeforeCompaction) { + // Skip compaction checks if this assistant message is older than the latest + // compaction boundary. This prevents stale pre-compaction usage from + // retriggering threshold or overflow compaction on the first prompt after one. + if (compactionEntry !== null && assistantMessage.timestamp <= new Date(compactionEntry.timestamp).getTime()) { return false; } // Automatic cases 1 and 2: context overflow. // A length stop is recoverable when output ended below the model's original desired limit, // independent of the configured context size or any context-clamped provider request limit. - const contextOverflow = sameModel && isContextOverflow(assistantMessage, contextWindow); - const recoverableLength = sameModel && isRecoverableLength(assistantMessage, this.model?.maxTokens ?? 0); - if (contextOverflow || recoverableLength) { - const willRetry = assistantMessage.stopReason !== "stop"; + const overflowKind = this._getOverflowKind(assistantMessage); + if (overflowKind) { + const contextOverflow = overflowKind === "context"; + // An empty turn is never replayed. Case 1 removes the assistant message and + // calls `agent.continue()`, and a compaction that restores that message + // leaves an assistant at the tail, which `continue()` rejects. The + // incomplete-turn recovery sends its own prompt instead, so this path only + // has to make room. + const willRetry = assistantMessage.stopReason !== "stop" && !isIncompleteAssistantTurn(assistantMessage); // Case 2: the response completed successfully. Compact, but do not retry because // agent.continue() cannot continue from a completed assistant response. @@ -3241,6 +3498,9 @@ export class AgentSession { } if (this._overflowRecoveryAttempted) { + // The attempt that just failed is the one worth reporting; a detail left + // from an earlier compaction would describe a different failure. + this._lastAutoCompactionFailure = undefined; const errorMessage = contextOverflow ? "Context overflow recovery failed after one compact-and-retry attempt. Try reducing context or switching to a larger-context model." : "Truncated response recovery failed after one compact-and-retry attempt."; @@ -3332,6 +3592,7 @@ export class AgentSession { * @returns Whether the post-run loop should call `agent.continue()` */ private async _runAutoCompaction(reason: "overflow" | "threshold", willRetry: boolean): Promise { + this._lastAutoCompactionFailure = undefined; const runSignal = this._agentRunAbortController?.signal; if (runSignal?.aborted) return false; if (reason === "threshold" && this._blocksNativeThreshold()) return false; @@ -3343,6 +3604,7 @@ export class AgentSession { try { if (!this.model) { + this._lastAutoCompactionFailure = "No model is selected."; return false; } @@ -3360,6 +3622,9 @@ export class AgentSession { preparation && this._getLiveContext()?.prepareCompaction(preparation, this.agent.state.messages); if (liveCompaction) preparation = liveCompaction.preparation; if (!preparation) { + // Nothing old enough to summarize. The caller reports this as the reason + // it could not make room, instead of a bare "did not complete". + this._lastAutoCompactionFailure = "No older history could be summarized."; return false; } @@ -3381,6 +3646,8 @@ export class AgentSession { })) as SessionBeforeCompactResult | undefined; if (extensionResult?.cancel) { + // Not an exception, but still the reason a caller could not make room. + this._lastAutoCompactionFailure = "An extension cancelled compaction."; this._emit({ type: "compaction_end", reason, @@ -3513,11 +3780,13 @@ export class AgentSession { if (started) { const aborted = this._autoCompactionAbortController?.signal.aborted ?? false; const failureDetails = aborted ? undefined : this._recordCompactionFailure(error, reason); + if (aborted) this._lastAutoCompactionFailure = "Automatic compaction was cancelled."; let formattedErrorMessage = aborted ? undefined : reason === "overflow" ? `Context overflow recovery failed: ${failureDetails}` : `Auto-compaction failed: ${failureDetails}`; + if (formattedErrorMessage) this._lastAutoCompactionFailure = formattedErrorMessage; // A throw after the compaction was persisted (for example from a session_compact // handler) is not a failed compaction, so it does not count toward the pause. if (!aborted && !committed && reason === "threshold") { diff --git a/packages/coding-agent/src/core/compaction/live-context/auto-compaction.ts b/packages/coding-agent/src/core/compaction/live-context/auto-compaction.ts index 93c511aad..88d9cebd9 100644 --- a/packages/coding-agent/src/core/compaction/live-context/auto-compaction.ts +++ b/packages/coding-agent/src/core/compaction/live-context/auto-compaction.ts @@ -1,6 +1,12 @@ import { randomUUID } from "node:crypto"; import type { AgentContext, AgentMessage, StreamFn, ThinkingLevel } from "@step-harness/agent-core"; -import type { Context, Model, SimpleStreamOptions, Usage } from "@step-harness/providers"; +import { + type Context, + isIncompleteAssistantTurn, + type Model, + type SimpleStreamOptions, + type Usage, +} from "@step-harness/providers"; import { raceWithAbortSignal } from "../../../utils/abort.ts"; import { convertToLlm } from "../../messages.ts"; import type { SessionManager } from "../../session-manager.ts"; @@ -91,10 +97,10 @@ export class AutoClmController { if (entry.type !== "message" || entry.message.role !== "assistant") continue; const message = entry.message; if (message.stopReason !== "stop" && message.stopReason !== "toolUse") continue; + if (isIncompleteAssistantTurn(message)) continue; const calls = message.content.filter((part) => part.type === "toolCall"); if (calls.length === 0) { - if (!message.content.some((part) => part.type === "text" && /|(); for (let next = index + 1; next < branch.length; next++) { diff --git a/packages/coding-agent/src/core/compaction/live-context/manager.ts b/packages/coding-agent/src/core/compaction/live-context/manager.ts index 60c696fcc..23f0286d0 100644 --- a/packages/coding-agent/src/core/compaction/live-context/manager.ts +++ b/packages/coding-agent/src/core/compaction/live-context/manager.ts @@ -32,7 +32,7 @@ interface Checkpoint { sourceDigest: string; sourceHashes?: string[]; retrySourceIndexes?: number[]; - /** Messages omitted by the native loop (for example tool-parser resampling). */ + /** Messages omitted from a host's request projection. */ hostExcludedIndexes?: number[]; messages: AgentMessage[]; sourceIndexes: (number | null)[]; @@ -366,8 +366,8 @@ export class LiveContextManager { mkdirSync(this.mirrorDirectory, { recursive: true, mode: 0o700 }); atomicWrite(this.path, document.text); atomicWrite(this.indexPath, renderLiveContextIndex(document, this.path)); - // The loop can omit resampled attempts while state/transcript retain them. Anchor - // to state explicitly rather than guessing from text what the loop discarded. + // A host projection can omit messages while state and the transcript retain them. + // Anchor to state explicitly rather than guessing from text what the host discarded. const positions: number[] = []; let cursor = 0; for (const message of raw) { diff --git a/packages/coding-agent/src/core/extensions/types.ts b/packages/coding-agent/src/core/extensions/types.ts index 66a774279..07d8d9490 100644 --- a/packages/coding-agent/src/core/extensions/types.ts +++ b/packages/coding-agent/src/core/extensions/types.ts @@ -55,6 +55,7 @@ import type { KeybindingsManager } from "../keybindings.ts"; import type { CustomMessage } from "../messages.ts"; import type { ModelRegistry } from "../model-registry.ts"; import type { ScopedModel } from "../model-resolver.ts"; +import type { AgentRunFailure } from "../run-failure.ts"; import type { BranchSummaryEntry, CompactionEntry, @@ -849,9 +850,17 @@ export interface AgentEndEvent { messages: AgentMessage[]; } -/** Fired after an agent run has fully settled and no automatic retry, compaction, or queued continuation will run. */ +/** + * Fired after an agent run has fully settled and no automatic retry, compaction, + * or queued continuation will run. + * + * `failure` is set when the run produced no usable answer. A provider error is + * already visible as the assistant message's `stopReason`; an empty turn keeps + * the provider's own value, so its outcome can only be read here. + */ export interface AgentSettledEvent { type: "agent_settled"; + failure?: AgentRunFailure; } export type UIPromptKind = "select" | "confirm" | "input" | "editor" | "custom"; diff --git a/packages/coding-agent/src/core/incomplete-turn-recovery.ts b/packages/coding-agent/src/core/incomplete-turn-recovery.ts new file mode 100644 index 000000000..ae2a2647b --- /dev/null +++ b/packages/coding-agent/src/core/incomplete-turn-recovery.ts @@ -0,0 +1,115 @@ +import { + type AssistantMessage, + appendAssistantMessageDiagnostic, + contentBlocks, + INCOMPLETE_TURN_MESSAGES, + type IncompleteTurnReason, +} from "@step-harness/providers"; +import type { CustomMessage } from "./messages.ts"; + +export const INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE = "step-incomplete-turn-recovery"; + +/** + * How many recoveries one run may spend on empty turns before it ends. + * + * The existing policy permits one recovery before a repeated empty turn fails. + */ +export const INCOMPLETE_TURN_RECOVERY_ATTEMPTS = 1; + +/** + * Recovery instructions for a turn that produced nothing actionable. + * + * The wording does not refer to "your previous response": a response with no + * content blocks is dropped by the provider serializers, so the model may only + * see the original prompt followed by this message. + */ +const RECOVERY_PROMPTS: Record = { + no_visible_output: [ + "[Step runtime recovery] The last turn produced no visible answer and no tool call.", + "If work remains, call the appropriate tool now; otherwise finish with a clear answer for me.", + "Do not put tool calls or commands inside reasoning only.", + ].join("\n"), + missing_tool_call: [ + "[Step runtime recovery] The last turn ended as a tool-use turn but carried no executable tool call.", + "If work remains, call the appropriate tool now; otherwise finish with a clear answer for me.", + "Do not put tool calls or commands inside reasoning only.", + ].join("\n"), +}; + +export interface IncompleteTurnRecoveryOptions { + /** Session entry this follow-up answers. Metadata only, never sent to the model. */ + recoversEntryId?: string; + /** 1-based number of this recovery within the run. */ + attempt?: number; + maxAttempts?: number; +} + +/** + * The hidden follow-up that asks the model to continue an empty turn. + * + * `recoversEntryId` names the session entry this message was sent for, so a reader + * can pair the two by identity instead of guessing from their position: a session + * writes other entries in between (usage provenance, a compaction, a model or + * thinking-level change). It is metadata, never sent to the model. + */ +export function createIncompleteTurnRecoveryMessage( + reason: IncompleteTurnReason, + options: IncompleteTurnRecoveryOptions = {}, +): CustomMessage { + return { + role: "custom", + customType: INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE, + content: RECOVERY_PROMPTS[reason], + display: false, + details: { + ...(options.recoversEntryId ? { recoversEntryId: options.recoversEntryId } : {}), + attempt: options.attempt ?? 1, + maxAttempts: options.maxAttempts ?? INCOMPLETE_TURN_RECOVERY_ATTEMPTS, + }, + timestamp: Date.now(), + }; +} + +/** + * The failure wording for the consecutive-empty-turn limit. + * + * It says what happened and that retrying stopped, because "the model returned no + * visible answer" alone reads as a per-turn remark rather than the end of the run. + */ +export function incompleteTurnFailureMessage( + reason: IncompleteTurnReason, + options: { maxAttempts?: number } = {}, +): string { + const max = options.maxAttempts ?? INCOMPLETE_TURN_RECOVERY_ATTEMPTS; + return `${INCOMPLETE_TURN_MESSAGES[reason]} Retry limit reached (${max}/${max}); the run ended without an answer.`; +} + +/** + * Record why an incomplete turn could not stand, on the turn itself. + * + * The message keeps the provider's stop reason: this only adds the diagnostic and + * never rewrites a provider fact. Classification does not establish whether a + * recovery was started or a run ended. + */ +export function recordIncompleteTurn( + message: AssistantMessage, + reason: IncompleteTurnReason, + options: { streak: number; maxAttempts?: number; details?: Record }, +): void { + const content = contentBlocks(message); + appendAssistantMessageDiagnostic(message, { + type: "incomplete_turn", + timestamp: Date.now(), + details: { + reason, + consecutiveTurns: options.streak, + maxAttempts: options.maxAttempts ?? INCOMPLETE_TURN_RECOVERY_ATTEMPTS, + hadThinking: content.some((block) => block.type === "thinking"), + hadVisibleText: content.some( + (block) => block.type === "text" && typeof block.text === "string" && block.text.trim().length > 0, + ), + ...(message.rawStopReason ? { rawStopReason: message.rawStopReason } : {}), + ...options.details, + }, + }); +} diff --git a/packages/coding-agent/src/core/index.ts b/packages/coding-agent/src/core/index.ts index f2cd0a129..0afd552af 100644 --- a/packages/coding-agent/src/core/index.ts +++ b/packages/coding-agent/src/core/index.ts @@ -2,7 +2,9 @@ * Core modules shared between all run modes. */ +export { INCOMPLETE_TURN_MESSAGES, type IncompleteTurnReason } from "@step-harness/providers"; export { + type AgentRunFailure, AgentSession, type AgentSessionConfig, type AgentSessionEvent, @@ -77,6 +79,11 @@ export { type TurnStartEvent, type WorkingIndicatorOptions, } from "./extensions/index.ts"; +export { + createIncompleteTurnRecoveryMessage, + INCOMPLETE_TURN_RECOVERY_ATTEMPTS, + INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE, +} from "./incomplete-turn-recovery.ts"; export { type ModelRequestCompleted, type ModelRequestObserver, diff --git a/packages/coding-agent/src/core/run-failure.ts b/packages/coding-agent/src/core/run-failure.ts new file mode 100644 index 000000000..50285853e --- /dev/null +++ b/packages/coding-agent/src/core/run-failure.ts @@ -0,0 +1,16 @@ +import type { IncompleteTurnReason } from "@step-harness/providers"; + +/** + * Why a run ended without a usable answer, when the transcript cannot say it. + * + * A provider error reaches hosts as the last assistant message's `stopReason`. + * An incomplete turn cannot: its message keeps the provider's own stop reason, + * because the run's outcome is a session decision, not a provider fact. Hosts + * that map a settled run to an exit code, a lane status, or a goal status read + * this instead of inferring success from the transcript. + */ +export interface AgentRunFailure { + kind: "incomplete_turn"; + reason: IncompleteTurnReason; + message: string; +} diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index 50a626f0d..e20136a8a 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -398,7 +398,8 @@ function getSessionContextSettings(path: SessionEntry[]): Pick { + pi.on("agent_settled", (event, ctx) => { setContext(ctx); const before = runtime.get()?.status; - runtime.onAgentSettled(); + runtime.onAgentSettled(event.failure); notifyAutoStop(before, ctx); updateStatus(ctx); }); diff --git a/packages/coding-agent/src/features/step-subagent.ts b/packages/coding-agent/src/features/step-subagent.ts index 11ad89bc1..c2709d69b 100644 --- a/packages/coding-agent/src/features/step-subagent.ts +++ b/packages/coding-agent/src/features/step-subagent.ts @@ -11,7 +11,7 @@ import { randomUUID } from "node:crypto"; import path from "node:path"; import type { AgentToolResult, AgentToolUpdateCallback, ThinkingLevel } from "@step-harness/agent-core"; import { Text } from "@step-harness/pi-tui"; -import { type Message, type ProviderErrorKind, StringEnum, type Usage } from "@step-harness/providers"; +import { contentBlocks, type Message, type ProviderErrorKind, StringEnum, type Usage } from "@step-harness/providers"; import { type Static, Type } from "typebox"; import { CONFIG_DIR_NAME } from "../config.ts"; import { CHILD_SESSION_ORIGIN } from "../core/child-session.ts"; @@ -355,7 +355,11 @@ function isAssistantMessage(message: Message): message is Extract part.type === "text") .map((part) => part.text) .join("\n"); @@ -405,6 +409,39 @@ export function isFailed(result: Pick): string | undefined { + if (event.type !== "agent_settled") return undefined; + const failure = isRecordValue(event.failure) ? event.failure : undefined; + if (!failure) return undefined; + return typeof failure.message === "string" && failure.message + ? failure.message + : "The agent run ended without a usable answer."; +} + export function resultText(result: StepSubagentRunResult): string { if (isFailed(result)) return result.errorMessage || result.stderr || finalOutput(result.messages) || "(no output)"; return finalOutput(result.messages) || "(no output)"; diff --git a/packages/coding-agent/src/features/subagent/rpc-adapter.ts b/packages/coding-agent/src/features/subagent/rpc-adapter.ts index b73ea1818..7bbdff943 100644 --- a/packages/coding-agent/src/features/subagent/rpc-adapter.ts +++ b/packages/coding-agent/src/features/subagent/rpc-adapter.ts @@ -25,8 +25,10 @@ import { MAX_JSON_LINE_BYTES, normalizeChildTools, parseJsonEvent, + readSettledFailure, type StepSubagentRunInput, type StepSubagentRunResult, + settledTurnOutcome, } from "../step-subagent.ts"; import { WORKFLOW_ACL_ENV } from "../workflow/acl-extension.ts"; import { liveSubagentSessions } from "./lane-lifecycle.ts"; @@ -458,12 +460,12 @@ export async function createSubagentRpcSession( parseJsonEvent(eventLine, active.current, active.input.onUpdate); if (event.type === "agent_settled") { // The run (including queued steer/follow_up messages) fully drained. + // A settled run is not automatically a successful one: a terminal + // incomplete turn reports itself here, because its assistant message + // keeps the provider's stop reason. + const failure = readSettledFailure(event); settleTurn((current, aborted) => { - current.exitCode = 0; - if (aborted) { - current.stopReason = "aborted"; - current.errorMessage = "Subagent was aborted"; - } + Object.assign(current, settledTurnOutcome(failure, aborted)); }); } }, diff --git a/packages/coding-agent/src/features/workflow/runtime.ts b/packages/coding-agent/src/features/workflow/runtime.ts index d419d4732..4c8a5b3a8 100644 --- a/packages/coding-agent/src/features/workflow/runtime.ts +++ b/packages/coding-agent/src/features/workflow/runtime.ts @@ -568,8 +568,8 @@ export class WorkflowRuntime { if (event.type === "succeeded") { this.semaphore.succeeded(requestEpoch); shared.succeeded(sharedEpoch); - // The next request in this turn (a resample after leaked tool-call - // markup) has no "started" event of its own. + // A provider-internal follow-up can begin without a new "started" + // event of its own. requestEpoch = this.semaphore.epoch; sharedEpoch = shared.epoch; } diff --git a/packages/coding-agent/src/index.ts b/packages/coding-agent/src/index.ts index 7f68dfc61..51134e1f3 100644 --- a/packages/coding-agent/src/index.ts +++ b/packages/coding-agent/src/index.ts @@ -1,5 +1,9 @@ // Core session management +// The user-facing wording for an empty turn, and its reason type. A host that +// renders a run needs both; re-exporting keeps it from depending on the provider +// package directly. +export { INCOMPLETE_TURN_MESSAGES, type IncompleteTurnReason } from "@step-harness/providers"; export { type Args, type Mode, parseArgs } from "./cli/args.ts"; // ----------------------------------------------------------------------------- // S4-0 barrel widening: symbols consumed by the interactive UI that moved to the @@ -30,6 +34,7 @@ export { VERSION, } from "./config.ts"; export { + type AgentRunFailure, AgentSession, type AgentSessionConfig, type AgentSessionEvent, @@ -229,6 +234,11 @@ export { FooterDataProvider } from "./core/footer-data-provider.ts"; // rather than reimplement them, or it would act on empty/detached local state. // ----------------------------------------------------------------------------- export { configureHttpDispatcher, formatHttpIdleTimeoutMs, HTTP_IDLE_TIMEOUT_CHOICES } from "./core/http-dispatcher.ts"; +export { + createIncompleteTurnRecoveryMessage, + INCOMPLETE_TURN_RECOVERY_ATTEMPTS, + INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE, +} from "./core/incomplete-turn-recovery.ts"; export { KeybindingsManager } from "./core/keybindings.ts"; export { type BranchSummaryMessage, diff --git a/packages/coding-agent/src/modes/json-event.ts b/packages/coding-agent/src/modes/json-event.ts index 2cfb0a962..9352ed819 100644 --- a/packages/coding-agent/src/modes/json-event.ts +++ b/packages/coding-agent/src/modes/json-event.ts @@ -42,8 +42,8 @@ type JsonAgentEndEvent = { * Length of the run's `messages` array — what this event used to carry. * * Informational, not a checksum: the loop can emit more `message_end` events - * than it lists here (a resampled tool-call leak, a throwing - * `shouldStopAfterTurn`), so a reader must not use it to detect truncation. + * than it lists here (a throwing `shouldStopAfterTurn`), so a reader must not + * use it to detect truncation. * A missing final message is visible as a message that never got its * `message_end`. */ diff --git a/packages/coding-agent/src/modes/print-mode.ts b/packages/coding-agent/src/modes/print-mode.ts index babaa77f0..53d472691 100644 --- a/packages/coding-agent/src/modes/print-mode.ts +++ b/packages/coding-agent/src/modes/print-mode.ts @@ -7,7 +7,7 @@ */ import type { AgentMessage } from "@step-harness/agent-core"; -import type { AssistantMessage, ImageContent } from "@step-harness/providers"; +import { type AssistantMessage, type ImageContent, INCOMPLETE_TURN_MESSAGES } from "@step-harness/providers"; import type { AgentSessionEvent } from "../core/agent-session.ts"; import type { AgentSessionRuntimeHost } from "../core/agent-session-runtime.ts"; import { flushRawStdout, waitForRawStdoutBackpressure, writeRawStdout } from "../core/output-guard.ts"; @@ -91,14 +91,35 @@ function getExternalPlanHandoff(event: AgentSessionEvent): ExternalPlanHandoff | return readExternalPlanHandoff(event.result); } -function getAssistantFailure(message: AgentMessage | undefined): AssistantMessage | undefined { +/** + * The assistant message that makes this run a failure, if any. + * + * `includeIncomplete` decides whether an empty turn is this invocation's result. + * A completion check owns missing output, so it passes `false`: the empty turn is + * that follow-up's input there, and the checker reports the exit code instead. + */ +function getAssistantFailure( + message: AgentMessage | undefined, + options: { includeIncomplete: boolean }, +): AssistantMessage | undefined { if (message?.role !== "assistant") return undefined; const assistant = message as AssistantMessage; - return assistant.stopReason === "error" || assistant.stopReason === "aborted" ? assistant : undefined; + if (assistant.stopReason === "error" || assistant.stopReason === "aborted") return assistant; + if (options.includeIncomplete && assistant.incomplete) return assistant; + return undefined; +} + +/** User-facing text for a failed assistant message. */ +function assistantFailureText(assistant: AssistantMessage): string { + // `incomplete` is read back from a session file, so an unknown value must fall + // through to the provider's own message rather than print `undefined`. + const described = assistant.incomplete ? INCOMPLETE_TURN_MESSAGES[assistant.incomplete] : undefined; + if (described) return described; + return assistant.errorMessage || `Request ${assistant.stopReason}`; } function hasFinalAssistantText(message: AgentMessage | undefined): boolean { - if (message?.role !== "assistant" || getAssistantFailure(message)) return false; + if (message?.role !== "assistant" || getAssistantFailure(message, { includeIncomplete: false })) return false; const assistant = message as AssistantMessage; return ( assistant.stopReason !== "toolUse" && @@ -179,6 +200,9 @@ export async function runPrintMode(runtimeHost: AgentSessionRuntimeHost, options unsubscribe?.(); unsubscribeBackpressure?.(); session = nextSession; + // completion-check owns the bounded follow-up for missing output. Running the + // generic recovery too would add a second model call and consume its budget. + session.setIncompleteTurnRecoveryEnabled(completionAttempts === undefined); // The handoff flags describe the previous session's last prompt. A rebound // session has run nothing yet, so keeping them would print a stale proposal // if this run ends without a new assistant message. @@ -193,7 +217,10 @@ export async function runPrintMode(runtimeHost: AgentSessionRuntimeHost, options latestPromptHasExternalHandoff = true; latestExternalHandoffText = handoff.text; } - if (event.type === "message_end") assistantFailure ??= getAssistantFailure(event.message); + // Only a completion check reads this, and it owns missing output: an empty + // turn is its follow-up's input, not this invocation's failure. + if (event.type === "message_end") + assistantFailure ??= getAssistantFailure(event.message, { includeIncomplete: false }); if (mode === "json") { // The projection decides what a record contains; a caller that filters // first would be a second, silently divergent policy. @@ -283,7 +310,7 @@ export async function runPrintMode(runtimeHost: AgentSessionRuntimeHost, options if (initialMessage && !(completionAttempts !== undefined && terminalOutcome())) { latestPromptHasExternalHandoff = false; await session.prompt(initialMessage, { images: initialImages }); - assistantFailure ??= getAssistantFailure(session.state.messages.at(-1)); + assistantFailure ??= getAssistantFailure(session.state.messages.at(-1), { includeIncomplete: false }); } // Keep all explicit user messages in order; the completion budget is for @@ -292,11 +319,13 @@ export async function runPrintMode(runtimeHost: AgentSessionRuntimeHost, options if (completionAttempts !== undefined && terminalOutcome()) break; latestPromptHasExternalHandoff = false; await session.prompt(message); - assistantFailure ??= getAssistantFailure(session.state.messages.at(-1)); + assistantFailure ??= getAssistantFailure(session.state.messages.at(-1), { includeIncomplete: false }); } if (completionAttempts !== undefined) { - assistantFailure ??= getAssistantFailure(session.state.messages.at(-1)); + // A completion check owns missing output here, so an empty turn is its + // bounded follow-up's input rather than this invocation's failure. + assistantFailure ??= getAssistantFailure(session.state.messages.at(-1), { includeIncomplete: false }); for (let attempt = 0; attempt <= completionAttempts && !automaticCompletionBlocked(); attempt++) { // Extension commands may explicitly replace the runtime. Keep normal // rebinding intact, but never carry automatic feedback to a new session. @@ -383,7 +412,7 @@ export async function runPrintMode(runtimeHost: AgentSessionRuntimeHost, options } finally { unsubscribeReview?.(); } - assistantFailure ??= getAssistantFailure(session.state.messages.at(-1)); + assistantFailure ??= getAssistantFailure(session.state.messages.at(-1), { includeIncomplete: false }); } } @@ -401,8 +430,13 @@ export async function runPrintMode(runtimeHost: AgentSessionRuntimeHost, options if (lastMessage?.role === "assistant") { const assistantMsg = lastMessage as AssistantMessage; - if (assistantMsg.stopReason === "error" || assistantMsg.stopReason === "aborted") { - console.error(assistantMsg.errorMessage || `Request ${assistantMsg.stopReason}`); + // An empty turn is only a failure when no completion check owns missing + // output; that path reports it as an incomplete result instead. + const lastFailure = getAssistantFailure(lastMessage, { + includeIncomplete: completionAttempts === undefined, + }); + if (lastFailure) { + console.error(assistantFailureText(lastFailure)); exitCode = 1; } else if (mode === "text") { for (const content of assistantMsg.content) { @@ -420,7 +454,7 @@ export async function runPrintMode(runtimeHost: AgentSessionRuntimeHost, options if (completionAttempts !== undefined && exitCode === 0 && !hasFinalAssistantText(lastMessage)) { if (assistantFailure) { - console.error(assistantFailure.errorMessage || `Request ${assistantFailure.stopReason}`); + console.error(assistantFailureText(assistantFailure)); exitCode = 1; } else if (!latestPromptHasExternalHandoff) { console.error("Completion check incomplete: no final answer text after bounded follow-up."); diff --git a/packages/coding-agent/src/step/stdio-host.ts b/packages/coding-agent/src/step/stdio-host.ts index b8b6aa5b0..a6f9c1f57 100644 --- a/packages/coding-agent/src/step/stdio-host.ts +++ b/packages/coding-agent/src/step/stdio-host.ts @@ -1094,6 +1094,10 @@ export class StepStdioHost { }, }); } else if (event.type === "agent_settled") { + // A settled run is not automatically a successful one: a terminal empty turn + // keeps the provider's stop reason on its message, so the outcome can only be + // read here. Without this the query would report success with no answer. + if (event.failure && query.errorMessage === undefined) query.errorMessage = event.failure.message; this.#maybeFinish(query); } } diff --git a/packages/coding-agent/test/auto-clm-runtime.test.ts b/packages/coding-agent/test/auto-clm-runtime.test.ts index 1a7695e63..0b662e7c8 100644 --- a/packages/coding-agent/test/auto-clm-runtime.test.ts +++ b/packages/coding-agent/test/auto-clm-runtime.test.ts @@ -5,6 +5,7 @@ import type { AgentMessage, StreamFn, ThinkingLevel } from "@step-harness/agent- import { createAssistantMessageEventStream, fauxAssistantMessage, + fauxThinking, fauxToolCall, type Model, } from "@step-harness/providers"; @@ -521,14 +522,24 @@ describe("bounded automatic CLM requests", () => { expect(result).toMatchObject({ attempted: false, reason: "no-reducible-context" }); expect(requests).toBe(0); }); - it("reconstructs cooldown from completed task turns while ignoring failed responses", async () => { + it("reconstructs cooldown from completed task turns while ignoring failed and incomplete responses", async () => { const state = setup(); state.session.appendCustomEntry("step-auto-clm", { version: 1, attempted: true, accepted: false }); for (const stopReason of ["error", "aborted", "length"] as const) state.session.appendMessage(fauxAssistantMessage("Failed response", { stopReason })); + state.session.appendMessage(fauxAssistantMessage([fauxThinking("reasoning only")], { stopReason: "stop" })); const result = await new AutoClmController(state.session, state.live).run(input(state, () => completed())); expect(result).toMatchObject({ attempted: false, reason: "cooldown" }); }); + it("counts a stop turn with tool-call markup as completed", async () => { + const state = setup(); + state.session.appendCustomEntry("step-auto-clm", { version: 1, attempted: true, accepted: false }); + for (let index = 0; index < 3; index++) { + state.session.appendMessage(fauxAssistantMessage("example only")); + } + const result = await new AutoClmController(state.session, state.live).run(input(state, () => completed())); + expect(result.reason).not.toBe("cooldown"); + }); it("caps actual provider output including legacy Anthropic thinking", async () => { const state = setup(); state.model = { diff --git a/packages/coding-agent/test/fixtures/subagent-settled-child.mjs b/packages/coding-agent/test/fixtures/subagent-settled-child.mjs new file mode 100644 index 000000000..b669d48d9 --- /dev/null +++ b/packages/coding-agent/test/fixtures/subagent-settled-child.mjs @@ -0,0 +1,88 @@ +// Fake `step --mode rpc` child for the subagent settled-failure regression. +// +// Speaks the real framing (one JSON object per LF-framed line). It drives the +// parent's production rpc adapter: the only thing synthetic here is the child's +// event stream. +// +// The empty turn keeps `stop` on its assistant message — that is what the kernel +// does — so the run's outcome can only arrive on `agent_settled`. `SETTLED_MODE` +// selects the two shapes a real child can produce: +// +// failure — the run ended without an actionable answer +// success — a normal settled run, whose transcript is the only evidence +const MODE = process.env.SETTLED_MODE ?? "success"; + +let buffer = ""; +let queue = Promise.resolve(); +process.stdin.setEncoding("utf8"); +process.stdin.on("data", (chunk) => { + buffer += chunk; + const lines = buffer.split("\n"); + buffer = lines.pop() ?? ""; + for (const line of lines) queue = queue.then(() => handle(line.trim())); +}); + +function write(value) { + process.stdout.write(`${JSON.stringify(value)}\n`); +} + +function assistant(content, stopReason = "stop") { + return { + role: "assistant", + content, + api: "anthropic-messages", + provider: "step", + model: "step-3.8-code-alpha", + usage: { input: 100, output: 50, cacheRead: 0, cacheWrite: 0, totalTokens: 150, cost: {} }, + stopReason, + timestamp: Date.now(), + }; +} + +async function handle(line) { + if (!line) return; + let command; + try { + command = JSON.parse(line); + } catch { + return; + } + if (command.type === "get_state") { + write({ + type: "response", + id: command.id, + command: "get_state", + success: true, + data: { + sessionFile: `${process.env.TMPDIR ?? "/tmp/"}settled-child-session.jsonl`, + }, + }); + return; + } + if (command.type !== "prompt") return; + + write({ type: "response", id: command.id, command: "prompt", success: true }); + write({ type: "agent_start" }); + write({ type: "turn_start" }); + + const empty = assistant([]); + write({ type: "message_start", message: empty }); + // The kernel classifies the turn and records it on the message it persists. + empty.incomplete = "no_visible_output"; + write({ type: "message_end", message: empty }); + write({ type: "turn_end", messageRole: "assistant", toolResultCount: 0 }); + write({ type: "agent_end", willRetry: false, messageCount: 2 }); + + if (MODE === "failure") { + write({ + type: "agent_settled", + failure: { + kind: "incomplete_turn", + reason: "no_visible_output", + message: "The model returned no visible answer or executable tool call.", + }, + }); + return; + } + write({ type: "agent_settled" }); +} diff --git a/packages/coding-agent/test/live-context-manager.test.ts b/packages/coding-agent/test/live-context-manager.test.ts index 98098b375..2afccf671 100644 --- a/packages/coding-agent/test/live-context-manager.test.ts +++ b/packages/coding-agent/test/live-context-manager.test.ts @@ -184,6 +184,20 @@ describe("live context manager", () => { expect(JSON.stringify(restored.project(withError))).toContain("survives retry and resume"); expect(JSON.stringify(restored.project(withError))).not.toContain("OLD DETAIL"); }); + it("keeps a host-projected omission across a retried provider error", async () => { + const { manager, raw, session, directory } = fixture(true); + const projected = raw.filter((_message, index) => index !== 2); + await manager.prepare(projected, raw); + edit(manager, "OLD DETAIL to replace", "host omission survived"); + expect((await manager.accept(raw))?.accepted).toBe(true); + + const restored = new LiveContextManager(session, { directory }); + const failed = fauxAssistantMessage("", { stopReason: "error", errorMessage: "transient", timestamp: 0 }); + const withError = [failed, ...raw]; + expect(JSON.stringify(restored.project(withError))).toContain("host omission survived"); + expect(JSON.stringify(restored.project(withError))).not.toContain("OLD DETAIL"); + }); + it("restores edits and readable archives after closing and reopening a JSONL session", async () => { const { manager, raw, session, directory } = fixture(true); await manager.prepare(raw); diff --git a/packages/coding-agent/test/print-mode.test.ts b/packages/coding-agent/test/print-mode.test.ts index 3c668b930..e65684e19 100644 --- a/packages/coding-agent/test/print-mode.test.ts +++ b/packages/coding-agent/test/print-mode.test.ts @@ -20,6 +20,7 @@ type FakeSession = { getUnknownToolSelectors: () => { tools: string[]; excludeTools: string[]; knownTools: string[] }; prompt: ReturnType; reload: ReturnType; + setIncompleteTurnRecoveryEnabled: ReturnType; emitToListeners: (event: unknown) => void; }; @@ -79,6 +80,7 @@ function createRuntimeHost(assistantMessage: AssistantMessage): FakeRuntimeHost getUnknownToolSelectors: () => ({ tools: [], excludeTools: [], knownTools: [] }), prompt: vi.fn(async () => {}), reload: vi.fn(async () => {}), + setIncompleteTurnRecoveryEnabled: vi.fn(), emitToListeners: (event: unknown) => { for (const listener of listeners) listener(event); }, @@ -118,6 +120,23 @@ describe("runPrintMode", () => { expect(session.extensionRunner.emit).toHaveBeenCalledWith({ type: "session_shutdown", reason: "quit" }); }); + it("owns incomplete-turn recovery unless a completion feature supersedes it", async () => { + const plain = createRuntimeHost(createAssistantMessage({ text: "done" })); + await runPrintMode(plain as unknown as Parameters[0], { + mode: "text", + initialMessage: "Say done", + }); + expect(plain.session.setIncompleteTurnRecoveryEnabled.mock.calls).toEqual([[true]]); + + const checked = createRuntimeHost(createAssistantMessage({ text: "done" })); + await runPrintMode(checked as unknown as Parameters[0], { + mode: "text", + initialMessage: "Say done", + completionReview: true, + }); + expect(checked.session.setIncompleteTurnRecoveryEnabled.mock.calls).toEqual([[false]]); + }); + it("emits session_shutdown in json mode", async () => { const runtimeHost = createRuntimeHost(createAssistantMessage({ text: "done" })); const { session } = runtimeHost; diff --git a/packages/coding-agent/test/rpc-prompt-response-semantics.test.ts b/packages/coding-agent/test/rpc-prompt-response-semantics.test.ts index 2ff159309..69f60daaf 100644 --- a/packages/coding-agent/test/rpc-prompt-response-semantics.test.ts +++ b/packages/coding-agent/test/rpc-prompt-response-semantics.test.ts @@ -1,7 +1,7 @@ import { existsSync, mkdirSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { Agent } from "@step-harness/agent-core"; +import { Agent, type StreamFn } from "@step-harness/agent-core"; import { type AssistantMessage, type AssistantMessageEvent, @@ -75,6 +75,13 @@ function createAssistantMessage(text: string): AssistantMessage { }; } +function createThinkingOnlyMessage(): AssistantMessage { + return { + ...createAssistantMessage(""), + content: [{ type: "thinking", thinking: "I should run a command." }], + }; +} + type ParsedOutputLine = Record; function parseOutputLines(outputLines: string[]): ParsedOutputLine[] { @@ -94,7 +101,13 @@ function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } -async function createRuntimeHost(options: { withAuth: boolean; responseDelayMs: number; model?: Model }): Promise<{ +async function createRuntimeHost(options: { + withAuth: boolean; + responseDelayMs: number; + model?: Model; + streamFn?: StreamFn; + settingsOverrides?: Parameters[0]; +}): Promise<{ runtimeHost: AgentSessionRuntime; cleanup: () => Promise; }> { @@ -113,20 +126,23 @@ async function createRuntimeHost(options: { withAuth: boolean; responseDelayMs: systemPrompt: "Test", tools: [], }, - streamFn: (_model, _context, _options) => { - const stream = new MockAssistantStream(); - queueMicrotask(() => { - stream.push({ type: "start", partial: createAssistantMessage("") }); - setTimeout(() => { - stream.push({ type: "done", reason: "stop", message: createAssistantMessage("done") }); - }, options.responseDelayMs); - }); - return stream; - }, + streamFn: + options.streamFn ?? + ((_model, _context, _options) => { + const stream = new MockAssistantStream(); + queueMicrotask(() => { + stream.push({ type: "start", partial: createAssistantMessage("") }); + setTimeout(() => { + stream.push({ type: "done", reason: "stop", message: createAssistantMessage("done") }); + }, options.responseDelayMs); + }); + return stream; + }), }); const sessionManager = SessionManager.inMemory(); const settingsManager = SettingsManager.create(tempDir, tempDir); + if (options.settingsOverrides) settingsManager.applyOverrides(options.settingsOverrides); const authStorage = AuthStorage.create(join(tempDir, "auth.json")); const modelRegistry = await createInMemoryModelRegistry(authStorage); registerTestStepProvider(getModelRuntime(modelRegistry)); @@ -170,7 +186,13 @@ async function createRuntimeHost(options: { withAuth: boolean; responseDelayMs: }; } -async function startRpcMode(options: { withAuth: boolean; responseDelayMs: number; model?: Model }): Promise<{ +async function startRpcMode(options: { + withAuth: boolean; + responseDelayMs: number; + model?: Model; + streamFn?: StreamFn; + settingsOverrides?: Parameters[0]; +}): Promise<{ lineHandler: (line: string) => void; cleanup: () => Promise; }> { @@ -286,6 +308,63 @@ describe("RPC prompt response semantics", () => { } }); + it("reports a post-acceptance recovery failure on the settled run, not as a second response", async () => { + let calls = 0; + const streamFn: StreamFn = () => { + const stream = new MockAssistantStream(); + queueMicrotask(() => { + if (calls++ === 0) { + const base = createThinkingOnlyMessage(); + const message = { + ...base, + usage: { ...base.usage, input: 2_000, totalTokens: 2_000 }, + }; + stream.push({ type: "start", partial: message }); + stream.push({ type: "done", reason: "stop", message }); + return; + } + stream.push({ + type: "error", + reason: "error", + error: { ...createAssistantMessage(""), stopReason: "error", errorMessage: "invalid_api_key" }, + }); + }); + return stream; + }; + const { lineHandler, cleanup } = await startRpcMode({ + withAuth: true, + responseDelayMs: 0, + model: { ...stepModel()!, contextWindow: 1_000, maxTokens: 100 }, + settingsOverrides: { + compaction: { enabled: true, contextProjection: "off", keepRecentTokens: 1, reserveTokens: 0 }, + }, + streamFn, + }); + + try { + lineHandler(JSON.stringify({ id: "b4", type: "prompt", message: "Hello" })); + + await vi.waitFor(() => { + const settled = parseOutputLines(rpcIo.outputLines).filter((record) => record.type === "agent_settled"); + expect(settled).toHaveLength(1); + const failure = settled[0].failure as Record | undefined; + expect(failure).toMatchObject({ kind: "incomplete_turn", reason: "no_visible_output" }); + expect(String(failure?.message)).toContain( + "Cannot recover an incomplete assistant turn because automatic compaction did not complete", + ); + expect(String(failure?.message)).toContain("Context overflow recovery failed"); + }); + + // An accepted prompt still gets exactly one response; the failure travels + // as a durable event so a record consumer sees it exactly once. + const responses = getPromptResponses(rpcIo.outputLines, "b4"); + expect(responses).toHaveLength(1); + expect(responses[0]).toMatchObject({ success: true }); + } finally { + await cleanup(); + } + }); + it("returns and clears queued steering and follow-up messages", async () => { const { lineHandler, cleanup } = await startRpcMode({ withAuth: true, responseDelayMs: 500 }); diff --git a/packages/coding-agent/test/step-schedule.test.ts b/packages/coding-agent/test/step-schedule.test.ts index d717e6e16..dfec2bf94 100644 --- a/packages/coding-agent/test/step-schedule.test.ts +++ b/packages/coding-agent/test/step-schedule.test.ts @@ -279,6 +279,43 @@ describe("StepGoalRuntime", () => { expect(failed.runtime.get()?.status).toBe("blocked"); }); + test("a settled run failure stops the goal even when the turn looks complete", () => { + // An empty turn keeps the provider's stop reason on its message, so `agent_end` + // alone reads as a normal stop. Only the settled failure says the run failed. + const failed = runtimeHarness(); + failed.runtime.start("Recover the service", "session-1"); + failed.runtime.onAgentStart(); + failed.runtime.onAgentEnd([assistant("stop")]); + expect(failed.runtime.get()?.status).toBe("active"); + failed.runtime.onAgentSettled({ + message: "The model returned no visible answer or executable tool call.", + }); + expect(failed.runtime.get()?.status).toBe("blocked"); + expect(failed.continuations).toHaveLength(0); + }); + + test("an empty turn alone does not fail the goal: the settled outcome decides", () => { + // A turn with no visible text or call can be recovered, and the session may + // hand completion to a host. Reading the message here would block a goal for a + // run the session never called a failure. + const recovered = runtimeHarness(); + recovered.runtime.start("Recover the service", "session-1"); + recovered.runtime.onAgentStart(); + recovered.runtime.onAgentEnd([assistant("stop")]); + expect(recovered.runtime.onAgentSettled()).toBeDefined(); + expect(recovered.runtime.get()?.status).toBe("active"); + expect(recovered.continuations).toHaveLength(1); + }); + + test("the user's own stop pauses the goal instead of blocking it", () => { + const interrupted = runtimeHarness(); + interrupted.runtime.start("Recover the service", "session-1"); + interrupted.runtime.onAgentStart(); + interrupted.runtime.onAgentEnd([assistant("aborted")]); + interrupted.runtime.onAgentSettled(); + expect(interrupted.runtime.get()?.status).toBe("paused"); + }); + test("restores a matching snapshot and honors a clear tombstone", () => { const source = runtimeHarness(); const goal = source.runtime.start("Persist me", "session-1"); diff --git a/packages/coding-agent/test/subagent-settled-failure.test.ts b/packages/coding-agent/test/subagent-settled-failure.test.ts new file mode 100644 index 000000000..11ca976ef --- /dev/null +++ b/packages/coding-agent/test/subagent-settled-failure.test.ts @@ -0,0 +1,100 @@ +import { expect, test } from "vitest"; +import type { StepSubagentRunResult } from "../src/features/step-subagent.ts"; +import { + isFailed, + parseJsonEvent, + readSettledFailure, + resultText, + settledTurnOutcome, +} from "../src/features/step-subagent.ts"; +import { emptyUsage } from "../src/features/subagent/helpers.ts"; + +function emptyResult(): StepSubagentRunResult { + return { messages: [], stderr: "", exitCode: 0, usage: emptyUsage() }; +} + +test("a settled run without failure is a success", () => { + expect(readSettledFailure({ type: "agent_settled" })).toBeUndefined(); +}); + +test("a settled run that reports a failure carries its message", () => { + expect( + readSettledFailure({ + type: "agent_settled", + failure: { + kind: "incomplete_turn", + reason: "no_visible_output", + message: "The model returned no visible answer or executable tool call.", + }, + }), + ).toBe("The model returned no visible answer or executable tool call."); +}); + +test("a settled run whose failure has no usable message still fails", () => { + expect(readSettledFailure({ type: "agent_settled", failure: { kind: "incomplete_turn" } })).toBe( + "The agent run ended without a usable answer.", + ); +}); + +test("only agent_settled reports a run-level failure", () => { + expect(readSettledFailure({ type: "agent_end", failure: { message: "ignored" } })).toBeUndefined(); + expect(readSettledFailure({ type: "message_end", failure: { message: "ignored" } })).toBeUndefined(); +}); + +test("the JSON projection of an empty turn keeps the provider stop reason", () => { + const current = emptyResult(); + // The assistant message keeps `stop`, so a parent reading only message_end would + // call this a success; the failure arrives on agent_settled instead. + parseJsonEvent( + JSON.stringify({ + type: "message_end", + message: { + role: "assistant", + content: [{ type: "thinking", thinking: "reasoning only" }], + api: "openai-responses", + provider: "openai", + model: "mock", + usage: { input: 1, output: 1, cacheRead: 0, cacheWrite: 0, totalTokens: 2 }, + stopReason: "stop", + incomplete: "no_visible_output", + timestamp: 1, + }, + }), + current, + undefined, + ); + expect(current.stopReason).toBe("stop"); + expect(isFailed(current)).toBe(false); + expect(resultText(current)).toBe("(no output)"); +}); + +test("a failed lane surfaces the run failure instead of an empty answer", () => { + const current = emptyResult(); + const failure = readSettledFailure({ + type: "agent_settled", + failure: { + kind: "incomplete_turn", + reason: "missing_tool_call", + message: "The model ended a tool-use turn without an executable tool call.", + }, + }); + Object.assign(current, settledTurnOutcome(failure, false)); + expect(isFailed(current)).toBe(true); + expect(resultText(current)).toBe("The model ended a tool-use turn without an executable tool call."); +}); + +test("a settled run without a reported failure succeeds even when the turn is empty", () => { + const current = emptyResult(); + // The assistant message keeps `stop`, so nothing in the transcript says the run + // failed. Only `agent_settled.failure` may, and this run did not report one. + Object.assign(current, settledTurnOutcome(undefined, false)); + expect(current.exitCode).toBe(0); + expect(current.stopReason).toBeUndefined(); +}); + +test("a cancelled lane is the caller's stop, not a failure", () => { + const current = emptyResult(); + Object.assign(current, settledTurnOutcome("whatever the run said", true)); + expect(current.stopReason).toBe("aborted"); + expect(current.errorMessage).toBe("Subagent was aborted"); +}); diff --git a/packages/coding-agent/test/subagent-settled-lane.test.ts b/packages/coding-agent/test/subagent-settled-lane.test.ts new file mode 100644 index 000000000..55cf6023a --- /dev/null +++ b/packages/coding-agent/test/subagent-settled-lane.test.ts @@ -0,0 +1,89 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { fauxAssistantMessage, fauxToolCall } from "@step-harness/providers"; +import { afterAll, afterEach, expect, test } from "vitest"; +import { createStepSubagentExtension, isFailed, type StepSubagentDetails } from "../src/features/step-subagent.ts"; +import { createHarness, type Harness } from "./suite/harness.ts"; + +/** + * A settled subagent run is not automatically a successful one, on the real path. + * + * A real `--mode rpc` child is spawned through the production rpc adapter and + * parsed by the production projection. Its assistant message keeps the provider's + * `stop` — that is what an empty kernel turn looks like — so the only evidence + * that the lane failed is `agent_settled.failure`. This test fails if the adapter + * goes back to treating settlement as success. + */ +const childPath = fileURLToPath(new URL("./fixtures/subagent-settled-child.mjs", import.meta.url)); +const agentRoot = mkdtempSync(join(tmpdir(), "step-subagent-settled-")); +const harnesses: Harness[] = []; +const originalArgv = [...process.argv]; +const originalExecArgv = [...process.execArgv]; +const CHILD_ENV_KEYS = ["SETTLED_MODE", "STEP_CODING_AGENT_DIR"]; +const originalChildEnv = Object.fromEntries(CHILD_ENV_KEYS.map((key) => [key, process.env[key]])); + +afterEach(() => { + process.argv = [...originalArgv]; + process.execArgv = [...originalExecArgv]; + for (const key of CHILD_ENV_KEYS) { + const value = originalChildEnv[key]; + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + while (harnesses.length > 0) harnesses.pop()?.cleanup(); +}); + +afterAll(() => { + rmSync(agentRoot, { recursive: true, force: true }); +}); + +async function runAgainstRealChild(mode: "failure" | "success"): Promise { + process.env.SETTLED_MODE = mode; + process.env.STEP_CODING_AGENT_DIR = agentRoot; + // The default runner spawns `process.argv[1]`; point it at the fake rpc child + // and keep the parent's TS loader out of a plain-ESM child process. + process.argv[1] = childPath; + process.execArgv = process.execArgv.filter((arg) => !/^(?:--import|--loader|--require|-r)$/u.test(arg)); + + const harness = await createHarness({ + extensionFactories: [{ name: "step-subagent", factory: createStepSubagentExtension({ agentDir: agentRoot }) }], + }); + harnesses.push(harness); + + let details: StepSubagentDetails | undefined; + harness.session.subscribe((event) => { + if (event.type !== "tool_execution_end") return; + const result = event.result as { details?: StepSubagentDetails } | undefined; + if (result?.details?.results) details = result.details; + }); + + harness.setResponses([ + fauxAssistantMessage(fauxToolCall("subagent", { agent: "general", task: "inspect" }), { stopReason: "toolUse" }), + fauxAssistantMessage("parent done"), + ]); + await harness.session.prompt("delegate"); + if (!details) throw new Error("the subagent tool published no details"); + return details; +} + +test("a settled child that reports a failure marks its lane failed", async () => { + const details = await runAgainstRealChild("failure"); + const [record] = details.results; + expect(record.status).toBe("failed"); + expect(record.exitCode).toBe(1); + expect(record.stopReason).toBe("error"); + expect(record.errorMessage).toBe("The model returned no visible answer or executable tool call."); + expect(isFailed(record)).toBe(true); +}); + +test("a settled child without a failure keeps its lane successful", async () => { + // The transcript is empty and the assistant message says `stop`: without the + // explicit outcome this is indistinguishable from the failing case. + const details = await runAgainstRealChild("success"); + const [record] = details.results; + expect(record.status).toBe("completed"); + expect(record.exitCode).toBe(0); + expect(isFailed(record)).toBe(false); +}); diff --git a/packages/coding-agent/test/suite/agent-session-auto-clm.test.ts b/packages/coding-agent/test/suite/agent-session-auto-clm.test.ts index 904b264d7..1349fad15 100644 --- a/packages/coding-agent/test/suite/agent-session-auto-clm.test.ts +++ b/packages/coding-agent/test/suite/agent-session-auto-clm.test.ts @@ -448,39 +448,6 @@ describe("automatic CLM maintenance", () => { expect(h.eventsOfType("compaction_start")).toHaveLength(0); expect(h.session.getLiveContextStatus()!.revision).toBe(0); }); - it("retains parser-resampling exclusions when maintenance is interrupted", async () => { - const noop: AgentTool = { - name: "noop", - label: "Noop", - description: "Check progress", - parameters: Type.Object({}), - execute: async () => ({ content: [{ type: "text", text: "current evidence" }], details: {} }), - }; - const h = await setup({ tools: [noop] }); - seed(h, "brief prior state"); - h.setResponses([fauxAssistantMessage(oldText)]); - await h.session.prompt("collect old findings"); - let nextContext: Context | undefined; - h.setResponses([ - fauxAssistantMessage("UNEXECUTED_PARSER_LEAK"), - fauxAssistantMessage(fauxToolCall("noop", {}), { stopReason: "toolUse" }), - async (context) => { - expect(context.tools?.map((tool) => tool.name)).toEqual(["apply_context_edit"]); - await h.session.prompt("STEERING: retain output order", { streamingBehavior: "steer" }); - return fauxAssistantMessage("No edit while user input pending"); - }, - (context) => { - nextContext = context; - return fauxAssistantMessage("done"); - }, - ]); - await h.session.prompt("inspect task"); - expect(h.eventsOfType("compaction_start")).toHaveLength(0); - expect(h.getPendingResponseCount()).toBe(0); - expect(nextContext).toBeDefined(); - expect(JSON.stringify(nextContext!.messages)).not.toContain("UNEXECUTED_PARSER_LEAK"); - expect(JSON.stringify(nextContext!.messages)).toContain("STEERING"); - }); it("waits for the complete tool batch and continues the task once", async () => { let finished = false; const bulk: AgentTool = { diff --git a/packages/coding-agent/test/suite/agent-session-live-context.test.ts b/packages/coding-agent/test/suite/agent-session-live-context.test.ts index 337d80239..d637f4b0f 100644 --- a/packages/coding-agent/test/suite/agent-session-live-context.test.ts +++ b/packages/coding-agent/test/suite/agent-session-live-context.test.ts @@ -1,7 +1,7 @@ import { execFileSync, spawnSync } from "node:child_process"; import { existsSync, readFileSync, writeFileSync } from "node:fs"; import type { AgentTool } from "@step-harness/agent-core"; -import { type Context, fauxAssistantMessage, fauxToolCall } from "@step-harness/providers"; +import { type Context, fauxAssistantMessage, fauxThinking, fauxToolCall } from "@step-harness/providers"; import { Type } from "typebox"; import { afterEach, describe, expect, it } from "vitest"; import { createHarness, type Harness } from "./harness.ts"; @@ -447,7 +447,7 @@ describe("AgentSession CLM context integration", () => { expect(next!.messages.filter((m) => m.role === "toolResult")).toHaveLength(2); }, ); - it("accepts a valid edit after the native loop resamples a leaked tool call", async () => { + it("keeps a recovered incomplete turn in CLM source history before a later edit", async () => { let h: Harness; const noop: AgentTool = { name: "noop", @@ -477,7 +477,7 @@ describe("AgentSession CLM context integration", () => { await h.session.prompt("next"); let next: Context | undefined; h.setResponses([ - fauxAssistantMessage("parser leak"), + fauxAssistantMessage(fauxThinking("recovered response retained in context")), fauxAssistantMessage(fauxToolCall("noop", {}), { stopReason: "toolUse" }), fauxAssistantMessage(fauxToolCall("organize", {}), { stopReason: "toolUse" }), (context) => { @@ -488,8 +488,8 @@ describe("AgentSession CLM context integration", () => { await h.session.prompt("organize after noop"); expect(h.session.getLiveContextStatus()!.revision).toBe(1); expect(JSON.stringify(next!.messages)).toContain("resampled finding"); - expect(JSON.stringify(next!.messages)).not.toContain("parser leak"); - expect(JSON.stringify(h.sessionManager.getEntries())).toContain("parser leak"); + expect(JSON.stringify(next!.messages)).toContain("recovered response retained in context"); + expect(JSON.stringify(h.sessionManager.getEntries())).toContain("recovered response retained in context"); }); it("keeps the existing raw-history threshold policy in lightweight mode", async () => { diff --git a/packages/coding-agent/test/suite/completion-check.test.ts b/packages/coding-agent/test/suite/completion-check.test.ts index cd05ba3d3..9bb62c362 100644 --- a/packages/coding-agent/test/suite/completion-check.test.ts +++ b/packages/coding-agent/test/suite/completion-check.test.ts @@ -359,14 +359,49 @@ describe("runPrintMode same-session completion check", () => { }, ); - it("leaves default-off behavior unchanged without Git or a final answer", async () => { + it("uses the generic recovery when completion checking is not enabled", async () => { const { harness, run } = await setup({}, false); - harness.setResponses([fauxAssistantMessage(fauxThinking("thinking only")), fauxAssistantMessage("unused")]); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("thinking only")), + fauxAssistantMessage("recovered without completion check"), + ]); expect(await run({ completionCheck: undefined })).toBe(0); - expect(harness.faux.state.callCount).toBe(1); + expect(harness.faux.state.callCount).toBe(2); + expect(stdout).toBe("recovered without completion check\n"); expect(console.error).not.toHaveBeenCalled(); }); + it("exits 1 when the model repeats an incomplete turn", async () => { + const { harness, run } = await setup({}, false); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("thinking only")), + fauxAssistantMessage(fauxThinking("thinking only again")), + ]); + expect(await run({ completionCheck: undefined })).toBe(1); + expect(harness.faux.state.callCount).toBe(2); + expect(stdout).toBe(""); + // The message keeps its provider stop reason; the exit code comes from the + // structural classification, so the diagnostic stays the only wording. + const assistants = harness.session.messages.filter((message) => message.role === "assistant"); + expect(assistants.at(-1)).toMatchObject({ stopReason: "stop", incomplete: "no_visible_output" }); + expect(console.error).toHaveBeenCalledWith("The model returned no visible answer or executable tool call."); + }); + + it("reports a repeated incomplete turn as exit 1 in JSON mode too", async () => { + const { harness, run } = await setup({}, false); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("thinking only")), + fauxAssistantMessage(fauxThinking("thinking only again")), + ]); + expect(await run({ mode: "json", completionCheck: undefined })).toBe(1); + expect(harness.faux.state.callCount).toBe(2); + // The record carries the classification and the settled failure, so a caller + // reading the event stream sees why the run ended without an answer. + expect(stdout).toContain('"incomplete":"no_visible_output"'); + expect(stdout).toContain('"kind":"incomplete_turn"'); + expect(stdout).not.toContain('"type":"completion_check"'); + }); + it("fails non-Git preflight before binding extensions or calling the model and cleans up", async () => { const { harness, host, run } = await setup({}, false); const bind = vi.spyOn(harness.session, "bindExtensions"); diff --git a/packages/coding-agent/test/suite/harness.ts b/packages/coding-agent/test/suite/harness.ts index f380c1767..dc964865b 100644 --- a/packages/coding-agent/test/suite/harness.ts +++ b/packages/coding-agent/test/suite/harness.ts @@ -74,6 +74,7 @@ export interface HarnessOptions { bindings?: Parameters[0]; withConfiguredAuth?: boolean; modelsJson?: Record; + sessionPersistence?: boolean; } export interface Harness { @@ -111,7 +112,7 @@ export async function createHarness(options: HarnessOptions = {}): Promise message.role === "assistant"); +} + +function recoveryEntries(harness: Harness) { + return harness.sessionManager + .getBranch() + .filter((entry) => entry.type === "custom_message" && entry.customType === INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE); +} + +function settledFailures(harness: Harness): AgentRunFailure[] { + return harness.eventsOfType("agent_settled").flatMap((event) => (event.failure ? [event.failure] : [])); +} + +describe("incomplete assistant turn recovery", () => { + const harnesses: Harness[] = []; + + afterEach(() => { + while (harnesses.length > 0) harnesses.pop()?.cleanup(); + }); + + it("preserves a thinking-only turn and retries with a recovery message", async () => { + const harness = await createHarness(); + harnesses.push(harness); + const requests: string[] = []; + harness.setResponses([ + fauxAssistantMessage([ + fauxThinking("I should run the command. echo hi"), + ]), + (context) => { + requests.push(JSON.stringify(context.messages)); + return fauxAssistantMessage("Recovered with a visible answer."); + }, + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(2); + expect(requests).toHaveLength(1); + expect(requests[0]).toContain("The last turn produced no visible answer"); + expect(requests[0]).toContain("I should run the command."); + + const messages = assistants(harness); + expect(messages).toHaveLength(2); + expect(messages[0].content).toEqual([ + fauxThinking("I should run the command. echo hi"), + ]); + // The empty turn keeps the provider's stop reason; the fact lives on `incomplete`. + expect(messages[0].stopReason).toBe("stop"); + expect(messages[0].incomplete).toBe("no_visible_output"); + expect(messages[1].incomplete).toBeUndefined(); + + const entries = recoveryEntries(harness); + expect(entries).toHaveLength(1); + expect(entries[0]).toMatchObject({ display: false }); + expect(getMessageText(entries[0])).toContain("The last turn produced no visible answer"); + expect(harness.eventsOfType("tool_execution_start")).toEqual([]); + expect(settledFailures(harness)).toEqual([]); + }); + + it("fails the run when the model repeats itself, without rewriting the stop reason", async () => { + const harness = await createHarness({ sessionPersistence: true }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first attempt")), + fauxAssistantMessage(fauxThinking("second attempt")), + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(2); + const messages = assistants(harness); + expect(messages).toHaveLength(2); + for (const message of messages) { + expect(message.stopReason).toBe("stop"); + expect(message.errorMessage).toBeUndefined(); + expect(message.incomplete).toBe("no_visible_output"); + } + expect(messages[1].diagnostics).toEqual([ + expect.objectContaining({ + type: "incomplete_turn", + details: expect.objectContaining({ + reason: "no_visible_output", + consecutiveTurns: 2, + maxAttempts: 1, + hadThinking: true, + hadVisibleText: false, + }), + }), + ]); + expect(settledFailures(harness)).toEqual([ + { + kind: "incomplete_turn", + reason: "no_visible_output", + message: expect.stringContaining("the run ended without an answer"), + }, + ]); + + const sessionFile = harness.sessionManager.getSessionFile(); + expect(sessionFile).toBeDefined(); + const reloaded = SessionManager.open(sessionFile!).getBranch(); + expect( + reloaded.flatMap((entry) => + entry.type === "message" && entry.message.role === "assistant" ? [entry.message] : [], + ), + ).toMatchObject([ + { stopReason: "stop", incomplete: "no_visible_output" }, + { + stopReason: "stop", + incomplete: "no_visible_output", + diagnostics: [expect.objectContaining({ type: "incomplete_turn" })], + }, + ]); + expect( + reloaded.filter( + (entry) => entry.type === "custom_message" && entry.customType === INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE, + ), + ).toHaveLength(1); + }); + + it("does not enter the generic retry path for an incomplete turn", async () => { + const harness = await createHarness(); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first attempt")), + fauxAssistantMessage(fauxThinking("second attempt")), + fauxAssistantMessage("generic retry must not run"), + ]); + const willRetry: boolean[] = []; + harness.session.subscribe((event) => { + if (event.type === "agent_end") willRetry.push(event.willRetry); + }); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(2); + expect(willRetry).toEqual([false, false]); + expect(harness.eventsOfType("auto_retry_start")).toEqual([]); + }); + + it("lets queued user input run instead of injecting a stale recovery prompt, and treats it as a new attempt", async () => { + const harness = await createHarness(); + harnesses.push(harness); + harness.setResponses([ + async () => { + await harness.session.prompt("Use the new direction.", { streamingBehavior: "steer" }); + return fauxAssistantMessage(fauxThinking("first attempt")); + }, + fauxAssistantMessage(fauxThinking("still nothing after the steer")), + fauxAssistantMessage("Visible answer to the recovery."), + ]); + + await harness.session.prompt("Inspect the workspace"); + + // 1 initial, 1 steered turn, 1 recovery — the same run keeps its recovery + // budget because user input is an attempt, not progress. + expect(harness.faux.state.callCount).toBe(3); + expect(recoveryEntries(harness)).toHaveLength(1); + const messages = assistants(harness); + expect(messages).toHaveLength(3); + expect(messages[2].content).toEqual([fauxText("Visible answer to the recovery.")]); + expect(settledFailures(harness)).toEqual([]); + }); + + it("gives the recovery budget back after an actionable tool turn", async () => { + const noop: AgentTool = { + name: "noop", + label: "Noop", + description: "Noop", + parameters: Type.Object({}), + execute: async () => ({ content: [{ type: "text", text: "noop done" }], details: {} }), + }; + const harness = await createHarness({ tools: [noop] }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first attempt")), + fauxAssistantMessage(fauxToolCall("noop", {}), { stopReason: "toolUse" }), + fauxAssistantMessage(fauxThinking("incomplete after the tool result")), + fauxAssistantMessage("Recovered after the tool result."), + ]); + + await harness.session.prompt("Inspect the workspace"); + + // A completed tool turn is progress, not another consecutive empty turn, so + // the model gets one more recovery instead of ending the run. + expect(harness.faux.state.callCount).toBe(4); + expect(recoveryEntries(harness)).toHaveLength(2); + expect(settledFailures(harness)).toEqual([]); + expect(assistants(harness).at(-1)?.content).toEqual([fauxText("Recovered after the tool result.")]); + }); + + it("recovers again after a long run of completed turns", async () => { + const noop: AgentTool = { + name: "noop", + label: "Noop", + description: "Noop", + parameters: Type.Object({}), + execute: async () => ({ content: [{ type: "text", text: "noop done" }], details: {} }), + }; + const harness = await createHarness({ tools: [noop] }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("empty early in the run")), + fauxAssistantMessage(fauxToolCall("noop", {}), { stopReason: "toolUse" }), + fauxAssistantMessage(fauxToolCall("noop", {}), { stopReason: "toolUse" }), + fauxAssistantMessage(fauxToolCall("noop", {}), { stopReason: "toolUse" }), + fauxAssistantMessage(fauxThinking("empty much later in the same run")), + fauxAssistantMessage("Recovered late in the run."), + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(6); + expect(recoveryEntries(harness)).toHaveLength(2); + expect(settledFailures(harness)).toEqual([]); + expect(harness.session.getLastAssistantText()).toBe("Recovered late in the run."); + }); + + it("finalizes the classification after extension content replacement", async () => { + const observedByExtension: string[] = []; + const observedBySession: Array<[string, string | undefined]> = []; + const harness = await createHarness({ + extensionFactories: [ + (pi) => { + pi.on("message_end", (event) => { + if (event.message.role === "assistant") observedByExtension.push(event.message.stopReason); + }); + }, + ], + }); + harnesses.push(harness); + harness.session.subscribe((event) => { + if (event.type === "message_end" && event.message.role === "assistant") { + observedBySession.push([event.message.stopReason, (event.message as AssistantMessage).incomplete]); + } + }); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first attempt")), + fauxAssistantMessage(fauxThinking("second attempt")), + ]); + + await harness.session.prompt("Inspect the workspace"); + + // Extensions see the provider's own stop reason for both turns; the session + // reports the same stop reason plus the structural classification. + expect(observedByExtension).toEqual(["stop", "stop"]); + expect(observedBySession).toEqual([ + ["stop", "no_visible_output"], + ["stop", "no_visible_output"], + ]); + }); + + it("does not recover when an extension adds visible text at message_end", async () => { + const harness = await createHarness({ + extensionFactories: [ + (pi) => { + pi.on("message_end", (event) => { + if (event.message.role !== "assistant") return undefined; + return { message: { ...event.message, content: [fauxText("Visible text added by an extension.")] } }; + }); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage(fauxThinking("the model produced no visible text"))]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(1); + expect(recoveryEntries(harness)).toHaveLength(0); + expect(assistants(harness).some((message) => message.incomplete !== undefined)).toBe(false); + }); + + it("recovers when an extension removes all visible text at message_end", async () => { + const harness = await createHarness({ + extensionFactories: [ + (pi) => { + pi.on("message_end", (event) => { + if (event.message.role !== "assistant") return undefined; + return { message: { ...event.message, content: [] } }; + }); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage("visible before the extension replaced it"), + fauxAssistantMessage("Recovered after the extension replacement."), + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(2); + expect(recoveryEntries(harness)).toHaveLength(1); + }); + + it("compacts a silent overflow before spending the recovery continuation", async () => { + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 1, maxTokens: 100 }], + settings: { compaction: { contextProjection: "off", enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async (event) => ({ + compaction: { + summary: "silent overflow compacted", + firstKeptEntryId: event.preparation.firstKeptEntryId, + tokensBefore: event.preparation.tokensBefore, + details: {}, + }, + })); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("incomplete after silent overflow")), + fauxAssistantMessage("Recovered after compaction."), + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(2); + const compactionIndex = harness.events.findIndex( + (event) => event.type === "compaction_start" && event.reason === "overflow", + ); + const recoveryIndex = harness.events.findIndex( + (event) => + event.type === "message_start" && + event.message.role === "custom" && + event.message.customType === INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE, + ); + expect(compactionIndex).toBeGreaterThanOrEqual(0); + expect(recoveryIndex).toBeGreaterThan(compactionIndex); + expect(harness.session.getLastAssistantText()).toBe("Recovered after compaction."); + expect(settledFailures(harness)).toEqual([]); + }); + + it("reports an unrecoverable silent overflow on the settled run, not by rewriting the message", async () => { + const harness = await createHarness({ + sessionPersistence: true, + models: [{ id: "faux-1", contextWindow: 1, maxTokens: 100 }], + settings: { compaction: { contextProjection: "off", enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", () => ({ cancel: true })); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("incomplete after silent overflow")), + fauxAssistantMessage("must not be used"), + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(1); + const [message] = assistants(harness); + expect(message.stopReason).toBe("stop"); + expect(message.errorMessage).toBeUndefined(); + // Recorded before the message was persisted, so a reload still has it. + expect(message.diagnostics).toEqual([ + expect.objectContaining({ + type: "incomplete_turn", + details: expect.objectContaining({ overflowKind: "context", consecutiveTurns: 1 }), + }), + ]); + const failures = settledFailures(harness); + expect(failures).toHaveLength(1); + expect(failures[0].message).toContain( + "Cannot recover an incomplete assistant turn because automatic compaction did not complete", + ); + // Nothing was spent: no recovery prompt and no second request. + expect(recoveryEntries(harness)).toHaveLength(0); + + const sessionFile = harness.sessionManager.getSessionFile(); + const reloaded = SessionManager.open(sessionFile!).getBranch(); + expect( + reloaded + .flatMap((entry) => (entry.type === "message" && entry.message.role === "assistant" ? [entry.message] : [])) + .map((entry) => entry.stopReason), + ).toEqual(["stop"]); + }); + + it("fails immediately when a silent overflow cannot be compacted because compaction is disabled", async () => { + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 1, maxTokens: 100 }], + settings: { compaction: { contextProjection: "off", enabled: false } }, + }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("incomplete after silent overflow")), + fauxAssistantMessage("must not be used"), + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(1); + const [message] = assistants(harness); + expect(message.stopReason).toBe("stop"); + expect(message.diagnostics).toEqual([ + expect.objectContaining({ + type: "incomplete_turn", + details: expect.objectContaining({ overflowKind: "context", consecutiveTurns: 1 }), + }), + ]); + expect(settledFailures(harness)).toEqual([ + { + kind: "incomplete_turn", + reason: "no_visible_output", + message: expect.stringContaining("automatic compaction is disabled"), + }, + ]); + expect(recoveryEntries(harness)).toHaveLength(0); + }); + + it("explains why an uncommittable silent overflow could not compact", async () => { + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 1, maxTokens: 100 }], + settings: { compaction: { contextProjection: "off", enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", () => ({ cancel: true })); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage(fauxThinking("incomplete after silent overflow"))]); + + await harness.session.prompt("Inspect the workspace"); + + expect(settledFailures(harness)[0].message).toContain("compaction did not complete"); + expect(settledFailures(harness)[0].message).not.toMatch(/compaction did not complete\.$/); + }); + + it("applies the silent-overflow guard to a tool-use turn that carries no tool call", async () => { + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 1, maxTokens: 100 }], + settings: { compaction: { contextProjection: "off", enabled: false } }, + }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage([], { stopReason: "toolUse" }), + fauxAssistantMessage("must not be used"), + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(1); + expect(assistants(harness)[0].incomplete).toBe("missing_tool_call"); + expect(recoveryEntries(harness)).toHaveLength(0); + expect(settledFailures(harness)).toEqual([ + { + kind: "incomplete_turn", + reason: "missing_tool_call", + message: expect.stringContaining("automatic compaction is disabled"), + }, + ]); + }); + + it("compacts a silent overflow on an empty tool-use turn without replaying it", async () => { + // The assistant message is restored by the compaction, so a retry would see an + // assistant at the tail and `agent.continue()` would reject the run. + const harness = await createHarness({ + sessionPersistence: true, + models: [{ id: "faux-1", contextWindow: 1, maxTokens: 100 }], + settings: { compaction: { contextProjection: "off", enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async (event) => ({ + compaction: { + summary: "silent overflow compacted", + firstKeptEntryId: event.preparation.firstKeptEntryId, + tokensBefore: event.preparation.tokensBefore, + details: {}, + }, + })); + }, + ], + }); + harnesses.push(harness); + // Overflow recovery needs history old enough to summarize. + for (const message of [ + { role: "user" as const, content: "Investigate the parser", timestamp: 1 }, + fauxAssistantMessage("prior observation ".repeat(200), { timestamp: 2 }), + { role: "user" as const, content: "Continue", timestamp: 3 }, + ]) { + harness.sessionManager.appendMessage(message); + } + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + harness.setResponses([ + fauxAssistantMessage([], { stopReason: "toolUse" }), + fauxAssistantMessage("Recovered after compaction."), + ]); + + await expect(harness.session.prompt("Inspect the workspace")).resolves.toBeUndefined(); + + expect(harness.faux.state.callCount).toBe(2); + expect(harness.session.getLastAssistantText()).toBe("Recovered after compaction."); + expect(settledFailures(harness)).toEqual([]); + // The recovered message is persisted, not mutated after the fact. + const sessionFile = harness.sessionManager.getSessionFile(); + const reloaded = SessionManager.open(sessionFile!).getBranch(); + expect( + reloaded + .flatMap((entry) => (entry.type === "message" && entry.message.role === "assistant" ? [entry.message] : [])) + .map((message) => message.incomplete), + ).toEqual([undefined, "missing_tool_call", undefined]); + }); + + it("does not refill the budget when an error or an abort alternates with an empty turn", async () => { + const harness = await createHarness({ settings: { retry: { enabled: true, maxRetries: 3, baseDelayMs: 0 } } }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("empty")), + fauxAssistantMessage("", { stopReason: "error", errorMessage: "503 upstream" }), + fauxAssistantMessage(fauxThinking("empty again")), + fauxAssistantMessage("", { stopReason: "error", errorMessage: "503 upstream" }), + fauxAssistantMessage(fauxThinking("never reached")), + fauxAssistantMessage("never reached"), + ]); + + await harness.session.prompt("Inspect the workspace"); + + // Two consecutive empty turns ended the run: the errors between them are not + // progress, so they must not refill the recovery budget. + const messages = assistants(harness); + expect(messages.filter((message) => message.incomplete !== undefined)).toHaveLength(2); + expect(settledFailures(harness)).toEqual([ + { + kind: "incomplete_turn", + reason: "no_visible_output", + message: expect.stringContaining("the run ended without an answer"), + }, + ]); + }); + + it("keeps the incomplete diagnostic after a reload", async () => { + const harness = await createHarness({ sessionPersistence: true }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first attempt")), + fauxAssistantMessage(fauxThinking("second attempt")), + ]); + + await harness.session.prompt("Inspect the workspace"); + + const sessionFile = harness.sessionManager.getSessionFile(); + const reloaded = SessionManager.open(sessionFile!) + .getBranch() + .flatMap((entry) => (entry.type === "message" && entry.message.role === "assistant" ? [entry.message] : [])); + expect(reloaded.map((message) => message.incomplete)).toEqual(["no_visible_output", "no_visible_output"]); + expect(reloaded[1].diagnostics).toEqual([ + expect.objectContaining({ + type: "incomplete_turn", + details: expect.objectContaining({ reason: "no_visible_output", consecutiveTurns: 2 }), + }), + ]); + }); + + it("keeps an executed tool batch paired with its call when the turn overflowed", async () => { + // A tool-use turn that did deliver its calls is not an incomplete turn, and + // overflow recovery must not remove it: the results already follow it. + const blocker: AgentTool = { + name: "blocker", + label: "Blocker", + description: "Ends the turn", + parameters: Type.Object({}), + execute: async () => ({ + content: [{ type: "text", text: "stopped here" }], + details: {}, + terminate: true, + }), + }; + const harness = await createHarness({ + tools: [blocker], + models: [{ id: "faux-1", contextWindow: 1, maxTokens: 100 }], + settings: { compaction: { contextProjection: "off", enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage(fauxToolCall("blocker", {}), { stopReason: "toolUse" })]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(1); + expect(recoveryEntries(harness)).toHaveLength(0); + expect(settledFailures(harness)).toEqual([]); + const messages = harness.session.messages; + const callIndex = messages.findIndex( + (message) => message.role === "assistant" && message.content.some((block) => block.type === "toolCall"), + ); + expect(callIndex).toBeGreaterThanOrEqual(0); + expect(messages[callIndex + 1]).toMatchObject({ role: "toolResult", toolName: "blocker" }); + }); + + it("settles quietly when the user aborts during silent-overflow compaction", async () => { + let harness: Harness; + harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 1, maxTokens: 100 }], + settings: { compaction: { contextProjection: "off", enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", () => { + void harness.session.abort(); + return { cancel: true }; + }); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("incomplete after silent overflow")), + fauxAssistantMessage("must not be used"), + ]); + + await expect(harness.session.prompt("Inspect the workspace")).resolves.toBeUndefined(); + + expect(harness.faux.state.callCount).toBe(1); + expect(settledFailures(harness)).toEqual([]); + expect(recoveryEntries(harness)).toHaveLength(0); + }); + + it("does not persist a recovery prompt when the run is aborted before continuation", async () => { + const harness = await createHarness(); + harnesses.push(harness); + harness.setResponses([ + async () => { + const message = fauxAssistantMessage(fauxThinking("incomplete before abort")); + void harness.session.abort(); + return message; + }, + fauxAssistantMessage("must not be used"), + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(1); + expect(recoveryEntries(harness)).toHaveLength(0); + expect(settledFailures(harness)).toEqual([]); + }); + + it("persists the recovery prompt once its continuation has started", async () => { + const harness = await createHarness(); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("incomplete before recovery")), + fauxAssistantMessage("the recovery request ran"), + ]); + harness.session.subscribe((event) => { + if ( + event.type === "message_start" && + event.message.role === "custom" && + event.message.customType === INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE + ) { + void harness.session.abort(); + } + }); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(2); + expect(recoveryEntries(harness)).toHaveLength(1); + }); + + it("gives a new top-level run its own recovery budget", async () => { + const harness = await createHarness(); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first run, first attempt")), + fauxAssistantMessage("first run recovered"), + fauxAssistantMessage(fauxThinking("second run, first attempt")), + fauxAssistantMessage("second run recovered"), + ]); + + await harness.session.prompt("First run"); + await harness.session.prompt("Second run"); + + expect(harness.faux.state.callCount).toBe(4); + expect(recoveryEntries(harness)).toHaveLength(2); + expect(settledFailures(harness)).toEqual([]); + }); + + it("leaves the decision to a host that owns completion", async () => { + const harness = await createHarness(); + harnesses.push(harness); + harness.session.setIncompleteTurnRecoveryEnabled(false); + harness.setResponses([fauxAssistantMessage(fauxThinking("the host owns completion"))]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(1); + expect(recoveryEntries(harness)).toHaveLength(0); + expect(settledFailures(harness)).toEqual([]); + // The structural fact still describes the turn that was persisted. + expect(assistants(harness)[0].incomplete).toBe("no_visible_output"); + }); + + it("does not recover a truncated turn", async () => { + const harness = await createHarness(); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage(fauxThinking("truncated reasoning"), { stopReason: "length" })]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(1); + expect(recoveryEntries(harness)).toHaveLength(0); + expect(assistants(harness).some((message) => message.incomplete !== undefined)).toBe(false); + }); + + it("does not treat tool-call-like text as an incomplete turn", async () => { + const harness = await createHarness(); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage("this is an example, not an executable call")]); + + await harness.session.prompt("Explain the markup"); + + expect(harness.faux.state.callCount).toBe(1); + expect(recoveryEntries(harness)).toHaveLength(0); + expect(assistants(harness).some((message) => message.incomplete !== undefined)).toBe(false); + }); + + it("delivers a follow-up queued after a terminal incomplete turn", async () => { + const harness = await createHarness(); + harnesses.push(harness); + let queued = false; + harness.session.subscribe((event) => { + if (event.type !== "agent_end" || queued || harness.eventsOfType("agent_end").length !== 2) return; + queued = true; + harness.session.agent.followUp({ + role: "user", + content: [{ type: "text", text: "Try a different approach." }], + timestamp: Date.now(), + }); + }); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first attempt")), + fauxAssistantMessage(fauxThinking("second attempt")), + fauxAssistantMessage("Answered the follow-up."), + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(3); + expect(harness.session.getLastAssistantText()).toBe("Answered the follow-up."); + // The queued turn resolved the run, so the earlier empty turns are not its outcome. + expect(settledFailures(harness)).toEqual([]); + }); + + it("lets the user's own stop outrank a terminal empty turn", async () => { + const harness = await createHarness(); + harnesses.push(harness); + let ends = 0; + harness.session.subscribe((event) => { + if (event.type !== "agent_end") return; + ends += 1; + // The second attempt is the terminal one; the user stops the run before it + // settles. + if (ends === 2) void harness.session.abort(); + }); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first attempt")), + fauxAssistantMessage(fauxThinking("second attempt")), + ]); + + await harness.session.prompt("Inspect the workspace"); + + // The second empty turn is terminal by classification, but the user ended the + // run: a pause must not be reported as a failure. + expect(assistants(harness)[1].incomplete).toBe("no_visible_output"); + expect(settledFailures(harness)).toEqual([]); + }); + + it("treats user input after a terminal empty turn as the run's outcome", async () => { + const harness = await createHarness(); + harnesses.push(harness); + let ends = 0; + harness.session.subscribe((event) => { + if (event.type !== "agent_end") return; + ends += 1; + // The second attempt is the terminal one; the user answers it themselves. + if (ends !== 2) return; + harness.session.agent.steer({ + role: "user", + content: [{ type: "text", text: "Use the new direction." }], + timestamp: Date.now(), + }); + }); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first attempt")), + fauxAssistantMessage(fauxThinking("second attempt")), + fauxAssistantMessage("Answered the direction."), + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.session.getLastAssistantText()).toBe("Answered the direction."); + // The user's own input decides the run, so the empty turn before it is not the + // outcome even though it was classified as terminal. + expect(settledFailures(harness)).toEqual([]); + }); + + it("classifies a hand-built string content without failing the run", async () => { + // An extension replacement is only normalized for a missing container, so a + // plain string reaches the classifier exactly as a legacy session file would. + const harness = await createHarness({ + extensionFactories: [ + (pi) => { + pi.on("message_end", (event) => { + if (event.message.role !== "assistant") return undefined; + return { message: { ...event.message, content: "an older transcript" } as never }; + }); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage(fauxThinking("provider text"))]); + + await expect(harness.session.prompt("Inspect the workspace")).resolves.toBeUndefined(); + + // No blocks can be read, so the turn is empty by structure; the diagnostic is + // written without throwing. + const [message] = assistants(harness); + expect(message.incomplete).toBe("no_visible_output"); + expect(message.diagnostics).toEqual([ + expect.objectContaining({ + type: "incomplete_turn", + details: expect.objectContaining({ reason: "no_visible_output", hadVisibleText: false }), + }), + ]); + }); + + it.each([ + ["no visible output", () => fauxAssistantMessage(fauxThinking("reasoning")), "no_visible_output"], + ["a tool-use turn with no call", () => fauxAssistantMessage([], { stopReason: "toolUse" }), "missing_tool_call"], + ] as const)("derives the turn and the run from one classification: %s", async (_label, respond, reason) => { + // The message-level `incomplete` and the run-level failure are two projections of + // the same decision. If they ever diverge, a consumer reading one of them is + // wrong while every single-shaped test still passes. + const harness = await createHarness(); + harnesses.push(harness); + harness.setResponses([respond(), respond()]); + + await harness.session.prompt("Inspect the workspace"); + + const messages = assistants(harness).filter((message) => message.incomplete !== undefined); + expect(messages.map((message) => message.incomplete)).toEqual([reason, reason]); + expect(settledFailures(harness)).toEqual([ + { kind: "incomplete_turn", reason, message: expect.stringContaining("the run ended without an answer") }, + ]); + }); + + it("names the turn it answers, so a reader can pair them without their positions", async () => { + const harness = await createHarness({ sessionPersistence: true }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage(fauxThinking("first attempt")), fauxAssistantMessage("Recovered.")]); + + await harness.session.prompt("Inspect the workspace"); + + const branch = harness.sessionManager.getBranch(); + const empty = branch.find( + (entry) => entry.type === "message" && entry.message.role === "assistant" && entry.message.incomplete, + ); + const recovery = branch.find( + (entry) => entry.type === "custom_message" && entry.customType === INCOMPLETE_TURN_RECOVERY_CUSTOM_TYPE, + ); + expect(recovery).toMatchObject({ details: { recoversEntryId: empty?.id } }); + }); + + it("delivers the user's own queued words while a decision is open, without recovering", async () => { + // A decision gate holds back work started on the user's behalf, not the words + // they wrote — those words are how they answer it. The empty turn itself must + // not spend a recovery on their behalf while the decision is open. + const harness = await createHarness({ + bindings: { mode: "print" }, + extensionFactories: [ + (pi) => { + pi.on("session_start", (_event, ctx) => { + ctx.registerAutomaticTurnGuard?.(() => true); + }); + }, + ], + }); + harnesses.push(harness); + let queued = false; + harness.session.subscribe((event) => { + if (event.type !== "agent_end" || queued) return; + queued = true; + void harness.session.prompt("Use the new direction.", { streamingBehavior: "steer" }); + }); + const requests: string[] = []; + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first attempt")), + (context) => { + requests.push(JSON.stringify(context.messages)); + return fauxAssistantMessage("Answered the direction."); + }, + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.session.hasPendingUserDecision).toBe(true); + // The queued words, and not a recovery prompt, are what the model saw next. + expect(harness.faux.state.callCount).toBe(2); + expect(requests[0]).toContain("Use the new direction."); + expect(requests[0]).not.toContain("The last turn produced no visible answer"); + expect(harness.session.getLastAssistantText()).toBe("Answered the direction."); + expect(recoveryEntries(harness)).toHaveLength(0); + }); + + it("does not recover when compaction itself makes a decision pending", async () => { + // The decision is re-read after compaction because that window awaits provider + // auth and extension hooks; a decision that appears there still holds the + // recovery back. + let harness: Harness; + harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 1, maxTokens: 100 }], + settings: { compaction: { contextProjection: "off", enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async (event) => { + // Only the post-run overflow compaction, so the decision appears after + // the post-run gate has already been evaluated once. + if (event.reason === "overflow") harness.session.registerAutomaticTurnGuard(() => true); + return { + compaction: { + summary: "silent overflow compacted", + firstKeptEntryId: event.preparation.firstKeptEntryId, + tokensBefore: event.preparation.tokensBefore, + details: {}, + }, + }; + }); + }, + ], + }); + harnesses.push(harness); + for (const message of [ + { role: "user" as const, content: "Investigate the parser", timestamp: 1 }, + fauxAssistantMessage("prior observation ".repeat(200), { timestamp: 2 }), + { role: "user" as const, content: "Continue", timestamp: 3 }, + ]) { + harness.sessionManager.appendMessage(message); + } + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + harness.setResponses([ + // Later than the pre-run compaction boundary, so the turn is not mistaken for + // one recorded before it. + fauxAssistantMessage(fauxThinking("incomplete after silent overflow"), { timestamp: Date.now() + 10_000 }), + fauxAssistantMessage("must not be used"), + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.session.hasPendingUserDecision).toBe(true); + expect(harness.faux.state.callCount).toBe(1); + expect(recoveryEntries(harness)).toHaveLength(0); + // A decision the user still owes is not a failure of their run. + expect(settledFailures(harness)).toEqual([]); + }); + + it("reports the terminal retry limit and records recovery metadata", async () => { + const harness = await createHarness(); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first attempt")), + fauxAssistantMessage(fauxThinking("second attempt")), + ]); + + await harness.session.prompt("Inspect the workspace"); + + const [failure] = settledFailures(harness); + expect(failure.message).toContain("Retry limit reached (1/1)"); + expect(failure.message).toContain("the run ended without an answer"); + // Classification records the budget, but only the recovery entry proves it started. + const last = assistants(harness)[1]; + expect(last.diagnostics?.[0]?.details).toMatchObject({ consecutiveTurns: 2, maxAttempts: 1 }); + expect(recoveryEntries(harness)[0]).toMatchObject({ details: { attempt: 1, maxAttempts: 1 } }); + }); + + it("preserves the same recovery instructions after an earlier failed run", async () => { + const harness = await createHarness(); + harnesses.push(harness); + const requests: string[] = []; + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first run, first attempt")), + (context) => { + requests.push(JSON.stringify(context.messages)); + return fauxAssistantMessage(fauxThinking("first run, second attempt")); + }, + fauxAssistantMessage(fauxThinking("second run, first attempt")), + (context) => { + requests.push(JSON.stringify(context.messages)); + return fauxAssistantMessage("Answered the second run."); + }, + ]); + + await harness.session.prompt("First run"); + await harness.session.prompt("Second run"); + + expect(requests).toHaveLength(2); + expect(requests[0]).toContain("The last turn produced no visible answer"); + const recoveries = recoveryEntries(harness).filter((entry) => entry.type === "custom_message"); + expect(recoveries).toHaveLength(2); + expect(recoveries[1].content).toEqual(recoveries[0].content); + expect(recoveries[1].content).toContain("Do not put tool calls or commands inside reasoning only."); + expect(harness.session.getLastAssistantText()).toBe("Answered the second run."); + }); + + it("does not recover a provider error", async () => { + const harness = await createHarness(); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage("", { stopReason: "error", errorMessage: "invalid_api_key" })]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(1); + const [message] = assistants(harness); + expect(message.stopReason).toBe("error"); + expect(message.incomplete).toBeUndefined(); + expect(recoveryEntries(harness)).toHaveLength(0); + expect(settledFailures(harness)).toEqual([]); + }); + + it("delivers queued input after a terminal incomplete turn instead of dropping it", async () => { + const harness = await createHarness(); + harnesses.push(harness); + let queued = false; + harness.session.subscribe((event) => { + if (event.type !== "agent_end" || queued) return; + queued = true; + harness.session.agent.steer({ + role: "user", + content: [{ type: "text", text: "Use the new direction." }], + timestamp: Date.now(), + }); + }); + harness.setResponses([ + fauxAssistantMessage(fauxThinking("first attempt")), + fauxAssistantMessage(fauxThinking("second attempt")), + fauxAssistantMessage("Visible answer after the queued input."), + ]); + + await harness.session.prompt("Inspect the workspace"); + + expect(harness.faux.state.callCount).toBe(3); + expect(assistants(harness).at(-1)?.content).toEqual([fauxText("Visible answer after the queued input.")]); + // The queued turn resolved the run, so the earlier failure is not the outcome. + expect(settledFailures(harness)).toEqual([]); + }); +}); diff --git a/packages/coding-agent/test/suite/lax-message-content.test.ts b/packages/coding-agent/test/suite/lax-message-content.test.ts index fa7db35f2..93797717d 100644 --- a/packages/coding-agent/test/suite/lax-message-content.test.ts +++ b/packages/coding-agent/test/suite/lax-message-content.test.ts @@ -70,6 +70,11 @@ describe("lax message content handling", () => { }, ]; const harness = await createHarness({ extensionFactories }); + // The replacement removes every block, so the turn is empty by structure and + // the session treats it exactly like a provider that returned nothing. The + // recovery is what turns it into a second model call here; this case is about + // the normalization, so the extra call is not what is under test. + harness.session.setIncompleteTurnRecoveryEnabled(false); try { harness.setResponses([fauxAssistantMessage("hello")]); @@ -78,6 +83,8 @@ describe("lax message content handling", () => { const assistantMessages = harness.session.messages.filter((message) => message.role === "assistant"); expect(assistantMessages).toHaveLength(1); expect(assistantMessages[0].content).toEqual([]); + // The classification still describes the message that was persisted. + expect(assistantMessages[0].incomplete).toBe("no_visible_output"); } finally { harness.cleanup(); } diff --git a/packages/coding-agent/test/workflow-runtime.test.ts b/packages/coding-agent/test/workflow-runtime.test.ts index 895e8ad40..851eaec8b 100644 --- a/packages/coding-agent/test/workflow-runtime.test.ts +++ b/packages/coding-agent/test/workflow-runtime.test.ts @@ -410,7 +410,7 @@ test("usage reported by a runner after cancellation is still charged to the budg expect(entries.at(-1)).toMatchObject({ status: "aborted", usage: { input: 7 } }); }); -test("a resampled request in the same turn is tagged with the current congestion epoch", async () => { +test("a follow-up request without a started event is tagged with the current congestion epoch", async () => { let finish!: () => void; const gate = new Promise((resolve) => { finish = resolve; @@ -430,7 +430,7 @@ test("a resampled request in the same turn is tagged with the current congestion runtime.semaphore.throttle(runtime.semaphore.epoch, 0); expect(runtime.semaphore.max).toBe(3); // The first success belongs to the pre-throttle epoch and is ignored. The - // resample that follows has no "started" event and must count again. + // follow-up request has no "started" event and must count again. for (let i = 0; i < 5; i++) input.onModelRequest?.({ type: "succeeded" }); capAfterRecovery = runtime.semaphore.max; finish(); diff --git a/packages/contracts/src/in-process/session-handle.ts b/packages/contracts/src/in-process/session-handle.ts index c8d030ba0..0b936ed57 100644 --- a/packages/contracts/src/in-process/session-handle.ts +++ b/packages/contracts/src/in-process/session-handle.ts @@ -43,7 +43,19 @@ export interface AgentMessagePreview { export type AgentEvent = | { readonly type: "agent_start" } | { readonly type: "agent_end"; readonly willRetry: boolean } - | { readonly type: "agent_settled" } + | { + readonly type: "agent_settled"; + /** + * Why the run produced no usable answer, when the transcript cannot say it. + * + * A provider error is visible as an assistant message's stop reason. An empty + * turn keeps the provider's own value, so a consumer that decides success from + * the transcript alone would be wrong. Opaque here on purpose: the concrete + * failure shape belongs to the product, which the contracts package must not + * depend on. + */ + readonly failure?: unknown; + } | { readonly type: "turn_start" } | { readonly type: "turn_end"; readonly message: AgentMessagePreview } | { readonly type: "message_start"; readonly message: AgentMessagePreview } diff --git a/packages/providers/README.md b/packages/providers/README.md index f33d33a0c..ef5f4784e 100644 --- a/packages/providers/README.md +++ b/packages/providers/README.md @@ -777,6 +777,8 @@ Every `AssistantMessage` includes a `stopReason` field that indicates how the ge - `"error"` - An error occurred during generation - `"aborted"` - Request was cancelled via abort signal +A successful stop reason does not promise anything a client can act on. A turn can end with `stop` while carrying neither visible text nor a tool call, and `toolUse` while carrying no call at all. `classifyIncompleteAssistantTurn()` classifies that from the stop reason and content-block types alone, and the agent loop records it as `AssistantMessage.incomplete` (`"no_visible_output"` or `"missing_tool_call"`) without changing the stop reason. Use `assistantTerminalFailure()` when a consumer needs to decide whether a run produced a usable answer; do not infer it from `stopReason` alone. + `AssistantMessage` may also include `responseId`, a provider-specific upstream response or message identifier when the underlying API exposes one. Do not assume it is always present across providers. ## Error Handling diff --git a/packages/providers/src/index.ts b/packages/providers/src/index.ts index b6fe9b154..300358b52 100644 --- a/packages/providers/src/index.ts +++ b/packages/providers/src/index.ts @@ -41,6 +41,7 @@ export * from "./types.ts"; export * from "./utils/diagnostics.ts"; export * from "./utils/error-kind.ts"; export * from "./utils/event-stream.ts"; +export * from "./utils/incomplete-turn.ts"; export * from "./utils/json-parse.ts"; export * from "./utils/overflow.ts"; export * from "./utils/retry.ts"; diff --git a/packages/providers/src/types.ts b/packages/providers/src/types.ts index 79159e50a..c16534c3e 100644 --- a/packages/providers/src/types.ts +++ b/packages/providers/src/types.ts @@ -5,6 +5,7 @@ import type { OpenAIResponsesOptions } from "./api/openai-responses.ts"; import type { AssistantMessageDiagnostic } from "./utils/diagnostics.ts"; import type { ProviderErrorKind } from "./utils/error-kind.ts"; import type { AssistantMessageEventStream } from "./utils/event-stream.ts"; +import type { IncompleteTurnReason } from "./utils/incomplete-turn.ts"; export type { AssistantMessageEventStream } from "./utils/event-stream.ts"; @@ -373,6 +374,12 @@ export interface AssistantMessage { diagnostics?: AssistantMessageDiagnostic[]; // Redacted provider/runtime diagnostics for failures and recoveries. usage: Usage; stopReason: StopReason; + /** + * Set when the finished turn carries nothing a client can act on: no visible + * text and no structured tool call. The stop reason keeps the provider's own + * value; consumers use this field instead of inferring an outcome from it. + */ + incomplete?: IncompleteTurnReason; deferred?: DeferredHandle; errorMessage?: string; /** diff --git a/packages/providers/src/utils/incomplete-turn.ts b/packages/providers/src/utils/incomplete-turn.ts new file mode 100644 index 000000000..2841c3d5b --- /dev/null +++ b/packages/providers/src/utils/incomplete-turn.ts @@ -0,0 +1,93 @@ +import type { AssistantMessage } from "../types.ts"; + +/** + * Why a finished provider turn carries nothing a client can act on. + * + * A client executes structured tool calls and renders text. A turn that reports + * a normal stop while offering neither will end a run with nothing visible and + * no work done, which reads as the task stopping early. + */ +export type IncompleteTurnReason = "missing_tool_call" | "no_visible_output"; + +/** Neutral, user-facing description of each reason. */ +export const INCOMPLETE_TURN_MESSAGES: Record = { + no_visible_output: "The model returned no visible answer or executable tool call.", + missing_tool_call: "The model ended a tool-use turn without an executable tool call.", +}; + +type ContentBlocks = AssistantMessage["content"]; + +/** + * A finished turn's content blocks, tolerating a persisted or hand-built message. + * + * Session files are read without validation, so `content` can be missing, or be a + * plain string from an older or hand-edited transcript. Only a real array can be + * inspected; anything else behaves as "no blocks", which classifies the turn + * rather than throwing inside every `message_end`. + */ +export function contentBlocks(message: { content?: unknown }): ContentBlocks { + if (!Array.isArray(message.content)) return []; + // Elements come off a JSON pipe too: a `null` entry must not turn every + // `message_end` into a TypeError. + return (message.content as ContentBlocks).filter( + (block): block is ContentBlocks[number] => typeof block === "object" && block !== null && "type" in block, + ); +} + +function hasVisibleText(blocks: ContentBlocks): boolean { + return blocks.some( + (block) => block.type === "text" && typeof block.text === "string" && block.text.trim().length > 0, + ); +} + +/** + * Classify a turn from its structured stop reason and content-block types. + * + * Only `stop` and `toolUse` can be incomplete: `error` and `aborted` carry their + * own outcome, `length` is truncation that compaction owns, and `pending` / + * `deferred` are not finished turns. Visible text or a structured tool call are + * both actionable, so either one makes the turn complete. + */ +export function classifyIncompleteAssistantTurn(message: AssistantMessage): IncompleteTurnReason | undefined { + const content = contentBlocks(message); + // A tool-use stop promises a call the client can execute. Text does not make + // that promise good: the call is what the provider said it produced. + if (message.stopReason === "toolUse") { + return content.some((block) => block.type === "toolCall") ? undefined : "missing_tool_call"; + } + if (message.stopReason !== "stop") return undefined; + const actionable = hasVisibleText(content) || content.some((block) => block.type === "toolCall"); + return actionable ? undefined : "no_visible_output"; +} + +/** Whether a finished turn carries nothing a client can act on. */ +export function isIncompleteAssistantTurn(message: AssistantMessage): boolean { + return classifyIncompleteAssistantTurn(message) !== undefined; +} + +/** Why a run may treat this assistant message as its terminal failure. */ +export interface AssistantTerminalFailure { + kind: "error" | "aborted" | "incomplete"; + message: string; + /** Set when `kind` is `incomplete`; the empty turn that ended the run. */ + incompleteReason?: IncompleteTurnReason; +} + +/** + * Describe a terminal run failure carried by an assistant message. + * + * Provider errors and cancellations are reported by `stopReason`. An incomplete + * turn keeps the provider's stop reason and is reported separately, so consumers + * must not decide "the run succeeded" from `stopReason` alone. + */ +export function assistantTerminalFailure(message: AssistantMessage): AssistantTerminalFailure | undefined { + if (message.stopReason === "error") { + return { kind: "error", message: message.errorMessage || "The model request failed." }; + } + if (message.stopReason === "aborted") { + return { kind: "aborted", message: message.errorMessage || "The model request was cancelled." }; + } + const reason = classifyIncompleteAssistantTurn(message); + if (reason) return { kind: "incomplete", message: INCOMPLETE_TURN_MESSAGES[reason], incompleteReason: reason }; + return undefined; +} diff --git a/packages/providers/test/incomplete-turn.test.ts b/packages/providers/test/incomplete-turn.test.ts new file mode 100644 index 000000000..9ffa872e1 --- /dev/null +++ b/packages/providers/test/incomplete-turn.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, it } from "vitest"; +import type { AssistantMessage, Usage } from "../src/types.ts"; +import { + assistantTerminalFailure, + classifyIncompleteAssistantTurn, + INCOMPLETE_TURN_MESSAGES, + isIncompleteAssistantTurn, +} from "../src/utils/incomplete-turn.ts"; + +const usage: Usage = { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, +}; + +function assistant(content: AssistantMessage["content"], stopReason: AssistantMessage["stopReason"]): AssistantMessage { + return { + role: "assistant", + content, + api: "openai-responses", + provider: "openai", + model: "mock", + usage, + stopReason, + timestamp: 1, + }; +} + +describe("isIncompleteAssistantTurn", () => { + it.each([ + ["an empty stop", [], "stop", "no_visible_output"], + ["thinking-only stop", [{ type: "thinking" as const, thinking: "reasoning" }], "stop", "no_visible_output"], + ["whitespace-only text", [{ type: "text" as const, text: " \n\t" }], "stop", "no_visible_output"], + ["a tool-use stop without a tool call", [], "toolUse", "missing_tool_call"], + [ + "a tool-use stop with visible text but no tool call", + [{ type: "text" as const, text: "I will inspect it." }], + "toolUse", + "missing_tool_call", + ], + ] as const)("classifies %s as incomplete", (_label, content, stopReason, reason) => { + expect(classifyIncompleteAssistantTurn(assistant([...content], stopReason))).toBe(reason); + expect(isIncompleteAssistantTurn(assistant([...content], stopReason))).toBe(true); + }); + + it.each([ + ["visible text", [{ type: "text" as const, text: "answer" }], "stop"], + ["a structured tool call", [{ type: "toolCall" as const, id: "call-1", name: "read", arguments: {} }], "toolUse"], + ["a length stop", [{ type: "thinking" as const, thinking: "reasoning" }], "length"], + ["an error stop", [], "error"], + ["an aborted stop", [], "aborted"], + ] as const)("does not classify %s as incomplete", (_label, content, stopReason) => { + expect(isIncompleteAssistantTurn(assistant([...content], stopReason))).toBe(false); + }); + + it("tolerates a text block whose text is missing", () => { + const malformed = assistant([{ type: "text" } as never], "stop"); + expect(classifyIncompleteAssistantTurn(malformed)).toBe("no_visible_output"); + }); + + it("tolerates a null entry inside an otherwise valid content array", () => { + const malformed = { + ...assistant([], "stop"), + content: [null, { type: "text", text: " " }], + } as unknown as AssistantMessage; + expect(classifyIncompleteAssistantTurn(malformed)).toBe("no_visible_output"); + }); + + it.each([ + ["a plain string", "answer"], + ["null", null], + ["undefined", undefined], + ["an object", { type: "text", text: "answer" }], + ])("tolerates content that is %s", (_label, content) => { + // Session files are read without validation, so this runs on real input. + const malformed = { ...assistant([], "stop"), content } as unknown as AssistantMessage; + expect(() => classifyIncompleteAssistantTurn(malformed)).not.toThrow(); + expect(classifyIncompleteAssistantTurn(malformed)).toBe("no_visible_output"); + }); +}); + +describe("assistantTerminalFailure", () => { + it("reports provider errors and cancellations from the stop reason", () => { + expect(assistantTerminalFailure(assistant([], "error"))).toEqual({ + kind: "error", + message: "The model request failed.", + }); + const aborted = { ...assistant([], "aborted"), errorMessage: "Operation aborted" }; + expect(assistantTerminalFailure(aborted)).toEqual({ kind: "aborted", message: "Operation aborted" }); + }); + + it("reports an incomplete turn without rewriting its stop reason", () => { + const message = assistant([{ type: "thinking" as const, thinking: "reasoning" }], "stop"); + expect(assistantTerminalFailure(message)).toEqual({ + kind: "incomplete", + message: INCOMPLETE_TURN_MESSAGES.no_visible_output, + incompleteReason: "no_visible_output", + }); + expect(message.stopReason).toBe("stop"); + }); + + it("reports nothing for a completed turn", () => { + expect(assistantTerminalFailure(assistant([{ type: "text" as const, text: "done" }], "stop"))).toBeUndefined(); + }); +});