Skip to content

Commit 9a5931c

Browse files
committed
fix(cloud): three review findings from #92, and the BYOK fallback it broke
1. A failed chat request threw instead of showing its error. handleSend declared `req` inside the try and read it in the catch, so every non-aborted failure — a BYOK provider error included — was a ReferenceError, and the webview was left in its streaming state. `req` now lives outside the try, with a value the catch can read even when preparing the request is the thing that threw. 2. A refresh that merely failed signed the user out. With (1) fixed, that same catch ran sessionExpired() on any gateway 401 — including one whose refresh was offline, a 503, or unparseable, which classifyRefresh deliberately answers with "keep the tokens". The catch no longer ends anything; exactly one caller does, the refresh endpoint answering 401. It shows the sign-in card only once the session HAS ended (!cloudSignedIn), the rule the agent hook already used. Otherwise the request's own error is shown, the credentials stay, and the next message tries the refresh again. 3. An expiry found with the chat closed was never shown. The focus check can end the session with no webview to hear about it: post() drops the card, and the next `ready` finds no token and returns. sessionExpired() now leaves a marker in globalState — written before the tokens are deleted, so a crash in between cannot lose it — and `ready` replays the card from it. Last, so the card lands under a replayed transcript instead of above it; that also covers a new chat, a resumed session, and the chat moving to an editor tab, each of which loads a fresh document. The marker goes when the user answers: signs in, signs out, or picks "Use my own key instead", which now takes the card away too. 4. Not in the review: this change made BYOK unreachable by default. prepProviderRequest answered "signed out" for ANY gateway-mode request with no token. Gateway is the default mode and levelcode.ai the default host, so that was everyone using LevelCode on their own key with no account: chat and agent said "Your session has expired", inline edit said "No API key set for LevelCode Cloud", and completions stopped — where the setting promises "automatically fall back to BYOK when you're signed out". The branch now hangs off the marker from (3). Only a session that ENDED stops a request to ask for a sign-in; it still never prompts for a key that user never needed. Verified: 43 suites, 722 cases, 0 failing. sessionExpiredHost.test.js is new (25 cases) and RUNS the host code — sliced out of extension.js, against stand-ins for SecretStorage, globalState, fetch and the adapter, on the defaults package.json ships. The string guards all passed with (1) in place, which is why reading the source was not enough. Each defect put back in turn fails its own case: `req` in the try -> "req is not defined"; sessionExpired() in the catch -> "access token kept"; no marker -> the closed-chat replay; the token-only branch -> a BYOK user's message never reaches the provider. sessionExpiredUi.test.js goes from 10 to 16: the order of `ready`, and the card run against a stand-in DOM. tsc --checkJs reports "Cannot find name 'req'" at the old line 2095 and no unresolved name in extension.js after. The card flows were also driven in headless Chrome against the real chat.html.
1 parent 4e1453f commit 9a5931c

4 files changed

Lines changed: 601 additions & 17 deletions

File tree

‎extensions/levelcode-ai/extension.js‎

