diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a328c26..cf99140 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,6 +42,13 @@ jobs: - name: Test run: npm test + # Whole classes of defect here are invisible to the unit suite because + # only a layout engine can answer them: whether a bar wraps, whether an + # answer actually reaches the screen while it streams. The runner image + # ships Chrome, and the runner refuses to skip when CI is set. + - name: Browser checks + run: npm run test:browser + install: name: Verify the documented install runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index a59c676..b475705 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,32 @@ ## [5.1.2] - 2026-07-26 -### Notes -- Version bump only; no functional changes. +### Added +- **A conversation can run a different model from the one the profile picks.** + The model shown beside the composer is now something to click: choose one the + runtime has listed, or type a name it has not, and that choice belongs to that + one conversation — never written back as a profile or personal default, and + never inherited by the next conversation. Where the runtime can change model + without restarting, it changes immediately; where it cannot, the choice is + saved and used the next time that conversation starts, and the reply says + which of the two happened rather than claiming success either way. The same + menu offers the way back to the runtime's own default. +- **Turns fold away.** Each turn's header can be collapsed to hide everything + under it, and a turn folds on its own once the next one begins, so a long + session reads as a list of what was asked rather than an endless scroll. The + turn index can open or close them all at once, and jumping to a turn from the + index opens it. A folded turn still says what it was about, and its copy and + branch actions keep working while it is shut. + +### Fixed +- **The runtime's own slash commands are there from the moment a conversation + opens.** They used to appear only after the first message had been sent, + because nothing knew what the agent supported until the agent had spoken. +- **`/clear` and `/new` really do start over.** The transcript went blank, but + the text was handed to the agent like any other message, so the process kept + the whole conversation in mind and the next answer brought it all back. They + now restart the agent on a genuinely empty conversation, which is what they + had appeared to do. ## [5.1.1] - 2026-07-26 diff --git a/src/client/shell/chat/Composer.tsx b/src/client/shell/chat/Composer.tsx index 999b8db..30f4c73 100644 --- a/src/client/shell/chat/Composer.tsx +++ b/src/client/shell/chat/Composer.tsx @@ -1231,6 +1231,34 @@ function ModelChip({ This runtime hasn't listed models — type one above. ) : null} + + {/* The way back. Picking a model is one click; without this, undoing + that choice was impossible — the text field refuses to submit + empty and every entry above carries a name — so a conversation + could be moved off its profile default but never returned to it, + and a typo stayed in force for every later launch. */} + ) : null} diff --git a/src/client/shell/chat/MessageList.tsx b/src/client/shell/chat/MessageList.tsx index b939517..b3367e7 100644 --- a/src/client/shell/chat/MessageList.tsx +++ b/src/client/shell/chat/MessageList.tsx @@ -248,11 +248,18 @@ export const MessageList = React.forwardRef // Looked up by id rather than iterated in transcript order: a turn's own // messages are already listed on it, and rendering turn-by-turn (below) is // what lets a collapsed turn hide its whole body as one unit. + // + // Keyed on `version`, not on `messages`: the transcript appends to its + // array in place, so its identity never changes and a `[messages]` map + // would be built once at mount and then never see another message. Every + // later id would miss and render nothing — a turn whose body stays empty + // while its strip keeps counting. const messageById = React.useMemo(() => { const map = new Map(); for (const message of messages) map.set(message.id, message); return map; - }, [messages]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [messages, version]); const lastTurnId = turns.length ? turns[turns.length - 1].id : undefined; diff --git a/src/client/shell/chat/TurnStrip.tsx b/src/client/shell/chat/TurnStrip.tsx index fecfbe1..1fa782a 100644 --- a/src/client/shell/chat/TurnStrip.tsx +++ b/src/client/shell/chat/TurnStrip.tsx @@ -147,7 +147,7 @@ export function TurnStrip({ > - {turn.label} + {turn.label} ) : null} @@ -188,8 +188,21 @@ export function TurnStrip({ ); } -/** A meta item that gives up its own width before the row wraps. */ -function Shrinkable({ children }: { children: React.ReactNode }): React.JSX.Element { +/** + * Text that gives up its width before anything else in the bar does. + * + * `nowrap` is what makes the ellipsis work at all: without it the text wraps + * instead of being cut, and since the bar is a fixed height the second line + * simply leaves it. The other users of this sit inside a span that already + * sets both that and a size; the turn label does not, so it says so itself. + */ +function Shrinkable({ + children, + fontSize, +}: { + children: React.ReactNode; + fontSize?: string | number; +}): React.JSX.Element { return ( {children} diff --git a/src/server/chat/manager.ts b/src/server/chat/manager.ts index 0ab04a2..adfa038 100644 --- a/src/server/chat/manager.ts +++ b/src/server/chat/manager.ts @@ -214,6 +214,11 @@ export class ChatSessionManager { return session.setModel(model); } + /** Carry a new model into the options an in-place `/clear` restart replays. */ + rememberModel(sessionId: string, model: string | undefined): void { + this.sessions.get(sessionId)?.rememberModel(model); + } + /** Drop a turn that was typed ahead and has not run yet. */ cancelQueued(sessionId: string, queuedId: string): boolean { return this.sessions.get(sessionId)?.cancelQueued(queuedId) ?? false; diff --git a/src/server/chat/session.ts b/src/server/chat/session.ts index 64f0c07..1037451 100644 --- a/src/server/chat/session.ts +++ b/src/server/chat/session.ts @@ -681,6 +681,25 @@ export class ChatSession { return true; } + /** + * Record the model an in-place restart must launch with. + * + * `restart()` replays the options this session was last started with, and + * those were resolved once, at launch. Everything in them is fixed for the + * life of the conversation except the model, which `chat_set_model` can + * change underneath them — so without this a `/clear` would quietly + * reinstate the model the conversation happened to open with, discarding a + * choice the browser has already been told was applied. + * + * Takes the effective model rather than the override, so clearing an + * override lands on the profile default here exactly as it would on a fresh + * launch. + */ + rememberModel(model: string | undefined): void { + if (!this.lastStartOptions) return; + this.lastStartOptions = { ...this.lastStartOptions, model }; + } + snapshot(): Promise { return this.deps.store.snapshot(this.ref).then((snapshot) => ({ ...snapshot, diff --git a/src/server/websocket/messages.ts b/src/server/websocket/messages.ts index 8892e03..94624e9 100644 --- a/src/server/websocket/messages.ts +++ b/src/server/websocket/messages.ts @@ -15,6 +15,29 @@ import { sendToWebSocket, broadcastChat, broadcastToSession } from './handler.js import { chatUnavailableReason, isChatRuntime } from '../../shared/chat-runtimes.js'; import { ChatNotRunningError } from '../chat/session.js'; +/** + * The longest model name worth storing. Real ones are far shorter; this only + * has to stop an unbounded string from being persisted and then handed to a + * spawn on every future launch of the conversation. + */ +const MAX_MODEL_NAME = 200; + +/** + * Tidy a typed model name into something safe to keep. + * + * Names are never validated against a list — a runtime knows its own models + * and new ones appear without us — so this only removes what can't belong in + * one: control characters, which would otherwise ride into the best-effort + * `/model ` turn as extra lines, and unbounded length. + */ +function normaliseModelName(raw: string): string | undefined { + const stripped = raw.replace(/[\u0000-\u001f\u007f-\u009f\u2028\u2029]+/g, ' ').trim(); + // Sliced by code point, not by code unit: cutting mid-character would store + // half a surrogate pair. + const cleaned = [...stripped].slice(0, MAX_MODEL_NAME).join('').trim(); + return cleaned || undefined; +} + export interface MessageProcessorDeps { dev: boolean; claudeSessions: Map; @@ -90,6 +113,8 @@ export interface ChatManagerLike { interrupt(sessionId: string): Promise; /** Switch a live session's model. False when nothing is running, or the adapter cannot. */ setModel(sessionId: string, model: string): Promise; + /** Carry a new model into the options an in-place `/clear` restart replays. */ + rememberModel(sessionId: string, model: string | undefined): void; cancelQueued(sessionId: string, queuedId: string): boolean; respondPermission(sessionId: string, requestId: string, optionId: string): boolean; stop(sessionId: string): Promise; @@ -1121,6 +1146,23 @@ export class MessageProcessor { const text = typeof data.text === 'string' ? data.text : ''; if (!text.trim() && !(data.attachments || []).length) return; + // The runtime's own `/model` reaches the same decision by the other door, + // so it has to leave the same trace. Without this the command is forwarded + // untouched, the conversation really does change model, and then the next + // `/clear` restarts it on the model it opened with — the same silent + // reversion the model picker was fixed for. Recorded, then forwarded + // unchanged: the runtime still runs its own command, and whether it + // accepted the name is still its answer to give, not ours. + const typedModel = /^\/model[ \t]+(\S.*)$/.exec(text.trim()); + if (typedModel) { + const model = normaliseModelName(typedModel[1]); + if (model) { + session.chatModelOverride = model; + await this.deps.saveSessionsToDisk(); + manager.rememberModel(session.id, model); + } + } + try { await manager.send(session.id, { text, @@ -1180,10 +1222,22 @@ export class MessageProcessor { if (!session) return; const raw = typeof data.model === 'string' ? data.model.trim() : ''; - const model = raw || undefined; + const model = raw ? normaliseModelName(raw) : undefined; session.chatModelOverride = model; await this.deps.saveSessionsToDisk(); + // A live session keeps the options it was launched with so that `/clear` + // can restart the process in place. The model is the one thing in there + // this handler can change, so it has to be carried across too — otherwise + // the next `/clear` reinstates the model the conversation opened with, + // after the browser has already been told the switch was applied. Resolved + // the way a launch resolves it, so clearing lands on the profile default. + const profile = this.deps.resolveRuntimeProfile( + session.agent as AgentKind, + session.workingDir, + ); + this.deps.chatManager?.rememberModel(session.id, model || profile?.model); + if (!model) { sendToWebSocket(wsInfo.ws, { type: 'chat_model_result', diff --git a/test/browser/checks.ts b/test/browser/checks.ts index d110f5a..3160a6a 100644 --- a/test/browser/checks.ts +++ b/test/browser/checks.ts @@ -225,6 +225,7 @@ async function run(): Promise { await checkATallDialogStaysOnScreen(); await checkTheComposerShrinksWithTheWorkspaceRail(); await checkTheFixedBarsNeverWrap(); + await checkALiveAnswerAppearsAsItStreams(); const pre = document.createElement('pre'); pre.id = 'results'; @@ -517,7 +518,7 @@ async function checkTheFixedBarsNeverWrap(): Promise { messages: [ { id: 'u1', seq: 1, turnId: 't1', role: 'user', ts: Date.now(), - blocks: [{ kind: 'text', text: 'a question long enough to need the whole column for itself' }], + blocks: [{ kind: 'text', text: 'please refactor the authentication middleware so that it validates the bearer token against the new issuer, then update every call site that still assumes the old claim shape, and make sure the integration tests cover the expired-token path as well as the malformed-header path' }], }, { id: 'a1', seq: 2, turnId: 't1', role: 'assistant', ts: Date.now(), @@ -531,6 +532,14 @@ async function checkTheFixedBarsNeverWrap(): Promise { ], usage: { inputTokens: 123456, outputTokens: 98765, costUsd: 1234.5678 }, }, + { + id: 'u2', seq: 3, turnId: 't2', role: 'user', ts: Date.now(), + blocks: [{ kind: 'text', text: 'and now a second turn, so the first one folds' }], + }, + { + id: 'a2', seq: 4, turnId: 't2', role: 'assistant', ts: Date.now(), + blocks: [{ kind: 'text', text: 'done' }], + }, ], pendingPermissions: [], queued: [], firstSeq: 1, replayFrom: 1, cursor: 2, live: true, bypassPermissions: true, @@ -611,6 +620,112 @@ async function checkTheFixedBarsNeverWrap(): Promise { host.remove(); } +/** + * The chat has to show the answer while it is being written. + * + * Every layer under this was already covered — the reducer applies the events, + * the transcript bumps its version, the strips re-render — and the chat still + * went blank in 5.1.2, because the list looked its messages up in a map that + * was built once and never rebuilt. Nothing short of mounting the real view + * and streaming into it catches that, so this asserts the only thing that + * actually matters: the words reach the screen. + */ +async function checkALiveAnswerAppearsAsItStreams(): Promise { + const host = document.createElement('div'); + host.style.cssText = 'width:1280px;height:760px;position:absolute;top:0;left:0;display:flex'; + document.body.appendChild(host); + + const controller = new ChatController('live-check', { send: () => {} }); + controller.handle({ + type: 'chat_snapshot', + sessionId: 'live-check', + snapshot: { + sessionId: 'live-check', runtime: 'claude', state: 'idle', + capabilities: { + streaming: true, thinking: true, toolCalls: true, diffs: true, permissions: true, + interrupt: true, resume: true, fork: false, attachments: true, usage: true, + cost: true, plan: true, commands: [], + }, + messages: [ + { + id: 'u1', seq: 1, turnId: 't1', role: 'user', ts: Date.now(), + blocks: [{ kind: 'text', text: 'the question already in the transcript' }], + }, + ], + pendingPermissions: [], queued: [], firstSeq: 1, replayFrom: 1, cursor: 1, + live: true, bypassPermissions: false, + }, + } as never); + + const root = createRoot(host); + root.render( + React.createElement(ChatView, { + controller, + runtime: 'claude', + runtimeLabel: 'Claude Code', + workingDir: '/home/dev/project', + branch: 'main', + view: { ...DEFAULT_CHAT_VIEW }, + onViewChange: () => {}, + } as never), + ); + await wait(400); + + // Exactly what arrives from the socket while an answer is being written: + // a new turn opens, then text lands in it a delta at a time. + const event = (payload: unknown): void => + controller.handle({ type: 'chat_event', sessionId: 'live-check', event: payload } as never); + + // Read from the rendered messages only. The turn strip repeats the opening + // question as its label, so asserting against the whole subtree would pass + // on the strip alone while the body it names is empty — which is the exact + // bug being guarded against. + const bodyText = (): string => + Array.from(host.querySelectorAll('[data-message-id]')) + .map((node) => node.textContent || '') + .join(' '); + + event({ t: 'msg_start', id: 'u2', seq: 2, turnId: 't2', role: 'user', ts: Date.now() }); + event({ t: 'block_start', msgId: 'u2', index: 0, block: { kind: 'text', text: '' } }); + event({ t: 'block_delta', msgId: 'u2', index: 0, text: 'a question typed while the app is open' }); + event({ t: 'msg_end', msgId: 'u2' }); + await wait(250); + + check( + 'a message sent while the app is open shows up', + bodyText().includes('a question typed while the app is open'), + 'the user\'s own turn never rendered', + ); + + event({ t: 'msg_start', id: 'a2', seq: 3, turnId: 't2', role: 'assistant', ts: Date.now() }); + event({ t: 'block_start', msgId: 'a2', index: 0, block: { kind: 'text', text: '' } }); + await wait(120); + event({ t: 'block_delta', msgId: 'a2', index: 0, text: 'the answer as it ' }); + await wait(120); + event({ t: 'block_delta', msgId: 'a2', index: 0, text: 'is being written' }); + await wait(250); + + const text = bodyText(); + check( + 'the answer is on screen before it has finished', + text.includes('the answer as it is being written'), + text.includes('the answer as it') + ? 'the first delta rendered but later ones did not' + : 'the streaming turn body stayed empty', + ); + + // And the transcript really did receive it — so a failure above is the view + // not rendering, not the events being wrong. + check( + 'the transcript itself holds the streamed message', + controller.transcript.messages.some((m: { id: string }) => m.id === 'a2'), + `ids=${controller.transcript.messages.map((m: { id: string }) => m.id).join(',')}`, + ); + + root.unmount(); + host.remove(); +} + run().catch((error: unknown) => { const pre = document.createElement('pre'); pre.id = 'results'; diff --git a/test/browser/run.js b/test/browser/run.js index fdc050c..0c8172c 100755 --- a/test/browser/run.js +++ b/test/browser/run.js @@ -11,6 +11,14 @@ const chrome = ['google-chrome', 'google-chrome-stable', 'chromium', 'chromium-b }); if (!chrome) { + // Skipping is a convenience for a machine that happens to have no browser, + // never for CI: these checks are the only thing covering defects that a + // layout engine has to be running to see, and a silent skip there would + // report a green build for a suite that never ran. + if (process.env.CI) { + console.error('No Chrome/Chromium on PATH. CI must run the browser checks, not skip them.'); + process.exit(1); + } console.log('Skipping browser checks: no Chrome/Chromium on PATH.'); process.exit(0); } diff --git a/test/chat-clear-reset.test.js b/test/chat-clear-reset.test.js index bf2e5d7..3180aa6 100644 --- a/test/chat-clear-reset.test.js +++ b/test/chat-clear-reset.test.js @@ -158,4 +158,51 @@ describe('/clear and /new actually reset the conversation', function () { assert.strictEqual(sendCalls.length, 1); assert.strictEqual(sendCalls[0].text, 'clear the table, please'); }); + + // The restart replays the options the session was launched with, and the + // model is the one thing in them that can change while the session is alive. + // Both features shipped in 5.1.2 and each one's own tests passed: the + // override was lost only where they met. + it('restarts with the model the conversation was moved to, not the one it opened with', async function () { + const { s } = session(); + s.lastStartOptions = { runtime: 'claude', workingDir: '/tmp', model: 'opened-with' }; + + // What `chat_set_model` does once the choice is persisted, whether or not + // the live adapter could take it. + s.rememberModel('moved-to'); + + let startedWith = null; + s.start = async (options) => { + startedWith = options; + s.adapter = fakeAdapter([]); + s.state = 'idle'; + }; + + await s.send({ text: '/clear' }); + + assert.strictEqual( + startedWith.model, + 'moved-to', + 'the fresh process must run the model the browser was told was applied', + ); + }); + + it('restarts on the profile default once the override is cleared', async function () { + const { s } = session(); + s.lastStartOptions = { runtime: 'claude', workingDir: '/tmp', model: 'an-override' }; + + // Clearing resolves to the profile default, so that is what arrives here. + s.rememberModel('profile-default'); + + let startedWith = null; + s.start = async (options) => { + startedWith = options; + s.adapter = fakeAdapter([]); + s.state = 'idle'; + }; + + await s.send({ text: '/clear' }); + + assert.strictEqual(startedWith.model, 'profile-default'); + }); }); diff --git a/test/chat-wiring.test.js b/test/chat-wiring.test.js index ad92737..9cd290c 100644 --- a/test/chat-wiring.test.js +++ b/test/chat-wiring.test.js @@ -38,7 +38,10 @@ function createSessionRecord(params = {}) { /** A chat manager that records what it was asked to do. */ function createChatManager(overrides = {}) { - const calls = { start: [], send: [], interrupt: [], permission: [], page: [], cancelQueued: [], setModel: [] }; + const calls = { + start: [], send: [], interrupt: [], permission: [], page: [], cancelQueued: [], + setModel: [], rememberModel: [], + }; return { calls, has: () => false, @@ -46,6 +49,9 @@ function createChatManager(overrides = {}) { calls.setModel.push({ sessionId, model }); return false; }, + rememberModel(sessionId, model) { + calls.rememberModel.push({ sessionId, model }); + }, async start(record, options) { calls.start.push({ record, options }); return { @@ -498,6 +504,75 @@ describe('chat wiring', function () { assert.strictEqual(result.model, null); }); + // A live session holds the options `/clear` will relaunch from, and the + // model is the only one this handler can change. Without carrying it over, + // the next `/clear` reinstated the model the conversation opened with. + it('carries the new model into the options a /clear restart replays', async function () { + const { processor, chatManager } = build({ surface: 'chat' }); + + await processor.handleMessage('ws-1', { type: 'chat_set_model', model: 'some-custom-model' }); + + assert.deepStrictEqual(chatManager.calls.rememberModel[0], { + sessionId: 'session-1', + model: 'some-custom-model', + }); + }); + + it('carries the profile default across when the override is cleared', async function () { + const { processor, chatManager } = build({ surface: 'chat' }); + processor.deps.resolveRuntimeProfile = () => ({ profileName: 'p', model: 'profile-default' }); + + await processor.handleMessage('ws-1', { type: 'chat_set_model', model: '' }); + + // The value itself, not just that something was carried: clearing has to + // land where a fresh launch would land, which is the profile's model and + // not the runtime's own default. + assert.deepStrictEqual(chatManager.calls.rememberModel[0], { + sessionId: 'session-1', + model: 'profile-default', + }); + }); + + // The same choice by the other door. Forwarding it untouched left the + // record unaware, so the next /clear restarted on the original model. + it('records a /model typed straight into the composer, and still forwards it', async function () { + const { processor, chatManager, session } = build({ surface: 'chat' }); + + await processor.handleMessage('ws-1', { type: 'chat_send', text: '/model haiku-3' }); + + assert.strictEqual(session.chatModelOverride, 'haiku-3'); + assert.deepStrictEqual(chatManager.calls.rememberModel[0], { + sessionId: 'session-1', + model: 'haiku-3', + }); + assert.strictEqual( + chatManager.calls.send[0].turn.text, + '/model haiku-3', + 'the runtime still has to receive its own command', + ); + }); + + it('leaves an ordinary message that merely mentions /model alone', async function () { + const { processor, chatManager, session } = build({ surface: 'chat' }); + + await processor.handleMessage('ws-1', { type: 'chat_send', text: 'what does /model do?' }); + + assert.strictEqual(session.chatModelOverride, undefined); + assert.strictEqual(chatManager.calls.rememberModel.length, 0); + }); + + it('strips control characters and caps the length of a typed model name', async function () { + const { processor, session } = build({ surface: 'chat' }); + + await processor.handleMessage('ws-1', { + type: 'chat_set_model', + model: `sneaky\nrm -rf /${'x'.repeat(400)}`, + }); + + assert.ok(!session.chatModelOverride.includes('\n'), 'a newline would become a second line of the /model turn'); + assert.ok(session.chatModelOverride.length <= 200, `stored ${session.chatModelOverride.length} characters`); + }); + it('lets a saved override outrank the profile default on the next launch', async function () { const { processor, chatManager, session } = build( { surface: 'chat' },