fix(web): persist server access token across tabs and browser restarts - #1567
Conversation
The web UI kept the server bearer credential in sessionStorage, which is tab-scoped and cleared on tab close, so users had to re-enter the token for every new tab and after every mobile tab eviction. Mirror it to localStorage instead: the token already lives on disk at <KIMI_CODE_HOME>/server.token and rides in the launch URL fragment, so browser-profile persistence does not materially widen exposure for this local tool. Existing sessionStorage copies are migrated on first boot, and a 401 (e.g. after `kimi server rotate-token`) still clears the stored credential.
🦋 Changeset detectedLatest commit: 2abac60 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
82a420f to
c7b388b
Compare
wbxl2000
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82a420f940
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // membership source Cmd+K search uses — so no session stays invisible. | ||
| const globalSessions = await listAllSessionsGlobal().catch(() => [] as AppSession[]); | ||
| const knownIds = new Set(sessions.map((s) => s.id)); | ||
| const missed = globalSessions.filter((s) => !knownIds.has(s.id)); |
There was a problem hiding this comment.
Because knownIds only contains the first pages loaded per workspace, this filter treats every older paginated session as “missed.” For any workspace with more than the initial page, listAllSessionsGlobal() now drains the full history at startup and sessions.push(...missed) inserts all remaining sessions while hasMore and cursors still describe only the first page, defeating the load-more path and producing states like a “load more” button with nothing actually remaining. Please restrict recovery to sessions whose workspace mapping is genuinely absent/inconsistent, or mark the affected pagination state as fully loaded when intentionally adding all sessions.
Useful? React with 👍 / 👎.
| export function clearCredential(): void { | ||
| memory = undefined; | ||
| try { | ||
| globalThis.localStorage?.removeItem(STORAGE_KEY); |
There was a problem hiding this comment.
Now that the credential is persisted in shared localStorage, this unconditional removal lets any tab with a stale in-memory token erase a newer valid token written by another tab. For example, after token rotation one tab can store the fresh fragment token, then an older tab’s delayed 401 calls markAuthRequired() → clearCredential() and removes the fresh persisted value, causing subsequent reloads/new tabs to prompt again even though a valid token had been saved. Please only clear the persisted value if it still matches the rejected credential, or otherwise avoid cross-tab deletion of a newer token.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2537c1f: clearCredential now only removes the persisted value when it still matches the credential this tab was using, so a stale tab can no longer erase a newer token another tab stored after rotation. Added cross-tab tests for the stale-tab, matching-credential, and no-credential cases.
localStorage is shared across tabs, so the unconditional removal let a tab holding a stale in-memory credential erase a newer token another tab had just persisted (e.g. after `kimi server rotate-token` one tab stores the fresh fragment token, then an older tab's delayed 401 wiped it). Only clear the persisted copy when it still matches the rejected credential this tab was using.
Drop return-await on the dynamic import, brace the void arrow in the toThrow assertion, and remove a redundant String() conversion — CI runs oxlint --type-aware, which flags these where the plain local run does not.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 127f4d42ef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| globalThis.localStorage?.setItem(STORAGE_KEY, value); | ||
| // Drop any legacy sessionStorage copy so the two stores cannot diverge. | ||
| globalThis.sessionStorage?.removeItem(STORAGE_KEY); | ||
| } catch { | ||
| // sessionStorage may be unavailable (private mode) — memory still works. | ||
| // Storage may be unavailable (private mode) — memory still works. |
There was a problem hiding this comment.
When localStorage.setItem throws but sessionStorage is still usable (for example, blocked/quota-limited local storage), this catch leaves the token only in module memory. ServerAuthDialog.submit() immediately calls window.location.reload() after setCredential, so that in-memory credential is lost and the user lands back on the token prompt even though the old sessionStorage path would have survived the reload. Please either fall back to sessionStorage for this case or avoid reloading when persistence fails.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62b0ff7257
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (memory.expiresAt <= Date.now()) { | ||
| clearExpiredCredential(memory); | ||
| return undefined; |
There was a problem hiding this comment.
When a browser tab keeps its WebSocket open past memory.expiresAt, this check is never reached for that socket: the WS client reads the token only once in connect() and later subscribe / terminal_input frames reuse the already-authenticated connection. That means a long-lived tab can keep using server access past the seven-day cap until the socket happens to reconnect; force a reconnect/close or auth-required transition when the in-memory expiry passes.
Useful? React with 👍 / 👎.
| persistCredential(stored); | ||
| // Drop any legacy sessionStorage copy so the two stores cannot diverge. | ||
| globalThis.sessionStorage?.removeItem(STORAGE_KEY); |
There was a problem hiding this comment.
If localStorage.setItem throws here (for example quota/private-mode storage) the try exits before the legacy sessionStorage value is removed. In a tab that still has an old session-scoped credential, submitting a fresh token after rotation will work only until reload, then initServerAuth() can read and migrate the stale session value again and re-prompt; run the session cleanup in a separate best-effort block even when local persistence fails.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 01b4c1f: the legacy sessionStorage cleanup in setCredential now runs in its own best-effort try block, so it executes even when localStorage.setItem throws (private mode / quota). Added a test covering the blocked-localStorage path: the credential stays memory-only, but the stale session copy is removed and cannot be re-migrated on reload.
setCredential ran persistCredential and the legacy sessionStorage cleanup in one try, so a localStorage.setItem failure (private mode, quota) skipped the cleanup: a stale session-scoped credential left behind was re-migrated on the next reload and 401'd into another token prompt. Split the session cleanup into its own best-effort block.
1 similar comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01b4c1fc0b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (memory.expiresAt <= Date.now()) { | ||
| clearExpiredCredential(memory); | ||
| return undefined; |
There was a problem hiding this comment.
When a tab stays open past the 7-day TTL, this branch drops the token but never invokes the auth-required listeners, so App.vue keeps authRequired false. In the case where the next auth use is only a WebSocket reconnect (for example after a daemon restart or network drop), DaemonEventSocket.connect() calls getCredential(), opens an unauthenticated upgrade, and the WS path reports a generic connection error/retry rather than showing the token dialog; the user is stuck disconnected until some separate REST call happens to 401. Expiry should drive the same prompt path as a rejected credential, or WS auth failures need to call it.
Useful? React with 👍 / 👎.
1 participant
Related Issue
No linked issue — reported as direct user feedback ("access token 隔一阵就需要重新输入"). This PR is split out from #1554, which bundled four fixes; of those, the workspace-path entry already landed via #1556 and the mid-turn resync fix was dropped in #1558 (to be reworked on the v2 kernel). The legacy session-recovery parts were held back for now.
Problem
The web UI stored the server bearer credential in sessionStorage, which is tab-scoped and cleared when the tab closes. Users had to re-enter the token for every new tab, after closing the browser, and after mobile browsers evicted background tabs. The server token itself is persistent (
<KIMI_CODE_HOME>/server.tokensurvives restarts and is only rotated explicitly); only the browser-side storage was short-lived.What changed
Persist the credential in localStorage with a fixed seven-day expiry, so it survives tab close and browser restarts without being retained indefinitely. Details:
kimi server rotate-token) clears the rejected credential and shows the token dialog. A different fresh token written by another tab is preserved.KIMI_CODE_PASSWORD, so browser-profile retention is capped at seven days for both.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.