diff --git a/src/servers/api/chat/websocket.test.ts b/src/servers/api/chat/websocket.test.ts index 94e1a2fd..a56b780a 100644 --- a/src/servers/api/chat/websocket.test.ts +++ b/src/servers/api/chat/websocket.test.ts @@ -28,17 +28,24 @@ describe('decideResume', () => { expect(decideResume(undefined, 'claude-code')).toEqual({ kind: 'adopt', model: 'claude-code' }); }); - it('stays unknown rather than defaulting to claude, which is the whole bug', () => { - // No record and no claim. Previously this became `claude-code` and was indistinguishable from a real - // Claude session; now it declines to adopt and declines to run the cut-off check. - expect(decideResume(undefined, undefined)).toEqual({ kind: 'replay-only' }); - expect(decideResume(undefined, '')).toEqual({ kind: 'replay-only' }); + it('marks a total unknown as an assumption instead of passing it off as knowledge', () => { + // No record and no claim. This still adopts — see below — but the caller can tell it apart, which is + // what keeps the durable cut-off row off a session nobody has identified. + expect(decideResume(undefined, undefined)).toEqual({ kind: 'assume', model: 'claude-code' }); + expect(decideResume(undefined, '')).toEqual({ kind: 'assume', model: 'claude-code' }); }); - it('never reports replay-only once anything is known, so a live turn is always re-bound', () => { - // The failure mode in the other direction: declining to adopt a session we could have identified - // would silently drop the reconnect that makes turn output resume. - expect(decideResume('claude-code', undefined).kind).toBe('known'); - expect(decideResume('', 'claude-code').kind).toBe('adopt'); + it('always yields a model to adopt with, because an unbound socket misses the whole turn', () => { + // Regression guard. Refusing to adopt when the harness was unknown looked principled and broke the + // app inside an hour: `useChat.ts` only sends `model` `if (modelRef.current)`, so a reconnect + // without one is routine. The socket never re-bound, the live turn's output went nowhere, and the + // transcript collapsed to "turn completed without output" until a refresh rebuilt it from the log. + for (const d of [ + decideResume('claude-code', undefined), + decideResume(undefined, 'opencode/x'), + decideResume(undefined, undefined), + ]) { + expect(d.model).toBeTruthy(); + } }); }); diff --git a/src/servers/api/chat/websocket.ts b/src/servers/api/chat/websocket.ts index ceeccce8..6429b283 100644 --- a/src/servers/api/chat/websocket.ts +++ b/src/servers/api/chat/websocket.ts @@ -626,19 +626,17 @@ async function handleResumeCursor( const known = sessionManager.getSession(sessionId); const decision = decideResume(known?.model, msg.model); - const model = decision.kind === 'replay-only' ? null : decision.model; - if (decision.kind === 'adopt') { + if (decision.kind !== 'known') { + // Adopt on an assumption too: an unbound socket misses the turn entirely, which is worse than a + // harness guess that only ever costs us the cut-off check below. adoptOrphanedSession(ws, sessionId, decision.model, msg.cwd ?? ''); - } else if (decision.kind === 'replay-only') { - // Replay the durable log and stop there. Not adopting costs a live re-subscription; adopting on a - // guess cost correctness — see decideResume. - logger.warn('resume-cursor names a session this process does not know, and no model; replaying only', { - sessionId, - }); + if (decision.kind === 'assume') { + logger.warn('resume-cursor for an unknown session with no model; adopting on the default', { sessionId }); + } } - sessionManager.attachWs(sessionId, ws); // a no-op when adoption was skipped + sessionManager.attachWs(sessionId, ws); wsToSessionMap.set(ws as any, sessionId); try { const events = await getChatEventsSince(sessionId, cursor ?? 0); @@ -648,15 +646,19 @@ async function handleResumeCursor( } catch (err) { logger.error('Failed to replay chat events on resume-cursor', { sessionId, error: String(err) }); } - // Only ask when the harness is actually known. The check writes a DURABLE row, so a wrong answer here - // is permanent — silence is the safe failure. - if (msg.generating && model) await endTurnIfAgentIsGone(ws, sessionId, model); + // Only ask when the harness is actually known — never on an assumption. This check writes a DURABLE + // row, so a wrong answer is permanent: that was B7, where a defaulted `claude-code` made officer ask + // the Claude sidecar about an OpenCode session, hear "not generating", and write "the agent went + // away" into a turn that was running perfectly well. + if (msg.generating && decision.kind !== 'assume') { + await endTurnIfAgentIsGone(ws, sessionId, decision.model); + } } export type ResumeDecision = | { kind: 'known'; model: string } | { kind: 'adopt'; model: string } - | { kind: 'replay-only' }; + | { kind: 'assume'; model: string }; /** * Which harness a resuming session belongs to, and what that permits. @@ -677,13 +679,21 @@ export type ResumeDecision = * * Two rules. **The server's own record beats the client's claim** — a session in memory already knows its * harness, and letting a socket re-declare it is how the wrong sidecar gets a session in the first place. - * **An unknown harness stays unknown**: no adoption, no cut-off check, just the replay. Defaulting is - * what made a guess indistinguishable from knowledge. + * **A guess is still a guess**: when neither knows, the session is adopted on the default so delivery + * keeps working, but it is marked `assume` and nothing durable may be written from it. + * + * The middle position — refusing to adopt at all when the harness is unknown — was tried and was WRONG, + * visibly so within the hour. `useChat.ts` sends `model` only `if (modelRef.current)`, so a reconnect + * without one is ordinary, not exotic; declining to adopt left the socket unbound to a live turn, and + * the running turn's output went nowhere. On screen: the transcript collapsed to "turn completed + * without output" and only a refresh — which rebuilds from the durable log — brought it back. + * + * So adoption is about DELIVERY and must be generous. Only the durable write needs certainty. */ export function decideResume(knownModel: string | undefined, claimedModel: string | undefined): ResumeDecision { if (knownModel) return { kind: 'known', model: knownModel }; if (claimedModel) return { kind: 'adopt', model: claimedModel }; - return { kind: 'replay-only' }; + return { kind: 'assume', model: DEFAULT_MODEL }; } /**