feat: connection state overlay for web app#197
Conversation
Add a connection state machine (features/connection) that tracks WebSocket health through 4 progressive states: CONNECTED → GRACE_PERIOD (2s) → RECONNECTING (show thin banner) → DISCONNECTED (30s+, show full banner, dim content). Includes ConnectionBanner at top of content area, ConnectionOrb in sidebar footer replacing BackendStatusIndicator, ServerOfflinePage with warm illustration for first-load/server-off states, and send-attempt-failed escalation from useSendMessage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 09874114ce65
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Lite Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughIntroduce a Zustand connection state machine, a small event bus, init hook, UI (ConnectionBanner, ConnectionOrb, ServerOfflinePage, ConnectionIllustration), and integrations that surface WebSocket send failures and connection lifecycle into layouts and session send-error handling. Changes
Sequence DiagramsequenceDiagram
participant Client as User / UI
participant WS as WebSocket Layer
participant Hooks as Connection Hooks
participant Store as Connection Store
participant Session as Session API
Client->>WS: network interruption / send message
WS->>Session: send error ("not connected"/"disconnected")
Session->>Hooks: emitSendAttemptFailed()
Hooks->>Store: markSendAttemptFailed()
Store->>Store: transition -> "disconnected" (escalation)
Store->>Client: notify state changes
Client->>Client: show ConnectionBanner / update ConnectionOrb
Client->>Client: Click "Retry now"
Client->>Store: retry()
Store->>WS: forceReconnect() / notifyConnectionChange(false)
WS->>Hooks: onConnectionChange(true)
Hooks->>Store: onConnected()
Store->>Client: hide banner, orb -> green
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/features/connection/hooks/useConnectionState.ts (1)
13-18: Extract granular hooks to prevent unnecessary re-renders of components that only need one field.Line 13–18 bundles three independent fields (
state,sendAttemptFailed,retry).ConnectionOrbandMainContentsubscribe to all three but only usestate, causing unnecessary re-renders when the other fields change. Extract individual hooks so each component subscribes only to what it needs.♻️ Suggested refactor
export { type ConnectionState }; -export function useConnectionState() { - const state = useConnectionStore((s) => s.state); - const sendAttemptFailed = useConnectionStore((s) => s.sendAttemptFailed); - const retry = useConnectionStore((s) => s.retry); - - return { state, sendAttemptFailed, retry }; -} +export const useConnectionState = () => useConnectionStore((s) => s.state); +export const useConnectionSendAttemptFailed = () => useConnectionStore((s) => s.sendAttemptFailed); +export const useConnectionRetry = () => useConnectionStore((s) => s.retry);Then
ConnectionBannerimports all three, whileConnectionOrbandMainContentimport onlyuseConnectionState.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/connection/hooks/useConnectionState.ts` around lines 13 - 18, The current useConnectionState hook selects three independent fields from useConnectionStore which causes components that only need one field (like ConnectionOrb and MainContent) to re-render when the others change; split this into granular selectors: create three hooks — e.g. useConnectionState (returns only s.state), useConnectionSendAttemptFailed (returns only s.sendAttemptFailed) and useConnectionRetry (returns only s.retry) — each implemented with useConnectionStore((s) => s.<field>); update ConnectionOrb and MainContent to import only useConnectionState and update ConnectionBanner to import all three hooks; keep existing names (useConnectionState, sendAttemptFailed, retry) referenced in components to make locating changes simple and ensure returned types/signatures remain consistent.apps/web/src/features/connection/ui/ConnectionBanner.tsx (1)
52-55: Consider disabling ping animation for reduced motion.The amber dot uses
animate-pingwhich runs indefinitely. Users withprefers-reduced-motionmay prefer this disabled.♻️ Proposed fix
- <span className="bg-accent-gold absolute inline-flex h-full w-full animate-ping rounded-full opacity-40" /> + {!reduceMotion && ( + <span className="bg-accent-gold absolute inline-flex h-full w-full animate-ping rounded-full opacity-40" /> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/connection/ui/ConnectionBanner.tsx` around lines 52 - 55, Replace the unconditional Tailwind animation class on the amber ping with motion-aware utilities so users with prefers-reduced-motion won't see it; in the ConnectionBanner component update the element using "animate-ping" (the inner span with className including animate-ping) to use "motion-safe:animate-ping motion-reduce:animate-none" (or equivalent CSS respecting prefers-reduced-motion) so the ping runs only when motion is allowed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/features/session/api/session.queries.ts`:
- Around line 362-365: The current detection of WS disconnects in the _err
handler (the block that calls emitSendAttemptFailed) is brittle because it only
checks message substrings; create or import a shared predicate named
isWsConnectivityError in the WS layer/util and use it here instead of substring
checks: replace the msg.includes(...) logic with a call to
isWsConnectivityError(_err) (or isWsConnectivityError(_err.message)) so errors
like "WebSocket connection failed" and other variants are consistently detected
and trigger emitSendAttemptFailed. Ensure the new predicate is exported from the
WS layer or shared utils and update imports in session.queries.ts where _err is
evaluated.
---
Nitpick comments:
In `@apps/web/src/features/connection/hooks/useConnectionState.ts`:
- Around line 13-18: The current useConnectionState hook selects three
independent fields from useConnectionStore which causes components that only
need one field (like ConnectionOrb and MainContent) to re-render when the others
change; split this into granular selectors: create three hooks — e.g.
useConnectionState (returns only s.state), useConnectionSendAttemptFailed
(returns only s.sendAttemptFailed) and useConnectionRetry (returns only s.retry)
— each implemented with useConnectionStore((s) => s.<field>); update
ConnectionOrb and MainContent to import only useConnectionState and update
ConnectionBanner to import all three hooks; keep existing names
(useConnectionState, sendAttemptFailed, retry) referenced in components to make
locating changes simple and ensure returned types/signatures remain consistent.
In `@apps/web/src/features/connection/ui/ConnectionBanner.tsx`:
- Around line 52-55: Replace the unconditional Tailwind animation class on the
amber ping with motion-aware utilities so users with prefers-reduced-motion
won't see it; in the ConnectionBanner component update the element using
"animate-ping" (the inner span with className including animate-ping) to use
"motion-safe:animate-ping motion-reduce:animate-none" (or equivalent CSS
respecting prefers-reduced-motion) so the ping runs only when motion is allowed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Lite
Run ID: edc84863-2adf-4c9e-866f-503f50baca71
📒 Files selected for processing (16)
.claude/agent-memory/code-reviewer/MEMORY.mdapps/web/src/app/layouts/MainContent.tsxapps/web/src/app/layouts/MainLayout.tsxapps/web/src/app/shells/DesktopShell.tsxapps/web/src/app/shells/ServerLayout.tsxapps/web/src/features/connection/hooks/useConnectionState.tsapps/web/src/features/connection/hooks/useConnectionStateInit.tsapps/web/src/features/connection/index.tsapps/web/src/features/connection/lib/connectionEvents.tsapps/web/src/features/connection/store/connectionStore.tsapps/web/src/features/connection/ui/ConnectionBanner.tsxapps/web/src/features/connection/ui/ConnectionIllustration.tsxapps/web/src/features/connection/ui/ConnectionOrb.tsxapps/web/src/features/connection/ui/ServerOfflinePage.tsxapps/web/src/features/session/api/session.queries.tsapps/web/src/features/sidebar/ui/SidebarFooter.tsx
| if (_err instanceof Error) { | ||
| const msg = _err.message.toLowerCase(); | ||
| if (msg.includes("not connected") || msg.includes("disconnected")) { | ||
| emitSendAttemptFailed(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current WS-layer error wording used by connect/send paths.
# Expected: enumerate all thrown/rejected connectivity error strings so we can
# confirm whether current substring checks are comprehensive.
set -euo pipefail
echo "== Candidate WS implementation files =="
fd 'ws.*\.ts$' apps/web/src
echo
echo "== Connectivity-related error sites =="
fd 'ws.*\.ts$' apps/web/src -x rg -n -C2 'throw new Error|Error\(|not connected|disconnected|socket|closed'
echo
echo "== sendCommand/connect usage context =="
rg -n -C2 '\bsendCommand\s*\(|\bconnect\s*\(' apps/web/src/features/session/api/session.queries.ts apps/web/src/platformRepository: zvadaadam/box-ide
Length of output: 727
🏁 Script executed:
# Search for WebSocket client setup and actual implementation
echo "=== Searching for WebSocket client initialization ==="
rg -n "new WebSocket|ws\(" apps/web/src --type ts --type tsx -B2 -A2
echo
echo "=== Look at the full context of lines 359-375 in session.queries.ts ==="
sed -n '350,380p' apps/web/src/features/session/api/session.queries.ts
echo
echo "=== Search for error handling patterns around WebSocket errors ==="
rg -n "instanceof Error|Error\(|disconnect|connectivity" apps/web/src/features/session/api/session.queries.ts -B1 -A1
echo
echo "=== Find where WebSocket is imported/used in the session module ==="
rg -n "import.*[Ww]eb[Ss]ocket|WebSocket|ws\b" apps/web/src/features/session --type ts --type tsxRepository: zvadaadam/box-ide
Length of output: 2168
🏁 Script executed:
# Search without problematic type flags
echo "=== WebSocket imports and setup ==="
rg -n "WebSocket|ws\(" apps/web/src/features/session -B2 -A2
echo
echo "=== Check useWsToolRequest hook mentioned in earlier search ==="
head -100 apps/web/src/shared/hooks/useWsToolRequest.ts
echo
echo "=== Search broadly for error message generation ==="
rg -n '"WebSocket|disconnected|not connected' apps/web/src
echo
echo "=== Check the full session.queries.ts file structure and imports ==="
head -50 apps/web/src/features/session/api/session.queries.tsRepository: zvadaadam/box-ide
Length of output: 13107
🏁 Script executed:
# Double-check: verify "WebSocket connection failed" is thrown in connectivity context
sed -n '620,635p' apps/web/src/platform/ws/query-protocol-client.tsRepository: zvadaadam/box-ide
Length of output: 519
Harden disconnect detection beyond message substrings.
Lines 364–365 rely on substring matching which misses error variants. The WS client throws "WebSocket connection failed" on failed connect attempts, but the current check only catches "not connected" or "disconnected"—causing missed state escalation.
💡 Suggested direction
- if (_err instanceof Error) {
- const msg = _err.message.toLowerCase();
- if (msg.includes("not connected") || msg.includes("disconnected")) {
- emitSendAttemptFailed();
- }
- }
+ if (isWsConnectivityError(_err)) {
+ emitSendAttemptFailed();
+ }Define isWsConnectivityError once in the WS layer (or a shared util) and reuse it here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/features/session/api/session.queries.ts` around lines 362 - 365,
The current detection of WS disconnects in the _err handler (the block that
calls emitSendAttemptFailed) is brittle because it only checks message
substrings; create or import a shared predicate named isWsConnectivityError in
the WS layer/util and use it here instead of substring checks: replace the
msg.includes(...) logic with a call to isWsConnectivityError(_err) (or
isWsConnectivityError(_err.message)) so errors like "WebSocket connection
failed" and other variants are consistently detected and trigger
emitSendAttemptFailed. Ensure the new predicate is exported from the WS layer or
shared utils and update imports in session.queries.ts where _err is evaluated.
Re: CodeRabbit ReviewFinding #1 (walkthrough/summary) — Informational, no action needed.Finding #2 (
|
Prettier formatting for 4 connection UI components and remove unused `capabilities` import from DesktopShell.tsx. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: e27357a10bbf
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/src/features/connection/ui/ConnectionBanner.tsx (1)
25-26: Setmode="wait"on allAnimatePresencewrappers in this TSX file.The copy block already uses
mode="wait", but the outer banner and retry-button presence wrappers do not.Suggested patch
- <AnimatePresence> + <AnimatePresence mode="wait"> @@ - <AnimatePresence> + <AnimatePresence mode="wait">Based on learnings: "Applies to src/**/*.{tsx} : Wrap conditional renders with
AnimatePresencewhen exit animations are needed... Always setmode='wait'to prevent layout jumps".Also applies to: 87-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/connection/ui/ConnectionBanner.tsx` around lines 25 - 26, The file has multiple AnimatePresence wrappers (the outer banner wrapper around the showBanner conditional and the retry-button presence wrapper) that are missing mode="wait", which can cause layout jumps; update every AnimatePresence instance in ConnectionBanner.tsx (including the one that wraps the showBanner conditional and the one that controls the retry button presence—note the copy block already uses mode="wait") to include mode="wait" so exit animations complete before new enters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/features/connection/ui/ConnectionBanner.tsx`:
- Around line 48-50: The pulsing dot and inner fade animations in
ConnectionBanner still run under reduced-motion; update the component
(ConnectionBanner) to read the same useReducedMotion() flag used for the
container and disable/short-circuit those animations when reducedMotion is true:
remove or omit the "animate-ping" class on the outer span and the
transition/animate props or motion wrappers on the inner fade elements when
reducedMotion is enabled, and instead render their static non-animated
equivalents so the visual remains but no continuous or secondary animations run;
apply this for the spans that contain "animate-ping" and any motion/transitions
referenced around the same blocks (also adjust the similar blocks at the other
indicated ranges).
- Around line 21-23: Replace the ad-hoc boolean checks for connection state in
ConnectionBanner with a ts-pattern match against the `state` value to get
compile-time exhaustiveness: import `match` from 'ts-pattern', compute
`showBanner` and `isEscalated` (and any other derived values used in the render
block between the existing `state` checks around lines 55–83) via
`match(state).with(...).exhaustive()` handling `"connected"`, `"grace_period"`,
`"reconnecting"`, and `"disconnected"` explicitly, and update the render logic
to consume those matched results; ensure you call `.exhaustive()` so the
compiler will force you to handle future states.
---
Nitpick comments:
In `@apps/web/src/features/connection/ui/ConnectionBanner.tsx`:
- Around line 25-26: The file has multiple AnimatePresence wrappers (the outer
banner wrapper around the showBanner conditional and the retry-button presence
wrapper) that are missing mode="wait", which can cause layout jumps; update
every AnimatePresence instance in ConnectionBanner.tsx (including the one that
wraps the showBanner conditional and the one that controls the retry button
presence—note the copy block already uses mode="wait") to include mode="wait" so
exit animations complete before new enters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Lite
Run ID: f564c866-810a-4487-890e-9708f4924577
📒 Files selected for processing (5)
apps/web/src/app/shells/DesktopShell.tsxapps/web/src/features/connection/ui/ConnectionBanner.tsxapps/web/src/features/connection/ui/ConnectionIllustration.tsxapps/web/src/features/connection/ui/ConnectionOrb.tsxapps/web/src/features/connection/ui/ServerOfflinePage.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/features/connection/ui/ServerOfflinePage.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/features/connection/ui/ConnectionIllustration.tsx
| const showBanner = state === "reconnecting" || state === "disconnected"; | ||
| const isEscalated = state === "disconnected"; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, find the ConnectionBanner.tsx file
find . -name "ConnectionBanner.tsx" -type fRepository: zvadaadam/box-ide
Length of output: 119
🏁 Script executed:
# Check the current content of the file
cat -n apps/web/src/features/connection/ui/ConnectionBanner.tsxRepository: zvadaadam/box-ide
Length of output: 4950
🏁 Script executed:
# Find the connection state hook to verify the type
find . -name "useConnectionState*" -o -name "*connection*hook*" | grep -E "\.ts$|\.tsx$"Repository: zvadaadam/box-ide
Length of output: 190
🏁 Script executed:
cat -n apps/web/src/features/connection/hooks/useConnectionState.tsRepository: zvadaadam/box-ide
Length of output: 816
🏁 Script executed:
find . -name "connectionStore.ts" -o -name "connectionStore.tsx" | head -5Repository: zvadaadam/box-ide
Length of output: 120
🏁 Script executed:
cat -n apps/web/src/features/connection/store/connectionStore.tsRepository: zvadaadam/box-ide
Length of output: 3714
🏁 Script executed:
# Check if ts-pattern is available in the project
grep -r "ts-pattern" package.json bun.lock 2>/dev/null | head -20Repository: zvadaadam/box-ide
Length of output: 298
Convert state branching to ts-pattern with .exhaustive() for compile-time state exhaustiveness.
The current boolean/ternary approach misses opportunities for compile-time safety. Using ts-pattern ensures all connection states ("connected", "grace_period", "reconnecting", "disconnected") are handled explicitly, preventing bugs when new states are added.
Suggested patch
+import { match } from "ts-pattern";
import { AnimatePresence, m, useReducedMotion } from "framer-motion";
import { useConnectionState } from "../hooks/useConnectionState";
@@
- const showBanner = state === "reconnecting" || state === "disconnected";
- const isEscalated = state === "disconnected";
+ const bannerState = match(state)
+ .with("connected", () => null)
+ .with("grace_period", () => null)
+ .with("reconnecting", () => ({ isEscalated: false }))
+ .with("disconnected", () => ({ isEscalated: true }))
+ .exhaustive();
+
+ if (!bannerState) return null;
+ const { isEscalated } = bannerState;
@@
- {showBanner && (
- <m.div
+ <m.div
key="connection-banner"
@@
- </m.div>
- )}
+ </m.div>
</AnimatePresence>Applies to lines 21–23 and 55–83.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/features/connection/ui/ConnectionBanner.tsx` around lines 21 -
23, Replace the ad-hoc boolean checks for connection state in ConnectionBanner
with a ts-pattern match against the `state` value to get compile-time
exhaustiveness: import `match` from 'ts-pattern', compute `showBanner` and
`isEscalated` (and any other derived values used in the render block between the
existing `state` checks around lines 55–83) via
`match(state).with(...).exhaustive()` handling `"connected"`, `"grace_period"`,
`"reconnecting"`, and `"disconnected"` explicitly, and update the render logic
to consume those matched results; ensure you call `.exhaustive()` so the
compiler will force you to handle future states.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 78fb175e4e65
- retry() now resets store to grace_period before forceReconnect() so user gets immediate "Reconnecting..." feedback instead of the banner staying stuck on "Connection lost" (forceReconnect skips notifyConnectionChange when ws is null after 30s+) - Add "connection failed" to WS error substring matching in useSendMessage.onError — catches connect() rejections that produce "WebSocket connection failed" - Replace built-in "easeInOut" with "linear" on WaitingDots per CLAUDE.md animation guidelines Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 4d09e1ce83e2
Suppress animate-ping on the amber dot and set inner fade transition durations to 0 when useReducedMotion() is true. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 9a14372914a6
Re: CodeRabbit Review (Round 2)Comment #1 — Correcting my earlier analysisMy earlier comment incorrectly stated that Comment #2 (ts-pattern in ConnectionBanner) — Not actionable.The current code derives 2 boolean flags from state: const showBanner = state === "reconnecting" || state === "disconnected";
const isEscalated = state === "disconnected";This isn't a switch/case or if/else chain — it's 2 assignments. The suggested ts-pattern version returns Comment #3 (reduced-motion inner animations) — Fixed in 0cc9b2a.
|
1 participant
Summary
features/connectionmodule with a Zustand state machine tracking WebSocket health through 4 progressive states: CONNECTED → GRACE_PERIOD (2s silence) → RECONNECTING (thin amber banner) → DISCONNECTED (30s+, full banner with retry, content dimmed to 60%)ConnectionBannerslides down from top of content area with amber accent line, escalates copy from "Reconnecting..." to "Connection lost — Your agents are still running" with a Retry buttonConnectionOrbreplacesBackendStatusIndicatorin sidebar footer — always-visible green/amber dot with breathing pulse animation and "Offline" labelServerOfflinePagewith warm SVG illustration replaces inline error JSX in bothDesktopShellandServerLayout, with relay-aware copy variant ("Desktop app not detected" vs "Your computer isn't connected")useSendMessageonError now emitssendAttemptFailedto immediately escalate connection state when a user action is blocked mid-flightTest plan
prefers-reduced-motionenabled — verify animations are disabled/reduced🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements