fix(chat): closing the chat tab closes it, instead of reopening on the right

Regression from #76, reported from a real build. A MOVE and a CLOSE both end in
panel.dispose(), and onDidDispose treated them identically — revealing the sidebar every
time. That was correct while the sidebar was home: surrendering the tab meant going home.

With chat.startLocation defaulting to `editor` it inverts into a trap. Closing the tab
pops the panel open on the right, and there is no way to put the chat away at all: close
the tab and the panel appears, close the panel and it returns with the next window. The
`else` branch was the sharp end — with no resolved sidebar view (the normal case now,
since the chat opens centred and nobody opens the secondary sidebar) it called
focusChatView() unconditionally, which is precisely what revealed the panel.

Dispose cannot see the difference on its own, so moveChatToSidebar announces itself with a
flag that dispose reads once and clears. A plain close now closes.

WHAT STILL HAPPENS ON A CLOSE: if a sidebar view is already resolved it still becomes live,
because otherwise the conversation would be live nowhere while a resolved view sat there
showing a stale hand-over card. Only the REVEAL is conditional — making a hidden view live
costs nothing and is correct the moment the user opens it.

Two existing tests asserted the old behaviour and were updated deliberately, not
mechanically:
  - "a sidebar that was never resolved is revealed rather than assumed" asserted the reveal
    on EVERY close. That was the bug, stated as a requirement. It now covers the move.
  - the activeWebview invariant pinned an exact list of assignments, so adding a second
    RESET read as a regression. It now says what it means: exactly one place points it at a
    live surface, and at least one releases it.

Guards, each bypass-verified by reverting the fix:
  - the resolved view force-opened on a plain close
  - the no-view reveal escaping the `moving` branch (the reported bug)
  - the flag never cleared, so the NEXT close reveals too
  - dispose unable to see a move at all
  - the flag set AFTER dispose, so the move reads it stale
  - the resolved view never becoming live, which would leave the chat live nowhere

NOT in this change, both reported alongside it:
  - The untitled tab at startup is not ours — LevelCode has no `workbench.startupEditor`
    override anywhere in its source. That is the editor default or a user setting.
  - The "ResizeObserver loop" console spam is INFO-level noise from VS Code's own webview
    harness; we register no ResizeObserver. I could not reproduce a loop: a harness running
    the real stylesheet at 280/320/340/360px, with the ASCII logo's clamp(5px, 3.6cqi, 13px)
    genuinely in its variable range, reported 1 resize callback and 0 loop errors. Fixing
    this bug removes the reported scenario anyway, since the sidebar view is no longer
    force-opened. Not shipping a speculative CSS change for something I have not reproduced.

25 tests in chatSurface, 34 suites green.

Author: ndemianc

extensions/levelcode-ai/extension.js

@@ -58,6 +58,9 @@ let sidebarChatView;    // the contributed view, so the panel can hand the slot
 /** @type {vscode.WebviewPanel | undefined} */
 let chatEditorPanel;    // set only while the chat is open as an editor tab
 let chatProvider;       // the single provider instance; both surfaces wire through it
+// Closing the tab and MOVING the chat both end in panel.dispose(), and they must not mean the same
+// thing. Set only by moveChatToSidebar, read once by onDidDispose, cleared immediately.
+let movingChatToSidebar = false;
 // The visible transcript lives in the webview's DOM, so swapping surfaces would blank it. Set before
 // handing over; the freshly-loaded surface replays on its `ready`, which is the first moment it can
 // receive anything at all.
