diff --git a/src/servers/sidecar/claude/claude-manager.ts b/src/servers/sidecar/claude/claude-manager.ts index 38a170b7..49f5b5e7 100644 --- a/src/servers/sidecar/claude/claude-manager.ts +++ b/src/servers/sidecar/claude/claude-manager.ts @@ -463,6 +463,30 @@ function armStall(session: PersistentSession): void { session.stallTimer = undefined; return; } + + // ── A turn WAITING ON A PERSON is not a stalled turn ── + // + // Parking on `AskUserQuestion` or a plan gate means emitting nothing until somebody answers — that is + // what parking is. This timer could not tell that from a wedged turn, and at 10 minutes against gates + // that wait 20, it always won: + // + // 10m the turn is declared dead, `isGenerating` goes false, and the session is torn down + // 20m the gate's own timeout would have resolved it truthfully — never reached + // + // The card stays on screen with nothing behind it, and clicking an answer does nothing, because an + // answer's only receipt is the `tool:result` that the gate emits when it unblocks — and the gate is + // gone. Reported as "the chat becomes unusable, I have to refresh and send another message", which is + // exactly what it would look like from the outside. + // + // So a session with an outstanding question or plan is left alone here. It is not unbounded: each gate + // holds its own 20-minute timer and resolves with a sentence written for the agent, after which the + // turn speaks again and this re-arms on the next event. + const waitingOnAPerson = session.pendingQuestions.size > 0 || session.pendingPlans.size > 0; + if (waitingOnAPerson) { + session.stallTimer = undefined; + return; + } + const compacting = session.compactStartedAt !== undefined; session.stallTimer = setTimeout( () => { @@ -1030,6 +1054,9 @@ async function questionGate(session: PersistentSession, toolCallId: string): Pro return new Promise((resolve) => { const timer = setTimeout(() => { session.pendingQuestions.delete(toolCallId); + // Re-arm: the turn is about to speak again, and from here a silence IS a stall. `armStall` skips + // itself while any gate is still outstanding, so a second unanswered question keeps the reprieve. + armStall(session); console.warn(`[claude:question:${session.sessionKey}] no answer in 20m`); resolve( 'The user has not answered yet — they may be away from the screen. The question is still on theirs, ' + @@ -1067,6 +1094,9 @@ export function answerClaudeQuestion(sessionKey: string, userId: number, toolCal if (!session || !pending) return false; clearTimeout(pending.timer); session.pendingQuestions.delete(toolCallId); + // The turn resumes now, so silence stops being legitimate and the watchdog goes back on. `armStall` + // skips itself while another gate is outstanding, so answering one of two questions keeps the reprieve. + armStall(session); pending.resolve(answer); return true; } diff --git a/src/servers/sidecar/claude/stall-parked.test.ts b/src/servers/sidecar/claude/stall-parked.test.ts new file mode 100644 index 00000000..9c4ecab6 --- /dev/null +++ b/src/servers/sidecar/claude/stall-parked.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, test } from 'bun:test'; + +// The rule the fix encodes, exercised as the predicate rather than through a live harness: a session +// waiting on a person must not be treated as stalled. +const stallApplies = (isGenerating: boolean, questions: number, plans: number) => + isGenerating && questions === 0 && plans === 0; + +describe('stall watchdog vs a parked turn', () => { + test('a generating turn with nothing pending is watched', () => { + expect(stallApplies(true, 0, 0)).toBe(true); + }); + test('a turn parked on a question is NOT stalled — it is waiting', () => { + expect(stallApplies(true, 1, 0)).toBe(false); + }); + test('a turn parked on a plan decision is not either', () => { + expect(stallApplies(true, 0, 1)).toBe(false); + }); + test('answering one of two questions keeps the reprieve', () => { + expect(stallApplies(true, 1, 0)).toBe(false); + }); + test('a turn that is not generating is never stalled', () => { + expect(stallApplies(false, 0, 0)).toBe(false); + }); +});