Skip to content

Commit b0a00d7

Browse files
committed
fix(chat): the startup open cannot become an unhandled rejection
Review on #76. `revealChatAtStartup()` is async and nothing awaits the timer, so a rejection from createWebviewPanel or from the focus command would surface as an unhandled rejection in the extension host — noisy, and attributed to nothing in particular. Caught, but LOGGED rather than swallowed. A chat that never appears with no trace of why is the exact failure this setting exists to make explicable; `.catch(() => {})` would have answered the review and made the product worse. The window still starts and both surfaces stay openable by hand. The same defect was in moveChatToSidebar, which I wrote in this PR — but it wants the OPPOSITE treatment. That one is an explicit click, and `registerCommand` awaits whatever the handler returns, so the reveal is now RETURNED: a failure reaches the user as a failed command instead of leaving them pressing a button that does nothing. Silence is right for an implicit startup action and wrong for a deliberate one. Guards, each bypass-verified by reverting the fix: - the .catch removed (the reviewer's bug, restored) — caught - caught but swallowed with no dbg — caught - move-back firing and forgetting again — caught - the registration dropping the returned promise, which would make returning it pointless NOT swept into this commit: seven other bare `executeCommand('levelcodeAi.chat.focus')` calls predate this PR and have the same shape. Rewriting unrelated call sites inside a review fix would bury the change under a diff nobody asked for; filed separately. 21 tests in chatSurface, 32 suites green.
1 parent 9378f05 commit b0a00d7

2 files changed

Lines changed: 38 additions & 3 deletions

File tree

extensions/levelcode-ai/extension.js

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2310,9 +2310,14 @@ async function openChatInEditor(opts) {
23102310
* hand-over changed.
23112311
*/
23122312
function moveChatToSidebar() {
2313-
if (chatEditorPanel) { chatEditorPanel.dispose(); return; }
2313+
if (chatEditorPanel) { chatEditorPanel.dispose(); return undefined; }
23142314
// Already there (or never moved) — just reveal it, so the command is never a silent no-op.
2315-
vscode.commands.executeCommand('levelcodeAi.chat.focus');
2315+
//
2316+
// RETURNED, not fired and forgotten. `registerCommand` awaits whatever the handler returns, so a
2317+
// failure here reaches the user as a failed command instead of an unhandled rejection. That is the
2318+
// opposite of the startup path on purpose: this is an explicit click, and silence would leave the
2319+
// user pressing a button that does nothing.
2320+
return vscode.commands.executeCommand('levelcodeAi.chat.focus');
23162321
}
23172322

23182323
/**
@@ -2788,7 +2793,15 @@ function activate(context) {
27882793
// broken webview forcing the panel open forever; `chat.startLocation: none` is a better answer
27892794
// to that, and a chat that silently stops appearing after five launches is worse to diagnose
27902795
// than one that keeps showing you it is broken.
2791-
setTimeout(() => { revealChatAtStartup(); }, 600);
2796+
//
2797+
// `.catch` because this is fire-and-forget: nothing awaits the timer, so a rejection from
2798+
// `createWebviewPanel` or the focus command would surface as an unhandled rejection in the
2799+
// extension host — noisy, and attributed to nothing in particular. Logged rather than swallowed:
2800+
// a chat that never appears, with no trace of why, is the one failure mode this whole setting is
2801+
// supposed to make explicable. The window still starts, and both surfaces remain openable by hand.
2802+
setTimeout(() => {
2803+
revealChatAtStartup().catch((e) => dbg('chat.startLocation.failed', { msg: String((e && e.message) || e) }));
2804+
}, 600);
27922805

27932806
// First-launch onboarding: open the "Welcome to LevelCode" walkthrough once. Only mark it shown
27942807
// AFTER it actually opens (previously the flag was set up-front, so a first-launch race that failed

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,28 @@ test('START: the startup open cannot be triggered by a menu click', () => {
266266
'preserveFocus must be read strictly, so a stray truthy argument cannot enable it');
267267
});
268268

269+
test('START: the fire-and-forget startup call cannot become an unhandled rejection', () => {
270+
// Nothing awaits the startup timer, so a rejection from createWebviewPanel or from the focus
271+
// command would land in the extension host attributed to nothing. Caught — but LOGGED, not
272+
// swallowed: a chat that never appears with no trace of why is the exact failure this setting is
273+
// supposed to make explicable.
274+
const call = /revealChatAtStartup\(\)([\s\S]{0,160}?)\}, 600\)/.exec(ext);
275+
assert.ok(call, 'the startup call site moved — this guard no longer covers it');
276+
assert.match(call[1], /\.catch\(/, 'the fire-and-forget startup call has no .catch — unhandled rejection');
277+
assert.match(call[1], /dbg\(/, 'the failure is swallowed silently; log the reason so it can be diagnosed');
278+
});
279+
280+
test('MOVE BACK: a failed move reaches the user instead of vanishing', () => {
281+
// Deliberately the OPPOSITE of the startup path. This is an explicit click, and registerCommand
282+
// awaits what the handler returns — so returning the thenable turns a failure into a reported
283+
// command error, where swallowing it would leave the user pressing a button that does nothing.
284+
const body = fnBody(ext, 'moveChatToSidebar');
285+
assert.match(body, /return vscode\.commands\.executeCommand\('levelcodeAi\.chat\.focus'\)/,
286+
'the reveal must be RETURNED, or a failure is an unhandled rejection and the click looks inert');
287+
assert.match(ext, /registerCommand\('levelcode\.ai\.moveChatToSidebar', \(\) => moveChatToSidebar\(\)\)/,
288+
'the registration must return the handler result, or returning it inside buys nothing');
289+
});
290+
269291
test('MOVE BACK: there is a button on the tab, and it reuses the dispose hand-over', () => {
270292
// The chat now opens centred for everyone, so the way BACK has to be visible from the centre.
271293
// Before this it existed only on the sidebar card — which you cannot see while the chat is a tab.

0 commit comments

Comments
 (0)