From bbe3ebc19e5c4d2021ba0dd2dfb3c86ef4b54af0 Mon Sep 17 00:00:00 2001 From: JD Date: Fri, 12 Jun 2026 12:38:12 +0000 Subject: [PATCH] fix(ui): SSE connection status indicators for mobile network drops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements immediate visual feedback when SSE transport disconnects (wifi drops, Cloudflare idle timeouts, mobile network switches). ## Problem Per-instance status dots remained frozen at 'connected' (green) when SSE transport dropped because: - Instance status updates travel over SSE (impossible when SSE is down) - EventSource.onerror doesn't fire immediately on mobile wifi drops - No user-facing indication of workspace-level SSE state ## Solution 1. server-events.ts: Add onDisconnect() handler (mirrors existing onOpen) - Fire disconnectHandlers in scheduleReconnect() when connection drops 2. sse-manager.ts: Register SSE lifecycle handlers - onDisconnect: set ALL instances to 'connecting' (amber dot) - onOpen: clear 'connecting' status (green dot restored by events) - Log transitions for debug visibility 3. AGENTS.md: Add PR Review Principles section - Check regressions, look for better implementations - Be the PR gatekeeper, ruthless code quality - Test before responding, UI/server version parity ## Verification - TypeScript compilation clean (pre-existing SDK errors unrelated) - Vite build successful - Mobile testing: SSE disconnect → immediate amber dots - Reconnect → green dots restored ## Edge Cases Handled - Transient drop: amber → green on reconnect (no false modal) - Instance dies during outage: amber → green → 'disconnected' event → red - 0 instances: Map empty, loop is no-op - Rapid reconnect cycles: idempotent (setting 'connecting' on 'connecting' is no-op) Complements PR #519 (pong retry with timeout) - merged upstream. --- AGENTS.md | 8 ++++++++ packages/ui/src/lib/server-events.ts | 8 ++++++++ packages/ui/src/lib/sse-manager.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 3a8b08cc9..1cd5c66e4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,6 +15,14 @@ - Prefer composable primitives (signals, hooks, utilities) over deep inheritance or implicit global state. - When adding platform integrations (SSE, IPC, SDK), isolate them in thin adapters that surface typed events/actions. +## PR Review Principles +- **Check for regressions first.** Before approving any change, verify the existing behavior still works — run the test suite, test manually on both mobile and desktop, and confirm no unintended side effects in related subsystems. +- **Look for better possible implementations.** Don't settle for the first working approach. Ask: is there a simpler way? Does the codebase already have a pattern for this? Would a different abstraction reduce future maintenance cost? +- **Be the PR gatekeeper.** Every line merged becomes technical debt someone else will read. If it's unclear, fragile, or lacks tests, push back. The reviewer's job is to protect the codebase, not to be nice. +- **Be ruthless about code quality.** Surface-level "LGTM" is negligence. Inspect: naming, error handling, edge cases, type safety, logging (is it useful or just noise?), performance (any unnecessary allocations or re-renders?), and whether the change respects existing architectural boundaries. +- **Test before responding to review comments.** Never reply "works for me" or "this fixes it" without deploying the exact commit and verifying the behavior. Untested responses waste reviewer time and erode trust. +- **UI and server must be built from the same version.** Version mismatches between UI and server cause subtle bugs (e.g., sessions disappearing). Always build both from the same commit before testing. + ## Multi-Language Support (i18n) The UI uses a small custom i18n layer (no ICU/messageformat). When building features, never hardcode user-visible strings. diff --git a/packages/ui/src/lib/server-events.ts b/packages/ui/src/lib/server-events.ts index 4145367dd..e8704193b 100644 --- a/packages/ui/src/lib/server-events.ts +++ b/packages/ui/src/lib/server-events.ts @@ -21,6 +21,7 @@ function logSse(message: string, context?: Record) { class ServerEvents { private handlers = new Map void>>() private openHandlers = new Set<() => void>() + private disconnectHandlers = new Set<() => void>() private connection: WorkspaceEventConnection | null = null private connectGeneration = 0 private retryDelay = RETRY_BASE_DELAY @@ -105,6 +106,8 @@ class ServerEvents { this.connection = null } + this.disconnectHandlers.forEach((handler) => handler()) + logSse("Events stream disconnected, scheduling reconnect", { delayMs: this.retryDelay }) this.retryTimer = setTimeout(() => { this.retryTimer = null @@ -154,6 +157,11 @@ class ServerEvents { return () => this.openHandlers.delete(handler) } + onDisconnect(handler: () => void): () => void { + this.disconnectHandlers.add(handler) + return () => this.disconnectHandlers.delete(handler) + } + restart(reason = "manual restart"): void { this.retryDelay = RETRY_BASE_DELAY this.clearReconnectTimer() diff --git a/packages/ui/src/lib/sse-manager.ts b/packages/ui/src/lib/sse-manager.ts index 47b9ea802..f0f113af1 100644 --- a/packages/ui/src/lib/sse-manager.ts +++ b/packages/ui/src/lib/sse-manager.ts @@ -101,6 +101,8 @@ const [connectionStatus, setConnectionStatus] = createSignal { const payload = event as InstanceStatusPayload this.updateConnectionStatus(payload.instanceId, payload.status) @@ -118,6 +120,30 @@ class SSEManager { this.updateConnectionStatus(payload.instanceId, "connected") this.handleEvent(payload.instanceId, payload.event as SSEEvent) }) + + serverEvents.onDisconnect(() => { + log.info("SSE transport disconnected → setting all instances to 'connecting'") + setConnectionStatus((prev) => { + const next = new Map(prev) + for (const [id] of next) { + next.set(id, "connecting") + } + return next + }) + }) + + serverEvents.onOpen(() => { + log.info("SSE transport reconnected → clearing 'connecting' status") + setConnectionStatus((prev) => { + const next = new Map(prev) + for (const [id, status] of next) { + if (status === "connecting") { + next.delete(id) + } + } + return next + }) + }) } seedStatus(instanceId: string, status: ConnectionStatus) {