Merge pull request #77 from levelcodeai/fix/unhandled-focus-rejections

fix(chat): every reveal of the chat view handles its own rejection

Author: ndemianc

extensions/levelcode-ai/extension.js

@@ -416,11 +416,37 @@ function captureSelection() {
 	};
 }
 
+/**
+ * Reveal the chat view for its side effect only, and never reject.
+ *
+ * `executeCommand` returns a Thenable, so a bare call in a void context turns any rejection into an
+ * unhandled promise rejection in the extension host — noisy, and attributed to nothing in particular,
+ * which is the part that makes it useless.
+ *
+ * Every caller here is a BACKGROUND reveal: the work it accompanies (a selection added, a session
+ * resumed, a login launched) has already succeeded by the time this runs. Failing that work because
+ * the panel would not come forward would be worse than the panel not coming forward.
+ *
+ * Logged, never swallowed. `.catch(() => {})` would hide the one failure that is genuinely hard to
+ * diagnose — a chat surface that silently never appears — so `why` names the caller in the log.
+ *
+ * NOT for a command handler whose whole job IS the reveal: `levelcode.ai.focus` returns the thenable
+ * instead, so VS Code reports the failure to the user who asked for it. See moveChatToSidebar.
+ */
+function focusChatView(why) {
+	return Promise.resolve(vscode.commands.executeCommand('levelcodeAi.chat.focus'))
+		.then(undefined, (e) => {
+			const msg = String((e && e.message) || e);
+			console.warn('[levelcode-ai] chat.focus.failed', { why, msg });
+			dbg('chat.focus.failed', { why, msg });
+		});
+}
+
 function addSelection() {
 	const sel = captureSelection();
 	if (!sel) { vscode.window.showInformationMessage('LevelCode AI: select some code first.'); return; }
 	pendingContext = sel.block;
-	vscode.commands.executeCommand('levelcodeAi.chat.focus');
+	focusChatView('addSelection');
 	post({ type: 'context', label: sel.label });
 }
 
@@ -465,7 +491,7 @@ async function addContext() {
 			}
 		}
 	}
-	vscode.commands.executeCommand('levelcodeAi.chat.focus');
+	focusChatView('addContext');
 	postContextFiles();
 }
 
@@ -1052,7 +1078,7 @@ async function resumeSession(id) {
 	post({ type: 'sessionResumed', id, title: (r.entry && r.entry.title) || 'Session', note: r.note || '', tier: r.plan && r.plan.tier, turns });
 	postContextFiles();
 	refreshSessions();                          // the resumed session bumps to the top — keep both surfaces current
-	vscode.commands.executeCommand('levelcodeAi.chat.focus');
+	focusChatView('resumeSession');
 	dbg('sessions.resumed', { id, tier: r.plan && r.plan.tier, restored: agentMessages.length, shown: turns.length });
 }
 
@@ -2295,7 +2321,7 @@ async function openChatInEditor(opts) {
 			// resolveWebviewView then makes it live, and without this the chat would have no surface at all.
 			activeWebview = undefined;
 			pendingTranscriptReplay = 'Back in the sidebar';
-			vscode.commands.executeCommand('levelcodeAi.chat.focus');
+			focusChatView('editorClosed');
 		}
 		dbg('chat.closedEditor', {});
 	});
