fix(desktop): migrate terminal reconnect onto createRelaySocket (forever-retry + retryCount diagnosis)

Merged
KI
4mo ago
+601-369

Summary

  • Rebuild the desktop terminal WebSocket transport on createRelaySocket (partysocket) — the same shared reconnecting socket the events channel and web terminal use — so it retries forever instead of giving up after ~66s, gets the fly-affinity preflight, and re-signs a fresh token per dial.
  • Fixes the reported bug: after laptop sleep/wake, a remote workspace terminal fails to reconnect and sits "offline" until an incidental window focus/visibility event.
  • Adds an additive onProbe hook to packages/workspace-client so the transport can classify why a stream is down (host-offline / unauthorized / relay-routing).

Why / Context

The desktop terminal was the one relay socket still hand-rolled on a raw WebSocket with a 10-attempt (~66s) give-up. On a shared sleep/wake, the host-service can take longer than that window to re-register its relay tunnel (its inbound-silence watchdog alone is ~75s), so the client gave up right before the host returned and never retried on its own. PostHog terminal_connect_failed confirmed the pattern: 100% of these failures exhausted all attempts (host-offline bucket: 624 events / 31 users / 30 days).

How It Works

  • Transport → createRelaySocket. Reconnection, backoff, preflight affinity, and per-dial token signing move into the shared socket (partysocket, forever-retry). The transport keeps the pieces partysocket doesn't model: the wall-clock liveness watchdog (the dependable sleep/wake signal — app-suspend doesn't reliably fire focus/visibility), the write coalescer, replay=0 after first bytes, and terminated-on-exit/error.
  • Diagnosis off retryCount, not close-counting. partysocket delivers dial failures (host unreachable, upgrade rejected) as an error + a synthetic close whose code is the string "close". Counting close codes misses those, so a genuinely-offline terminal would retry silently. The offline diagnosis is now driven by partysocket's authoritative retryCount (resets after a stable connection), surfaced after 10 consecutive failures — matching the events channel's relaySocket.ts.
  • Focus gate on the diagnosis, not the retries. A hidden/minimized window keeps retrying (background agents stay live and reattach), but doesn't accrue a spurious "offline" header. On resume, the focus/visibility/online listener force-redials and resets the budget.
  • onProbe hook surfaces the existing _whoowns preflight result to the caller (previously only 403 leaked out via onAccessDenied). No new network calls, no protocol change.

Manual QA — verified live over CDP (dev desktop)

  • Two real terminals connect end-to-end via createRelaySocket → connectionState: "open" (new code confirmed live: _diagnosisLogged present, old _reconnectAttempt gone)
  • Injected a dead-host failure → retryCount climbs 10 → 11 → 12 (forever-retry, never self-closes)
  • Dial-failure diagnosis surfaces at retryCount ≥ 10 (was the silent-failure bug) and keeps retrying
  • Repoint to the real host → recovers to "open" in ~1.6s, diagnosis cleared
  • Terminal streams PTY output / attach / resize intact

Testing

  • bun run typecheck — 35/35 packages ✅
  • bun run lint — clean ✅
  • bunx sherif — no issues ✅
  • bun test apps/desktop/.../terminal-ws-transport.test.ts — 14/14; the new guards (forever-retry, retryCount threshold, dial-failure diagnosis, hidden-window gate, wall-clock watchdog) are each mutation-verified (fail on the broken code)
  • bun test packages/workspace-client/src — 20/20 ✅

Design Decisions

  • Full migration (option A) over a slow-poll patch (option B): deletes the hand-rolled reconnect loop entirely and inherits forever-retry + preflight from the shared socket, killing the whole bug class rather than the one instance.
  • retryCount over close-counting: robust to partysocket's synthetic-close/error double-fire on dial failures (same lesson as relaySocket.ts). Caught by the live CDP run, not the unit tests — the fake initially dispatched numeric closes.
  • Kept retrying while hidden: desktop backgrounds windows while agents run; pausing retries would delay reattach. Only the diagnosis is gated on visibility.

Known Limitations / Follow-ups

  • Web terminal (apps/web) shares the give-up-at-12 shape but is an unused surface (desktop is the real one) — left for a parity pass.
  • ACP subscribeToSession lacks the affinity preflight — separate, deferred.
  • 401/unauthorized give-up storm (larger bucket in telemetry) is a token-refresh-on-wake issue — separate investigation.

Risks / Rollout

  • Blast radius: desktop + web only. No server/hosted service consumes @superset/workspace-client (confirmed: relay, api, host-service, electric-proxy, mobile, marketing, admin, docs = 0). No server code, endpoint, or protocol change.
  • Risk: the one server-facing effect is a load-pattern change — a client stuck against an offline host now hits the relay's _whoowns + WS upgrade every ≤10s indefinitely (per open terminal) instead of giving up. Bounded by the 10s backoff cap and identical to the pattern the events channel already sustains in prod; not a Vercel-auth-storm class. If relay metrics ever flag it, we can pause retries (not just the diagnosis) while hidden.
  • Rollback: revert the two commits; no data or schema involved.
Open in Stage

Summary by cubic

Migrated the desktop terminal to the shared createRelaySocket with forever-retry, fresh token per dial, and preflight affinity to fix sleep/wake reconnects and provide clear, thresholded offline diagnosis. Added a reconnect() path and guarded late socket events so manual retries and teardown behave correctly.

  • Bug Fixes

    • Replaced the hand-rolled WebSocket with createRelaySocket for indefinite retries and parity with events/web terminal.
    • Drove outage diagnosis off retryCount after 10 consecutive failures; keeps retrying and logs once.
    • Keeps retrying while hidden but suppresses the “offline” banner until focus/visibility returns.
    • Wall-clock watchdog forces reconnect after sleep/wake gaps.
    • Preserves write coalescing, replay=0 after first bytes, and termination on PTY exit/fatal attach.
    • Exposed reconnect() (used in the desktop registry) and guarded late events from detached sockets to prevent state/log flips after disconnect/teardown.
    • Hardened tests by stubbing only createRelaySocket in @superset/workspace-client/relay-socket while preserving other exports; added regressions for never-open dial failures and teardown event handling.

  • New Features

    • @superset/workspace-client/relay-socket: added onProbe to expose relay _whoowns preflight results for better outage classification (host offline/unauthorized/routing), with an epoch guard to avoid publishing stale probes after URL swaps or reconnects.

Summary by CodeRabbit

  • Bug Fixes

    • Improved terminal reconnection across temporary outages, sleep/wake gaps, and stalled connections.
    • Prevented unnecessary reconnect/teardown when tokens rotate.
    • Ensured queued terminal output flushes correctly on close.
    • Reworked relay-driven close/error handling to avoid state resurrection from late events.

  • Improvements

    • Enhanced “connection outage” diagnosis: logged once per outage cycle and suppressed while the app is hidden.

  • Tests

    • Updated terminal transport tests to use a deterministic relay-socket simulation and expanded reconnect/diagnosis coverage.