fix(chat): closing the tab tears the conversation down, not just seals it

Both review points on #82. They landed on that PR's stale diff — I branched the wordmark
off feat/chat-editor-only instead of develop, so #82 carried #81's commits until #81
merged. Both are real, and both are about code that is now on develop, so they are fixed
here rather than in the wordmark PR (whose diff is now the two files it should always
have been).

1. SEALING WAS ONLY HALF THE TEARDOWN

sealLiveSession ends the SESSION — liveId() goes null, so the next chat opens visually
empty — while `conversation` and `agentMessages` still held every previous turn. The next
message therefore shipped the old history to the model. An empty-looking chat that secretly
remembers is worse than either honest option.

The out-of-process half was worse: background commands and MCP servers are DETACHED
children, so they outlived the surface that was reporting on them, and an in-flight agent
run kept editing files with nothing left to show for it.

newChat's teardown moves into resetConversationState() and the close path calls it. One
implementation, or the close path drifts — and it is the path nobody watches. It
deliberately does NOT post: New Chat re-renders afterwards because it has a surface to
re-render; the close path is tearing one down.

2. A LEGACY `secondarySidebar` SETTING PRODUCED A LYING LOG

The value was valid until the chat became editor-only, so it is still sitting in real
settings.json files. chatStartLocation still accepted it, so revealChatAtStartup logged
`where: secondarySidebar` and then opened the editor tab. Right behaviour, wrong story —
and the accepted set no longer matched the enum the package ships. Now mapped explicitly.

Guards, each bypass-verified by reverting the fix:
  - the close sealing but leaving history loaded (the reported bug)
  - conversation, checkpoints, abort, reapCommands and reapMcp each removed individually
  - newChat growing its own copy of the teardown
  - the shared teardown starting to post, which is wrong on the close path
  - the legacy value no longer mapped; the removed surface accepted again

Two bypasses initially looked like misses: commenting out `reapMcp()` and neutering the
abort with `if (false)` both left the strings present, and these are presence checks. Redone
as deletions — which is how they would actually regress — they fail correctly. Worth stating
plainly: these guards catch removal, not disabling.

21 tests in chatSurface, 34 suites green.

Author: ndemianc

extensions/levelcode-ai/extension.js

@@ -1106,19 +1106,39 @@ function sealLiveSession(why) {
 	}
 }
 
-function newChat() {
-	// Seal the outgoing session (its terminal state + a final index row) BEFORE the transcript is cleared,
-	// so it lands in History as a finished session and the next turn opens a fresh one.
-	sealLiveSession('newChat');
+/**
+ * Drop everything the finished conversation was holding, in memory and out of process.
+ *
+ * Extracted so the CLOSING TAB does the same teardown New Chat does. Sealing alone was not enough and
+ * left the two halves disagreeing: `sealLiveSession` ends the session, so `liveId()` goes null and the
+ * next chat opens visually empty — while `conversation` and `agentMessages` still hold every previous
+ * turn, so the next message silently ships the old history to the model. An empty-looking chat that
+ * secretly remembers is worse than either honest option.
+ *
+ * The out-of-process half matters just as much. Background commands and MCP servers are DETACHED
+ * children: without reaping them they outlive the surface that was reporting on them, and an
+ * in-flight agent run keeps editing files with nothing left to show for it.
+ *
+ * Deliberately does NOT post to the webview. New Chat re-renders afterwards because it has a surface
+ * to re-render; the close path is tearing one down.
+ */
+function resetConversationState() {
 	conversation = [];
 	agentMessages = [];
 	checkpoints.length = 0; currentCheckpoint = null;   // drop the per-turn restore stack
 	pendingContext = null;
 	contextFiles = [];
-	if (abort) { abort.abort(); }
+	if (abort) { abort.abort(); }          // stop an in-flight run — its surface is going away
 	reapCommands();                        // kill any background servers/watchers from the old session
 	reapMcp();                             // …and any MCP servers: they are detached children too
-	if (review) { review.finalizeAll(); } // drop review UI without reverting the user's files
+	if (review) { review.finalizeAll(); }  // drop review UI without reverting the user's files
+}
+
+function newChat() {
+	// Seal the outgoing session (its terminal state + a final index row) BEFORE the transcript is cleared,
+	// so it lands in History as a finished session and the next turn opens a fresh one.
+	sealLiveSession('newChat');
+	resetConversationState();
 	post({ type: 'reset' });
 	postContextFiles();
 	postMemoryDigest();                    // the fresh empty state shows the welcome-back strip
@@ -2341,6 +2361,7 @@ async function openChatInEditor(opts) {
 		chatEditorPanel = undefined;
 		activeWebview = undefined;      // nothing may post into a disposed webview
 		sealLiveSession('chatClosed');
+		resetConversationState();       // …and nothing may survive into the next one
 		dbg('chat.closedEditor', {});
 	});
 }
