Skip to content

Commit e2ae79b

Browse files
committed
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.
1 parent b2b6848 commit e2ae79b

5 files changed

Lines changed: 194 additions & 30 deletions

File tree

src/tui-opentui/runtime-bridge.test.ts

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,116 @@ describe("attachSessionBridge", () => {
214214
)
215215
})
216216

217+
test("queued item delivers on a tool-less turn (inference.done, no tool calls)", async () => {
218+
// Regression for CL-5563: reactor.done only fires once, at agent
219+
// shutdown, never between turns — a plain-text reply with no tool calls
220+
// must still drain the queue, or a queued message sits forever.
221+
await withTestRenderer(
222+
async (h) => {
223+
const shell = createAppShell(h.renderer, {
224+
terminal: { columns: 80, rows: 24 },
225+
wireKeys: false,
226+
run: "busy",
227+
})
228+
const port = createRecordingPort()
229+
const bridge = attachSessionBridge(shell, port)
230+
try {
231+
bridge.submit("follow up", "queue")
232+
expect(badgeCount(shell.session)).toBe(1)
233+
port.clear()
234+
bridge.handle({ type: "inference.start" })
235+
bridge.handle({
236+
type: "inference.text.delta",
237+
data: { token: "hi" },
238+
})
239+
bridge.handle({ type: "inference.done" })
240+
expect(badgeCount(shell.session)).toBe(0)
241+
const deliver = port.calls.find((c) => c.op === "deliver")
242+
expect(deliver).toEqual({
243+
op: "deliver",
244+
item: expect.objectContaining({
245+
text: "follow up",
246+
kind: "queue",
247+
}),
248+
})
249+
} finally {
250+
bridge.dispose()
251+
shell.dispose()
252+
}
253+
},
254+
{ width: 80, height: 24 },
255+
)
256+
})
257+
258+
test("run and the phase ramp both return to idle after a tool-less inference.done, with no connector.reply", async () => {
259+
// Regression: a goal-governor / workflow cycle that keeps self-continuing
260+
// may never emit connector.reply, the only other event that clears
261+
// `run` and the turn's `isProcessing`. Without this, every future Enter
262+
// resolves to "queue" (busy is sticky) and, once the workflow stops
263+
// producing cycles, that queued message is never drained — CL-5563's
264+
// bug moved one layer over. The ramp indicator has the same failure
265+
// mode: it reads `isProcessing`, not `run`, so it can say "working"
266+
// forever even once dispatch itself is fixed.
267+
await withTestRenderer(
268+
async (h) => {
269+
const shell = createAppShell(h.renderer, {
270+
terminal: { columns: 80, rows: 24 },
271+
wireKeys: false,
272+
run: "busy",
273+
})
274+
const port = createRecordingPort()
275+
const bridge = attachSessionBridge(shell, port)
276+
try {
277+
bridge.handle({ type: "inference.start" })
278+
bridge.handle({
279+
type: "inference.text.delta",
280+
data: { token: "hi" },
281+
})
282+
bridge.handle({ type: "inference.done" })
283+
expect(shell.session.run).toBe("idle")
284+
expect(shell.turnPhase).toBeNull()
285+
286+
port.clear()
287+
bridge.submit("are you still there", "queue")
288+
expect(port.calls).toEqual([
289+
{ op: "sendImmediate", text: "are you still there" },
290+
])
291+
} finally {
292+
bridge.dispose()
293+
shell.dispose()
294+
}
295+
},
296+
{ width: 80, height: 24 },
297+
)
298+
})
299+
300+
test("run stays busy after inference.done while a tool call is still outstanding", async () => {
301+
await withTestRenderer(
302+
async (h) => {
303+
const shell = createAppShell(h.renderer, {
304+
terminal: { columns: 80, rows: 24 },
305+
wireKeys: false,
306+
run: "busy",
307+
})
308+
const port = createRecordingPort()
309+
const bridge = attachSessionBridge(shell, port)
310+
try {
311+
bridge.handle({ type: "inference.start" })
312+
bridge.handle({
313+
type: "inference.tool_call.start",
314+
data: { call: { id: "c1", name: "bash" } },
315+
})
316+
bridge.handle({ type: "inference.done" })
317+
expect(shell.session.run).toBe("busy")
318+
} finally {
319+
bridge.dispose()
320+
shell.dispose()
321+
}
322+
},
323+
{ width: 80, height: 24 },
324+
)
325+
})
326+
217327
test("token-by-token deltas grow one assistant row", async () => {
218328
await withTestRenderer(
219329
async (h) => {

src/tui-opentui/runtime-bridge.ts

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
streamRowAt,
3030
streamRowCount,
3131
truncateStreamRows,
32+
userRowText,
3233
type AppShell,
3334
} from "./shell.js"
3435
import { rampFor, rampLine } from "./ramp.js"
@@ -55,10 +56,7 @@ import {
5556
turnStateOnSubmit,
5657
type TurnState,
5758
} from "./turn-state.js"
58-
import {
59-
formatAttachmentSummary,
60-
type PendingImageAttachment,
61-
} from "../tui/image-attachments.js"
59+
import type { PendingImageAttachment } from "../tui/image-attachments.js"
6260
import { toolCallRow } from "./diff.js"
6361
import { toolResultRow } from "./mcp-view.js"
6462
import {
@@ -68,16 +66,6 @@ import {
6866
} from "./tool-rows.js"
6967
import type { StreamRow } from "./stream.js"
7068
import { advanceRevealChars, flattenReasoningText, type Thought } from "./thinking.js"
71-
72-
/** Transcript echo for a user message, annotated with its attachments. */
73-
function userRowText(
74-
text: string,
75-
attachments: readonly PendingImageAttachment[],
76-
): string {
77-
const summary = formatAttachmentSummary(attachments)
78-
if (summary.length === 0) return text
79-
return text.length === 0 ? `[${summary}]` : `${text}\n[${summary}]`
80-
}
8169
import {
8270
PRODUCTION_REACTOR_TYPES,
8371
createStreamMapContext,
@@ -764,6 +752,13 @@ export function attachSessionBridge(
764752
)) {
765753
applyInbound(shell, bag, mapped)
766754
}
755+
// inference.done with tool calls still outstanding doesn't settle the
756+
// turn (see turn-state.ts) — the cycle continues, but a boundary still
757+
// passed, so a queued message waiting on it should not wait for the
758+
// turn's eventual end too.
759+
if (event.type === "inference.done" && bag.turn.activeToolCalls.length > 0) {
760+
drainAtBoundary(shell, bag)
761+
}
767762
if (settled) settleRun()
768763
return
769764
}
@@ -800,10 +795,13 @@ export function attachSessionBridge(
800795
? enqueueSteer(shell.session, t, undefined, attachments)
801796
: enqueue(shell.session, t, "queue", undefined, attachments)
802797
bag.port.enqueue(t, kind)
798+
// Show the message itself, not the internal transition ("queue +1 →
799+
// pending N") — the notice row already carries the depth once, in plain
800+
// language, so this row's job is making the pending item identifiable.
803801
appendStreamRow(shell, {
804-
role: "system",
805-
text: `${kind} +1 → pending ${badgeCount(shell.session)}`,
806-
meta: "queue",
802+
role: "user",
803+
text: userRowText(t, attached),
804+
meta: kind === "steer" ? "steer" : "queue",
807805
})
808806
paintChrome(shell)
809807
}

src/tui-opentui/shell.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { stringWidth } from "../tui/view/height.js"
3232
import { listPathSuggestions } from "../tui/components/at-mention/list.js"
3333
import { parseAtState } from "../tui/components/at-mention/parse.js"
3434
import {
35+
formatAttachmentSummary,
3536
readClipboardImage,
3637
type ClipboardImageResult,
3738
type PendingImageAttachment,
@@ -2761,6 +2762,16 @@ export function setShellRunState(shell: AppShell, run: RunState): void {
27612762
paintChrome(shell)
27622763
}
27632764

2765+
/** Transcript echo for a user message, annotated with its attachments. */
2766+
export function userRowText(
2767+
text: string,
2768+
attachments: readonly PendingImageAttachment[],
2769+
): string {
2770+
const summary = formatAttachmentSummary(attachments)
2771+
if (summary.length === 0) return text
2772+
return text.length === 0 ? `[${summary}]` : `${text}\n[${summary}]`
2773+
}
2774+
27642775
/** Submit prompt as queue (busy) or immediate user send (idle). */
27652776
export function submitPrompt(
27662777
shell: AppShell,
@@ -2801,14 +2812,18 @@ export function submitPrompt(
28012812
}
28022813

28032814
shell.session =
2804-
kind === "steer" ? enqueueSteer(shell.session, t) : enqueue(shell.session, t)
2815+
kind === "steer"
2816+
? enqueueSteer(shell.session, t, undefined, attachments)
2817+
: enqueue(shell.session, t, "queue", undefined, attachments)
28052818
shell.prompt.value = ""
28062819
clearPendingAttachments(shell)
2807-
const tag = kind === "steer" ? "steer" : "queue"
2820+
// Show the message itself, not the internal transition ("queue +1 →
2821+
// pending N") — the notice row already carries the depth once, in plain
2822+
// language, so this row's job is making the pending item identifiable.
28082823
appendStreamRow(shell, {
2809-
role: "system",
2810-
text: `${tag} +1 → pending ${badgeCount(shell.session)}`,
2811-
meta: "queue",
2824+
role: "user",
2825+
text: userRowText(t, attachments),
2826+
meta: kind === "steer" ? "steer" : "queue",
28122827
})
28132828
paintChrome(shell)
28142829
}

src/tui-opentui/turn-state.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,30 @@ describe("turnStateFromEvent", () => {
8282
expect(s.isProcessing).toBe(false)
8383
})
8484

85+
test("inference.done with no active tool calls settles the turn", () => {
86+
// Regression for CL-5563/CL-5570: a workflow/goal-governor cycle that
87+
// keeps self-continuing may never emit connector.reply, the usual
88+
// terminator. Without settling here too, isProcessing (and the "working"
89+
// ramp it drives) stays true forever once nothing else arrives.
90+
const s = fold([
91+
{ type: "inference.start" },
92+
{ type: "inference.text.delta" },
93+
{ type: "inference.done" },
94+
])
95+
expect(s.status).toBe("done")
96+
expect(s.isProcessing).toBe(false)
97+
})
98+
99+
test("inference.done with a tool call still outstanding does not settle", () => {
100+
const s = fold([
101+
{ type: "inference.start" },
102+
{ type: "inference.tool_call.end", data: { name: "bash" } },
103+
{ type: "inference.done" },
104+
])
105+
expect(s.isProcessing).toBe(true)
106+
expect(s.status).toBe("running")
107+
})
108+
85109
test("activity clock advances with every event", () => {
86110
const s = fold([{ type: "inference.start" }, { type: "inference.text.delta" }])
87111
expect(s.lastActivityAt).toBe(2)

src/tui-opentui/turn-state.ts

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -285,19 +285,36 @@ export function turnStateFromEvent(
285285
),
286286
}
287287

288+
/**
289+
* A cycle with no active tool calls left is also a turn's real
290+
* terminator: `connector.reply` (below) is the usual signal, but a
291+
* workflow/goal-governor cycle that keeps self-continuing may never
292+
* emit one, and `reactor.done` fires once at shutdown, never between
293+
* turns. Without settling here, the phase line stays hot ("working")
294+
* forever once nothing more arrives. A cycle that just requested tools
295+
* only ends here, not the turn — those calls are already reflected in
296+
* `activeToolCalls` (streamed before `inference.done`).
297+
*/
288298
case "inference.done":
299+
if (state.activeToolCalls.length > 0) {
300+
return {
301+
...state,
302+
awaitingResponse: false,
303+
streamingType: null,
304+
lastActivityAt: nowMs,
305+
}
306+
}
289307
return {
290-
...state,
291-
awaitingResponse: false,
292-
streamingType: null,
293-
lastActivityAt: nowMs,
308+
...initialTurnState(nowMs),
309+
status: "done",
310+
quota: state.quota,
294311
}
295312

296313
/**
297-
* The turn's real terminator. `agent.send()` resolves on connector.reply,
298-
* and a chat session emits no `reactor.done` until it closes — so without
299-
* this the phase line would stay hot for the rest of the session. A reply
300-
* with tools still outstanding only ends the cycle, not the turn.
314+
* The other turn terminator: `agent.send()` resolves on connector.reply,
315+
* and for the ordinary case above `inference.done` already settled the
316+
* turn a beat earlier, so this is a harmless idempotent re-settle. A
317+
* reply with tools still outstanding only ends the cycle, not the turn.
301318
*/
302319
case "connector.reply":
303320
if (state.activeToolCalls.length > 0) {

0 commit comments

Comments
 (0)