@@ -2311,19 +2314,37 @@ async function openChatInEditor(opts) {
 	dbg('chat.openInEditor', {});
 
 	panel.onDidDispose(() => {
+		// A MOVE and a CLOSE both land here. Until the chat opened centred by default they were the same
+		// thing — the sidebar was home, so surrendering the tab meant going home — and this handler
+		// revealed the sidebar unconditionally. With `chat.startLocation: editor` that turns ⌘W into
+		// "reopen on the right", and there is no way to put the chat away at all: close the tab, the
+		// panel appears; close the panel, it is still bound to come back next time.
+		//
+		// So the reveal now happens ONLY for a deliberate move. A plain close closes.
+		const moving = movingChatToSidebar;
+		movingChatToSidebar = false;
 		chatEditorPanel = undefined;
+
 		if (sidebarChatView) {
+			// The view exists, so hand the conversation back to it either way — otherwise the chat would
+			// be live nowhere while a resolved view sits there showing a stale hand-over card. Only the
+			// REVEAL is conditional: making a hidden view live costs nothing and is correct the moment
+			// the user opens it.
 			pendingTranscriptReplay = 'Back in the sidebar';
 			chatProvider.makeLive(sidebarChatView.webview);
-			sidebarChatView.show?.(true);
-		} else {
-			// The view was never resolved (the container has not been opened this session). Reveal it —
-			// resolveWebviewView then makes it live, and without this the chat would have no surface at all.
+			if (moving) { sidebarChatView.show?.(true); }
+		} else if (moving) {
+			// Moving with no resolved view: reveal it, and resolveWebviewView makes it live on arrival.
 			activeWebview = undefined;
 			pendingTranscriptReplay = 'Back in the sidebar';
-			focusChatView('editorClosed');
+			focusChatView('movedToSidebar');
+		} else {
+			// Plain close, nothing resolved: the chat has no surface, which is exactly what was asked
+			// for. Drop the reference so nothing posts into a disposed webview; reopening from the
+			// command, the sidebar, or the next window restores it.
+			activeWebview = undefined;
 		}
-		dbg('chat.closedEditor', {});
+		dbg('chat.closedEditor', { moving });
 	});
 }
 
