diff --git a/packages/ui/src/components/layout/ConnectionStatusIndicator.tsx b/packages/ui/src/components/layout/ConnectionStatusIndicator.tsx index 3235d95860..4bff2ef651 100644 --- a/packages/ui/src/components/layout/ConnectionStatusIndicator.tsx +++ b/packages/ui/src/components/layout/ConnectionStatusIndicator.tsx @@ -5,7 +5,7 @@ import { TooltipContent, TooltipTrigger, } from '@/components/ui/tooltip'; -import { useI18n, type I18nKey, type I18nParams } from '@/lib/i18n'; +import { useI18n, type I18nKey } from '@/lib/i18n'; import { cn } from '@/lib/utils'; import { useConfigStore } from '@/stores/useConfigStore'; import { @@ -55,12 +55,6 @@ const toneToDotClass = (tone: ConnectionTone): string => { type ConnectionStatusIndicatorBodyProps = { viewModel: ConnectionStatusViewModel; - /** - * The i18n `t` function, passed down from the parent so the body can be - * memoized purely on `viewModel`. When the locale changes, the parent - * re-renders with a new `t` reference and the body picks it up. - */ - t: (key: I18nKey, params?: I18nParams) => string; }; /** @@ -72,8 +66,8 @@ type ConnectionStatusIndicatorBodyProps = { */ const ConnectionStatusIndicatorBody = React.memo(function ConnectionStatusIndicatorBody({ viewModel, - t, }: ConnectionStatusIndicatorBodyProps) { + const { t } = useI18n(); const dotClass = toneToDotClass(viewModel.tone); const stateLabel = t(viewModel.overallLabelKey as I18nKey); const ariaLabel = t('connectionStatus.aria.indicator', { state: stateLabel }); @@ -145,16 +139,14 @@ const ConnectionStatusIndicatorBody = React.memo(function ConnectionStatusIndica * - does NOT subscribe to session list, streaming deltas, or message * state — the source of truth is updated by the existing event * pipeline and health-check paths - * - reads `navigator.onLine` once at mount; the existing sync-context + * - reads `navigator.onLine` on each render; the existing sync-context * browser online/offline listener already updates - * `runtimeTransportState` when the browser reports a change, so the - * ref snapshot is a redundancy check, not a primary signal + * `runtimeTransportState` when the browser reports a change, so this + * stays in sync without installing a second listener here * * No new polling loop is introduced. */ export const ConnectionStatusIndicator: React.FC = React.memo(function ConnectionStatusIndicator() { - const { t } = useI18n(); - // Narrow leaf selectors. Each call returns the same reference when the // corresponding field is unchanged (Zustand uses Object.is on the // selector return value), so this component does not re-render on @@ -162,21 +154,21 @@ export const ConnectionStatusIndicator: React.FC = React.memo(function Connectio const runtimeTransport = useConfigStore((s) => s.runtimeTransportState); const openCodeRuntime = useConfigStore((s) => s.openCodeRuntimeState); - // navigator.onLine is captured once at mount via a lazy ref initializer. - // We intentionally do not install a listener here — the existing - // sync-context browser online/offline listener already mirrors - // navigator.onLine transitions into `runtimeTransportState`, and the - // view model consults `navigatorOffline` only as a redundancy check for - // the case where the transport still believes it is connected. - const navigatorOfflineRef = React.useRef( - typeof navigator === 'object' && navigator !== null && navigator.onLine === false, - ); - const navigatorOffline = navigatorOfflineRef.current; + // Read `navigator.onLine` dynamically on every render. The browser's + // online/offline transitions are mirrored into `runtimeTransportState` + // by the existing sync-context listener, so this component re-renders + // exactly when the value could have changed — the snapshot stays + // fresh and a mount-while-offline app correctly transitions to a + // non-offline state once the network is restored. We intentionally do + // not install a separate `navigator.onLine` listener here; sync-context + // owns that. + const navigatorOffline = + typeof navigator === 'object' && navigator !== null && navigator.onLine === false; const viewModel = React.useMemo( () => buildConnectionStatusViewModel({ runtimeTransport, openCodeRuntime, navigatorOffline }), [runtimeTransport, openCodeRuntime, navigatorOffline], ); - return ; + return ; }); diff --git a/packages/ui/src/stores/useConfigStore.ts b/packages/ui/src/stores/useConfigStore.ts index 6910e19ed2..34194d4fd8 100644 --- a/packages/ui/src/stores/useConfigStore.ts +++ b/packages/ui/src/stores/useConfigStore.ts @@ -977,7 +977,7 @@ interface ConfigStore { selectionSource: "auto" | "manual"; isConnected: boolean; hasEverConnected: boolean; - connectionPhase: "connecting" | "connected" | "reconnecting"; + connectionPhase: "connecting" | "connected" | "reconnecting" | "disconnected"; lastDisconnectReason: string | null; // Hop-separated normalized state (issue #1737 / #1556). The legacy fields // above still drive readiness gating; the two new fields drive the diff --git a/packages/ui/src/sync/__tests__/session-status-snapshot.test.ts b/packages/ui/src/sync/__tests__/session-status-snapshot.test.ts index b6b0247737..40424351a8 100644 --- a/packages/ui/src/sync/__tests__/session-status-snapshot.test.ts +++ b/packages/ui/src/sync/__tests__/session-status-snapshot.test.ts @@ -7,6 +7,7 @@ import type { DirectoryStore } from "../child-store" import { applySessionStatusSnapshot, needsSnapshotAfterStatusPoll, + shouldUseDisconnectedTransportPhase, } from "../sync-context" type StatusSnapshot = Record @@ -115,3 +116,21 @@ describe("needsSnapshotAfterStatusPoll", () => { expect(needsSnapshotAfterStatusPoll(store.getState(), "ses_a", undefined)).toBe(false) }) }) + +describe("shouldUseDisconnectedTransportPhase", () => { + test("returns true for auth-like terminal disconnect reasons", () => { + expect(shouldUseDisconnectedTransportPhase("ws_auth_token_unavailable")).toBe(true) + expect(shouldUseDisconnectedTransportPhase("401")).toBe(true) + expect(shouldUseDisconnectedTransportPhase("forbidden")).toBe(true) + }) + + test("returns true when websocket closes before ready", () => { + expect(shouldUseDisconnectedTransportPhase("ws_closed_before_ready")).toBe(true) + }) + + test("returns false for transient reconnect reasons", () => { + expect(shouldUseDisconnectedTransportPhase("ws_closed:code=1006")).toBe(false) + expect(shouldUseDisconnectedTransportPhase("ws_heartbeat_timeout")).toBe(false) + expect(shouldUseDisconnectedTransportPhase(null)).toBe(false) + }) +}) diff --git a/packages/ui/src/sync/sync-context.tsx b/packages/ui/src/sync/sync-context.tsx index 80ebc7a6df..fc80d5c18d 100644 --- a/packages/ui/src/sync/sync-context.tsx +++ b/packages/ui/src/sync/sync-context.tsx @@ -540,21 +540,37 @@ async function resyncDirectorySessionStatuses( // resync: the store believes the session is active but the snapshot reports it // idle/absent — a suspected missed idle that the monotonic poll deliberately // won't lower on its own. The authoritative resync is the recovery path. -export function needsSnapshotAfterStatusPoll( - state: DirectoryStore, - sessionId: string, - snapshotEntry: DirectorySessionStatusSnapshot[string] | undefined, -): boolean { - const incoming = toSessionStatus(snapshotEntry) - if (incoming && incoming.type !== "idle") return false - const currentStatus = state.session_status?.[sessionId] - return Boolean(currentStatus && currentStatus.type !== "idle") -} - -type EventRoutingIndex = { - sessionDirectoryById: Map - messageSessionById: Map - sessionMessageIdsById: Map> +export function needsSnapshotAfterStatusPoll( + state: DirectoryStore, + sessionId: string, + snapshotEntry: DirectorySessionStatusSnapshot[string] | undefined, +): boolean { + const incoming = toSessionStatus(snapshotEntry) + if (incoming && incoming.type !== "idle") return false + const currentStatus = state.session_status?.[sessionId] + return Boolean(currentStatus && currentStatus.type !== "idle") +} + +export function shouldUseDisconnectedTransportPhase(reason: string | null): boolean { + if (typeof reason !== "string" || reason.length === 0) return false + // A close before the WS stream ever becomes ready typically means the + // server rejected the upgrade (for example missing/expired URL auth). Treat + // it as a terminal disconnect so the indicator stays red instead of looking + // like an in-progress reconnect that can self-heal without user action. + if (reason === "ws_closed_before_ready") return true + return ( + reason.includes("auth") + || reason === "401" + || reason === "403" + || reason === "unauthorized" + || reason === "forbidden" + ) +} + +type EventRoutingIndex = { + sessionDirectoryById: Map + messageSessionById: Map + sessionMessageIdsById: Map> } const SHOULD_DISPATCH_VSCODE_NOTIFICATIONS = isVSCodeRuntime() @@ -1826,19 +1842,22 @@ export function SyncProvider(props: { triggerDirectoryResync(dir) } }, - onDisconnect: (reason) => { - const { hasEverConnected } = useConfigStore.getState() - useConfigStore.setState({ - isConnected: false, - connectionPhase: hasEverConnected ? "reconnecting" : "connecting", - lastDisconnectReason: reason, - }) - useConfigStore.getState().setRuntimeTransportState({ - phase: "disconnected", - reason, - updatedAt: Date.now(), - }) - }, + onDisconnect: (reason) => { + const { hasEverConnected } = useConfigStore.getState() + const transportPhase = shouldUseDisconnectedTransportPhase(reason) + ? "disconnected" + : hasEverConnected ? "reconnecting" : "connecting" + useConfigStore.setState({ + isConnected: false, + connectionPhase: transportPhase, + lastDisconnectReason: reason, + }) + useConfigStore.getState().setRuntimeTransportState({ + phase: transportPhase, + reason, + updatedAt: Date.now(), + }) + }, onTransportSwitch: () => { // Transport changes are gap-prone in real networks. Treat them like a // reconnect and refresh active session snapshots from HTTP.