Skip to content

Commit 1a968f2

Browse files
committed
fix(chat): every reveal of the chat view handles its own rejection
Follow-up to #76, which fixed the two instances in the code it introduced and left the older ones alone to keep that review diff readable. `executeCommand` returns a Thenable, so a bare call in a void context turns any rejection into an unhandled promise rejection in the extension host — attributed to nothing, which is the part that makes it useless. SIX SITES, NOT SEVEN. The brief for this change listed the `levelcode.ai.focus` registration as needing a `return`. It does not: `() => vscode.commands.executeCommand(…)` is a concise arrow body, so it already returns the thenable and VS Code already reports its failures. Left alone. The other six are all BACKGROUND reveals — addSelection, addContext, resumeSession, the editor panel's onDidDispose, the sessions view's newSession, and handleLaunch. In each the accompanying work has already succeeded by the time the reveal runs, so failing that work because the panel would not come forward would be worse than the panel not coming forward. They route through one `focusChatView(why)` helper that logs and never rejects. Logged, not swallowed: the complaint was "attributed to nothing", so `.catch(() => {})` would answer the letter of it and none of the substance. `why` names the caller in the log. ONE GUARD FOR THE WHOLE CLASS. Rather than six assertions naming six functions, the test scans every occurrence of the call and accepts only returned / awaited / concise-arrow / helper-wrapped / inline-handled forms. That covers the seventh site and the eighth nobody has written yet — which matters, because this pattern reached six copies precisely because nothing was watching for it. Verified non-vacuous against develop: the same scan reports exactly the six bare sites there (lines 423, 468, 1055, 2298, 2435, 2531) and zero here. Bypasses, each reverted and confirmed to fail: - a single site returned to a bare call; the switch-case site returned to a bare call - the focus command wrapped in a block body so it stops returning (proves the guard protects the site that was already correct) - the helper swallowing silently; the log dropping its caller attribution One assertion deliberately relaxed after bypassing it: pinning `.then(undefined, …)` over `.catch(…)` failed a refactor with no behavioural difference. It now accepts either and still fails when the handler stops logging. 23 tests in chatSurface, 32 suites green.
1 parent b7b4472 commit 1a968f2

2 files changed

Lines changed: 73 additions & 7 deletions

File tree

extensions/levelcode-ai/extension.js

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -416,11 +416,33 @@ function captureSelection() {
416416
};
417417
}
418418

419+
/**
420+
* Reveal the chat view for its side effect only, and never reject.
421+
*
422+
* `executeCommand` returns a Thenable, so a bare call in a void context turns any rejection into an
423+
* unhandled promise rejection in the extension host — noisy, and attributed to nothing in particular,
424+
* which is the part that makes it useless.
425+
*
426+
* Every caller here is a BACKGROUND reveal: the work it accompanies (a selection added, a session
427+
* resumed, a login launched) has already succeeded by the time this runs. Failing that work because
428+
* the panel would not come forward would be worse than the panel not coming forward.
429+
*
430+
* Logged, never swallowed. `.catch(() => {})` would hide the one failure that is genuinely hard to
431+
* diagnose — a chat surface that silently never appears — so `why` names the caller in the log.
432+
*
433+
* NOT for a command handler whose whole job IS the reveal: `levelcode.ai.focus` returns the thenable
434+
* instead, so VS Code reports the failure to the user who asked for it. See moveChatToSidebar.
435+
*/
436+
function focusChatView(why) {
437+
return Promise.resolve(vscode.commands.executeCommand('levelcodeAi.chat.focus'))
438+
.then(undefined, (e) => dbg('chat.focus.failed', { why, msg: String((e && e.message) || e) }));
439+
}
440+
419441
function addSelection() {
420442
const sel = captureSelection();
421443
if (!sel) { vscode.window.showInformationMessage('LevelCode AI: select some code first.'); return; }
422444
pendingContext = sel.block;
423-
vscode.commands.executeCommand('levelcodeAi.chat.focus');
445+
focusChatView('addSelection');
424446
post({ type: 'context', label: sel.label });
425447
}
426448

@@ -465,7 +487,7 @@ async function addContext() {
465487
}
466488
}
467489
}
468-
vscode.commands.executeCommand('levelcodeAi.chat.focus');
490+
focusChatView('addContext');
469491
postContextFiles();
470492
}
471493

@@ -1052,7 +1074,7 @@ async function resumeSession(id) {
10521074
post({ type: 'sessionResumed', id, title: (r.entry && r.entry.title) || 'Session', note: r.note || '', tier: r.plan && r.plan.tier, turns });
10531075
postContextFiles();
10541076
refreshSessions(); // the resumed session bumps to the top — keep both surfaces current
1055-
vscode.commands.executeCommand('levelcodeAi.chat.focus');
1077+
focusChatView('resumeSession');
10561078
dbg('sessions.resumed', { id, tier: r.plan && r.plan.tier, restored: agentMessages.length, shown: turns.length });
10571079
}
10581080