@@ -2336,7 +2357,13 @@ async function openChatInEditor(opts) {
  * hand-over changed.
  */
 function moveChatToSidebar() {
-	if (chatEditorPanel) { chatEditorPanel.dispose(); return undefined; }
+	if (chatEditorPanel) {
+		// Tell onDidDispose this is a MOVE. Disposing is how the move is performed, so without this flag
+		// the hand-over cannot tell it apart from the user simply closing the tab.
+		movingChatToSidebar = true;
+		chatEditorPanel.dispose();
+		return undefined;
+	}
 	// Already there (or never moved) — just reveal it, so the command is never a silent no-op.
 	//
 	// RETURNED, not fired and forgotten. `registerCommand` awaits whatever the handler returns, so a

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

@@ -61,9 +61,14 @@ function fnBody(src, name) {
 test('SURFACE: only makeLive() ever moves the conversation, so two surfaces cannot both be live', () => {
 	// `post()` writes to activeWebview. If anything else assigned it, a hand-over could leave the
 	// pointer on a webview the user is no longer looking at — messages vanish into a hidden DOM.
+	// Stated as an invariant rather than an exact list: the number of RESETS is allowed to grow (the
+	// dispose path now has one per close route), but the number of places that point it at a live
+	// surface is not. Pinning the whole list meant adding a reset looked like a regression.
 	const writes = [...ext.matchAll(/activeWebview\s*=\s*([^;]+);/g)].map((m) => m[1].trim());
-	assert.deepStrictEqual(writes.sort(), ['undefined', 'webview'],
-		'activeWebview is assigned somewhere other than makeLive()/the dispose reset: ' + writes.join(' | '));
+	const live = writes.filter((w) => w !== 'undefined');
+	assert.deepStrictEqual(live, ['webview'],
+		'activeWebview is pointed at a surface somewhere other than makeLive(): ' + writes.join(' | '));
+	assert.ok(writes.length > live.length, 'nothing ever releases activeWebview — a disposed webview stays addressable');
 	assert.match(fnBody(ext, 'openChatInEditor'), /chatProvider\.makeLive\(panel\.webview\)/,
 		'the panel never becomes the live surface');
 });
@@ -140,14 +145,18 @@ test('RESTORE: closing the tab and "Bring it back" are the SAME path', () => {
 	assert.match(open, /chatEditorPanel = undefined/, 'the panel ref outlives the panel');
 });
 
-test('RESTORE: a sidebar that was never resolved is revealed rather than assumed', () => {
+test('RESTORE: a MOVE to a sidebar that was never resolved reveals it rather than assuming it', () => {
 	// If the container has not been opened this session, sidebarChatView is undefined — restoring by
-	// writing to it would throw, and doing nothing would leave the chat with no surface at all.
+	// writing to it would throw, and for a MOVE, doing nothing would leave the chat with no surface at
+	// all after the user explicitly asked for it on the right.
+	//
+	// This used to assert the reveal happened on EVERY close, which was right while the sidebar was
+	// home and became a bug the moment the editor became the default: it turned closing the tab into
+	// reopening the panel. The reveal is now scoped to the move, and the plain-close half is pinned by
+	// the CLOSE tests below.
 	const open = fnBody(ext, 'openChatInEditor');
-	// The reveal now goes through focusChatView() so its rejection cannot go unhandled; what this test
-	// cares about is unchanged — the else-branch must still reveal the view rather than assume it.
-	assert.match(open, /if \(sidebarChatView\) \{[\s\S]*\} else \{[\s\S]*focusChatView\(/,
-		'the never-resolved sidebar case is unhandled');
+	assert.match(open, /if \(sidebarChatView\) \{[\s\S]*\} else if \(moving\) \{[\s\S]*focusChatView\(/,
+		'a move with no resolved sidebar view no longer reveals it — the chat would land nowhere');
 });
 
 test('RESTORE: while detached, the sidebar shows the hand-off card, not a second chat', () => {
@@ -354,4 +363,44 @@ test('FOCUS: the shared helper logs the failure and names who caused it', () =>
 	assert.ok(callers >= 6, 'expected the background reveals to route through the helper, found ' + callers);
 });
 
+test('CLOSE: closing the tab closes the chat — it does not reopen on the right', () => {
+	// The regression this pins, reported from a real build: with chat.startLocation defaulting to the
+	// editor, onDidDispose revealed the sidebar unconditionally. Closing the tab therefore POPPED THE
+	// PANEL OPEN on the right, and there was no way to put the chat away at all — close the tab, the
+	// panel appears; close the panel, it comes back with the next window.
+	//
+	// A move and a close both end in panel.dispose(), so they are told apart by an explicit flag rather
+	// than by anything dispose itself can see.
+	const dispose = ext.slice(ext.indexOf('panel.onDidDispose'), ext.indexOf('panel.onDidDispose') + 1800);
+	assert.match(dispose, /const moving = movingChatToSidebar;/, 'dispose cannot tell a move from a close');
+	assert.match(dispose, /movingChatToSidebar = false;/, 'the flag must be cleared, or the NEXT close reveals too');
+
+	// The two reveals are the whole bug. Both must now be conditional.
+	assert.match(dispose, /if \(moving\) \{ sidebarChatView\.show\?\.\(true\); \}/,
+		'a plain close still forces the resolved sidebar view open');
+	// The no-view path is the one that actually bit: with nothing resolved, dispose used to call
+	// focusChatView() unconditionally, which is what POPPED THE PANEL OPEN. It must now sit inside the
+	// `moving` branch — checked positionally, because that is the property, and a regex trying to
+	// describe the surrounding block shape is how the first version of this assertion broke.
+	const revealIdx = dispose.indexOf('focusChatView(');
+	const movingBranchIdx = dispose.indexOf('else if (moving)');
+	assert.ok(movingBranchIdx > 0, 'the no-view close path is no longer split on `moving`');
+	assert.ok(revealIdx > movingBranchIdx,
+		'focusChatView is reachable on a plain close — closing the tab reopens the chat on the right');
+
+	// …and a close must still release the surface, or messages post into a disposed webview.
+	assert.match(dispose, /activeWebview = undefined;/, 'the disposed webview is still referenced');
+});
+
+test('CLOSE: a deliberate move still hands the conversation over', () => {
+	// The other half — it would be easy to fix the close by making the move stop working.
+	assert.match(fnBody(ext, 'moveChatToSidebar'), /movingChatToSidebar = true;[\s\S]{0,120}chatEditorPanel\.dispose\(\)/,
+		'the move must announce itself BEFORE disposing, or dispose reads a stale flag');
+	const dispose = ext.slice(ext.indexOf('panel.onDidDispose'), ext.indexOf('panel.onDidDispose') + 1800);
+	assert.match(dispose, /chatProvider\.makeLive\(sidebarChatView\.webview\)/,
+		'a resolved sidebar view must still become live, or the chat is live nowhere');
+	assert.match(dispose, /pendingTranscriptReplay = 'Back in the sidebar'/,
+		'the transcript must still replay into whichever surface takes over');
+});
+
 console.log('\nchatSurface: ' + n + ' tests passed.');