Lines changed: 84 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -397,24 +397,75 @@ async function refreshCloudToken() {
397397
* next sign-in overwrites it anyway. What must go is anything the editor would otherwise keep
398398
* presenting as a live session: the tokens, and the `cloudSignedIn` flag the footer and model gate
399399
* read. Idempotent, so every path that discovers the expiry can call it without coordination.
400+
*
401+
* It also leaves ACCOUNT_EXPIRED_KEY behind, because "the tokens are gone" says nothing afterwards:
402+
* that is just as true of someone who signed out, or who never signed in. The marker is what
403+
* records that a session ENDED here and the user has not answered it yet — sessionExpiredPending()
404+
* has the two things that hang off it.
400405
*/
401406
let sessionExpiredAnnounced = false;
402407
async function sessionExpired() {
403408
const hadToken = !!(ctx && await ctx.secrets.get(ACCOUNT_TOKEN_KEY));
404409
cloudSignedIn = false;
405410
if (ctx) {
411+
// Marker FIRST: if the editor dies between these writes, tokens-without-marker would read as an
412+
// ordinary sign-out and the card would be lost; marker-with-tokens just finds the expiry again.
413+
await ctx.globalState.update(ACCOUNT_EXPIRED_KEY, true);
406414
await ctx.secrets.delete(ACCOUNT_TOKEN_KEY);
407415
await ctx.secrets.delete(ACCOUNT_REFRESH_KEY);
408416
}
409417
if (!hadToken && sessionExpiredAnnounced) { return; }
410418
sessionExpiredAnnounced = true;
411419
const p = (ctx && ctx.globalState.get(ACCOUNT_PROFILE_KEY)) || {};
412420
dbg('cloud.sessionExpired', { name: p.name || p.email || '' });
413-
post({ type: 'sessionExpired', name: p.name || '', message: session.SESSION_EXPIRED_MESSAGE });
421+
postSessionExpired();
414422
await postAccount(false);
415423
sendConfigToWebview();
416424
}
417425

426+
/** Show the sign-in card. The profile outlives the session so the card can say who it is talking to. */
427+
function postSessionExpired() {
428+
const p = (ctx && ctx.globalState.get(ACCOUNT_PROFILE_KEY)) || {};
429+
post({ type: 'sessionExpired', name: p.name || '', message: session.SESSION_EXPIRED_MESSAGE });
430+
}
431+
432+
/**
433+
* True while an ended session is still waiting on the user: sessionExpired() left its marker, and
434+
* nothing has answered it — a sign-in, a sign-out, or "use my own key" (clearSessionExpired).
435+
*
436+
* Two things read it, and both need more than "there is no token":
437+
*
438+
* 1. The replay on a webview's `ready`. The session can end with no chat open to hear about it
439+
* (the focus check does not need one), and post() has nowhere to send the card.
440+
* 2. prepProviderRequest. Gateway is the DEFAULT mode, so "gateway mode, no token" describes
441+
* everyone who uses LevelCode on their own key with no account. Only a session that ended may
442+
* stop a request and ask for a sign-in; the rest fall back to BYOK, as the setting promises.
443+
*
444+
* Scoped to where the card can be acted on: gateway mode, with a host to sign in to.
445+
*/
446+
function sessionExpiredPending() {
447+
return !!(ctx && ctx.globalState.get(ACCOUNT_EXPIRED_KEY)) && providerMode() === 'gateway' && !!cloudEndpoint();
448+
}
449+
450+
/** The user has answered the expiry, so stop asking: the marker goes. Unconditional — another window
451+
* may have set it, and this window's view of globalState can lag behind the write. */
452+
async function clearSessionExpired() {
453+
if (ctx) { await ctx.globalState.update(ACCOUNT_EXPIRED_KEY, undefined); }
454+
}
455+
456+
/**
457+
* Show the card in a webview that was not there to see the session end — the chat was closed, or
458+
* this is a fresh document (a new chat, a resumed session, the chat moved to an editor tab). Called
459+
* LAST on `ready`, so the card lands under a replayed transcript instead of above it. When the
460+
* check earlier in the same `ready` is what found the expiry, this posts the card a second time;
461+
* the webview keeps one, at the bottom, which is where it belongs.
462+
*/
463+
async function replaySessionExpired() {
464+
if (!sessionExpiredPending() || await ctx.secrets.get(ACCOUNT_TOKEN_KEY)) { return; }
465+
dbg('cloud.sessionExpired.replay', {});
466+
postSessionExpired();
467+
}
468+
418469
/**
419470
* Check the session BEFORE the user types anything, so an expiry found on launch shows up as a
420471
* sign-in card at the top of the chat rather than as the reply to their first message.
@@ -427,6 +478,8 @@ let lastSessionCheck = 0;
427478
async function checkCloudSession(reason) {
428479
if (!ctx || providerMode() !== 'gateway') { return; }
429480
const token = await ctx.secrets.get(ACCOUNT_TOKEN_KEY);
481+
// No token, so no session to check. One that already ENDED — found while no chat was open to hear
482+
// about it — is not announced from here: `ready` replays it last, via replaySessionExpired().
430483
if (!token) { return; }
431484
lastSessionCheck = Date.now();
432485
if (!session.accessNeedsRefresh(token)) { return; }
@@ -444,18 +497,21 @@ async function refreshGatewayToken() {
444497
* Resolve everything needed to call the active provider: id, key, model, baseURL, maxTokens.
445498
* Returns { ok:false, reason } when a required key/baseURL is missing (after optional prompting) or
446499
* a custom endpoint would leak the key over plaintext — the caller renders providerErrorMessage().
447-
* @returns {Promise<{ok:boolean, providerId?:string, apiKey?:string, model?:string, baseURL?:string, maxTokens?:number, label?:string, reason?:string}>}
500+
* @returns {Promise<{ok:boolean, providerId?:string, apiKey?:string, model?:string, baseURL?:string, maxTokens?:number, label?:string, reason?:string, gateway?:boolean}>}
448501
*/
449502
async function prepProviderRequest(opts) {
450503
const cfg = aiConfig();
451504
// Gateway mode: when signed in (a cloud token is present) and mode==='gateway', route AI through the
452505
// LevelCode Cloud metered gateway (an openai-kind endpoint) instead of the user's own provider/key. Falls
453506
// back to the BYOK path below when signed out or in byok mode — the default is untouched.
454507
const token = ctx ? await ctx.secrets.get(ACCOUNT_TOKEN_KEY) : null;
455-
// Gateway mode with no token at all is a signed-out gateway user — most often one whose session
456-
// just expired — not a BYOK user who forgot a key. Say so, with a sign-in card, instead of "No API
457-
// key set for OpenAI", which sends them hunting for a key they never needed.
458-
if (providerMode() === 'gateway' && !token && cloudEndpoint()) {
508+
// No token in gateway mode is NOT, on its own, a reason to stop. Gateway is the default mode, so
509+
// that describes everyone who uses LevelCode on their own key with no account, and they take the
510+
// BYOK path below — "falls back to BYOK when you're signed out" is the setting's own promise.
511+
// The exception is a session that ENDED and has not been answered: that user was on their plan a
512+
// moment ago, and "No API key set for OpenAI" would send them hunting for a key they never needed.
513+
// They get the sign-in card, until they sign in again or choose their own key.
514+
if (!token && sessionExpiredPending()) {
459515
return { ok: false, providerId: 'openai', label: 'LevelCode Cloud', reason: 'signedOut', gateway: true };
460516
}
461517
const gw = resolveGateway({ mode: providerMode(), endpoint: cloudApiUrl(), token: token || '' });
@@ -2056,8 +2112,12 @@ async function handleSend(text, images) {
20562112
let assistant = '';
20572113
const onDelta = (d) => { assistant += d; post({ type: 'assistantDelta', text: d }); };
20582114

2115+
// Declared out here, not inside the try: the catch below reads it, and must be able to even when
2116+
// preparing the request is the thing that threw.
2117+
/** @type {Awaited<ReturnType<typeof prepProviderRequest>>} */
2118+
let req = { ok: false, gateway: false };
20592119
try {
2060-
const req = await prepProviderRequest({ prompt: true });
2120+
req = await prepProviderRequest({ prompt: true });
20612121
if (!req.ok) {
20622122
conversation.pop();
20632123
post({ type: 'assistantError', message: providerErrorMessage(req), code: req.reason === 'signedOut' ? 'session_expired' : undefined });
@@ -2090,10 +2150,14 @@ async function handleSend(text, images) {
20902150
post({ type: 'assistantDone' });
20912151
} else {
20922152
conversation.pop();
2093-
// A gateway 401 the refresh above could not recover is a dead session, not an error to read:
2094-
// name it so the webview shows the sign-in card instead of the adapter's raw message.
2095-
if (req.gateway && session.isSessionExpiredError(e)) {
2096-
await sessionExpired();
2153+
// A gateway 401 is a dead session only once the session has actually ENDED, and one place
2154+
// decides that: the refresh endpoint answering 401 (refreshCloudToken → sessionExpired, which
2155+
// is what clears cloudSignedIn). A 401 whose refresh merely failed — offline, a 5xx, a
2156+
// malformed reply — is this request failing, not the session: the tokens stay, the error is
2157+
// shown as the error it is, and the next message tries the refresh again. Ending the session
2158+
// from here would delete credentials classifyRefresh deliberately kept. Same rule as the
2159+
// agent's isSessionExpired hook.
2160+
if (req.gateway && !cloudSignedIn && session.isSessionExpiredError(e)) {
20972161
post({ type: 'assistantError', message: session.SESSION_EXPIRED_MESSAGE, code: 'session_expired' });
20982162
} else {
20992163
post({ type: 'assistantError', message: String((e && e.message) || e), code: e && e.code });
@@ -2579,8 +2643,9 @@ class ChatViewProvider {
25792643
webview.onDidReceiveMessage(async (msg) => {
25802644
switch (msg.type) {
25812645
// `ready` is the earliest a freshly-loaded webview can hear anything, so it is also where a
2582-
// surface that just took over replays the conversation it inherited (openChatInEditor).
2583-
case 'ready': cloudSignedIn = !!(ctx && await ctx.secrets.get(ACCOUNT_TOKEN_KEY)); await checkCloudSession('ready'); autopilot = aiConfig().get('agent.autopilot', false); sendConfigToWebview(); postActiveFile(); postContextFiles(); post({ type: 'mode', agent: agentMode }); post({ type: 'autopilot', on: autopilot }); postAccount(); buildFileIndex(); post({ type: 'contextUsage', input: 0, limit: currentContextLimit() }); if (review) { review.resync(); } postMemoryDigest(); if (pendingTranscriptReplay) { const t = pendingTranscriptReplay; pendingTranscriptReplay = ''; replayLiveTranscript(t); } break;
2646+
// surface that just took over replays the conversation it inherited (openChatInEditor) — and,
2647+
// after that, a session expiry this document was not there to see (replaySessionExpired).
2648+
case 'ready': cloudSignedIn = !!(ctx && await ctx.secrets.get(ACCOUNT_TOKEN_KEY)); await checkCloudSession('ready'); autopilot = aiConfig().get('agent.autopilot', false); sendConfigToWebview(); postActiveFile(); postContextFiles(); post({ type: 'mode', agent: agentMode }); post({ type: 'autopilot', on: autopilot }); postAccount(); buildFileIndex(); post({ type: 'contextUsage', input: 0, limit: currentContextLimit() }); if (review) { review.resync(); } postMemoryDigest(); if (pendingTranscriptReplay) { const t = pendingTranscriptReplay; pendingTranscriptReplay = ''; replayLiveTranscript(t); } await replaySessionExpired(); break;
25842649
case 'setMode': agentMode = !!msg.agent; post({ type: 'mode', agent: agentMode }); break;
25852650
case 'setAutopilot': autopilot = !!msg.on; aiConfig().update('agent.autopilot', autopilot, vscode.ConfigurationTarget.Global); dbg('autopilot.set', { on: autopilot }); post({ type: 'autopilot', on: autopilot }); break;
25862651
case 'send': await handleSend(msg.text, msg.images); break;
@@ -2612,7 +2677,9 @@ class ChatViewProvider {
26122677
case 'accountManage': await accountManage(); break;
26132678
case 'accountUpgrade': await openUpgrade(); break;
26142679
// The sign-in card's second button: the same setting the model picker's BYOK row opens.
2615-
case 'byokSettings': vscode.commands.executeCommand('workbench.action.openSettings', 'levelcode.ai.providerMode'); break;
2680+
// Choosing your own key ANSWERS the expiry, so the marker goes first — from the next message
2681+
// on this is an ordinary signed-out user, and gateway mode falls back to BYOK by itself.
2682+
case 'byokSettings': await clearSessionExpired(); vscode.commands.executeCommand('workbench.action.openSettings', 'levelcode.ai.providerMode'); break;
26162683
case 'openExternal': await openLegal(msg.target); break;
26172684
case 'copy': try { await vscode.env.clipboard.writeText(String(msg.text || '')); } catch (e) { /* clipboard unavailable */ } break;
26182685
case 'retry': if (lastAgentGoal && !abort) { dbg('retry', { goalChars: lastAgentGoal.length }); await agentFlow(lastAgentGoal); } break;
@@ -2849,6 +2916,7 @@ const ACCOUNT_TOKEN_KEY = 'levelcode.cloud.token'; // access token → Sec
28492916
const ACCOUNT_REFRESH_KEY = 'levelcode.cloud.refresh'; // refresh token → SecretStorage (gateway silent renewal)
28502917
const ACCOUNT_VERIFIER_KEY = 'levelcode.cloud.verifier'; // PKCE code_verifier → SecretStorage (per sign-in)
28512918
const ACCOUNT_PROFILE_KEY = 'levelcode.cloud.profile'; // {name,email,plan} → globalState
2919+
const ACCOUNT_EXPIRED_KEY = 'levelcode.cloud.sessionExpired'; // true while an ENDED session is unanswered → globalState
28522920

28532921
/** base64url without padding — the encoding PKCE (RFC 7636) uses for both verifier and challenge. */
28542922
function b64url(buf) { return buf.toString('base64').replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, ''); }
@@ -2953,6 +3021,7 @@ async function accountSignOut() {
29533021
await ctx.secrets.delete(ACCOUNT_TOKEN_KEY);
29543022
await ctx.secrets.delete(ACCOUNT_REFRESH_KEY);
29553023
await ctx.globalState.update(ACCOUNT_PROFILE_KEY, undefined);
3024+
await clearSessionExpired(); // signing out on purpose answers an expiry: no card, BYOK from here
29563025
}
29573026
dbg('account.signout');
29583027
await postAccount();
@@ -3003,6 +3072,7 @@ async function storeSession(access, refresh, profile) {
30033072
await ctx.globalState.update(ACCOUNT_PROFILE_KEY, {
30043073
name: (profile && profile.name) || '', email: (profile && profile.email) || '', plan: (profile && profile.plan) || ''
30053074
});
3075+
await clearSessionExpired(); // signed in again: the expiry is answered
30063076
}
30073077

30083078
// The browser redirects to levelcode://levelcode.levelcode-ai/auth/callback with either:

‎extensions/levelcode-ai/media/chat.html‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2753,7 +2753,12 @@
27532753
+ '<button class="ucbtn ghost" data-act="byok">Use my own key instead</button></div>';
27542754
log.appendChild(card); scrollIfStuck();
27552755
card.querySelector('[data-act="signin"]').onclick = () => vscode.postMessage({ type: 'accountSignIn' });
2756-
card.querySelector('[data-act="byok"]').onclick = () => vscode.postMessage({ type: 'byokSettings' });
2756+
// Choosing your own key answers the expiry — the host stops asking — so the card goes too,
2757+
// rather than sitting in the transcript offering a sign-in nobody is waiting on.
2758+
card.querySelector('[data-act="byok"]').onclick = () => {
2759+
vscode.postMessage({ type: 'byokSettings' });
2760+
card.remove(); if (sessionCard === card){ sessionCard = null; }
2761+
};
27572762
sessionCard = card;
27582763
}
27592764
// Our-side outage / transient issue → a neutral "service" notice, NOT the red error and NOT the

0 commit comments

Comments
 (0)