From afefa3e7653c763482212c989c07982c00e47ff8 Mon Sep 17 00:00:00 2001 From: dnviti Date: Sun, 26 Jul 2026 17:15:19 +0200 Subject: [PATCH] fix(chat): make /clear and /new actually reset the conversation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/clear` and `/new` forwarded their text to the still-running agent process like any other message, then only added a cosmetic marker that hid prior messages in the UI. The next message sent it right back into the same process, so the agent kept whatever context it already had — the reset never really happened, only its display did. Route clearing commands through the same restart path a manual "start fresh" relaunch already uses: stop the live adapter and start a new one with no resume id, so the next turn talks to a process that was never handed the prior conversation. This also fixes the id persisted for later reconnects, so a rejoin can't resurrect it either. Fixes #43 Co-Authored-By: Claude Sonnet 5 --- src/server/chat/session.ts | 45 +++++++--- test/chat-clear-reset.test.js | 161 ++++++++++++++++++++++++++++++++++ 2 files changed, 195 insertions(+), 11 deletions(-) create mode 100644 test/chat-clear-reset.test.js diff --git a/src/server/chat/session.ts b/src/server/chat/session.ts index 0ce29d0..bfb4868 100644 --- a/src/server/chat/session.ts +++ b/src/server/chat/session.ts @@ -150,6 +150,8 @@ export class ChatSession { private nativeSessionId: string | null = null; private startedAt = 0; private cwd = ''; + /** The options this session was last launched with, kept for `/clear` and `/new`. */ + private lastStartOptions: ChatSessionStartOptions | null = null; /** * True between resuming a conversation and the first thing the user says in it. @@ -224,6 +226,7 @@ export class ChatSession { throw new Error(`${options.runtime} has no chat adapter`); } + this.lastStartOptions = options; this.runtime = options.runtime; this.cwd = options.workingDir; this.bypass = Boolean(options.bypassPermissions); @@ -544,20 +547,40 @@ export class ChatSession { }); } this.ingest({ t: 'msg_end', msgId: messageId }); - this.setState('thinking'); - - await this.adapter.send(turn); - // After the runtime has accepted it, not before: if the send throws, the - // conversation is still there and clearing the window would have thrown - // away a transcript for a command that never ran. - // - // The marker goes in the durable log like everything else, so a browser - // that rejoins later replays the clear rather than being handed back the - // messages this was supposed to remove. + // `/clear` and `/new` promise a conversation the agent has never seen + // before, not one that only looks that way. Forwarding the text to the + // still-alive process would just add "/clear" to its own context — the + // process would still remember everything said before it. A real reset + // means a new process with no resume id, the same thing a manual "start + // fresh" relaunch already does. if (isClearingCommand(turn.text)) { - this.ingest({ t: 'marker', kind: 'cleared' }); + await this.restart(); + return; } + + this.setState('thinking'); + await this.adapter.send(turn); + } + + /** + * Stop the running adapter and start a brand new one with no resume id, in + * place, without tearing down the `ChatSession` itself. + * + * The marker that tells a rejoining browser to stop paging back past this + * point is emitted by `start()` itself (`startFresh`), so this only has to + * get a fresh process running — the same path a manual "start fresh" + * relaunch takes, just triggered from inside a live conversation instead of + * from the recovery banner. + */ + private async restart(): Promise { + const options = this.lastStartOptions; + if (!options) return; + await this.stop(); + // Stale until the new process's own `init` event reports its id — cleared + // up front so nothing reads the old conversation's id in the meantime. + this.nativeSessionId = null; + await this.start({ ...options, resumeSessionId: undefined, startFresh: true }); } async interrupt(): Promise { diff --git a/test/chat-clear-reset.test.js b/test/chat-clear-reset.test.js new file mode 100644 index 0000000..bf2e5d7 --- /dev/null +++ b/test/chat-clear-reset.test.js @@ -0,0 +1,161 @@ +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { ChatSession } = require('../dist/server/chat/session.js'); + +// Issue #43: `/clear` and `/new` looked like they reset the conversation — +// the transcript went blank — but the text was sent to the still-running +// agent process like any other message, so the very next turn brought the +// old context right back. A real reset has to stop the process that +// remembers and start a new one; these pin that down at the point the bug +// lived, `ChatSession.send`, without spawning a real CLI. + +function memoryStore() { + const events = []; + return { + events, + append(_ref, batch) { + events.push(...batch); + }, + async stat() { + return { firstSeq: 1, cursor: events.length }; + }, + async read() { + return { events: [], firstSeq: 1, from: 1, cursor: events.length }; + }, + }; +} + +function fakeAdapter(sendCalls) { + return { + alive: true, + async send(turn) { + sendCalls.push(turn); + }, + async interrupt() {}, + respondPermission() {}, + async stop() {}, + }; +} + +function session() { + const store = memoryStore(); + const s = new ChatSession( + { id: 's1', ownerUserId: 7 }, + { + store, + socketDir: fs.mkdtempSync(path.join(os.tmpdir(), 'clear-reset-')), + hookScript: path.join(__dirname, '..', 'does-not-exist.js'), + broadcast: () => {}, + resolveCommand: () => 'claude', + }, + ); + const sendCalls = []; + s.adapter = fakeAdapter(sendCalls); + s.state = 'idle'; + // Stands in for whatever `start()` was last called with — set directly + // because these tests stub the adapter rather than spawning a real one. + s.lastStartOptions = { runtime: 'claude', workingDir: '/tmp' }; + return { s, store, sendCalls }; +} + +describe('/clear and /new actually reset the conversation', function () { + it('never forwards the clearing command to the live adapter', async function () { + const { s, sendCalls } = session(); + s.start = async () => { + s.adapter = fakeAdapter([]); + s.state = 'idle'; + }; + + await s.send({ text: '/clear' }); + + assert.deepStrictEqual( + sendCalls, + [], + 'the process that already holds the old context must never see this turn', + ); + }); + + it('stops the running adapter and starts a fresh one with no resume id', async function () { + const { s } = session(); + const startCalls = []; + const stopCalls = []; + const originalAdapter = s.adapter; + originalAdapter.stop = async () => stopCalls.push('stopped'); + s.start = async (options) => { + startCalls.push(options); + s.adapter = fakeAdapter([]); + s.state = 'idle'; + }; + + await s.send({ text: '/new' }); + + assert.strictEqual(stopCalls.length, 1, 'the old process must be torn down, not reused'); + assert.strictEqual(startCalls.length, 1); + assert.strictEqual(startCalls[0].runtime, 'claude'); + assert.strictEqual(startCalls[0].resumeSessionId, undefined, 'a resume would hand the new process the old context back'); + assert.strictEqual(startCalls[0].startFresh, true); + }); + + it('is case-insensitive and ignores arguments, matching isClearingCommand', async function () { + const { s, sendCalls } = session(); + let started = 0; + s.start = async () => { + started += 1; + s.adapter = fakeAdapter([]); + s.state = 'idle'; + }; + + await s.send({ text: '/CLEAR now please' }); + + assert.strictEqual(started, 1); + assert.deepStrictEqual(sendCalls, []); + }); + + it('still records the user turn in the transcript before resetting', async function () { + const { s, store } = session(); + s.start = async () => { + s.adapter = fakeAdapter([]); + s.state = 'idle'; + }; + + await s.send({ text: '/clear' }); + + const texts = store.events + .filter((e) => e.t === 'block_start') + .map((e) => e.block.text); + assert.deepStrictEqual(texts, ['/clear']); + }); + + it('clears the stale native session id before the new process reports its own', async function () { + const { s } = session(); + s.ingest({ t: 'session', nativeSessionId: 'native-old', capabilities: {} }); + assert.strictEqual(s.nativeId, 'native-old'); + + s.start = async () => { + // The new process has not announced itself yet. + s.adapter = fakeAdapter([]); + s.state = 'idle'; + }; + + await s.send({ text: '/new' }); + + assert.strictEqual(s.nativeId, null, 'must not still point at the pre-clear conversation'); + }); + + it('leaves an ordinary message alone', async function () { + const { s, sendCalls } = session(); + let started = 0; + s.start = async () => { + started += 1; + }; + + await s.send({ text: 'clear the table, please' }); + + assert.strictEqual(started, 0, 'not a slash command, so no restart'); + assert.strictEqual(sendCalls.length, 1); + assert.strictEqual(sendCalls[0].text, 'clear the table, please'); + }); +});