Skip to content

Commit 8e4be34

Browse files
committed
Never auto-abort a healthy wait for the model's next token
awaitingResponse flips true with no signal to tell "still coming" from "never coming" apart, both right after submit and the instant a tool batch resolves. Auto-abort now only fires on a stream that had already started producing tokens and then went dead mid-flight; the awaiting case still surfaces via the notice instead of killing the turn. Adds regression coverage for a healthy post-tool-batch wait and for live sub-agent progress under an outstanding task call, both of which must never auto-abort.
1 parent 2b523c2 commit 8e4be34

4 files changed

Lines changed: 226 additions & 54 deletions

File tree

docs/TUI.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,14 @@ threshold, so a resumed session with already-stale activity shows the settled
133133
glyph immediately rather than alarming about silence the operator missed, and
134134
a stall that breaks and re-arms bursts again.
135135

136+
Auto-abort (`shouldAbortForStall`) is reserved for a stream that had already
137+
started producing tokens and then went dead mid-flight — not for a run that
138+
is merely *awaiting* the model's next response (right after submit, or the
139+
instant a tool batch resolves and `awaitingResponse` flips back to true).
140+
That wait has no signal to tell "still coming" from "never coming" apart, so
141+
it is never auto-aborted no matter how long it runs; it still surfaces via
142+
the notice, keeping the operator in control of whether to give up on it.
143+
136144
An idle session animates nothing at all: the monitor tick stops entirely
137145
rather than repainting an unchanging frame.
138146

src/tui/stall-watchdog.test.ts

Lines changed: 113 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -14,28 +14,21 @@ import {
1414
} from "./stall-watchdog.js"
1515

