fix: centralized WebSocket liveness checks and UI status indicator#691
Open
menny wants to merge 4 commits intositeboon:mainfrom
Open
fix: centralized WebSocket liveness checks and UI status indicator#691menny wants to merge 4 commits intositeboon:mainfrom
menny wants to merge 4 commits intositeboon:mainfrom
Conversation
…g requests SessionManager loads all persisted Gemini sessions from ~/.gemini/sessions/*.json asynchronously in its constructor via `this.ready = this.init()`. Previously, `sessionManager.ready` was never awaited anywhere, so the server would start accepting WebSocket connections before loadSessions() had completed. This created a race condition on the very first request after a server restart: if a client tried to resume an existing Gemini session, `sessionManager.getSession(sessionId)` would return undefined (the session hadn't been loaded from disk yet), causing the `--resume <cliSessionId>` flag to be silently omitted when spawning the Gemini CLI process. The CLI would then start a blank new session instead of continuing the prior conversation, with no error or indication to the user. The fix adds `await sessionManager.ready` in startServer() immediately after `await initializeDatabase()`, following the same pattern already established for database initialization. This guarantees the session store is fully populated before any request handler can call getSession(), createSession(), or addMessage(). No other providers are affected: Claude, Cursor, and Codex session providers are stateless per-request (they read from disk or SQLite on demand) and have no equivalent eager-loading singleton. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This comprehensive update addresses Issue siteboon#12 and enhances overall system stability: 1. Safe WebSocket Writing: Enhanced WebSocketWriter in server/index.js with readyState checks and try-catch blocks to prevent server crashes on client disconnects. 2. Server-Side Heartbeat: Implemented a ping/pong mechanism to actively detect and terminate dead connections. 3. Automatic Process Cleanup: Updated the disconnect handler to proactively kill orphaned CLI processes (Gemini, Cursor, Codex, Claude) when their associated WebSocket closes. 4. Provider Migration: Migrated all major providers (Gemini, Cursor, Codex, Claude SDK) to use the safe WebSocketWriter instead of raw .send() calls. 5. UI Connection Indicator: Added a visual status dot in the SidebarHeader to indicate WebSocket connectivity state (Green: Connected, Red: Disconnected).
Used StringDecoder from the string_decoder module instead of Buffer.toString() in gemini-cli.js and cursor-cli.js. This ensures that multi-byte characters that are split across process stdout/stderr data chunks are correctly decoded, preventing garbled text at chunk boundaries in streamed LLM responses.
Cleaned up duplicate export keywords from function definitions that were also being exported in the centralized export blocks at the end of the files. This fixes the SyntaxError that was causing the server to crash on startup.
Contributor
📝 WalkthroughWalkthroughThe PR refactors WebSocket communication across the server by introducing a Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant WS as WebSocket Server
participant Provider as Provider (CLI)
participant SessionMgr as Session Manager
rect rgba(100, 200, 150, 0.5)
Note over Client,SessionMgr: New Heartbeat & Keep-Alive
Client->>WS: Connect
WS->>WS: Set ws.isAlive = true
loop Every 30s (or configured interval)
WS->>Client: ping
Client->>WS: pong
WS->>WS: Update ws.isAlive = true
end
alt Client unresponsive
WS->>Client: ping (no pong received)
WS->>WS: ws.isAlive still false
WS->>Client: terminate connection
end
end
rect rgba(200, 100, 150, 0.5)
Note over Client,SessionMgr: Disconnect & Session Cleanup
Client->>WS: disconnect
WS->>Provider: abortXSessionsForWebSocket(ws)
activate Provider
Provider->>SessionMgr: identify sessions with proc.ws === disconnected_ws
Provider->>SessionMgr: abort each orphaned session
deactivate Provider
Provider-->>WS: return count of aborted sessions
WS->>WS: log cleanup results per provider
end
Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/claude-sdk.js (1)
236-244:⚠️ Potential issue | 🟠 MajorKeep the writer on active sessions so reconnects still work.
addSessionnow stores onlyws, butreconnectSessionWritercheckssession.writer. This makes reconnects always fail and leaves active Claude output bound to the stale socket.Proposed state fix
function addSession(sessionId, queryInstance, tempImagePaths = [], tempDir = null, writer = null) { + const sessionWs = writer && writer.isWebSocketWriter + ? writer.ws + : (writer && typeof writer.send === 'function' ? writer : null); + activeSessions.set(sessionId, { instance: queryInstance, startTime: Date.now(), status: 'active', tempImagePaths, tempDir, - ws: writer && writer.isWebSocketWriter ? writer.ws : (writer && typeof writer.send === 'function' ? writer : null) + writer, + ws: sessionWs }); } @@ function reconnectSessionWriter(sessionId, newRawWs) { const session = getSession(sessionId); if (!session?.writer?.updateWebSocket) return false; session.writer.updateWebSocket(newRawWs); + session.ws = newRawWs; console.log(`[RECONNECT] Writer swapped for session ${sessionId}`); return true; }Also applies to: 838-843
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/claude-sdk.js` around lines 236 - 244, The session object created by addSession only stores ws, but reconnectSessionWriter expects session.writer, so reconnects fail; modify addSession to keep the original writer under the key writer (in addition to setting ws for convenience) so reconnectSessionWriter can find and reuse it (handle writer when writer.isWebSocketWriter and when writer has send), and apply the same fix to the other addSession occurrence around the 838-843 area to preserve writer on activeSessions.server/gemini-cli.js (1)
232-355:⚠️ Potential issue | 🟠 MajorMake the no-writer path consistently safe.
Line 253 allows
writerto be absent, but the fallback and error/completion paths still callwriter.send(...). A no-writer invocation will crash instead of using the intended raw/recording fallback.Proposed safe-send helper
+ const send = (message) => { + if (writer && typeof writer.send === 'function') { + return writer.send(message); + } + return false; + }; + const startTimeout = () => { if (timeout) clearTimeout(timeout); timeout = setTimeout(() => { const socketSessionId = writer && typeof writer.getSessionId === 'function' ? writer.getSessionId() : (capturedSessionId || sessionId || processKey); terminalFailureReason = `Gemini CLI timeout - no response received for ${timeoutMs / 1000} seconds`; - writer.send(createNormalizedMessage({ kind: 'error', content: terminalFailureReason, sessionId: socketSessionId, provider: 'gemini' })); + send(createNormalizedMessage({ kind: 'error', content: terminalFailureReason, sessionId: socketSessionId, provider: 'gemini' })); try { geminiProcess.kill('SIGTERM'); } catch (e) { } }, timeoutMs); }; @@ - writer.send(createNormalizedMessage({ kind: 'session_created', newSessionId: capturedSessionId, sessionId: capturedSessionId, provider: 'gemini' })); + send(createNormalizedMessage({ kind: 'session_created', newSessionId: capturedSessionId, sessionId: capturedSessionId, provider: 'gemini' })); @@ - writer.send(createNormalizedMessage({ kind: 'stream_delta', content: rawOutput, sessionId: socketSessionId, provider: 'gemini' })); + send(createNormalizedMessage({ kind: 'stream_delta', content: rawOutput, sessionId: socketSessionId, provider: 'gemini' })); @@ - writer.send(createNormalizedMessage({ kind: 'error', content: errorMsg, sessionId: socketSessionId, provider: 'gemini' })); + send(createNormalizedMessage({ kind: 'error', content: errorMsg, sessionId: socketSessionId, provider: 'gemini' })); @@ - writer.send(createNormalizedMessage({ kind: 'complete', exitCode: code, isNewSession: !sessionId && !!command, sessionId: finalSessionId, provider: 'gemini' })); + send(createNormalizedMessage({ kind: 'complete', exitCode: code, isNewSession: !sessionId && !!command, sessionId: finalSessionId, provider: 'gemini' })); @@ - writer.send(createNormalizedMessage({ kind: 'error', content: 'Gemini CLI is not installed. Please install it first: https://github.com/google-gemini/gemini-cli', sessionId: socketSessionId, provider: 'gemini' })); + send(createNormalizedMessage({ kind: 'error', content: 'Gemini CLI is not installed. Please install it first: https://github.com/google-gemini/gemini-cli', sessionId: socketSessionId, provider: 'gemini' })); @@ - writer.send(createNormalizedMessage({ kind: 'error', content: errorContent, sessionId: errorSessionId, provider: 'gemini' })); + send(createNormalizedMessage({ kind: 'error', content: errorContent, sessionId: errorSessionId, provider: 'gemini' }));Also applies to: 377-423
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/gemini-cli.js` around lines 232 - 355, The code calls writer.send(...) in several places (stdout handler, stderr handler, completion paths) without guarding that writer may be null, causing crashes when running in no-writer/raw-recording mode; add a small safe-send helper (e.g., safeSend(normalizedMsg, fallbackSessionId)) and replace direct writer.send(...) calls with it so the helper checks writer && typeof writer.send === 'function' before calling writer.send, and otherwise records the same normalized message into sessionManager via sessionManager.addMessage(capturedSessionId || sessionId, 'assistant' or 'error', ...) or performs the existing raw/recording fallback; update uses in the stdout data handler (where responseHandler is absent), stderr handler, and any completion/error cleanup code that currently calls writer.send to ensure consistent, safe behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/index.js`:
- Around line 1564-1578: The cleanup helpers abortClaudeSDKSessionsForWebSocket,
abortCursorSessionsForWebSocket, abortCodexSessionsForWebSocket, and
abortGeminiSessionsForWebSocket are being called but not imported; add imports
for these helper functions at the top of the module (using the same module
system as this file—require(...) or ES import) from the module that exports
them, e.g. const { abortClaudeSDKSessionsForWebSocket,
abortCursorSessionsForWebSocket, abortCodexSessionsForWebSocket,
abortGeminiSessionsForWebSocket } = require('...') or the equivalent import
statement, ensuring the names match the exported symbols and then rerun
lint/tests.
- Around line 251-260: The heartbeat interval currently calls ws.ping()
unguarded which can throw if the socket's readyState isn't OPEN; update the
setInterval handler where heartbeatInterval and wss are used to check each
connection's readyState against WebSocket.OPEN before calling ws.ping(), and
wrap the ping call in a try/catch to swallow/log any errors instead of letting
the interval crash; ensure you still terminate when ws.isAlive === false and
retain setting ws.isAlive = false for live sockets (referencing ws.isAlive,
ws.ping, readyState, and WebSocket.OPEN).
In `@src/components/sidebar/view/subcomponents/SidebarHeader.tsx`:
- Around line 52-58: The connection-dot is currently only a colored empty div
with a title, which is not accessible or motion-safe; update the SidebarHeader
indicator (the div using isConnected, cn, and title) to include an accessible
status label and respect reduced-motion: add a visually-hidden text node or
aria-label/aria-live (e.g., a sr-only span announcing "Connected"/"Disconnected"
derived from isConnected) so screen readers and touch users get the status, and
replace animate-pulse with Tailwind motion-aware classes
(motion-safe:animate-pulse and motion-reduce:animate-none) so the pulse is
disabled for users who prefer reduced motion. Ensure the element retains its
role/status semantics (role="status" or aria-live) and keep the existing
className logic using cn/isConnected.
- Line 44: SidebarHeader subscribes to the entire WebSocket context by calling
useWebSocket(), causing re-renders on every latestMessage update; instead
extract only the connection state by creating and using a new hook (e.g.,
useWebSocketConnection or useIsWebSocketConnected) that selects and returns only
isConnected from the WebSocket context, update SidebarHeader to call that new
hook (replace useWebSocket() with useIsWebSocketConnected()) so the component no
longer re-renders on latestMessage changes, and ensure the original WebSocket
provider/context still exposes latestMessage for other consumers.
---
Outside diff comments:
In `@server/claude-sdk.js`:
- Around line 236-244: The session object created by addSession only stores ws,
but reconnectSessionWriter expects session.writer, so reconnects fail; modify
addSession to keep the original writer under the key writer (in addition to
setting ws for convenience) so reconnectSessionWriter can find and reuse it
(handle writer when writer.isWebSocketWriter and when writer has send), and
apply the same fix to the other addSession occurrence around the 838-843 area to
preserve writer on activeSessions.
In `@server/gemini-cli.js`:
- Around line 232-355: The code calls writer.send(...) in several places (stdout
handler, stderr handler, completion paths) without guarding that writer may be
null, causing crashes when running in no-writer/raw-recording mode; add a small
safe-send helper (e.g., safeSend(normalizedMsg, fallbackSessionId)) and replace
direct writer.send(...) calls with it so the helper checks writer && typeof
writer.send === 'function' before calling writer.send, and otherwise records the
same normalized message into sessionManager via
sessionManager.addMessage(capturedSessionId || sessionId, 'assistant' or
'error', ...) or performs the existing raw/recording fallback; update uses in
the stdout data handler (where responseHandler is absent), stderr handler, and
any completion/error cleanup code that currently calls writer.send to ensure
consistent, safe behavior.
🪄 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: cdcaf0e7-c83b-4d0a-bc83-52fa09ff17ac
📒 Files selected for processing (7)
server/claude-sdk.jsserver/cursor-cli.jsserver/gemini-cli.jsserver/gemini-response-handler.jsserver/index.jsserver/openai-codex.jssrc/components/sidebar/view/subcomponents/SidebarHeader.tsx
Comment on lines
+251
to
+260
| // Setup heartbeat interval to detect dead connections | ||
| const heartbeatInterval = setInterval(() => { | ||
| wss.clients.forEach((ws) => { | ||
| if (ws.isAlive === false) { | ||
| console.log('🔌 Terminating dead WebSocket connection'); | ||
| return ws.terminate(); | ||
| } | ||
| ws.isAlive = false; | ||
| ws.ping(); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In the npm ws WebSocket library, can websocket.ping() throw when readyState is CLOSING or CLOSED?
💡 Result:
Yes, in the npm ws WebSocket library, websocket.ping can throw an error when the readyState is CLOSING (2) or CLOSED (3). Multiple GitHub issues document this behavior with stack traces showing "Error: WebSocket is not open: readyState 2 (CLOSING)" and "readyState 3 (CLOSED)" thrown from websocket.ping. Official documentation states it throws only if readyState is CONNECTING (0), but real-world evidence from issues #1410, #1551, #1724 shows it throws for non-OPEN states. Users are advised to check readyState === WebSocket.OPEN before calling ping or use the callback to handle errors. In older versions (<7.0.0), send behaved similarly by throwing; later versions pass errors to callbacks, but ping retains throwing behavior per reports up to recent years.
Citations:
- 1: ping throws an exception if connection is broken (not an "error" event) websockets/ws#1551
- 2: Error: WebSocket is not open: readyState 3 (CLOSED) websockets/ws#1724
- 3: Error thrown from library when pinging clients which went to closing state. websockets/ws#1410
- 4: https://github.com/websockets/ws/blob/8.8.1/doc/ws.md
- 5: https://github.com/websockets/ws/blob/master/doc/ws.md
Guard heartbeat pings against closing sockets.
ws.ping() throws an error when readyState is not OPEN (including CLOSING and CLOSED states). The current code calls ping without checking the socket state or catching exceptions, which can crash the server's heartbeat interval.
Per npm ws library behavior (confirmed in GitHub issues #1410, #1551, #1724), calling ping() on a non-OPEN socket throws "Error: WebSocket is not open: readyState X" rather than failing silently. Check readyState === WebSocket.OPEN before pinging and wrap the call in try/catch to handle edge cases.
Proposed hardening
wss.clients.forEach((ws) => {
if (ws.isAlive === false) {
console.log('🔌 Terminating dead WebSocket connection');
return ws.terminate();
}
+ if (ws.readyState !== WebSocket.OPEN) {
+ return;
+ }
ws.isAlive = false;
- ws.ping();
+ try {
+ ws.ping();
+ } catch (error) {
+ console.warn('[WebSocket] Heartbeat ping failed:', error.message);
+ ws.terminate();
+ }
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/index.js` around lines 251 - 260, The heartbeat interval currently
calls ws.ping() unguarded which can throw if the socket's readyState isn't OPEN;
update the setInterval handler where heartbeatInterval and wss are used to check
each connection's readyState against WebSocket.OPEN before calling ws.ping(),
and wrap the ping call in a try/catch to swallow/log any errors instead of
letting the interval crash; ensure you still terminate when ws.isAlive === false
and retain setting ws.isAlive = false for live sockets (referencing ws.isAlive,
ws.ping, readyState, and WebSocket.OPEN).
Comment on lines
+1564
to
+1578
| // Abort any active sessions associated with this specific WebSocket | ||
| try { | ||
| const counts = { | ||
| claude: abortClaudeSDKSessionsForWebSocket(ws), | ||
| cursor: abortCursorSessionsForWebSocket(ws), | ||
| codex: abortCodexSessionsForWebSocket(ws), | ||
| gemini: abortGeminiSessionsForWebSocket(ws) | ||
| }; | ||
| const total = counts.claude + counts.cursor + counts.codex + counts.gemini; | ||
| if (total > 0) { | ||
| console.log(`🧹 Cleaned up ${total} orphaned sessions (Claude:${counts.claude}, Cursor:${counts.cursor}, Codex:${counts.codex}, Gemini:${counts.gemini})`); | ||
| } | ||
| } catch (err) { | ||
| console.error('[Cleanup] Error during session cleanup:', err.message); | ||
| } |
Contributor
There was a problem hiding this comment.
Import the cleanup helpers before calling them.
Line 1567 calls helper functions that are not imported in this file, so the first call throws ReferenceError and the orphaned-session cleanup does not run.
Proposed import fix
-import { queryClaudeSDK, abortClaudeSDKSession, isClaudeSDKSessionActive, getActiveClaudeSDKSessions, resolveToolApproval, getPendingApprovalsForSession, reconnectSessionWriter } from './claude-sdk.js';
-import { spawnCursor, abortCursorSession, isCursorSessionActive, getActiveCursorSessions } from './cursor-cli.js';
-import { queryCodex, abortCodexSession, isCodexSessionActive, getActiveCodexSessions } from './openai-codex.js';
-import { spawnGemini, abortGeminiSession, isGeminiSessionActive, getActiveGeminiSessions } from './gemini-cli.js';
+import { queryClaudeSDK, abortClaudeSDKSession, abortClaudeSDKSessionsForWebSocket, isClaudeSDKSessionActive, getActiveClaudeSDKSessions, resolveToolApproval, getPendingApprovalsForSession, reconnectSessionWriter } from './claude-sdk.js';
+import { spawnCursor, abortCursorSession, abortCursorSessionsForWebSocket, isCursorSessionActive, getActiveCursorSessions } from './cursor-cli.js';
+import { queryCodex, abortCodexSession, abortCodexSessionsForWebSocket, isCodexSessionActive, getActiveCodexSessions } from './openai-codex.js';
+import { spawnGemini, abortGeminiSession, abortGeminiSessionsForWebSocket, isGeminiSessionActive, getActiveGeminiSessions } from './gemini-cli.js';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/index.js` around lines 1564 - 1578, The cleanup helpers
abortClaudeSDKSessionsForWebSocket, abortCursorSessionsForWebSocket,
abortCodexSessionsForWebSocket, and abortGeminiSessionsForWebSocket are being
called but not imported; add imports for these helper functions at the top of
the module (using the same module system as this file—require(...) or ES import)
from the module that exports them, e.g. const {
abortClaudeSDKSessionsForWebSocket, abortCursorSessionsForWebSocket,
abortCodexSessionsForWebSocket, abortGeminiSessionsForWebSocket } =
require('...') or the equivalent import statement, ensuring the names match the
exported symbols and then rerun lint/tests.
| onCollapseSidebar, | ||
| t, | ||
| }: SidebarHeaderProps) { | ||
| const { isConnected } = useWebSocket(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that SidebarHeader consumes useWebSocket and that the provider value changes with latestMessage.
rg -n -C4 'useWebSocket\(\)|const value: WebSocketContextType|latestMessage|isConnected' \
src/components/sidebar/view/subcomponents/SidebarHeader.tsx \
src/contexts/WebSocketContext.tsxRepository: siteboon/claudecodeui
Length of output: 4509
🏁 Script executed:
#!/bin/bash
# Find the implementation of useWebSocket hook
rg -n 'const useWebSocket|export.*useWebSocket|function useWebSocket' src/contexts/WebSocketContext.tsxRepository: siteboon/claudecodeui
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Check all usages of useWebSocket in the codebase
rg -r 'useWebSocket\(\)' --type ts --type tsx | head -20Repository: siteboon/claudecodeui
Length of output: 461
🏁 Script executed:
#!/bin/bash
# Check if useContext is used in WebSocketContext and look for selector patterns
rg -n 'useContext|useMemo|useCallback|selector' src/contexts/WebSocketContext.tsxRepository: siteboon/claudecodeui
Length of output: 374
🏁 Script executed:
#!/bin/bash
# Read the useWebSocket implementation
sed -n '14,28p' src/contexts/WebSocketContext.tsxRepository: siteboon/claudecodeui
Length of output: 705
🏁 Script executed:
#!/bin/bash
# Find all usages of useWebSocket throughout the codebase
rg 'useWebSocket' --type ts --type tsx -nRepository: siteboon/claudecodeui
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Find all usages of useWebSocket throughout the codebase
rg 'useWebSocket' -nRepository: siteboon/claudecodeui
Length of output: 1121
🏁 Script executed:
#!/bin/bash
# Check if there's already a selector pattern or useWebSocketConnectionStatus
rg 'useWebSocketConnectionStatus|selector|select' src/contexts/WebSocketContext.tsxRepository: siteboon/claudecodeui
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check for any optimization patterns in the codebase (useMemo, memo, etc.)
rg 'useMemo|memo|useCallback' src/components/sidebar/view/subcomponents/SidebarHeader.tsxRepository: siteboon/claudecodeui
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check the full SidebarHeader component to understand its complexity
cat -n src/components/sidebar/view/subcomponents/SidebarHeader.tsxRepository: siteboon/claudecodeui
Length of output: 12963
🏁 Script executed:
#!/bin/bash
# Check how latestMessage is updated in WebSocketContext
rg -n 'setLatestMessage' src/contexts/WebSocketContext.tsx -A2 -B2Repository: siteboon/claudecodeui
Length of output: 926
Avoid subscribing the whole header to WebSocket message updates.
useWebSocket() returns a context value that changes whenever latestMessage updates (on every WebSocket message). This causes SidebarHeader to re-render for every message even though it only needs isConnected for the status badge. Extract connection-status-only logic into a separate hook to prevent unnecessary re-renders.
Suggested implementation
// In WebSocketContext.tsx, add:
+export const useWebSocketConnectionStatus = () => {
+ const context = useContext(WebSocketContext);
+ if (!context) {
+ throw new Error('useWebSocketConnectionStatus must be used within a WebSocketProvider');
+ }
+ return useMemo(() => context.isConnected, [context.isConnected]);
+};
// In SidebarHeader.tsx:
-import { useWebSocket } from '../../../../contexts/WebSocketContext';
+import { useWebSocketConnectionStatus } from '../../../../contexts/WebSocketContext';
@@
- const { isConnected } = useWebSocket();
+ const isConnected = useWebSocketConnectionStatus();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/sidebar/view/subcomponents/SidebarHeader.tsx` at line 44,
SidebarHeader subscribes to the entire WebSocket context by calling
useWebSocket(), causing re-renders on every latestMessage update; instead
extract only the connection state by creating and using a new hook (e.g.,
useWebSocketConnection or useIsWebSocketConnected) that selects and returns only
isConnected from the WebSocket context, update SidebarHeader to call that new
hook (replace useWebSocket() with useIsWebSocketConnected()) so the component no
longer re-renders on latestMessage changes, and ensure the original WebSocket
provider/context still exposes latestMessage for other consumers.
Comment on lines
+52
to
+58
| <div | ||
| className={cn( | ||
| "absolute -right-0.5 -top-0.5 h-2 w-2 rounded-full border border-background shadow-sm", | ||
| isConnected ? "bg-emerald-500" : "bg-red-500 animate-pulse" | ||
| )} | ||
| title={isConnected ? t('websocket.connected', { defaultValue: 'Connected' }) : t('websocket.disconnected', { defaultValue: 'Disconnected' })} | ||
| /> |
Contributor
There was a problem hiding this comment.
Make the connection status accessible and motion-safe.
The empty dot relies on color/title, which is easy to miss for screen readers and touch users. Also gate the pulse animation behind reduced-motion support.
♿ Proposed fix
+ const connectionStatusLabel = isConnected
+ ? t('websocket.connected', { defaultValue: 'Connected' })
+ : t('websocket.disconnected', { defaultValue: 'Disconnected' });
+
const LogoBlock = () => (
@@
- <div
+ <span
className={cn(
"absolute -right-0.5 -top-0.5 h-2 w-2 rounded-full border border-background shadow-sm",
- isConnected ? "bg-emerald-500" : "bg-red-500 animate-pulse"
+ isConnected ? "bg-emerald-500" : "bg-red-500 motion-safe:animate-pulse"
)}
- title={isConnected ? t('websocket.connected', { defaultValue: 'Connected' }) : t('websocket.disconnected', { defaultValue: 'Disconnected' })}
- />
+ title={connectionStatusLabel}
+ role="status"
+ aria-live="polite"
+ >
+ <span className="sr-only">{connectionStatusLabel}</span>
+ </span>📝 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
| <div | |
| className={cn( | |
| "absolute -right-0.5 -top-0.5 h-2 w-2 rounded-full border border-background shadow-sm", | |
| isConnected ? "bg-emerald-500" : "bg-red-500 animate-pulse" | |
| )} | |
| title={isConnected ? t('websocket.connected', { defaultValue: 'Connected' }) : t('websocket.disconnected', { defaultValue: 'Disconnected' })} | |
| /> | |
| const connectionStatusLabel = isConnected | |
| ? t('websocket.connected', { defaultValue: 'Connected' }) | |
| : t('websocket.disconnected', { defaultValue: 'Disconnected' }); | |
| const LogoBlock = () => ( | |
| <span | |
| className={cn( | |
| "absolute -right-0.5 -top-0.5 h-2 w-2 rounded-full border border-background shadow-sm", | |
| isConnected ? "bg-emerald-500" : "bg-red-500 motion-safe:animate-pulse" | |
| )} | |
| title={connectionStatusLabel} | |
| role="status" | |
| aria-live="polite" | |
| > | |
| <span className="sr-only">{connectionStatusLabel}</span> | |
| </span> | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/sidebar/view/subcomponents/SidebarHeader.tsx` around lines 52
- 58, The connection-dot is currently only a colored empty div with a title,
which is not accessible or motion-safe; update the SidebarHeader indicator (the
div using isConnected, cn, and title) to include an accessible status label and
respect reduced-motion: add a visually-hidden text node or aria-label/aria-live
(e.g., a sr-only span announcing "Connected"/"Disconnected" derived from
isConnected) so screen readers and touch users get the status, and replace
animate-pulse with Tailwind motion-aware classes (motion-safe:animate-pulse and
motion-reduce:animate-none) so the pulse is disabled for users who prefer
reduced motion. Ensure the element retains its role/status semantics
(role="status" or aria-live) and keep the existing className logic using
cn/isConnected.
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.
Summary
This Pull Request addresses several critical stability issues across all LLM providers and adds a visual connection indicator to the UI.
Key Changes
WebSocketWriterinserver/index.jswithreadyStatechecks andtry-catchblocks to prevent unhandled exceptions from crashing the server when a client disconnects.Buffer.toString()withStringDecoderingemini-cli.jsandcursor-cli.jsto correctly handle multi-byte characters split across data chunks.claude-sdk.jsand corrected the Codex SDK import name.SidebarHeader(Green: Connected, Red: Disconnected).Impact
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes