From 22b4030057f4b438e1acf8b57f2d93f7299bdbad Mon Sep 17 00:00:00 2001 From: xarmian Date: Sat, 9 May 2026 01:00:05 -0400 Subject: [PATCH] feat(collab): connection-state badge on item editor (TASK-1264) (#461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surfaces the WS connection state ('connecting' | 'synced' | 'reconnecting' | 'offline') as a small badge in the item-detail meta-info row. Visible only when the WS provider exists (i.e. canEdit && !rawMode), so share-page / read-only / raw mode don't render it. Adds CollabProvider.state $state field with transitions on the real-sync edges only. reconnectAttempts is now reset in the syncStep2 branch and the grace timer (not on raw open/close), so a flaky proxy that OPEN→CLOSE-before-sync still reaches the OFFLINE_THRESHOLD. State preserves 'offline' across retries to avoid flicker; pre-first-sync failures stay 'connecting', post-sync drops become 'reconnecting'. Three rounds of Codex review. --- web/src/lib/collab/wsProvider.svelte.ts | 106 ++++++++++++++++-- .../[collection]/[slug]/+page.svelte | 70 +++++++++++- 2 files changed, 168 insertions(+), 8 deletions(-) diff --git a/web/src/lib/collab/wsProvider.svelte.ts b/web/src/lib/collab/wsProvider.svelte.ts index 757d2c5f..fa1ec720 100644 --- a/web/src/lib/collab/wsProvider.svelte.ts +++ b/web/src/lib/collab/wsProvider.svelte.ts @@ -39,6 +39,15 @@ const MESSAGE_AWARENESS = 1; const RECONNECT_BASE_MS = 1_000; const RECONNECT_MAX_MS = 30_000; +/** After this many consecutive failed reconnect attempts, the public + * `state` signal flips to `'offline'` so UX (TASK-1264 indicator) can + * surface a hard failure rather than a perpetual yellow "reconnecting" + * spinner. The provider keeps trying — backoff continues — but the + * user-visible state is honest about the situation. + * + * Three attempts ≈ 1s + 2s + 4s of failure before declaring offline. */ +const OFFLINE_THRESHOLD = 3; + /** Fallback grace before declaring `synced` true on connections that * never receive an explicit syncStep2. The dumb-relay server replays * the op-log as a sequence of BinaryMessage frames but doesn't @@ -72,6 +81,24 @@ export type ApplierRequestHandler = ( expiresAtMillis: number, ) => boolean | Promise; +/** + * Public connection state surfaced to UX (TASK-1264 pending-sync + * indicator). Strictly more informative than `connected` + `synced` + * because it distinguishes the initial handshake from a mid-session + * reconnect from a hard "we've given up trying" failure: + * + * - `connecting` — socket attempting initial open OR open but + * handshake not yet done. Show a neutral spinner. + * - `synced` — socket open AND server has answered our + * syncStep1 (or grace expired). Show green dot. + * - `reconnecting` — socket dropped after a successful session; + * backoff retry in flight. Show yellow. + * - `offline` — multiple consecutive reconnect failures past + * `OFFLINE_THRESHOLD`. Provider keeps trying but + * the UI surfaces a hard-failure colour (red). + */ +export type CollabConnectionState = 'connecting' | 'synced' | 'reconnecting' | 'offline'; + export interface CollabProviderOptions { /** * Override the WebSocket URL. Defaults to a same-origin URL based @@ -110,6 +137,15 @@ export class CollabProvider { */ synced = $state(false); + /** + * Public connection state for UX consumers (TASK-1264). Always + * derive UI from this instead of `connected` + `synced` + * separately — the four-state machine encodes "we never made it" + * vs "we made it then dropped" vs "we've given up", which the + * two-bool combination loses. + */ + state = $state('connecting'); + private ws: WebSocket | null = null; private readonly WebSocketImpl: typeof WebSocket; private readonly onApplierRequest?: ApplierRequestHandler; @@ -235,8 +271,23 @@ export class CollabProvider { } private readonly onOpen = (): void => { - this.reconnectAttempts = 0; this.connected = true; + // Public state ONLY moves on real progress: an OPEN socket + // alone is not progress (a flaky proxy can OPEN→CLOSE-before- + // sync repeatedly). State stays at whatever the most recent + // close handler set it to (or 'connecting' on first attempt) + // until the sync handshake or grace timer below flips it to + // 'synced'. Specifically: + // - 'offline' stays 'offline' (don't flicker to 'connecting' + // until the proxy actually delivers a sync). Per Codex + // round 2 [P2]. + // - 'connecting'/'reconnecting' stay as-is, awaiting sync. + // 'synced' is unreachable here in practice because onClose + // always demotes it before scheduling a reconnect, so we + // don't need an explicit guard. + // NB: reconnectAttempts is also NOT reset here — same reason. + // Reset is owned by the actual-sync paths (syncStep2 branch + + // syncGraceTimer below). Per Codex round 1 [P2]. // Initial syncStep1: send our current state vector. Server // replays the op-log (which contains all prior peer ops) so @@ -288,6 +339,15 @@ export class CollabProvider { clearTimeout(this.syncGraceTimer); this.syncGraceTimer = setTimeout(() => { if (!this.synced) this.synced = true; + if (this.connected) { + this.state = 'synced'; + // Treat the grace expiry as a successful sync — + // the dumb-relay design means an empty/pruned op-log + // + first peer is the canonical "everything is fine" + // case. Reset backoff so a subsequent disconnect + // starts fresh. Per Codex review round 1 [P2]. + this.reconnectAttempts = 0; + } }, SYNC_GRACE_MS); }; @@ -323,6 +383,13 @@ export class CollabProvider { } if (subtype === syncProtocol.messageYjsSyncStep2) { this.synced = true; + if (this.connected) { + this.state = 'synced'; + // Successful sync — reset the backoff counter + // so a subsequent disconnect starts fresh. + // Per Codex review round 1 [P2]. + this.reconnectAttempts = 0; + } } break; } @@ -412,7 +479,6 @@ export class CollabProvider { } private readonly onClose = (): void => { - const wasConnected = this.connected; this.connected = false; // The sync-grace timer is per-open; if it hasn't fired yet // it would set synced=true even after we lost the socket. @@ -433,11 +499,26 @@ export class CollabProvider { if (this.destroyed) return; - // If we never even completed an open, this counts as a failed - // attempt and bumps the backoff. wasConnected==true means we - // had a real session that dropped — start backoff at the floor. - if (wasConnected) { - this.reconnectAttempts = 0; + // NB: do NOT reset reconnectAttempts based on `wasConnected` + // here. Reaching OPEN is not proof of a real working session + // (a flaky proxy can ESTABLISH then immediately CLOSE before + // any sync frame lands); only `synced` is. The reset is owned + // by the syncStep2 branch and the grace timer in onOpen — the + // actual successful-sync edges of the state machine. Per Codex + // review round 1 [P2]. + // Drop the public state to a backoff variant unless we already + // declared 'offline' on a prior cycle — preserving 'offline' + // across the close→scheduleReconnect handoff prevents a + // visible 'offline'→'reconnecting'→'offline' flicker on every + // backoff retry. The variant depends on whether we ever + // actually synced: pre-first-sync failures stay 'connecting' + // (we never reached a working session, so calling it + // "reconnecting" would be misleading); post-sync drops become + // 'reconnecting'. Per Codex round 2 [NIT]. + // scheduleReconnect re-confirms the final state once it's done + // bumping reconnectAttempts. + if (this.state !== 'offline') { + this.state = this.synced ? 'reconnecting' : 'connecting'; } this.scheduleReconnect(); }; @@ -455,6 +536,17 @@ export class CollabProvider { RECONNECT_BASE_MS * 2 ** this.reconnectAttempts, ); this.reconnectAttempts++; + // Public state stays at the close-time variant + // ('connecting' before any sync, 'reconnecting' after) until + // we've burned through OFFLINE_THRESHOLD attempts; after that + // the UX flips to 'offline' so the user knows it's not coming + // back on its own. Provider keeps trying in the background. + // Per Codex rounds 1-2 [P2/NIT]. + if (this.reconnectAttempts > OFFLINE_THRESHOLD) { + this.state = 'offline'; + } else if (this.state !== 'offline') { + this.state = this.synced ? 'reconnecting' : 'connecting'; + } this.reconnectTimer = setTimeout(() => { this.reconnectTimer = undefined; this.connect(); diff --git a/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte b/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte index be9f3b16..433e7cc6 100644 --- a/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte +++ b/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte @@ -11,7 +11,7 @@ import RawMarkdownEditor from '$lib/components/editor/RawMarkdownEditor.svelte'; import type { Editor as EditorType } from '@tiptap/core'; import * as Y from 'yjs'; - import { CollabProvider } from '$lib/collab/wsProvider.svelte'; + import { CollabProvider, type CollabConnectionState } from '$lib/collab/wsProvider.svelte'; import FieldEditor from '$lib/components/fields/FieldEditor.svelte'; import ItemTimeline from '$lib/components/timeline/ItemTimeline.svelte'; import ChildItems from '$lib/components/ChildItems.svelte'; @@ -1385,6 +1385,26 @@ moving = false; } } + + // TASK-1264: human-readable label + tooltip for the four-state + // CollabConnectionState. Centralized here so the markup branches on + // the variant only once. + function collabStateLabel(s: CollabConnectionState): string { + switch (s) { + case 'synced': return 'Synced'; + case 'connecting': return 'Connecting…'; + case 'reconnecting': return 'Reconnecting…'; + case 'offline': return 'Offline'; + } + } + function collabStateTitle(s: CollabConnectionState): string { + switch (s) { + case 'synced': return 'Real-time collaboration active. Changes sync instantly.'; + case 'connecting': return 'Connecting to the collaboration server…'; + case 'reconnecting': return 'Connection dropped. Trying to reconnect…'; + case 'offline': return 'Could not reconnect. Edits are saved locally and will sync when the connection is restored.'; + } + } {#if loading} @@ -1466,6 +1486,21 @@ {#if saveStatus === 'saving'}Saving...{:else}✓ Saved{/if} + {#if collabProvider} + + + + {collabStateLabel(collabProvider.state)} + + {/if} @@ -2224,6 +2259,38 @@ .save-status.saving { color: var(--text-muted); } .save-status.saved { color: var(--accent-green); } + /* Collab connection state badge (TASK-1264). Always-visible while + the WS provider exists; colour communicates state. The dot is the + primary signal, the label exists for accessibility and clarity + but is small and unobtrusive per the Plan body's "no flashing, + green dot" guidance. */ + .collab-state { + display: inline-flex; + align-items: center; + gap: 0.35em; + font-size: 0.85em; + margin-left: var(--space-2); + color: var(--text-muted); + } + .collab-state-dot { + width: 0.55em; + height: 0.55em; + border-radius: 50%; + background: var(--text-muted); + } + .collab-state-synced { color: var(--accent-green); } + .collab-state-synced .collab-state-dot { background: var(--accent-green); } + .collab-state-connecting, + .collab-state-reconnecting { color: var(--accent-yellow, #d4a017); } + .collab-state-connecting .collab-state-dot, + .collab-state-reconnecting .collab-state-dot { + background: var(--accent-yellow, #d4a017); + } + .collab-state-offline { color: var(--accent-red, #c0392b); } + .collab-state-offline .collab-state-dot { + background: var(--accent-red, #c0392b); + } + /* Layout variants */ .item-body { display: flex; @@ -2825,6 +2892,7 @@ .editor-mode-toggle, .add-relationship-section, .save-status, + .collab-state, .copy-ref-btn, .copied-tooltip, .link-delete-btn {