Skip to content

Commit 906a207

Browse files
committed
Keep cancellation a flag, not a text rewrite, and prove it on screen
paintStreamRow now owns turning a cancelled user row into the "[cancelled]" prefix, reading a new cancelled flag on StreamRow. row.text stays exactly what the operator typed, so copy-mode and anything else reading it back are unaffected by the cancel. Regression tests now assert on captureCharFrame() before and after a cancel, not only on shell.streamLog, and cover a steer-kind cancel alongside queue in session-queue.test.ts, shell.test.ts and keybindings.test.ts.
1 parent fc8b452 commit 906a207

5 files changed

Lines changed: 112 additions & 20 deletions

File tree

‎src/tui-opentui/keybindings.test.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ const PROBES: Readonly<Record<string, { readonly group: Group; readonly probe: P
458458

459459
"Ctrl+G": {
460460
group: "session",
461-
probe: ({ h, shell, chords }) => {
461+
probe: async ({ h, shell, chords }) => {
462462
clearShellBridgeHooks(shell)
463463
setShellRunState(shell, "busy")
464464
shell.prompt.value = "keep"
@@ -477,6 +477,13 @@ const PROBES: Readonly<Record<string, { readonly group: Group; readonly probe: P
477477
// at this pulled).
478478
expect(rows).toEqual(["queue", "cancelled"])
479479

480+
// The chord's whole job is what lands on screen, not the model alone —
481+
// assert on the rendered frame, not just streamLog.
482+
await h.renderOnce()
483+
const frame = h.captureCharFrame()
484+
expect(frame).toContain("[cancelled] drop me")
485+
expect(frame).toContain("keep")
486+
480487
setShellRunState(shell, "idle")
481488
},
482489
},

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, test } from "bun:test"
22
import {
33
badgeCount,
4+
cancelLast,
45
clearInterruptFlash,
56
createSessionQueue,
67
drainOne,
@@ -63,6 +64,34 @@ describe("session-queue", () => {
6364
expect(s.interruptFlash).toBe(false)
6465
})
6566

67+
test("cancelLast retracts the newest queue item", () => {
68+
let s = createSessionQueue("busy")
69+
s = enqueue(s, "keep")
70+
s = enqueue(s, "drop")
71+
const { state, item } = cancelLast(s)
72+
expect(item?.text).toBe("drop")
73+
expect(badgeCount(state)).toBe(1)
74+
expect(state.items[0]!.text).toBe("keep")
75+
})
76+
77+
test("cancelLast retracts the newest steer item, same as queue", () => {
78+
let s = createSessionQueue("busy")
79+
s = enqueue(s, "queued")
80+
s = enqueueSteer(s, "steered")
81+
const { state, item } = cancelLast(s)
82+
expect(item?.kind).toBe("steer")
83+
expect(item?.text).toBe("steered")
84+
expect(badgeCount(state)).toBe(1)
85+
expect(state.items[0]!.kind).toBe("queue")
86+
})
87+
88+
test("cancelLast on an empty queue is a no-op", () => {
89+
const s = createSessionQueue("busy")
90+
const { state, item } = cancelLast(s)
91+
expect(item).toBeNull()
92+
expect(state).toBe(s)
93+
})
94+
6695
test("setRunState toggles busy/idle", () => {
6796
let s = createSessionQueue("idle")
6897
s = setRunState(s, "busy")

‎src/tui-opentui/shell.test.ts‎

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ describe("product skin: stream + queue + overlay", () => {
479479
)
480480
})
481481

482-
test("Ctrl+G cancels the last queued message and rewrites its row", async () => {
482+
test("Ctrl+G cancels the last queued message and the screen shows it", async () => {
483483
await withTestRenderer(
484484
async (h) => {
485485
const shell = createAppShell(h.renderer, {
@@ -494,24 +494,76 @@ describe("product skin: stream + queue + overlay", () => {
494494
submitPrompt(shell, "queue")
495495
expect(shell.pendingQueue).toBe(2)
496496

497-
const before = shell.streamLog.map((row) => ({ text: row.text, meta: row.meta }))
497+
const before = shell.streamLog.map((row) => ({
498+
text: row.text,
499+
meta: row.meta,
500+
cancelled: row.cancelled,
501+
}))
498502
expect(before).toEqual([
499-
{ text: "keep this one", meta: "queue" },
500-
{ text: "oops wrong message", meta: "queue" },
503+
{ text: "keep this one", meta: "queue", cancelled: undefined },
504+
{ text: "oops wrong message", meta: "queue", cancelled: undefined },
501505
])
506+
await h.renderOnce()
507+
const frameBefore = h.captureCharFrame()
508+
expect(frameBefore).toContain("keep this one")
509+
expect(frameBefore).toContain("oops wrong message")
510+
expect(frameBefore).not.toContain("[cancelled]")
502511

503512
applyShellCancelLast(shell)
504513

505514
expect(shell.pendingQueue).toBe(1)
506515
expect(shell.session.items[0]!.text).toBe("keep this one")
507516

508-
const after = shell.streamLog.map((row) => ({ text: row.text, meta: row.meta }))
509-
// The cancelled row is rewritten, not left claiming "queue" — the
510-
// first attempt's bug this test exists to catch.
517+
const after = shell.streamLog.map((row) => ({
518+
text: row.text,
519+
meta: row.meta,
520+
cancelled: row.cancelled,
521+
}))
522+
// The stored text is untouched — the cancel is a flag the paint
523+
// layer reads, not a rewrite of what the operator typed.
511524
expect(after).toEqual([
512-
{ text: "keep this one", meta: "queue" },
513-
{ text: "[cancelled] oops wrong message", meta: "cancelled" },
525+
{ text: "keep this one", meta: "queue", cancelled: undefined },
526+
{ text: "oops wrong message", meta: "cancelled", cancelled: true },
514527
])
528+
529+
// The screen, not just the model, is asserted on: this is exactly
530+
// what the first attempt at this issue got wrong (the row read
531+
// back unchanged from streamLog while the model looked cancelled).
532+
await h.renderOnce()
533+
const frameAfter = h.captureCharFrame()
534+
expect(frameAfter).toContain("[cancelled] oops wrong message")
535+
expect(frameAfter).toContain("keep this one")
536+
} finally {
537+
shell.dispose()
538+
}
539+
},
540+
{ width: 80, height: 24 },
541+
)
542+
})
543+
544+
test("Ctrl+G cancels a steered message the same way", async () => {
545+
await withTestRenderer(
546+
async (h) => {
547+
const shell = createAppShell(h.renderer, {
548+
terminal: { columns: 80, rows: 24 },
549+
wireKeys: false,
550+
run: "busy",
551+
})
552+
try {
553+
shell.prompt.value = "steer me now"
554+
submitPrompt(shell, "steer")
555+
expect(shell.pendingQueue).toBe(1)
556+
expect(shell.session.items[0]!.kind).toBe("steer")
557+
558+
applyShellCancelLast(shell)
559+
560+
expect(shell.pendingQueue).toBe(0)
561+
expect(shell.session.items).toHaveLength(0)
562+
expect(shell.streamLog[0]?.cancelled).toBe(true)
563+
expect(shell.streamLog[0]?.meta).toBe("cancelled")
564+
565+
await h.renderOnce()
566+
expect(h.captureCharFrame()).toContain("[cancelled] steer me now")
515567
} finally {
516568
shell.dispose()
517569
}

‎src/tui-opentui/shell.ts‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2924,15 +2924,11 @@ export function applyShellCancelLast(shell: AppShell): void {
29242924
if (index !== undefined) {
29252925
const row = streamRowAt(shell, index)
29262926
if (row !== undefined) {
2927-
// The bar-and-bubble paint for a user row shows only `text`, not
2928-
// `meta` (see `paintStreamRow`) — copy mode reads `text` too — so the
2929-
// word has to land in the body itself or the visible transcript still
2930-
// reads as a message that will dispatch.
2931-
replaceStreamRowAt(shell, index, {
2932-
...row,
2933-
text: `[cancelled] ${row.text}`,
2934-
meta: "cancelled",
2935-
})
2927+
// `cancelled` stays a flag, not a `text` rewrite — `paintStreamRow`
2928+
// owns turning it into the "[cancelled]" prefix, so `row.text` still
2929+
// holds what the operator actually typed for anything else that reads
2930+
// it (copy mode, a resumed transcript).
2931+
replaceStreamRowAt(shell, index, { ...row, meta: "cancelled", cancelled: true })
29362932
}
29372933
}
29382934
paintChrome(shell)

‎src/tui-opentui/stream.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,13 @@ export type StreamRow = {
8686
* guessing by position once other rows have interleaved.
8787
*/
8888
readonly queueItemId?: string
89+
/**
90+
* The queued/steered message this row echoed was cancelled before it
91+
* dispatched. Kept as a flag rather than baked into `text` so the stored
92+
* body stays what the operator actually typed — the paint layer alone
93+
* decides how a cancelled row reads.
94+
*/
95+
readonly cancelled?: boolean
8996
/**
9097
* Row standing for a run of repeated calls. Its subject stays the call the
9198
* run repeats (never a total across them, which would be a claim the
@@ -639,7 +646,8 @@ export function paintStreamRow(
639646
): PaintedStreamLine {
640647
const fg = rowFg(row)
641648
if (row.role === "user") {
642-
return { content: userBubbleLines(row.text, layout.width).join("\n"), fg }
649+
const text = row.cancelled === true ? `[cancelled] ${row.text}` : row.text
650+
return { content: userBubbleLines(text, layout.width).join("\n"), fg }
643651
}
644652
if (isThinkingRow(row)) {
645653
return {

0 commit comments

Comments
 (0)