1616
describe("shouldAbortForStall", () => {
17+
// Mid-stream hang: tokens already flowed, then everything went silent —
18+
// the one shape auto-abort is willing to act on. The generic guards
19+
// (status, threshold, exemptions) are exercised against this base.
1720
const base = {
1821
status: "running" as const,
19-
awaitingResponse: true,
22+
awaitingResponse: false,
2023
lastActivityAt: 0,
2124
nowMs: STALL_TIMEOUT_MS,
2225
stallTimeoutMs: STALL_TIMEOUT_MS,
2326
isProcessing: true,
24-
streamingType: null,
27+
streamingType: "text" as const,
2528
activeToolCalls: [],
2629
}
2730

28-
test("a parallel fan-out with sibling tools still running is not a stall", () => {
29-
const args = {
30-
...base,
31-
activeToolCalls: ["call-2"],
32-
lastActivityAt: 0,
33-
nowMs: 20 * 60_000,
34-
}
35-
expect(shouldAbortForStall(args)).toBe(false)
36-
})
37-
38-
test("aborts an awaiting run past the timeout", () => {
31+
test("aborts a mid-stream hang past the timeout", () => {
3932
expect(shouldAbortForStall(base)).toBe(true)
4033
})
4134

@@ -52,60 +45,89 @@ describe("shouldAbortForStall", () => {
5245
expect(shouldAbortForStall({ ...base, status: "stopping" })).toBe(false)
5346
})
5447

55-
// Two independent exemptions (a gate open on the operator, a sibling tool
56-
// call still outstanding) must both keep exempting when combined — neither
57-
// one's guard may accidentally require the other's condition to also hold.
58-
test("a gate open and a sibling tool call each exempt alone, and together", () => {
59-
const gateOnly = { ...base, status: "blocked" as const }
60-
const toolCallOnly = { ...base, activeToolCalls: ["call-2"] }
61-
const both = { ...base, status: "blocked" as const, activeToolCalls: ["call-2"] }
62-
63-
expect(shouldAbortForStall(gateOnly)).toBe(false)
64-
expect(shouldAbortForStall(toolCallOnly)).toBe(false)
65-
expect(shouldAbortForStall(both)).toBe(false)
66-
})
67-
6848
test("a settled turn with nothing in flight is not a stall", () => {
6949
expect(
7050
shouldAbortForStall({
7151
...base,
72-
awaitingResponse: false,
52+
streamingType: null,
7353
isProcessing: false,
7454
}),
7555
).toBe(false)
7656
})
7757

7858
test("mid-thinking silence fires, recent thinking tokens do not", () => {
79-
const thinking = {
80-
...base,
81-
awaitingResponse: false,
82-
streamingType: "thinking" as const,
83-
}
59+
const thinking = { ...base, streamingType: "thinking" as const }
8460
expect(shouldAbortForStall(thinking)).toBe(true)
8561
expect(
8662
shouldAbortForStall({ ...thinking, lastActivityAt: STALL_TIMEOUT_MS - 1 }),
8763
).toBe(false)
8864
})
8965

9066
test("mid-stream text hang aborts", () => {
91-
expect(
92-
shouldAbortForStall({
93-
...base,
94-
awaitingResponse: false,
95-
streamingType: "text",
96-
}),
97-
).toBe(true)
67+
expect(shouldAbortForStall(base)).toBe(true)
9868
})
9969

10070
test("long tool runs are not stalls", () => {
71+
expect(shouldAbortForStall({ ...base, streamingType: "tool" })).toBe(false)
72+
})
73+
})
74+
75+
// The other shape silence can take: awaiting the model's next response, with
76+
// no tokens yet — set right after submit and again the instant the last
77+
// outstanding tool call resolves (`turnStateOnSubmit`, the `tool.done`
78+
// handler). A slow model produces exactly this state for as long as it takes
79+
// to reply, so it is never auto-aborted, however long the silence — only the
80+
// notice may surface. This is the regression coverage for CL-5640.
81+
describe("shouldAbortForStall — awaiting the model's next token is never auto-aborted", () => {
82+
const awaiting = {
83+
status: "running" as const,
84+
awaitingResponse: true,
85+
lastActivityAt: 0,
86+
nowMs: STALL_TIMEOUT_MS,
87+
stallTimeoutMs: STALL_TIMEOUT_MS,
88+
isProcessing: true,
89+
streamingType: null,
90+
activeToolCalls: [],
91+
}
92+
93+
test("does not abort a run merely awaiting a response, however long", () => {
94+
expect(shouldAbortForStall(awaiting)).toBe(false)
10195
expect(
102-
shouldAbortForStall({
103-
...base,
104-
awaitingResponse: false,
105-
streamingType: "tool",
106-
}),
96+
shouldAbortForStall({ ...awaiting, nowMs: STALL_TIMEOUT_MS * 10 }),
97+
).toBe(false)
98+
})
99+
100+
// Mirrors the tool.done handler: the last outstanding call just resolved,
101+
// awaitingResponse flips true and streamingType resets to null, then the
102+
// model itself takes a long-but-healthy while to start its next reply.
103+
test("healthy post-tool-batch wait never auto-aborts", () => {
104+
expect(shouldAbortForStall({ ...awaiting, activeToolCalls: [] })).toBe(
105+
false,
106+
)
107+
})
108+
109+
test("a parallel fan-out with sibling tools still running is not a stall", () => {
110+
expect(
111+
shouldAbortForStall({ ...awaiting, activeToolCalls: ["call-2"] }),
107112
).toBe(false)
108113
})
114+
115+
// Two independent exemptions (a gate open on the operator, a sibling tool
116+
// call still outstanding) must both keep exempting when combined — neither
117+
// one's guard may accidentally require the other's condition to also hold.
118+
test("a gate open and a sibling tool call each exempt alone, and together", () => {
119+
const gateOnly = { ...awaiting, status: "blocked" as const }
120+
const toolCallOnly = { ...awaiting, activeToolCalls: ["call-2"] }
121+
const both = {
122+
...awaiting,
123+
status: "blocked" as const,
124+
activeToolCalls: ["call-2"],
125+
}
126+
127+
expect(shouldAbortForStall(gateOnly)).toBe(false)
128+
expect(shouldAbortForStall(toolCallOnly)).toBe(false)
129+
expect(shouldAbortForStall(both)).toBe(false)
130+
})
109131
})
110132

111133
describe("applyStallRecovery", () => {
@@ -232,8 +254,26 @@ describe("shouldNoticeStall", () => {
232254
expect(shouldNoticeStall({ ...base, nowMs: STALL_NOTICE_MS - 1 })).toBe(false)
233255
})
234256

235-
test("hands over to the abort once the run is aborted", () => {
236-
expect(shouldNoticeStall({ ...base, nowMs: STALL_TIMEOUT_MS })).toBe(false)
257+
test("hands over to the abort once a mid-stream hang is aborted", () => {
258+
const midStream = {
259+
...base,
260+
awaitingResponse: false,
261+
streamingType: "text" as const,
262+
}
263+
expect(shouldNoticeStall({ ...midStream, nowMs: STALL_TIMEOUT_MS })).toBe(
264+
false,
265+
)
266+
})
267+
268+
test("a healthy wait for the model's next token keeps noticing rather than handing over to an abort", () => {
269+
// Unlike the mid-stream case above, this shape never reaches "abort" —
270+
// see the shouldAbortForStall describe block above — so the notice keeps
271+
// surfacing indefinitely instead of going silent once the old timeout
272+
// would have fired.
273+
expect(shouldNoticeStall({ ...base, nowMs: STALL_TIMEOUT_MS })).toBe(true)
274+
expect(
275+
shouldNoticeStall({ ...base, nowMs: STALL_TIMEOUT_MS * 10 }),
276+
).toBe(true)
237277
})
238278

239279
test("a long tool run is not stuck", () => {
@@ -261,21 +301,41 @@ describe("the stall level the indicator reads", () => {
261301
repeating: false,
262302
}
263303

264-
test("quiet, notice and abort partition the same silence clock", () => {
265-
expect(stallLevel({ ...base, nowMs: STALL_NOTICE_MS - 1 })).toBe("quiet")
304+
test("quiet, notice and abort partition the same silence clock for a mid-stream hang", () => {
305+
const midStream = { ...base, awaitingResponse: false, streamingType: "text" as const }
306+
expect(stallLevel({ ...midStream, nowMs: STALL_NOTICE_MS - 1 })).toBe(
307+
"quiet",
308+
)
309+
expect(stallLevel({ ...midStream, nowMs: STALL_NOTICE_MS })).toBe("notice")
310+
expect(stallLevel({ ...midStream, nowMs: STALL_TIMEOUT_MS })).toBe("abort")
311+
})
312+
313+
// Awaiting the model's next token (right after submit, or right after a
314+
// tool batch resolves) never escalates to "abort" — see
315+
// shouldAbortForStall's dedicated describe block — so this shape stays at
316+
// "notice" indefinitely instead of handing over.
317+
test("a healthy wait for the model's next token stays at notice, never abort", () => {
266318
expect(stallLevel({ ...base, nowMs: STALL_NOTICE_MS })).toBe("notice")
267-
expect(stallLevel({ ...base, nowMs: STALL_TIMEOUT_MS })).toBe("abort")
319+
expect(stallLevel({ ...base, nowMs: STALL_TIMEOUT_MS })).toBe("notice")
320+
expect(stallLevel({ ...base, nowMs: STALL_TIMEOUT_MS * 10 })).toBe(
321+
"notice",
322+
)
268323
})
269324

270325
test("the indicator keeps reading stalled across the abort threshold", () => {
271326
// The notice hands over to the abort so the two never speak at once, but
272327
// the phase must not flip back to healthy at the exact moment the run is
273328
// most stuck — that was the whole complaint the indicator answers.
274-
expect(shouldNoticeStall({ ...base, nowMs: STALL_TIMEOUT_MS })).toBe(false)
275-
expect(isStalledForDisplay({ ...base, nowMs: STALL_TIMEOUT_MS })).toBe(true)
276-
expect(isStalledForDisplay({ ...base, nowMs: STALL_TIMEOUT_MS * 3 })).toBe(
277-
true,
329+
const midStream = { ...base, awaitingResponse: false, streamingType: "text" as const }
330+
expect(shouldNoticeStall({ ...midStream, nowMs: STALL_TIMEOUT_MS })).toBe(
331+
false,
278332
)
333+
expect(
334+
isStalledForDisplay({ ...midStream, nowMs: STALL_TIMEOUT_MS }),
335+
).toBe(true)
336+
expect(
337+
isStalledForDisplay({ ...midStream, nowMs: STALL_TIMEOUT_MS * 3 }),
338+
).toBe(true)
279339
})
280340

281341
test("a repeating run is not a stall on any surface", () => {

src/tui/stall-watchdog.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,33 @@ function silentPastThreshold(
104104
)
105105
}
106106

107+
/**
108+
* Whether the run has gone silent while merely *awaiting* the model's next
109+
* response — right after submit or the instant a tool batch resolves, before
110+
* any token of the reply has arrived. `turnStateOnSubmit` and the `tool.done`
111+
* handler both reset `streamingType` to null exactly when they flip
112+
* `awaitingResponse` true, so this state can persist for as long as the model
113+
* takes to start replying: a slow model or a long thinking pass, not
114+
* necessarily a dead one. There is no signal available here to tell "still
115+
* coming" from "never coming" apart, so this case is deliberately excluded
116+
* from auto-abort and left to the notice instead — see `shouldAbortForStall`.
117+
*/
118+
function awaitingFirstToken(args: ShouldAbortForStallArgs): boolean {
119+
return args.awaitingResponse && args.streamingType === null
120+
}
121+
107122
// Pure decision helper: returns true when the run is genuinely stuck and should
108123
// be aborted. Extracted so the timeout logic is unit-testable without timers.
124+
//
125+
// Auto-abort is reserved for a stream that had already started producing
126+
// tokens and then went dead mid-flight — the one case silence cannot be
127+
// explained by "still waiting on the model." A long-but-healthy wait for the
128+
// model to start (right after submit, or right after a tool batch resolves)
129+
// is exempted here even past the timeout: it still surfaces via the notice
130+
// (`stallLevel` / `shouldNoticeStall`), but the operator stays in control of
131+
// whether to give up on it rather than having the turn discarded for them.
109132
export function shouldAbortForStall(args: ShouldAbortForStallArgs): boolean {
133+
if (awaitingFirstToken(args)) return false
110134
return silentPastThreshold(args, args.stallTimeoutMs)
111135
}
112136

src/tui/turn-monitor.test.ts

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,14 @@ describe("stall watchdog", () => {
326326
})
327327
})
328328

329-
test("aborts and flashes after the stall timeout", async () => {
329+
test("aborts and flashes once a mid-stream hang crosses the stall timeout", async () => {
330330
await withTestRenderer(async (h) => {
331331
const t: Harness = await setup(h)
332332
try {
333333
t.bridge.submit("build it", "immediate")
334+
// Tokens actually started flowing, then everything went silent —
335+
// the one shape auto-abort still acts on.
336+
t.bridge.handle({ type: "inference.text.delta", data: { token: "ok" } })
334337
t.port.clear()
335338

336339
t.advance(500)
@@ -352,6 +355,83 @@ describe("stall watchdog", () => {
352355
})
353356
})
354357

358+
// CL-5640: a healthy wait for the model to start its next reply — right
359+
// after submit, or right after the last outstanding tool call resolves —
360+
// must never be auto-aborted just because the parent stream is quiet. Only
361+
// a stream that had already started producing tokens and then went dead
362+
// (covered above) earns the abort; this shape gets the notice at most.
363+
test("a long-but-healthy wait right after submit is never auto-aborted, only noticed", async () => {
364+
await withTestRenderer(async (h) => {
365+
const t: Harness = await setup(h)
366+
try {
367+
t.bridge.submit("build it", "immediate")
368+
t.port.clear()
369+
370+
// No delta ever arrives — the model is just slow to start — held
371+
// far past the stall timeout.
372+
t.advance(20 * 60_000)
373+
t.tick()
374+
expect(t.port.calls).toEqual([])
375+
expect(t.shell.statusFlash).toBe(STALL_NOTICE_MESSAGE)
376+
} finally {
377+
t.bridge.dispose()
378+
}
379+
})
380+
})
381+
382+
test("a long-but-healthy wait right after a tool batch resolves is never auto-aborted", async () => {
383+
await withTestRenderer(async (h) => {
384+
const t: Harness = await setup(h)
385+
try {
386+
t.bridge.submit("build it", "immediate")
387+
t.bridge.handle({
388+
type: "inference.tool_call.end",
389+
data: { name: "bash", callId: "c1" },
390+
})
391+
t.bridge.handle({
392+
type: "tool.done",
393+
data: { result: { callId: "c1" } },
394+
})
395+
t.port.clear()
396+
397+
// The last outstanding call resolved; the model just takes a long
398+
// while to start its next reply. Held far past the stall timeout.
399+
t.advance(20 * 60_000)
400+
t.tick()
401+
expect(t.port.calls).toEqual([])
402+
expect(t.shell.statusFlash).toBe(STALL_NOTICE_MESSAGE)
403+
} finally {
404+
t.bridge.dispose()
405+
}
406+
})
407+
})
408+
409+
// CL-5640: live sub-agent progress must keep the parent stream's silence
410+
// exempt from abort even though the parent's own `task` call is the only
411+
// thing in `activeToolCalls` — a future change to task-lifecycle handling
412+
// must not silently drop this exemption.
413+
test("live sub-agent progress under an outstanding task call is never auto-aborted", async () => {
414+
await withTestRenderer(async (h) => {
415+
const t: Harness = await setup(h)
416+
try {
417+
t.bridge.submit("build it", "immediate")
418+
t.bridge.handle({
419+
type: "inference.tool_call.end",
420+
data: { name: "task", callId: "c1" },
421+
})
422+
t.port.clear()
423+
424+
// The parent stream stays quiet while the sub-agent works — far past
425+
// the stall timeout.
426+
t.advance(20 * 60_000)
427+
t.tick()
428+
expect(t.port.calls).toEqual([])
429+
} finally {
430+
t.bridge.dispose()
431+
}
432+
})
433+
})
434+
355435
test("an open gate is exempt no matter how long the operator takes", async () => {
356436
await withTestRenderer(async (h) => {
357437
const t: Harness = await setup(h)

0 commit comments

Comments
 (0)