fix: chat WebSocket reconnect reuses a stale token after refresh - #1016
Open
kuhlsnu wants to merge 2 commits into
Open
fix: chat WebSocket reconnect reuses a stale token after refresh#1016kuhlsnu wants to merge 2 commits into
kuhlsnu wants to merge 2 commits into
Conversation
authenticatedFetch persists a refreshed JWT to localStorage on X-Refreshed-Token, but WebSocketContext builds the chat WS URL from AuthContext in-memory token, which is never re-synced. On a long-lived tab the chat WS reconnect reuses the stale token and eventually fails at the token true expiry while REST keeps refreshing. Read the freshest auth-token from localStorage at (re)connect, matching the shell-WS and file-upload paths.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe WebSocket connection flow now reads the latest authentication token from ChangesWebSocket authentication
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/contexts/WebSocketContext.tsx (1)
108-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared storage-key constant.
src/components/auth/constants.ts:1already definesAUTH_TOKEN_STORAGE_KEY; use it here instead of duplicating'auth-token'to prevent silent contract drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/contexts/WebSocketContext.tsx` around lines 108 - 112, Update the freshToken lookup in the WebSocket reconnect logic to use the shared AUTH_TOKEN_STORAGE_KEY constant from the auth constants module instead of the duplicated 'auth-token' literal, while preserving the existing localStorage fallback to token.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/contexts/WebSocketContext.tsx`:
- Around line 108-113: Update the token lookup in connect() to use
AUTH_TOKEN_STORAGE_KEY and isolate localStorage.getItem() in a narrow try/catch.
If storage is unavailable or the read throws, retain the in-memory token
fallback so reconnect proceeds; keep buildWebSocketUrl using the resolved token.
---
Nitpick comments:
In `@src/contexts/WebSocketContext.tsx`:
- Around line 108-112: Update the freshToken lookup in the WebSocket reconnect
logic to use the shared AUTH_TOKEN_STORAGE_KEY constant from the auth constants
module instead of the duplicated 'auth-token' literal, while preserving the
existing localStorage fallback to token.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 972d3501-ebd0-4aea-b175-444c7f996e44
📒 Files selected for processing (1)
src/contexts/WebSocketContext.tsx
Comment on lines
+108
to
+113
| // Read the freshest token from localStorage (authenticatedFetch updates it | ||
| // on an X-Refreshed-Token) so a reconnect uses the refreshed token rather | ||
| // than a stale in-memory one. Mirrors the shell-WS / file-upload paths. | ||
| const freshToken = | ||
| (typeof localStorage !== 'undefined' && localStorage.getItem('auth-token')) || token; | ||
| const wsUrl = buildWebSocketUrl(freshToken); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File map ==\n'
git ls-files 'src/contexts/WebSocketContext.tsx' 'src/components/auth/constants.ts'
printf '\n== WebSocketContext outline ==\n'
ast-grep outline src/contexts/WebSocketContext.tsx --view expanded || true
printf '\n== constants outline ==\n'
ast-grep outline src/components/auth/constants.ts --view expanded || true
printf '\n== WebSocketContext relevant lines ==\n'
nl -ba src/contexts/WebSocketContext.tsx | sed -n '70,150p'
printf '\n== Auth token constant file ==\n'
nl -ba src/components/auth/constants.ts | sed -n '1,120p'
printf '\n== Search for auth-token key usage ==\n'
rg -n "auth-token|AUTH_TOKEN_STORAGE_KEY" src || trueRepository: siteboon/claudecodeui
Length of output: 990
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== WebSocketContext excerpt ==\n'
sed -n '90,130p' src/contexts/WebSocketContext.tsx | cat -n
printf '\n== Auth token constant ==\n'
sed -n '1,20p' src/components/auth/constants.ts | cat -n
printf '\n== Exact usages ==\n'
rg -n "AUTH_TOKEN_STORAGE_KEY|auth-token|localStorage\.getItem|try \{|catch" src/contexts/WebSocketContext.tsx src/components/auth/constants.tsRepository: siteboon/claudecodeui
Length of output: 3064
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== WebSocketContext 105-170 ==\n'
sed -n '105,170p' src/contexts/WebSocketContext.tsx | cat -n
printf '\n== Any storage/error handling nearby ==\n'
rg -n "localStorage|AUTH_TOKEN_STORAGE_KEY|catch \\(error\\)|setTimeout|reconnect" src/contexts/WebSocketContext.tsx src/components/auth/constants.tsRepository: siteboon/claudecodeui
Length of output: 4930
Preserve the fallback when storage access fails.
localStorage.getItem() can throw when browser storage is unavailable, and the outer try aborts connect() before the in-memory token fallback runs. Wrap only the storage read so reconnects still proceed with token, and use AUTH_TOKEN_STORAGE_KEY instead of the duplicated 'auth-token' literal.
Proposed fix
- const freshToken =
- (typeof localStorage !== 'undefined' && localStorage.getItem('auth-token')) || token;
+ let freshToken = token;
+ try {
+ const storedToken =
+ typeof localStorage !== 'undefined' ? localStorage.getItem(AUTH_TOKEN_STORAGE_KEY) : null;
+ freshToken = storedToken || token;
+ } catch {
+ // Use the in-memory token when storage is unavailable.
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Read the freshest token from localStorage (authenticatedFetch updates it | |
| // on an X-Refreshed-Token) so a reconnect uses the refreshed token rather | |
| // than a stale in-memory one. Mirrors the shell-WS / file-upload paths. | |
| const freshToken = | |
| (typeof localStorage !== 'undefined' && localStorage.getItem('auth-token')) || token; | |
| const wsUrl = buildWebSocketUrl(freshToken); | |
| // Read the freshest token from localStorage (authenticatedFetch updates it | |
| // on an X-Refreshed-Token) so a reconnect uses the refreshed token rather | |
| // than a stale in-memory one. Mirrors the shell-WS / file-upload paths. | |
| let freshToken = token; | |
| try { | |
| const storedToken = | |
| typeof localStorage !== 'undefined' ? localStorage.getItem(AUTH_TOKEN_STORAGE_KEY) : null; | |
| freshToken = storedToken || token; | |
| } catch { | |
| // Use the in-memory token when storage is unavailable. | |
| } | |
| const wsUrl = buildWebSocketUrl(freshToken); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/contexts/WebSocketContext.tsx` around lines 108 - 113, Update the token
lookup in connect() to use AUTH_TOKEN_STORAGE_KEY and isolate
localStorage.getItem() in a narrow try/catch. If storage is unavailable or the
read throws, retain the in-memory token fallback so reconnect proceeds; keep
buildWebSocketUrl using the resolved token.
Replace the duplicated 'auth-token' string literal with the shared AUTH_TOKEN_STORAGE_KEY constant from components/auth/constants to prevent silent storage-key contract drift. Addresses CodeRabbit review on siteboon#1016.
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
1 participant
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.
authenticatedFetch(src/utils/api.js) persists a refreshed JWT tolocalStorage['auth-token']when the server returnsX-Refreshed-Token. ButWebSocketContextbuilds the chat WS URL from AuthContext's in-memorytoken, which is seeded once at mount and only updated by login/logout, never re-synced fromlocalStorage.As a result, on a long-lived tab the chat WS auto-reconnect keeps reusing the original in-memory token and eventually fails at that token's true expiry, even though REST calls keep silently refreshing. The shell WS (
src/components/shell/utils/socket.ts) and file upload already readlocalStoragefresh and are unaffected.This reads the freshest
auth-tokenfromlocalStorageat (re)connect (falling back to the in-memory token), matching the shell-WS and file-upload paths.One-line frontend change, no new dependencies. I didn't add a test since the repo has no frontend test harness, but I have a regression test for this from a downstream fork and am happy to contribute it if you add one.
Summary by CodeRabbit