diff --git a/src/servers/api/chat/types.ts b/src/servers/api/chat/types.ts index 0845f4ba..5517ceae 100644 --- a/src/servers/api/chat/types.ts +++ b/src/servers/api/chat/types.ts @@ -282,6 +282,27 @@ export type ServerMessage = answered, as `from` — so it says what actually happened rather than what was asked for. */ | { type: 'model:switched'; from: string | null; to: string; ok: boolean; error?: string } + /* + What the harness says it is actually running, as opposed to what the turn asked for. + + Officer's `DEFAULT_MODEL` is the BARE HARNESS, `claude-code` — sent when the client names no model and + the account has no default. It routes correctly (no `--model` is passed, so the CLI picks for itself) + and it names no catalogue entry, so the picker had nothing to render and showed the literal string + `claude-code`. The server could not do better because it deliberately did not choose: only the harness + knows, and it says so in its `system.init`. + + So this is DISPLAY ONLY, and three things follow from that: + + - It is live-only, like `session:claude`. A replayed log gets the model from `session:init` and from + `GET /chat/sessions/:id`, which reads the transcript. + - It is sent only when the requested model does NOT name a tier. The alias map is lossy — `opus[1m]` + and `opusplan` both resolve back to plain `opus` — so a user who picked either must not watch their + choice silently flatten. + - It is never stored and never becomes the client's `selectedModel`. Storing it would pin a session to + whatever the CLI happened to default to on its first turn, which is exactly the freedom the bare + harness exists to preserve. + */ + | { type: 'model:resolved'; model: string } /* Plan mode, in three messages. @@ -322,6 +343,7 @@ export type TurnMessageType = | 'compact:start' | 'compact:done' | 'model:switched' + | 'model:resolved' | 'plan:mode' | 'plan:proposed' | 'plan:decided'; @@ -361,6 +383,9 @@ export type ChatEvent = | { type: 'compact:done'; trigger: 'manual' | 'auto'; preTokens: number; durationMs?: number } // Emitted by `applyModelChange` when a live session moves onto a different tier — see claude-manager. | { type: 'model:switched'; from: string | null; to: string; ok: boolean; error?: string } + // The harness's own `system.init` model, as an ANTHROPIC api id (`claude-opus-5`). Translated to an + // Officer id in `turn-stream`, because everything downstream of the sidecar speaks Officer ids. + | { type: 'model:resolved'; apiModel: string } // Plan mode — see the matching ServerMessage members for what each one means. | { type: 'plan:mode'; enabled: boolean; ok: boolean; error?: string } | { type: 'plan:proposed'; requestId: string; plan: string } diff --git a/src/servers/api/chat/websocket.ts b/src/servers/api/chat/websocket.ts index 87e22dd7..7e7f7f61 100644 --- a/src/servers/api/chat/websocket.ts +++ b/src/servers/api/chat/websocket.ts @@ -415,6 +415,28 @@ function createMessageHandler(sessionId: string, model: string, resumeSessionId? record(); } + /* + What the harness reports it is running — forwarded ONLY when the turn did not name a tier. + + `effective` without a `/` is the bare harness, `claude-code`: the client chose nothing, officer passed + no `--model`, and the CLI picked. That is the only case where the harness knows something officer does + not, and it is the only case where this may speak. + + When the turn DID name a tier, this is dropped on the floor, because the translation back from an API + id is lossy in exactly the directions that matter: `claude-code/opus[1m]` and `claude-code/opusplan` + both answer as `claude-opus-5`, so forwarding would quietly relabel the user's own choice as plain + opus. `model:switched` already covers a real change. + + Not recorded, not folded, not `sessionManager.setModel`. `session.model` is what dispatch and the stop + path route on, and it is correct as the bare harness; and `record()` refuses a model without a `/` for + a reason worth reading before changing any of this. + */ + if (msg.type === 'model:resolved') { + if (effective.includes('/')) return; + for (const socket of session.sockets) sendToClient(socket as ServerWebSocket, msg, seq); + return; + } + // Follow the harness, never the request — a refused switch (`ok: false`) leaves the session where it was, // and recording the attempt would restore a mode the agent is not in. if (msg.type === 'plan:mode' && msg.ok) { diff --git a/src/servers/sidecar/claude/stream-parser.test.ts b/src/servers/sidecar/claude/stream-parser.test.ts index 59e510ff..641e02cd 100644 --- a/src/servers/sidecar/claude/stream-parser.test.ts +++ b/src/servers/sidecar/claude/stream-parser.test.ts @@ -150,6 +150,27 @@ describe('processLine', () => { expect(sessionIds).toEqual(['sess_abc123']); }); + test('system init reports the model the harness actually chose', () => { + // The whole reason `model:resolved` exists: officer passes no `--model` when the client named no tier, + // so this line is the only place the answering model is ever stated. + const state = createParseState(); + const { events, sessionIds, callbacks } = makeCallbacks(); + const line = JSON.stringify({ type: 'system', subtype: 'init', session_id: 'sess_1', model: 'claude-opus-5' }); + processLine(line, state, callbacks); + expect(events).toEqual([ + { type: 'model:resolved', apiModel: 'claude-opus-5' }, + { type: 'session', claudeSessionId: 'sess_1' }, + ]); + expect(sessionIds).toEqual(['sess_1']); + }); + + test('system init without a model says nothing about one', () => { + const state = createParseState(); + const { events, callbacks } = makeCallbacks(); + processLine(JSON.stringify({ type: 'system', subtype: 'init', session_id: 'sess_1' }), state, callbacks); + expect(events.some((e) => e.type === 'model:resolved')).toBe(false); + }); + test('handles compact_boundary — emits compact:done with what it dropped', () => { const state = createParseState(); const { events, callbacks } = makeCallbacks(); diff --git a/src/servers/sidecar/claude/stream-parser.ts b/src/servers/sidecar/claude/stream-parser.ts index 624808a0..d3ebdc99 100644 --- a/src/servers/sidecar/claude/stream-parser.ts +++ b/src/servers/sidecar/claude/stream-parser.ts @@ -147,6 +147,14 @@ function handleResult(msg: Record, state: ParseState, callbacks function handleSystem(msg: Record, callbacks: StreamParserCallbacks): void { const subtype = msg.subtype as string | undefined; if (subtype === 'init') { + // The one place the harness says which model it is actually running. Officer often does not know — + // it passes no `--model` when the client named no tier — so without this the picker had nothing to + // show but the bare harness string. See `model:resolved` in api/chat/types.ts. + const initModel = msg.model; + if (typeof initModel === 'string' && initModel) { + callbacks.onEvent({ type: 'model:resolved', apiModel: initModel }); + } + const sessionId = msg.session_id as string | undefined; if (sessionId) { callbacks.onSessionId(sessionId); diff --git a/src/servers/sidecar/claude/turn-stream.test.ts b/src/servers/sidecar/claude/turn-stream.test.ts index 0e11e595..afdf0a99 100644 --- a/src/servers/sidecar/claude/turn-stream.test.ts +++ b/src/servers/sidecar/claude/turn-stream.test.ts @@ -17,6 +17,34 @@ function run(events: ChatEvent[]) { }; } +describe('the model the harness resolved for itself', () => { + test('travels as an Officer id and is never persisted', () => { + const { all, durable, types } = run([{ type: 'model:resolved', apiModel: 'claude-opus-5' }]); + expect(types).toEqual(['model:resolved']); + expect(all[0]!.msg).toEqual({ type: 'model:resolved', model: 'claude-code/opus' }); + // Live-only for the same reason as `session:claude`: it marks no boundary, and a replay learns the + // model from the session row instead. + expect(durable).toHaveLength(0); + }); + + test('an id with no honest Officer equivalent is dropped rather than forwarded raw', () => { + // mythos has no catalogue entry on this machine, and `` is not a model at all. Offering + // either to the picker is worse than leaving it as it was. + expect(run([{ type: 'model:resolved', apiModel: 'claude-mythos-5' }]).all).toHaveLength(0); + expect(run([{ type: 'model:resolved', apiModel: '' }]).all).toHaveLength(0); + }); + + test('does not flush an open delta buffer', () => { + // It says nothing about WHERE in the conversation anything happened, so cutting the sentence the + // agent is mid-way through would be a boundary the event does not describe. + const { types } = run([ + { type: 'delta', text: 'thinking' }, + { type: 'model:resolved', apiModel: 'claude-sonnet-5' }, + ]); + expect(types).toEqual(['assistant:delta', 'model:resolved']); + }); +}); + describe('createTurnStream', () => { test('deltas are live-only and never durable', () => { const { all, durable } = run([ diff --git a/src/servers/sidecar/claude/turn-stream.ts b/src/servers/sidecar/claude/turn-stream.ts index 6402693d..e0b1b070 100644 --- a/src/servers/sidecar/claude/turn-stream.ts +++ b/src/servers/sidecar/claude/turn-stream.ts @@ -1,4 +1,5 @@ import type { ChatEvent, TurnMessage } from '../../api/chat/types'; +import { officerModelForApiModel } from '../../api/chat/model-aliases'; // Translation from a harness's ChatEvent stream to the browser-facing turn messages, moved here from the // main server (it was `createEventHandler` in `chat/websocket.ts`, once per harness). Both the agent and @@ -150,6 +151,18 @@ export function createTurnStream(sessionId: string): TurnStream { }, ]; + case 'model:resolved': { + // Not durable and not flushed: it says nothing about WHERE in the conversation anything happened, + // only what the harness is running. `session:claude` is the same shape for the same reason. + // + // A null mapping is dropped rather than forwarded raw. ``, mythos and every OpenCode id + // resolve to null, and offering the picker a model this machine may not reach is worse than + // leaving it as it was. + const officerId = officerModelForApiModel(event.apiModel); + if (!officerId) return []; + return [{ msg: { type: 'model:resolved', model: officerId }, durable: false }]; + } + case 'plan:mode': // Flushed like `model:switched`, and for the same reason: the marker's job is to say WHERE in the // conversation the rules changed, which is worthless if it lands mid-paragraph. diff --git a/src/workspaces/officerdev/src/apps/Chat/types.ts b/src/workspaces/officerdev/src/apps/Chat/types.ts index 07be40d2..ed22f0c5 100644 --- a/src/workspaces/officerdev/src/apps/Chat/types.ts +++ b/src/workspaces/officerdev/src/apps/Chat/types.ts @@ -193,6 +193,14 @@ export type ServerMessage = | { type: 'compact:done'; trigger: 'manual' | 'auto'; preTokens: number; durationMs?: number } /** The conversation moved onto a different model. See the `model` message role. */ | { type: 'model:switched'; from: string | null; to: string; ok: boolean; error?: string } + /** + * What the harness is actually running, when officer did not choose. + * + * Sent only for a turn that named no tier — the composer's `model` (never `selectedModel`) so the picker + * stops showing the bare harness string `claude-code`. No transcript row: it is not a boundary, nothing + * changed, and it says only what has been true since the turn began. + */ + | { type: 'model:resolved'; model: string } /** Plan mode went on or off. See the `planmode` message role. */ | { type: 'plan:mode'; enabled: boolean; ok: boolean; error?: string } /** The agent is blocked, waiting for this plan to be approved. See the `plan` message role. */ diff --git a/src/workspaces/officerdev/src/hooks/useChat.ts b/src/workspaces/officerdev/src/hooks/useChat.ts index 4b8c3c03..9f842651 100644 --- a/src/workspaces/officerdev/src/hooks/useChat.ts +++ b/src/workspaces/officerdev/src/hooks/useChat.ts @@ -504,6 +504,18 @@ export function useChat(initialSessionId?: string, initialModel?: string | null, } break; + case 'model:resolved': + /* + Display only, and the two lines it does NOT contain are the point. + + `setSelectedModel` would make the next turn request this tier explicitly, pinning a session that + was deliberately left to the CLI's own default. `modelRef.current` is the reattach handshake and + would do the same after any reconnect. The server sends this only when the turn named no tier, so + both would be inventing a choice the user never made. + */ + setModel(msg.model); + break; + case 'plan:mode': // Committed first, like every other boundary marker: the notice says where the rules changed, so // whatever was streamed under the old ones has to land above it.