adopt on a resume even when the harness is a guess
Regression from my own B7 change, reported within the hour: turns collapsing to "turn completed without output" and coming back only on refresh. B7 stopped resume-cursor defaulting an unidentified session to claude-code. Correct for the durable cut-off row, wrong for adoption: useChat sends model only if modelRef.current is set, so a reconnect without one is routine, not exotic. Declining to adopt left the socket unbound to the live session, so the running turn output went nowhere — and a refresh looked like a fix because it rebuilds from the durable log. Adoption is about DELIVERY and must be generous; only the durable write needs certainty. So adopt on the default again, mark it as an assumption, and skip the cut-off check on it. That keeps B7 fixed — no false "agent went away" written against an opencode session — with no unbound sockets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -28,17 +28,24 @@ describe('decideResume', () => {
|
|||||||
expect(decideResume(undefined, 'claude-code')).toEqual({ kind: 'adopt', model: 'claude-code' });
|
expect(decideResume(undefined, 'claude-code')).toEqual({ kind: 'adopt', model: 'claude-code' });
|
||||||
});
|
});
|
||||||
|
|
||||||
it('stays unknown rather than defaulting to claude, which is the whole bug', () => {
|
it('marks a total unknown as an assumption instead of passing it off as knowledge', () => {
|
||||||
// No record and no claim. Previously this became `claude-code` and was indistinguishable from a real
|
// No record and no claim. This still adopts — see below — but the caller can tell it apart, which is
|
||||||
// Claude session; now it declines to adopt and declines to run the cut-off check.
|
// what keeps the durable cut-off row off a session nobody has identified.
|
||||||
expect(decideResume(undefined, undefined)).toEqual({ kind: 'replay-only' });
|
expect(decideResume(undefined, undefined)).toEqual({ kind: 'assume', model: 'claude-code' });
|
||||||
expect(decideResume(undefined, '')).toEqual({ kind: 'replay-only' });
|
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', () => {
|
it('always yields a model to adopt with, because an unbound socket misses the whole turn', () => {
|
||||||
// The failure mode in the other direction: declining to adopt a session we could have identified
|
// Regression guard. Refusing to adopt when the harness was unknown looked principled and broke the
|
||||||
// would silently drop the reconnect that makes turn output resume.
|
// app inside an hour: `useChat.ts` only sends `model` `if (modelRef.current)`, so a reconnect
|
||||||
expect(decideResume('claude-code', undefined).kind).toBe('known');
|
// without one is routine. The socket never re-bound, the live turn's output went nowhere, and the
|
||||||
expect(decideResume('', 'claude-code').kind).toBe('adopt');
|
// 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();
|
||||||
|
}
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -626,19 +626,17 @@ async function handleResumeCursor(
|
|||||||
|
|
||||||
const known = sessionManager.getSession(sessionId);
|
const known = sessionManager.getSession(sessionId);
|
||||||
const decision = decideResume(known?.model, msg.model);
|
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 ?? '');
|
adoptOrphanedSession(ws, sessionId, decision.model, msg.cwd ?? '');
|
||||||
} else if (decision.kind === 'replay-only') {
|
if (decision.kind === 'assume') {
|
||||||
// Replay the durable log and stop there. Not adopting costs a live re-subscription; adopting on a
|
logger.warn('resume-cursor for an unknown session with no model; adopting on the default', { sessionId });
|
||||||
// guess cost correctness — see decideResume.
|
}
|
||||||
logger.warn('resume-cursor names a session this process does not know, and no model; replaying only', {
|
|
||||||
sessionId,
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
|
|
||||||
sessionManager.attachWs(sessionId, ws); // a no-op when adoption was skipped
|
sessionManager.attachWs(sessionId, ws);
|
||||||
wsToSessionMap.set(ws as any, sessionId);
|
wsToSessionMap.set(ws as any, sessionId);
|
||||||
try {
|
try {
|
||||||
const events = await getChatEventsSince(sessionId, cursor ?? 0);
|
const events = await getChatEventsSince(sessionId, cursor ?? 0);
|
||||||
@@ -648,15 +646,19 @@ async function handleResumeCursor(
|
|||||||
} catch (err) {
|
} catch (err) {
|
||||||
logger.error('Failed to replay chat events on resume-cursor', { sessionId, error: String(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
|
// Only ask when the harness is actually known — never on an assumption. This check writes a DURABLE
|
||||||
// is permanent — silence is the safe failure.
|
// row, so a wrong answer is permanent: that was B7, where a defaulted `claude-code` made officer ask
|
||||||
if (msg.generating && model) await endTurnIfAgentIsGone(ws, sessionId, model);
|
// 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 =
|
export type ResumeDecision =
|
||||||
| { kind: 'known'; model: string }
|
| { kind: 'known'; model: string }
|
||||||
| { kind: 'adopt'; model: string }
|
| { kind: 'adopt'; model: string }
|
||||||
| { kind: 'replay-only' };
|
| { kind: 'assume'; model: string };
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Which harness a resuming session belongs to, and what that permits.
|
* 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
|
* 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.
|
* 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
|
* **A guess is still a guess**: when neither knows, the session is adopted on the default so delivery
|
||||||
* what made a guess indistinguishable from knowledge.
|
* 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 {
|
export function decideResume(knownModel: string | undefined, claimedModel: string | undefined): ResumeDecision {
|
||||||
if (knownModel) return { kind: 'known', model: knownModel };
|
if (knownModel) return { kind: 'known', model: knownModel };
|
||||||
if (claimedModel) return { kind: 'adopt', model: claimedModel };
|
if (claimedModel) return { kind: 'adopt', model: claimedModel };
|
||||||
return { kind: 'replay-only' };
|
return { kind: 'assume', model: DEFAULT_MODEL };
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user