a chat can change model mid-conversation
the selector locked on `hasStarted`, so the tier was fixed for the life of a conversation. nothing justified it — the commit that added the flag says only "default model" — and the hazard it resembles is narrower than what it blocked: crossing HARNESS hands an opencode `ses_…` id to `claude --resume`, which reads as a dead agent rather than a wrong dispatch. changing TIER was always safe. so tier is free, harness is pinned, and the server refuses a crossing rather than relying on the UI to. the claude side does it the way the CLI's /model does: `Query.setModel()`, a control request that needs streaming input mode, which is the only mode this file uses. nothing is torn down and nothing is re-resumed — cwd, permission mode and transcript all continue and only the next response changes. an earlier plan here was to kill the session and re-create it with the stored resume id; that would have worked and was unnecessary. opencode already switched per turn (serve-runner POSTs the model to the live session), so only the UI was in its way. FIXES A SILENT MISMATCH while it is here. model reached the SDK once, in createSession's query() options, and every later turn was pushed onto that same call — so a changed model was dropped while session:init, the cost fold and the durable log all reported the new tier. reachable today by leaving a chat idle past the 30-minute GC: the UI said sonnet, opus answered, nothing logged it. the fold now tracks what the harness actually did rather than what the client asked for, and a refused switch reverts the picker. the switch is a durable `model:switched` event and a centred marker in the transcript, because the useful question is the BOUNDARY — which half of this conversation is opus. per-message model was already stamped server-side and shown nowhere, and claude-sessions.ts reads the FIRST model in a transcript, so a switch was invisible in history. tests: turn-stream proves the marker is durable and lands after the previous model's last words; the selector tests pull in opposite directions, so unlocking everything fails one and restoring the blanket lock fails the other. verified by re-breaking both. not done: opencode's `steer` path (a send mid-turn) returns before the model POST, so a switch there applies on the next full turn. session.model also stays stale for opencode tier changes — harness routing is unaffected.
This commit is contained in:
@@ -220,6 +220,16 @@ type PersistentSession = {
|
||||
interrupted: boolean;
|
||||
/** What we passed as `--resume`, so a failure can name it and decide whether it was to blame. */
|
||||
resumeId?: string;
|
||||
/**
|
||||
* The model this session is currently answering as, `harness/tier`.
|
||||
*
|
||||
* Recorded because `model` reaches the SDK exactly once, in `createSession`'s `query()` options, while
|
||||
* every later turn is pushed onto that same call — so without this there is nothing to compare a turn's
|
||||
* requested model against, and a change was silently dropped. Officer's UI, its cost folding and its
|
||||
* durable log all took the client's word for it, so a switch produced a session that reported one tier
|
||||
* and answered as another.
|
||||
*/
|
||||
model?: string;
|
||||
/** The tail of the child's stderr, so its exit can carry a reason instead of just a number. */
|
||||
stderrTail: string;
|
||||
/** Whether the child ever produced a single message. False plus a non-zero exit means it never started. */
|
||||
@@ -468,6 +478,7 @@ function createSession(params: ClaudeSpawnStreamingParams, onEvent: (event: Chat
|
||||
const resumeId = getClaudeSession(sessionKey, params.userId) ?? params.resumeSessionId;
|
||||
const subModel = params.model?.split('/')[1];
|
||||
session.resumeId = resumeId;
|
||||
session.model = params.model;
|
||||
|
||||
/**
|
||||
* What we are about to run, before we run it.
|
||||
@@ -662,12 +673,48 @@ export async function spawnClaudeStreaming(
|
||||
}
|
||||
if (session) {
|
||||
session.emit = onEvent; // adopt the latest emitter (equivalent across turns; keeps events flowing)
|
||||
await applyModelChange(session, params.model);
|
||||
} else {
|
||||
session = createSession(params, onEvent);
|
||||
}
|
||||
session.pushTurn(params.prompt, params.images);
|
||||
}
|
||||
|
||||
/**
|
||||
* Move a LIVE session onto a different model, the way the CLI's `/model` does.
|
||||
*
|
||||
* `model` is a `query()` option and options are read once, so before this a changed model was dropped on
|
||||
* the floor — the session kept answering as whatever it started as while Officer's UI, its cost folding and
|
||||
* its durable log all reported the new tier. The mismatch was invisible: no log line, no error, and the one
|
||||
* way to trigger it was to leave a chat idle past the 30-minute GC.
|
||||
*
|
||||
* `setModel` is a control request, so it needs streaming input mode — which is the only mode this file uses.
|
||||
* That is why nothing has to be torn down and re-resumed: the conversation, its cwd, its permission mode and
|
||||
* its transcript all continue, and only the next response comes from a different model.
|
||||
*
|
||||
* A refusal is reported and swallowed. The turn the user just typed is worth more than the tier they picked,
|
||||
* so a `setModel` the harness rejects must not lose the prompt — it answers as the old model and says so.
|
||||
*/
|
||||
async function applyModelChange(session: PersistentSession, model: string | undefined): Promise<void> {
|
||||
if (!model || model === session.model) return;
|
||||
|
||||
const previous = session.model;
|
||||
const subModel = model.split('/')[1];
|
||||
if (!subModel) return;
|
||||
|
||||
try {
|
||||
await session.query.setModel(subModel);
|
||||
session.model = model;
|
||||
console.log(`[claude:model:${session.sessionKey}] ${previous ?? 'default'} -> ${model}`);
|
||||
session.emit({ type: 'model:switched', from: previous ?? null, to: model, ok: true });
|
||||
} catch (err) {
|
||||
// Leave `session.model` alone, so the next turn tries again rather than believing a switch that failed.
|
||||
const reason = err instanceof Error ? err.message : String(err);
|
||||
console.error(`[claude:model:${session.sessionKey}] refused ${previous ?? 'default'} -> ${model}: ${reason}`);
|
||||
session.emit({ type: 'model:switched', from: previous ?? null, to: model, ok: false, error: reason });
|
||||
}
|
||||
}
|
||||
|
||||
/** Interrupt the current turn but KEEP the session alive (the "stop" button). */
|
||||
export async function interruptClaudeSession(sessionKey: string, userId: number): Promise<boolean> {
|
||||
const session = ownedSession(sessionKey, userId);
|
||||
|
||||
@@ -170,6 +170,33 @@ describe('createTurnStream', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
// A tier switch has to survive a reload, and it has to land in the right PLACE. The marker says "opus
|
||||
// wrote everything above this line", which is false if it is emitted before the last thing opus said is
|
||||
// flushed out of the delta buffer.
|
||||
test('a model switch is durable and lands after the previous model’s last words', () => {
|
||||
const { durable } = run([
|
||||
{ type: 'delta', text: 'the old model speaking' },
|
||||
{ type: 'model:switched', from: 'claude-code/opus', to: 'claude-code/sonnet', ok: true },
|
||||
{ type: 'text', text: 'the new one' },
|
||||
]);
|
||||
expect(durable).toEqual([
|
||||
{ type: 'assistant:text', text: 'the old model speaking' },
|
||||
{ type: 'model:switched', from: 'claude-code/opus', to: 'claude-code/sonnet', ok: true },
|
||||
{ type: 'assistant:text', text: 'the new one' },
|
||||
]);
|
||||
});
|
||||
|
||||
test('a refused switch is recorded too, with its reason', () => {
|
||||
// Silence here would be the worst outcome: the turn answers as the OLD model while the picker shows the
|
||||
// new one, which is the exact mismatch this whole change exists to remove.
|
||||
const { durable } = run([
|
||||
{ type: 'model:switched', from: 'claude-code/opus', to: 'claude-code/haiku', ok: false, error: 'nope' },
|
||||
]);
|
||||
expect(durable).toEqual([
|
||||
{ type: 'model:switched', from: 'claude-code/opus', to: 'claude-code/haiku', ok: false, error: 'nope' },
|
||||
]);
|
||||
});
|
||||
|
||||
test('streams are independent', () => {
|
||||
const a = createTurnStream('a');
|
||||
const b = createTurnStream('b');
|
||||
|
||||
@@ -133,6 +133,23 @@ export function createTurnStream(sessionId: string): TurnStream {
|
||||
// notice belongs after it rather than in the middle of a paragraph it interrupted.
|
||||
return [...flushAll(), { msg: { type: 'compact:start', trigger: event.trigger }, durable: true }];
|
||||
|
||||
case 'model:switched':
|
||||
// Flushed for the same reason, and it matters more here: the marker's whole job is to say where in
|
||||
// the conversation the tier changed, so it must land after the last thing the old model said.
|
||||
return [
|
||||
...flushAll(),
|
||||
{
|
||||
msg: {
|
||||
type: 'model:switched',
|
||||
from: event.from,
|
||||
to: event.to,
|
||||
ok: event.ok,
|
||||
...(event.error ? { error: event.error } : {}),
|
||||
},
|
||||
durable: true,
|
||||
},
|
||||
];
|
||||
|
||||
case 'compact:done':
|
||||
return [
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user