@@ -2358,7 +2379,12 @@ async function openChatInEditor(opts) {
  */
 function chatStartLocation() {
 	const raw = String(aiConfig().get('chat.startLocation', 'editor') || 'editor');
-	return ['editor', 'secondarySidebar', 'none'].includes(raw) ? raw : 'editor';
+	// `secondarySidebar` was a valid value until the chat became editor-only, so it is still sitting in
+	// real settings.json files. Mapped explicitly rather than left to fall through the unknown-value
+	// path: the result is the same, but this way the debug log names the location we actually opened
+	// instead of reporting a surface that no longer exists.
+	if (raw === 'secondarySidebar') { return 'editor'; }
+	return ['editor', 'none'].includes(raw) ? raw : 'editor';
 }
 
 /** Open the chat where `chat.startLocation` says, once, as the window finishes starting. */

extensions/levelcode-ai/test/chatSurface.test.js

@@ -218,7 +218,7 @@ test('START: the chat opens centred by default, and the setting is the only plac
 
 	// One reader, so a second caller cannot quietly disagree about what an unknown value means.
 	const body = fnBody(ext, 'chatStartLocation');
-	assert.match(body, /'editor', 'secondarySidebar', 'none'/, 'the reader no longer validates against the enum');
+	assert.match(body, /'editor', 'none'/, 'the reader must validate against exactly the enum the package ships');
 	assert.match(body, /: 'editor'/, 'an unknown value must fall back to the default, not leave the window with no chat');
 });
 
@@ -348,4 +348,57 @@ test('CLOSE: nothing resurrects the chat on the right', () => {
 		'Sessions must KEEP its view — it is the right-hand container\'s reason to exist');
 });
 
+test('CLOSE: the conversation is torn down, not just sealed', () => {
+	// Review caught the two halves disagreeing. sealLiveSession ends the SESSION — liveId() goes null,
+	// so the next chat opens visually empty — while `conversation` and `agentMessages` still held every
+	// previous turn, so the next message shipped the old history to the model anyway. An empty-looking
+	// chat that secretly remembers is worse than either honest option.
+	const dispose = ext.slice(ext.indexOf('panel.onDidDispose'), ext.indexOf('panel.onDidDispose') + 1400);
+	assert.match(dispose, /sealLiveSession\('chatClosed'\)/, 'closing no longer seals the session');
+	assert.match(dispose, /resetConversationState\(\)/,
+		'closing seals but leaves conversation/agentMessages loaded — the next send replays the old history');
+
+	// ONE teardown, shared with New Chat, or the close path drifts — and it is the path nobody watches.
+	assert.match(fnBody(ext, 'newChat'), /resetConversationState\(\)/,
+		'New Chat has its own copy of the teardown again');
+	const reset = fnBody(ext, 'resetConversationState');
+	for (const [frag, why] of [
+		['conversation = []', 'the model history survives the close'],
+		['agentMessages = []', 'the agent history survives the close'],
+		['checkpoints.length = 0', 'the restore stack still points at a finished turn'],
+		['contextFiles = []', 'stale attachments carry into the next chat'],
+		['abort.abort()', 'an in-flight run keeps going with no surface to report to'],
+		['reapCommands()', 'background commands outlive the chat — they are detached children'],
+		['reapMcp()', 'MCP servers outlive the chat — they are detached children too']
+	]) {
+		assert.ok(reset.includes(frag), why + ' (missing: ' + frag + ')');
+	}
+
+	// It must NOT post: the close path is tearing the surface down, and New Chat re-renders itself.
+	assert.ok(!/\bpost\(/.test(reset),
+		'resetConversationState posts to the webview — on the close path that webview is being disposed');
+});
+
+test('START: a legacy secondarySidebar setting maps to the editor, and says so', () => {
+	// The value was valid until the chat became editor-only, so it is still sitting in real
+	// settings.json files. Left to fall through the unknown-value path it produced the right BEHAVIOUR
+	// with a lying debug log — `where: secondarySidebar` while opening the editor tab.
+	const body = fnBody(ext, 'chatStartLocation');
+	assert.match(body, /raw === 'secondarySidebar'/, 'the legacy value is not mapped explicitly');
+	assert.ok(!/\['editor', 'secondarySidebar', 'none'\]/.test(body),
+		'secondarySidebar is still an accepted value — it names a surface that no longer exists');
+	assert.match(body, /\['editor', 'none'\]/, 'the accepted set should be exactly what the enum ships');
+
+	// Behaviour, evaluated from the SHIPPED source rather than a copy of it: fnBody hands back the
+	// braces, so wrapping it in a declaration gives the real function with aiConfig injected.
+	// aiConfig is CALLED and returns the config object, so the stub has to be a function that returns
+	// one — passing the object itself is the obvious thing and it is wrong.
+	const run = (v) => new Function('aiConfig',
+		'function chatStartLocation() ' + body + '\nreturn chatStartLocation();')(() => ({ get: () => v }));
+	assert.strictEqual(run('secondarySidebar'), 'editor', 'a legacy setting must resolve to the editor');
+	assert.strictEqual(run('none'), 'none', 'the opt-out must survive');
+	assert.strictEqual(run('editor'), 'editor');
+	assert.strictEqual(run('nonsense'), 'editor', 'an unknown value must fall back to the default');
+});
+
 console.log('\nchatSurface: ' + n + ' tests passed.');