@@ -2432,7 +2458,7 @@ class SessionsViewProvider {
 				// The real session index for this workspace (empty on a fresh install — the view shows its
 				// own empty state). Posted to THIS view's webview, not the chat's.
 				case 'listSessions': view.webview.postMessage({ type: 'sessions', entries: sessionList() }); break;
-				case 'newSession': newChat(); vscode.commands.executeCommand('levelcodeAi.chat.focus'); break;
+				case 'newSession': newChat(); focusChatView('sessions.newSession'); break;
 				case 'sessionAction': await handleSessionAction(msg.action, msg.id); break;
 				// Memory tab (§M3): list the recorded outcomes + facts; act on them; open the file.
 				case 'listMemory': { const mm = sessionsManager(); view.webview.postMessage({ type: 'memoryList', items: mm ? mm.memoryItems() : [], facts: mm ? mm.factsList() : [] }); break; }
@@ -2528,7 +2554,7 @@ async function postAccount(open) {
 // second login and no interceptable code ever travels through the custom scheme.
 async function handleLaunch() {
 	dbg('account.launch', {});
-	vscode.commands.executeCommand('levelcodeAi.chat.focus');
+	focusChatView('account.launch');
 	const token = ctx ? await ctx.secrets.get(ACCOUNT_TOKEN_KEY) : null;
 	if (!token) { await accountSignIn(); }
 }

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

@@ -144,7 +144,9 @@ test('RESTORE: a sidebar that was never resolved is revealed rather than assumed
 	// 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.
 	const open = fnBody(ext, 'openChatInEditor');
-	assert.match(open, /if \(sidebarChatView\) \{[\s\S]*\} else \{[\s\S]*levelcodeAi\.chat\.focus/,
+	// 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');
 });
 
@@ -310,4 +312,46 @@ test('MOVE BACK: there is a button on the tab, and it reuses the dispose hand-ov
 	assert.match(body, /levelcodeAi\.chat\.focus/, 'with no panel open the command must still reveal the chat, not do nothing');
 });
 
+test('FOCUS: no reveal of the chat view is left to reject unhandled', () => {
+	// One guard for the whole class, rather than six assertions that each name a function. `executeCommand`
+	// returns a Thenable, so a bare call in a void context makes any rejection an unhandled promise
+	// rejection in the extension host — attributed to nothing, which is what makes it useless.
+	//
+	// Scanning every call site means the NEXT one is covered too. That matters here: this pattern was
+	// copied into six places over time precisely because nothing was watching for it.
+	const CALL = "vscode.commands.executeCommand('levelcodeAi.chat.focus')";
+	const bare = [];
+	for (let i = ext.indexOf(CALL); i >= 0; i = ext.indexOf(CALL, i + 1)) {
+		const before = ext.slice(Math.max(0, i - 40), i);
+		const after = ext.slice(i + CALL.length, i + CALL.length + 40);
+		const handled = /\breturn\s+$/.test(before)         // returned — a command handler VS Code awaits
+			|| /\bawait\s+$/.test(before)                    // awaited by a caller that catches
+			|| /=>\s*$/.test(before)                         // concise arrow body: also a return
+			|| /Promise\.resolve\($/.test(before)            // wrapped by focusChatView
+			|| /^\s*\)?\s*\.(then|catch)\(/.test(after);     // handled inline
+		if (!handled) { bare.push('line ' + ext.slice(0, i).split('\n').length); }
+	}
+	assert.deepStrictEqual(bare, [],
+		'these reveals are fire-and-forget — a rejection becomes an unhandled promise rejection.\n'
+		+ 'Use focusChatView(why) for a background reveal, or `return` it when the command IS the reveal:\n  '
+		+ bare.join('\n  '));
+});
+
+test('FOCUS: the shared helper logs the failure and names who caused it', () => {
+	// The whole complaint was "attributed to nothing", so swallowing it silently would answer the letter
+	// of the review and none of it. A chat surface that never appears, with no trace, is the failure
+	// that costs an afternoon.
+	const body = fnBody(ext, 'focusChatView');
+	assert.match(body, /dbg\('chat\.focus\.failed'/, 'the failure is not logged — .catch(() => {}) is not a fix');
+	assert.match(body, /\bwhy\b/, 'the log must name the caller, or it is as unattributed as the rejection was');
+	assert.ok(!/\bthrow\b/.test(body), 'the helper must not rethrow — every caller uses it in a void context');
+	// Either `.then(undefined, …)` or `.catch(…)`. They are equivalent here and pinning one would fail a
+	// refactor that changes nothing; what must not disappear is the rejection handler itself.
+	assert.match(body, /\.then\(undefined,|\.catch\(/, 'no rejection handler — the helper can still reject');
+
+	// And it must be the thing the background callers actually use.
+	const callers = (ext.match(/focusChatView\('/g) || []).length;
+	assert.ok(callers >= 6, 'expected the background reveals to route through the helper, found ' + callers);
+});
+
 console.log('\nchatSurface: ' + n + ' tests passed.');