From e2ae79b6a456e58361b78bbe1bfca2568f5d1a34 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 6 Aug 2026 22:13:55 -0700 Subject: [PATCH] Drain the mid-run queue and settle the turn on inference.done reactor.done fires once, at agent shutdown, never between turns, and connector.reply never comes for a workflow/goal-governor cycle that keeps self-continuing. Both are dead ends a text-only or tool-only reply can hit, and both left session state stuck once nothing else arrived: a queued message with nowhere to drain to, `run` stuck busy so every later Enter queued instead of sending, and the phase ramp stuck saying "working" forever. inference.done is the one turn boundary every reactor cycle actually guarantees. Settle isProcessing there too, using the same criterion connector.reply already uses (no tool calls left outstanding, which are already known by then since tool-call events stream in before inference.done fires) -- reusing that path fixes `run`, the ramp, and the queue drain together, with no new event type needed. When tool calls are still outstanding the turn continues, so only the queued messages waiting on that boundary are drained. Also replace the queued-item transcript row's internal-state text ("queue +1 -> pending N", with the word "queue" duplicated on the same row) with the queued message's own text and attachments, so the depth reads once, in the notice row, and each item is identifiable. Cancelling a queued message before it sends is deliberately out of scope here; it needs its own transcript-row handling and its own review, not a rider on this fix. --- src/tui-opentui/runtime-bridge.test.ts | 110 +++++++++++++++++++++++++ src/tui-opentui/runtime-bridge.ts | 32 ++++--- src/tui-opentui/shell.ts | 25 ++++-- src/tui-opentui/turn-state.test.ts | 24 ++++++ src/tui-opentui/turn-state.ts | 33 ++++++-- 5 files changed, 194 insertions(+), 30 deletions(-) diff --git a/src/tui-opentui/runtime-bridge.test.ts b/src/tui-opentui/runtime-bridge.test.ts index 5ba250cd8..bf4521628 100644 --- a/src/tui-opentui/runtime-bridge.test.ts +++ b/src/tui-opentui/runtime-bridge.test.ts @@ -214,6 +214,116 @@ describe("attachSessionBridge", () => { ) }) + test("queued item delivers on a tool-less turn (inference.done, no tool calls)", async () => { + // Regression for CL-5563: reactor.done only fires once, at agent + // shutdown, never between turns — a plain-text reply with no tool calls + // must still drain the queue, or a queued message sits forever. + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + wireKeys: false, + run: "busy", + }) + const port = createRecordingPort() + const bridge = attachSessionBridge(shell, port) + try { + bridge.submit("follow up", "queue") + expect(badgeCount(shell.session)).toBe(1) + port.clear() + bridge.handle({ type: "inference.start" }) + bridge.handle({ + type: "inference.text.delta", + data: { token: "hi" }, + }) + bridge.handle({ type: "inference.done" }) + expect(badgeCount(shell.session)).toBe(0) + const deliver = port.calls.find((c) => c.op === "deliver") + expect(deliver).toEqual({ + op: "deliver", + item: expect.objectContaining({ + text: "follow up", + kind: "queue", + }), + }) + } finally { + bridge.dispose() + shell.dispose() + } + }, + { width: 80, height: 24 }, + ) + }) + + test("run and the phase ramp both return to idle after a tool-less inference.done, with no connector.reply", async () => { + // Regression: a goal-governor / workflow cycle that keeps self-continuing + // may never emit connector.reply, the only other event that clears + // `run` and the turn's `isProcessing`. Without this, every future Enter + // resolves to "queue" (busy is sticky) and, once the workflow stops + // producing cycles, that queued message is never drained — CL-5563's + // bug moved one layer over. The ramp indicator has the same failure + // mode: it reads `isProcessing`, not `run`, so it can say "working" + // forever even once dispatch itself is fixed. + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + wireKeys: false, + run: "busy", + }) + const port = createRecordingPort() + const bridge = attachSessionBridge(shell, port) + try { + bridge.handle({ type: "inference.start" }) + bridge.handle({ + type: "inference.text.delta", + data: { token: "hi" }, + }) + bridge.handle({ type: "inference.done" }) + expect(shell.session.run).toBe("idle") + expect(shell.turnPhase).toBeNull() + + port.clear() + bridge.submit("are you still there", "queue") + expect(port.calls).toEqual([ + { op: "sendImmediate", text: "are you still there" }, + ]) + } finally { + bridge.dispose() + shell.dispose() + } + }, + { width: 80, height: 24 }, + ) + }) + + test("run stays busy after inference.done while a tool call is still outstanding", async () => { + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + wireKeys: false, + run: "busy", + }) + const port = createRecordingPort() + const bridge = attachSessionBridge(shell, port) + try { + bridge.handle({ type: "inference.start" }) + bridge.handle({ + type: "inference.tool_call.start", + data: { call: { id: "c1", name: "bash" } }, + }) + bridge.handle({ type: "inference.done" }) + expect(shell.session.run).toBe("busy") + } finally { + bridge.dispose() + shell.dispose() + } + }, + { width: 80, height: 24 }, + ) + }) + test("token-by-token deltas grow one assistant row", async () => { await withTestRenderer( async (h) => { diff --git a/src/tui-opentui/runtime-bridge.ts b/src/tui-opentui/runtime-bridge.ts index 6003797b6..1b4769d16 100644 --- a/src/tui-opentui/runtime-bridge.ts +++ b/src/tui-opentui/runtime-bridge.ts @@ -29,6 +29,7 @@ import { streamRowAt, streamRowCount, truncateStreamRows, + userRowText, type AppShell, } from "./shell.js" import { rampFor, rampLine } from "./ramp.js" @@ -55,10 +56,7 @@ import { turnStateOnSubmit, type TurnState, } from "./turn-state.js" -import { - formatAttachmentSummary, - type PendingImageAttachment, -} from "../tui/image-attachments.js" +import type { PendingImageAttachment } from "../tui/image-attachments.js" import { toolCallRow } from "./diff.js" import { toolResultRow } from "./mcp-view.js" import { @@ -68,16 +66,6 @@ import { } from "./tool-rows.js" import type { StreamRow } from "./stream.js" import { advanceRevealChars, flattenReasoningText, type Thought } from "./thinking.js" - -/** Transcript echo for a user message, annotated with its attachments. */ -function userRowText( - text: string, - attachments: readonly PendingImageAttachment[], -): string { - const summary = formatAttachmentSummary(attachments) - if (summary.length === 0) return text - return text.length === 0 ? `[${summary}]` : `${text}\n[${summary}]` -} import { PRODUCTION_REACTOR_TYPES, createStreamMapContext, @@ -764,6 +752,13 @@ export function attachSessionBridge( )) { applyInbound(shell, bag, mapped) } + // inference.done with tool calls still outstanding doesn't settle the + // turn (see turn-state.ts) — the cycle continues, but a boundary still + // passed, so a queued message waiting on it should not wait for the + // turn's eventual end too. + if (event.type === "inference.done" && bag.turn.activeToolCalls.length > 0) { + drainAtBoundary(shell, bag) + } if (settled) settleRun() return } @@ -800,10 +795,13 @@ export function attachSessionBridge( ? enqueueSteer(shell.session, t, undefined, attachments) : enqueue(shell.session, t, "queue", undefined, attachments) bag.port.enqueue(t, kind) + // Show the message itself, not the internal transition ("queue +1 → + // pending N") — the notice row already carries the depth once, in plain + // language, so this row's job is making the pending item identifiable. appendStreamRow(shell, { - role: "system", - text: `${kind} +1 → pending ${badgeCount(shell.session)}`, - meta: "queue", + role: "user", + text: userRowText(t, attached), + meta: kind === "steer" ? "steer" : "queue", }) paintChrome(shell) } diff --git a/src/tui-opentui/shell.ts b/src/tui-opentui/shell.ts index 75b570e5d..c35fd1eaf 100644 --- a/src/tui-opentui/shell.ts +++ b/src/tui-opentui/shell.ts @@ -32,6 +32,7 @@ import { stringWidth } from "../tui/view/height.js" import { listPathSuggestions } from "../tui/components/at-mention/list.js" import { parseAtState } from "../tui/components/at-mention/parse.js" import { + formatAttachmentSummary, readClipboardImage, type ClipboardImageResult, type PendingImageAttachment, @@ -2761,6 +2762,16 @@ export function setShellRunState(shell: AppShell, run: RunState): void { paintChrome(shell) } +/** Transcript echo for a user message, annotated with its attachments. */ +export function userRowText( + text: string, + attachments: readonly PendingImageAttachment[], +): string { + const summary = formatAttachmentSummary(attachments) + if (summary.length === 0) return text + return text.length === 0 ? `[${summary}]` : `${text}\n[${summary}]` +} + /** Submit prompt as queue (busy) or immediate user send (idle). */ export function submitPrompt( shell: AppShell, @@ -2801,14 +2812,18 @@ export function submitPrompt( } shell.session = - kind === "steer" ? enqueueSteer(shell.session, t) : enqueue(shell.session, t) + kind === "steer" + ? enqueueSteer(shell.session, t, undefined, attachments) + : enqueue(shell.session, t, "queue", undefined, attachments) shell.prompt.value = "" clearPendingAttachments(shell) - const tag = kind === "steer" ? "steer" : "queue" + // Show the message itself, not the internal transition ("queue +1 → + // pending N") — the notice row already carries the depth once, in plain + // language, so this row's job is making the pending item identifiable. appendStreamRow(shell, { - role: "system", - text: `${tag} +1 → pending ${badgeCount(shell.session)}`, - meta: "queue", + role: "user", + text: userRowText(t, attachments), + meta: kind === "steer" ? "steer" : "queue", }) paintChrome(shell) } diff --git a/src/tui-opentui/turn-state.test.ts b/src/tui-opentui/turn-state.test.ts index 47b003f55..2cdc813aa 100644 --- a/src/tui-opentui/turn-state.test.ts +++ b/src/tui-opentui/turn-state.test.ts @@ -82,6 +82,30 @@ describe("turnStateFromEvent", () => { expect(s.isProcessing).toBe(false) }) + test("inference.done with no active tool calls settles the turn", () => { + // Regression for CL-5563/CL-5570: a workflow/goal-governor cycle that + // keeps self-continuing may never emit connector.reply, the usual + // terminator. Without settling here too, isProcessing (and the "working" + // ramp it drives) stays true forever once nothing else arrives. + const s = fold([ + { type: "inference.start" }, + { type: "inference.text.delta" }, + { type: "inference.done" }, + ]) + expect(s.status).toBe("done") + expect(s.isProcessing).toBe(false) + }) + + test("inference.done with a tool call still outstanding does not settle", () => { + const s = fold([ + { type: "inference.start" }, + { type: "inference.tool_call.end", data: { name: "bash" } }, + { type: "inference.done" }, + ]) + expect(s.isProcessing).toBe(true) + expect(s.status).toBe("running") + }) + test("activity clock advances with every event", () => { const s = fold([{ type: "inference.start" }, { type: "inference.text.delta" }]) expect(s.lastActivityAt).toBe(2) diff --git a/src/tui-opentui/turn-state.ts b/src/tui-opentui/turn-state.ts index abf96e142..2f681d23c 100644 --- a/src/tui-opentui/turn-state.ts +++ b/src/tui-opentui/turn-state.ts @@ -285,19 +285,36 @@ export function turnStateFromEvent( ), } + /** + * A cycle with no active tool calls left is also a turn's real + * terminator: `connector.reply` (below) is the usual signal, but a + * workflow/goal-governor cycle that keeps self-continuing may never + * emit one, and `reactor.done` fires once at shutdown, never between + * turns. Without settling here, the phase line stays hot ("working") + * forever once nothing more arrives. A cycle that just requested tools + * only ends here, not the turn — those calls are already reflected in + * `activeToolCalls` (streamed before `inference.done`). + */ case "inference.done": + if (state.activeToolCalls.length > 0) { + return { + ...state, + awaitingResponse: false, + streamingType: null, + lastActivityAt: nowMs, + } + } return { - ...state, - awaitingResponse: false, - streamingType: null, - lastActivityAt: nowMs, + ...initialTurnState(nowMs), + status: "done", + quota: state.quota, } /** - * The turn's real terminator. `agent.send()` resolves on connector.reply, - * and a chat session emits no `reactor.done` until it closes — so without - * this the phase line would stay hot for the rest of the session. A reply - * with tools still outstanding only ends the cycle, not the turn. + * The other turn terminator: `agent.send()` resolves on connector.reply, + * and for the ordinary case above `inference.done` already settled the + * turn a beat earlier, so this is a harmless idempotent re-settle. A + * reply with tools still outstanding only ends the cycle, not the turn. */ case "connector.reply": if (state.activeToolCalls.length > 0) {