@@ -2295,7 +2317,7 @@ async function openChatInEditor(opts) {
22952317
// resolveWebviewView then makes it live, and without this the chat would have no surface at all.
22962318
activeWebview = undefined;
22972319
pendingTranscriptReplay = 'Back in the sidebar';
2298-
vscode.commands.executeCommand('levelcodeAi.chat.focus');
2320+
focusChatView('editorClosed');
22992321
}
23002322
dbg('chat.closedEditor', {});
23012323
});
@@ -2432,7 +2454,7 @@ class SessionsViewProvider {
24322454
// The real session index for this workspace (empty on a fresh install — the view shows its
24332455
// own empty state). Posted to THIS view's webview, not the chat's.
24342456
case 'listSessions': view.webview.postMessage({ type: 'sessions', entries: sessionList() }); break;
2435-
case 'newSession': newChat(); vscode.commands.executeCommand('levelcodeAi.chat.focus'); break;
2457+
case 'newSession': newChat(); focusChatView('sessions.newSession'); break;
24362458
case 'sessionAction': await handleSessionAction(msg.action, msg.id); break;
24372459
// Memory tab (§M3): list the recorded outcomes + facts; act on them; open the file.
24382460
case 'listMemory': { const mm = sessionsManager(); view.webview.postMessage({ type: 'memoryList', items: mm ? mm.memoryItems() : [], facts: mm ? mm.factsList() : [] }); break; }
@@ -2528,7 +2550,7 @@ async function postAccount(open) {
25282550
// second login and no interceptable code ever travels through the custom scheme.
25292551
async function handleLaunch() {
25302552
dbg('account.launch', {});
2531-
vscode.commands.executeCommand('levelcodeAi.chat.focus');
2553+
focusChatView('account.launch');
25322554
const token = ctx ? await ctx.secrets.get(ACCOUNT_TOKEN_KEY) : null;
25332555
if (!token) { await accountSignIn(); }
25342556
}

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

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,9 @@ test('RESTORE: a sidebar that was never resolved is revealed rather than assumed
144144
// If the container has not been opened this session, sidebarChatView is undefined — restoring by
145145
// writing to it would throw, and doing nothing would leave the chat with no surface at all.
146146
const open = fnBody(ext, 'openChatInEditor');
147-
assert.match(open, /if \(sidebarChatView\) \{[\s\S]*\} else \{[\s\S]*levelcodeAi\.chat\.focus/,
147+
// The reveal now goes through focusChatView() so its rejection cannot go unhandled; what this test
148+
// cares about is unchanged — the else-branch must still reveal the view rather than assume it.
149+
assert.match(open, /if \(sidebarChatView\) \{[\s\S]*\} else \{[\s\S]*focusChatView\(/,
148150
'the never-resolved sidebar case is unhandled');
149151
});
150152

@@ -310,4 +312,46 @@ test('MOVE BACK: there is a button on the tab, and it reuses the dispose hand-ov
310312
assert.match(body, /levelcodeAi\.chat\.focus/, 'with no panel open the command must still reveal the chat, not do nothing');
311313
});
312314

315+
test('FOCUS: no reveal of the chat view is left to reject unhandled', () => {
316+
// One guard for the whole class, rather than six assertions that each name a function. `executeCommand`
317+
// returns a Thenable, so a bare call in a void context makes any rejection an unhandled promise
318+
// rejection in the extension host — attributed to nothing, which is what makes it useless.
319+
//
320+
// Scanning every call site means the NEXT one is covered too. That matters here: this pattern was
321+
// copied into six places over time precisely because nothing was watching for it.
322+
const CALL = "vscode.commands.executeCommand('levelcodeAi.chat.focus')";
323+
const bare = [];
324+
for (let i = ext.indexOf(CALL); i >= 0; i = ext.indexOf(CALL, i + 1)) {
325+
const before = ext.slice(Math.max(0, i - 40), i);
326+
const after = ext.slice(i + CALL.length, i + CALL.length + 40);
327+
const handled = /\breturn\s+$/.test(before) // returned — a command handler VS Code awaits
328+
|| /\bawait\s+$/.test(before) // awaited by a caller that catches
329+
|| /=>\s*$/.test(before) // concise arrow body: also a return
330+
|| /Promise\.resolve\($/.test(before) // wrapped by focusChatView
331+
|| /^\s*\)?\s*\.(then|catch)\(/.test(after); // handled inline
332+
if (!handled) { bare.push('line ' + ext.slice(0, i).split('\n').length); }
333+
}
334+
assert.deepStrictEqual(bare, [],
335+
'these reveals are fire-and-forget — a rejection becomes an unhandled promise rejection.\n'
336+
+ 'Use focusChatView(why) for a background reveal, or `return` it when the command IS the reveal:\n '
337+
+ bare.join('\n '));
338+
});
339+
340+
test('FOCUS: the shared helper logs the failure and names who caused it', () => {
341+
// The whole complaint was "attributed to nothing", so swallowing it silently would answer the letter
342+
// of the review and none of it. A chat surface that never appears, with no trace, is the failure
343+
// that costs an afternoon.
344+
const body = fnBody(ext, 'focusChatView');
345+
assert.match(body, /dbg\('chat\.focus\.failed'/, 'the failure is not logged — .catch(() => {}) is not a fix');
346+
assert.match(body, /\bwhy\b/, 'the log must name the caller, or it is as unattributed as the rejection was');
347+
assert.ok(!/\bthrow\b/.test(body), 'the helper must not rethrow — every caller uses it in a void context');
348+
// Either `.then(undefined, …)` or `.catch(…)`. They are equivalent here and pinning one would fail a
349+
// refactor that changes nothing; what must not disappear is the rejection handler itself.
350+
assert.match(body, /\.then\(undefined,|\.catch\(/, 'no rejection handler — the helper can still reject');
351+
352+
// And it must be the thing the background callers actually use.
353+
const callers = (ext.match(/focusChatView\('/g) || []).length;
354+
assert.ok(callers >= 6, 'expected the background reveals to route through the helper, found ' + callers);
355+
});
356+
313357
console.log('\nchatSurface: ' + n + ' tests passed.');

0 commit comments

Comments
 (0)