fix(cloud): an expired session is a sign-in card, found before the first message — not a 401 in red - #92
Open
ndemianc wants to merge 3 commits into
Open
fix(cloud): an expired session is a sign-in card, found before the first message — not a 401 in red#92ndemianc wants to merge 3 commits into
ndemianc wants to merge 3 commits into
Conversation
…rst message — not a 401 in red A user back from a week away typed a goal and got "LevelCode Cloud API 401: Signature has expired" in the transcript, while the account popover said they were signed in. Three things made that possible, and each is closed here. The session could die on a schedule nobody could see. The editor stored the access token the refresh endpoint returned and kept its ORIGINAL refresh token forever, so the 30 days ran from the last sign-in. The server now rotates the refresh token (thin.ly #438) and the editor stores it, so the window slides with use. "Signed in" meant "a secret exists". cloudSignedIn was the presence of a token in SecretStorage, never its validity, so dead credentials kept the footer on "Gateway · Max" and the popover on "Manage account". refreshCloudToken now classifies the refresh reply (providers/session.js): only an explicit 401 ends the session — offline and 5xx keep the tokens, because clearing credentials on a network blip would log someone out for closing their laptop on the train. When it IS over, sessionExpired() forgets the tokens, flips the flag, resyncs the popover, and tells the webview. The failure was discovered by the user's first message. The access token's own `exp` is now read locally at the webview's ready and on window focus (throttled), and the network is touched only when it is expired or within five minutes of it — so an expiry found on launch is a sign-in card at the top of an empty chat, not the reply to a goal they just typed. A send that still finds the session dead — chat, agent, or a prep failure in gateway mode with no token — posts code 'session_expired', which the webview routes to the card ahead of the cap and service cards and the red-text fallback. Gateway mode with no token is named for what it is, "signed out", rather than "No API key set for OpenAI". Verified: session 12 tests, sessionExpiredUi 10; full suite 42 suites, 621 cases, 0 failing. Each guard reverted in turn fails only its own assertion: the session check moved behind the cap card -> the ordering guard; agent.js no longer naming the dead session -> the agent assertion; the ready check removed -> the startup guard. No tsconfig exists, so node --check is the syntax gate.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Chat error handling can throw, transient refresh failures can erase credentials, and expiry notifications can be lost while chat is closed.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Improves LevelCode Cloud session handling so expired sessions prompt sign-in rather than displaying raw authentication errors.
Changes:
- Adds proactive expiry checks and rotated refresh-token storage.
- Routes session failures to a sign-in card.
- Adds session logic tests and UI integration guards.
| File | Description |
|---|---|
| extensions/levelcode-ai/test/sessionExpiredUi.test.js | Guards session-card routing and host integration. |
| extensions/levelcode-ai/test/session.test.js | Tests expiry decoding and refresh classification. |
| extensions/levelcode-ai/providers/session.js | Adds session classification helpers. |
| extensions/levelcode-ai/media/chat.html | Adds the sign-in card and dismissal handling. |
| extensions/levelcode-ai/extension.js | Integrates refresh rotation, checks, and notifications. |
| extensions/levelcode-ai/agent.js | Routes expired-session errors to the card. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…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.
…dline, in-place resets 1. A refresh's answer could be applied to a session it was not about. The request is out for as long as the network takes, and nothing tied its result to the session it started against. A 401 arriving after the user had signed in again deleted the pair that sign-in had just stored and marked it expired. A 200 arriving after a sign-out signed them back in; after a sign-in as someone else it filed the old account's tokens under the new name. Two checks now, because there are two ways for the session to change. A generation counter — bumped by sign-in, sign-out and expiry — covers this window exactly. Comparing the stored refresh token with the one that was sent covers another window, which no counter here can see. And every credential change runs through one queue (withSessionLock), because each is several awaits long: without it a sign-in can land between an expiry's "is this still the dead session?" and its deletes — the reviewer's point that comparing tokens is not enough on its own. The queue is per process; between windows the comparison narrows that gap to milliseconds, it does not close it. A superseded answer changes nothing and reports whether a token is in place to retry with. If there is none and this window still believes it is signed in, another window ended the session first, and this one catches up: the card, and a popover and footer that stop claiming it is live. 2. The refresh had no deadline, and `ready` waits on it before it restores the chat. A host that accepted the connection and then went quiet held the transcript, the config and the review controls behind it. The whole exchange now gets ten seconds: the timer is cleared after the body is read, not at the headers. Running out is "this attempt failed", so the tokens stay. 3. The replay ran only on `ready`, and I was wrong about what sends one. New Chat and a session resumed from History do not reload the document: they post `reset`, which empties the log in place, card included. The previous commit's comment and message said otherwise. A checkpoint restore does the same by dropping every node after the restored turn. All three now replay an unanswered expiry afterwards — under the restored transcript when resuming. Verified: 43 suites, 741 cases, 0 failing. sessionExpiredHost.test.js goes from 25 to 44 cases, and its stand-in stores now answer a turn of the event loop later, as the real ones do: that gap is where two changes interleave, and a stand-in that answered at once would have hidden what the lock is for. Each defect put back fails its own case: every answer applied -> "the new access token survived"; no token comparison -> another window's sign-in; no counter -> a sign-in that brought no refresh token of its own; no lock -> the sign-in started mid-expiry, with the interleaved writes in the message; no signal -> the silent host never finishes; timer cleared at the headers -> the stalled body; timer never cleared -> aborted after the fact; each replay removed, or run before the transcript -> its own case. tsc --checkJs reports nothing new. In headless Chrome against the real chat.html, `reset`, a resume and a checkpoint restore each remove the card, and the replay puts it back at the bottom.
| // shown as the error it is, and the next message tries the refresh again. Ending the session | ||
| // from here would delete credentials classifyRefresh deliberately kept. Same rule as the | ||
| // agent's isSessionExpired hook. | ||
| if (req.gateway && !cloudSignedIn && session.isSessionExpiredError(e)) { |
| else if (m.type === 'debug'){ addDebug(m); } | ||
| else if (m.type === 'account'){ renderAccount(m); if (m.open) openAccount(); } | ||
| else if (m.type === 'account'){ | ||
| if (m.signedIn && sessionCard && sessionCard.isConnected){ sessionCard.remove(); sessionCard = null; } renderAccount(m); if (m.open) openAccount(); } |
| function classifyRefresh(res) { | ||
| if (!res) { return 'retry'; } | ||
| if (res.status === 401) { return 'expired'; } | ||
| if (res.status >= 200 && res.status < 300 && res.body && (res.body.access || res.body.token)) { return 'ok'; } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The incident
Back from a week away, the user typed a goal and got
LevelCode Cloud API 401: Signature has expiredin red — while the account popover beside it said signed in · Max. Nothing on screen said what to do.What was wrong, and what this changes
1. The session could die on a schedule nobody could see. The editor stored the access token the refresh endpoint returned and kept its original refresh token forever, so the 30 days ran from the last sign-in, not the last use. The server now rotates it (systemu-net/thin.ly#438 — systemu-net/thin.ly#438) and the editor stores the rotated token, so the window slides with use.
2. "Signed in" meant "a secret exists".
cloudSignedInwas the presence of a token in SecretStorage, never its validity — dead credentials kept the footer on Gateway · Max and the popover on Manage account.refreshCloudTokennow classifies the refresh reply (providers/session.js):Only an explicit 401 ends the session — clearing credentials on a network blip would log someone out for closing their laptop on the train.
3. The failure was discovered by the user's first message. The access token's own
expis now read locally at the webview'sreadyand on window focus (throttled to once per 10 min); the network is touched only when it's expired or within five minutes of it. An expiry found on launch is a sign-in card at the top of an empty chat, not the reply to a goal they just typed.A send that still finds the session dead — chat, agent, or a prep failure in gateway mode with no token — posts
code: 'session_expired', which the webview routes to the card ahead of the cap-reached and service cards and the red-text fallback. Gateway mode with no token is named for what it is ("signed out") rather than No API key set for OpenAI, which sent people hunting for a key they never needed.The card: Your session has expired — Welcome back, ⟨name⟩. Sign in again to keep using LevelCode Cloud; your chat and files here are untouched. [Sign in] [Use my own key instead]. It dismisses itself when the signed-in account message arrives.
Verification
session.test.js12 (the pure arithmetic:expdecoding incl. base64url padding, the 5-minute margin, refresh classification, the error shapes a dead session arrives in);sessionExpiredUi.test.js10 (static guards on the routing order in both error handlers, the startup/focus hooks, rotation storage, the signed-out reason). Full suite 42 suites, 621 cases, 0 failing.agent.jsno longer naming the dead session → the agent assertion; thereadycheck removed → the startup guard.tsconfigexists in the extension, sonode --checkis the syntax gate.Works against the old server too
A server that doesn't yet return
refreshis handled (the field is optional), and the old"Signature has expired"text is still recognised byisSessionExpiredError, so the card appears even before #438 deploys — the rotation just won't slide until it does.