Skip to content

Commit d28f432

Browse files
committed
Settle run state on inference.done too, and drop cancel from this PR
Review found that draining the queue on inference.done was not enough: run stayed busy until connector.reply, and a workflow/goal-governor cycle that keeps self-continuing may never emit one. Every future Enter then resolved to "queue" against a session nothing will ever drain again. Settle run the same way connector.reply already does -- once inference.done lands with no tool calls still outstanding -- using the turn state the bridge already tracks, so no new event type is needed. Also reverts the tool.boundary addition to inference.done entirely: settling (or draining, when tools are still outstanding) from runtime-bridge's own handle() covers both cases without overloading an event that elsewhere means "a tool call finished." Cancel-last (Ctrl+X) is pulled out of this PR -- it is a separate feature with its own defect (the transcript row it left behind never reflected the cancellation) and belongs in its own change, landed once that is fixed and covered.
1 parent fed83e5 commit d28f432

10 files changed

Lines changed: 80 additions & 102 deletions

src/tui-opentui/keybindings.test.ts

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,6 @@ const PROBES: Readonly<Record<string, { readonly group: Group; readonly probe: P
439439
onInterrupt: () => {
440440
interrupted++
441441
},
442-
onCancelLast: () => {},
443442
exclusive: true,
444443
})
445444
setShellExitHandler(shell, () => {
@@ -455,23 +454,6 @@ const PROBES: Readonly<Record<string, { readonly group: Group; readonly probe: P
455454
},
456455
},
457456

458-
"Ctrl+X": {
459-
group: "session",
460-
probe: ({ h, shell, chords }) => {
461-
let cancelled = 0
462-
setShellBridgeHooks(shell, {
463-
onSubmit: () => {},
464-
onInterrupt: () => {},
465-
onCancelLast: () => {
466-
cancelled++
467-
},
468-
exclusive: true,
469-
})
470-
press(h, chords[0])
471-
expect(cancelled).toBe(1)
472-
},
473-
},
474-
475457
"Ctrl+D": {
476458
group: "host",
477459
// Probed on a mounted host rather than a bare shell: the row claims a
@@ -554,7 +536,6 @@ function recordSubmits(
554536
setShellBridgeHooks(shell, {
555537
onSubmit: (text, kind) => sent.push({ text, kind }),
556538
onInterrupt: () => {},
557-
onCancelLast: () => {},
558539
exclusive: true,
559540
})
560541
return sent

src/tui-opentui/keybindings.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ export const SHELL_SHORTCUTS: readonly ShellShortcut[] = [
2323
{ keys: "Enter", description: "queue the message mid-run (badge); send straight through when idle" },
2424
{ keys: "Alt+Enter", description: "steer at the next tool boundary; does nothing unless a run is busy" },
2525
{ keys: "Ctrl+C", description: "interrupt the run, or clear the prompt when idle; press twice to exit" },
26-
{ keys: "Ctrl+X", description: "cancel the most recently queued message, if any" },
2726
{ keys: "Ctrl+O", description: "open the command palette; press again to close it" },
2827
{ keys: "Alt+C", description: "copy mode: pick a message, tool output, or diff; press again to close it" },
2928
{ keys: "Alt+M", description: "take the mouse for click-to-expand and drag-scroll; off by default so drag-select and copy work" },

src/tui-opentui/prompt-chrome.test.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ describe("bare exit / quit at the prompt", () => {
4545
setShellBridgeHooks(shell, {
4646
onSubmit: (text) => sent.push(text),
4747
onInterrupt: () => {},
48-
onCancelLast: () => {},
4948
exclusive: true,
5049
})
5150
setShellExitHandler(shell, () => {
@@ -67,7 +66,6 @@ describe("bare exit / quit at the prompt", () => {
6766
setShellBridgeHooks(shell, {
6867
onSubmit: (text) => sent.push(text),
6968
onInterrupt: () => {},
70-
onCancelLast: () => {},
7169
exclusive: true,
7270
})
7371
setShellExitHandler(shell, () => {
@@ -86,7 +84,6 @@ describe("bare exit / quit at the prompt", () => {
8684
setShellBridgeHooks(shell, {
8785
onSubmit: (text) => sent.push(text),
8886
onInterrupt: () => {},
89-
onCancelLast: () => {},
9087
exclusive: true,
9188
})
9289
shell.prompt.value = "exit"

src/tui-opentui/prompt-features.test.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,6 @@ describe("image attachments", () => {
146146
setShellBridgeHooks(shell, {
147147
onSubmit: (_text, _kind, attachments) => seen.push(attachments),
148148
onInterrupt: () => {},
149-
onCancelLast: () => {},
150149
exclusive: true,
151150
})
152151
setPromptImageSource(shell, async () => ({ ok: true, attachment: CLIP }))
@@ -165,7 +164,6 @@ describe("image attachments", () => {
165164
setShellBridgeHooks(shell, {
166165
onSubmit: (text) => texts.push(text),
167166
onInterrupt: () => {},
168-
onCancelLast: () => {},
169167
exclusive: true,
170168
})
171169
setPromptImageSource(shell, async () => ({ ok: true, attachment: CLIP }))
@@ -197,7 +195,6 @@ describe("text paste", () => {
197195
setShellBridgeHooks(shell, {
198196
onSubmit: (text) => submitted.push(text),
199197
onInterrupt: () => {},
200-
onCancelLast: () => {},
201198
exclusive: true,
202199
})
203200
setPromptImageSource(shell, async () => ({ ok: true, attachment: CLIP }))
@@ -286,7 +283,6 @@ describe("sent-message recall", () => {
286283
setShellBridgeHooks(shell, {
287284
onSubmit: () => {},
288285
onInterrupt: () => {},
289-
onCancelLast: () => {},
290286
exclusive: true,
291287
})
292288
shell.prompt.value = "remember me"

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,71 @@ describe("attachSessionBridge", () => {
255255
)
256256
})
257257

258+
test("run returns 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`. Without this, every future Enter resolves to "queue" (busy is
262+
// sticky) and, once the workflow stops producing cycles, that queued
263+
// message is never drained — CL-5563's bug moved one layer over.
264+
await withTestRenderer(
265+
async (h) => {
266+
const shell = createAppShell(h.renderer, {
267+
terminal: { columns: 80, rows: 24 },
268+
wireKeys: false,
269+
run: "busy",
270+
})
271+
const port = createRecordingPort()
272+
const bridge = attachSessionBridge(shell, port)
273+
try {
274+
bridge.handle({ type: "inference.start" })
275+
bridge.handle({
276+
type: "inference.text.delta",
277+
data: { token: "hi" },
278+
})
279+
bridge.handle({ type: "inference.done" })
280+
expect(shell.session.run).toBe("idle")
281+
282+
port.clear()
283+
bridge.submit("are you still there", "queue")
284+
expect(port.calls).toEqual([
285+
{ op: "sendImmediate", text: "are you still there" },
286+
])
287+
} finally {
288+
bridge.dispose()
289+
shell.dispose()
290+
}
291+
},
292+
{ width: 80, height: 24 },
293+
)
294+
})
295+
296+
test("run stays busy after inference.done while a tool call is still outstanding", async () => {
297+
await withTestRenderer(
298+
async (h) => {
299+
const shell = createAppShell(h.renderer, {
300+
terminal: { columns: 80, rows: 24 },
301+
wireKeys: false,
302+
run: "busy",
303+
})
304+
const port = createRecordingPort()
305+
const bridge = attachSessionBridge(shell, port)
306+
try {
307+
bridge.handle({ type: "inference.start" })
308+
bridge.handle({
309+
type: "inference.tool_call.start",
310+
data: { call: { id: "c1", name: "bash" } },
311+
})
312+
bridge.handle({ type: "inference.done" })
313+
expect(shell.session.run).toBe("busy")
314+
} finally {
315+
bridge.dispose()
316+
shell.dispose()
317+
}
318+
},
319+
{ width: 80, height: 24 },
320+
)
321+
})
322+
258323
test("token-by-token deltas grow one assistant row", async () => {
259324
await withTestRenderer(
260325
async (h) => {

src/tui-opentui/runtime-bridge.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import {
1717
} from "./session-queue.js"
1818
import {
1919
appendStreamRow,
20-
applyShellCancelLast,
2120
applyShellInterrupt,
2221
clearShellBridgeHooks,
2322
paintChrome,
@@ -765,6 +764,19 @@ export function attachSessionBridge(
765764
)) {
766765
applyInbound(shell, bag, mapped)
767766
}
767+
// inference.done is the only turn boundary every reactor cycle
768+
// guarantees — reactor.done fires once, at shutdown, and a
769+
// workflow/goal-governor cycle that keeps self-continuing may never
770+
// emit the connector.reply `settled` depends on. Settling here too
771+
// (once no tool calls are still outstanding, the same criterion
772+
// connector.reply itself uses) means a queued message — or the next
773+
// Enter — is never left waiting on an event that might not come
774+
// again. When tools are still outstanding the turn continues, so only
775+
// the drain runs; run state is untouched.
776+
if (event.type === "inference.done") {
777+
if (bag.turn.activeToolCalls.length === 0) settleRun()
778+
else drainAtBoundary(shell, bag)
779+
}
768780
if (settled) settleRun()
769781
return
770782
}
@@ -825,11 +837,6 @@ export function attachSessionBridge(
825837
paintPhase()
826838
}
827839

828-
const doCancelLast = (): void => {
829-
if (bag.disposed) return
830-
applyShellCancelLast(shell)
831-
}
832-
833840
const tick = (): void => {
834841
if (bag.disposed) return
835842
const nowMs = now()
@@ -893,7 +900,6 @@ export function attachSessionBridge(
893900
onInterrupt: () => {
894901
doInterrupt()
895902
},
896-
onCancelLast: doCancelLast,
897903
exclusive: true,
898904
})
899905

src/tui-opentui/session-queue.test.ts

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { describe, expect, test } from "bun:test"
22
import {
33
badgeCount,
4-
cancelLast,
54
clearInterruptFlash,
65
createSessionQueue,
76
drainOne,
@@ -52,25 +51,6 @@ describe("session-queue", () => {
5251
expect(d3.item?.text).toBe("q1")
5352
})
5453

55-
test("cancelLast retracts the most recently queued item, not necessarily the drain-order head", () => {
56-
let s = createSessionQueue("busy")
57-
s = enqueue(s, "q1")
58-
s = enqueueSteer(s, "s1")
59-
const c1 = cancelLast(s)
60-
expect(c1.item?.text).toBe("s1")
61-
expect(badgeCount(c1.state)).toBe(1)
62-
const c2 = cancelLast(c1.state)
63-
expect(c2.item?.text).toBe("q1")
64-
expect(badgeCount(c2.state)).toBe(0)
65-
})
66-
67-
test("cancelLast on an empty queue is a no-op", () => {
68-
const s = createSessionQueue("busy")
69-
const c = cancelLast(s)
70-
expect(c.item).toBeNull()
71-
expect(c.state).toBe(s)
72-
})
73-
7454
test("Ctrl+C interrupt clears pending + sets flash + idle", () => {
7555
let s = createSessionQueue("busy")
7656
s = enqueue(s, "a")

src/tui-opentui/session-queue.ts

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -121,21 +121,6 @@ export function drainOrder(
121121
return [...steers, ...queues]
122122
}
123123

124-
/**
125-
* Cancel the most recently queued item (undo-last), so an operator who
126-
* queued the wrong message can retract it before it dispatches.
127-
*/
128-
export function cancelLast(
129-
state: SessionQueueState,
130-
): { state: SessionQueueState; item: QueueItem | null } {
131-
const item = state.items[state.items.length - 1] ?? null
132-
if (!item) return { state, item: null }
133-
return {
134-
state: { ...state, items: state.items.slice(0, -1) },
135-
item,
136-
}
137-
}
138-
139124
/** Pop next delivery item (steer-first). */
140125
export function drainOne(
141126
state: SessionQueueState,

src/tui-opentui/shell.ts

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ import {
153153
} from "./copy-path.js"
154154
import {
155155
badgeCount,
156-
cancelLast,
157156
clearInterruptFlash,
158157
createSessionQueue,
159158
enqueue,
@@ -228,8 +227,6 @@ export type ShellBridgeHooks = {
228227
attachments?: readonly PendingImageAttachment[],
229228
) => void
230229
onInterrupt: () => void
231-
/** Cancel the most recently queued item; a no-op if the queue is empty. */
232-
onCancelLast: () => void
233230
exclusive: boolean
234231
}
235232

@@ -2661,25 +2658,6 @@ export function interruptShell(shell: AppShell): void {
26612658
applyShellInterrupt(shell)
26622659
}
26632660

2664-
/** Local cancel-last mutation (no bridge re-entry). */
2665-
export function applyShellCancelLast(shell: AppShell): void {
2666-
const { state, item } = cancelLast(shell.session)
2667-
if (!item) return
2668-
shell.session = state
2669-
setStatusFlash(shell, `cancelled queued message: ${item.text || "(no text)"}`)
2670-
paintChrome(shell)
2671-
}
2672-
2673-
/** Ctrl+X: cancel the most recently queued message, if any. */
2674-
export function cancelLastQueued(shell: AppShell): void {
2675-
const hooks = getShellBridgeHooks(shell)
2676-
if (hooks?.exclusive) {
2677-
hooks.onCancelLast()
2678-
return
2679-
}
2680-
applyShellCancelLast(shell)
2681-
}
2682-
26832661
export function clearShellInterruptFlash(shell: AppShell): void {
26842662
shell.session = clearInterruptFlash(shell.session)
26852663
paintChrome(shell)
@@ -4917,12 +4895,6 @@ export function createAppShell(
49174895
return
49184896
}
49194897

4920-
if (key.ctrl && key.name === "x") {
4921-
key.preventDefault()
4922-
cancelLastQueued(shell)
4923-
return
4924-
}
4925-
49264898
if (
49274899
(key.name === "return" || key.name === "enter") &&
49284900
(key.meta || key.option) &&

src/tui-opentui/stream-event-map.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -310,11 +310,8 @@ function mapEvent(
310310

311311
case "inference.done":
312312
// Cycle settled: disarm so a pre-commit retry belonging to the *next*
313-
// cycle cannot retract this one's rows. inference.done is the only
314-
// turn boundary the reactor guarantees per cycle (reactor.done fires
315-
// once, at agent shutdown, never between turns) — drain the mid-run
316-
// queue here so a text-only reply doesn't strand queued messages.
317-
return [...disarmAttempt(ctx), { type: "tool.boundary" }]
313+
// cycle cannot retract this one's rows.
314+
return disarmAttempt(ctx)
318315

319316
case "inference.retry": {
320317
const armed = ctx?.attemptArmed === true

0 commit comments

Comments
 (0)