Repository navigation
fix(desktop): migrate terminal reconnect onto createRelaySocket (forever-retry + retryCount diagnosis) - #5702
Merged
Merged
Conversation
createRelaySocket already runs the _whoowns affinity preflight before every dial but only surfaced a 403 (via onAccessDenied). Add an optional onProbe callback so callers can read the full preflight status (503 host-offline, 401 unauthorized, 502/200 relay-routing) to classify why a stream is down — the WS upgrade status is otherwise invisible to browser clients. Additive and opt-in; existing consumers are unaffected.
The desktop terminal was the one relay socket still hand-rolled on a raw WebSocket with a 10-attempt (~66s) give-up. After a laptop sleep/wake the host can take longer than that to re-register its relay tunnel, so the terminal gave up right before the host came back and sat "offline" until an incidental DOM resume event — the reported "won't reconnect after sleep" bug. Rebuild the transport on createRelaySocket (partysocket), same as the events channel and web terminal: forever-retry, fly-affinity preflight, fresh token per dial. Keep the desktop-specific pieces partysocket doesn't model — the wall-clock liveness watchdog (sleep/wake), write coalescer, replay=0 after first bytes, terminated-on-exit/error. Drive the offline diagnosis off partysocket's retryCount (not close-event counting), so dial failures that arrive as an error + synthetic string-code close still surface after the threshold instead of retrying silently. Gate the diagnosis (not the retries) on document.hidden so a backgrounded window doesn't flash a spurious "offline".
|
Ready to review this PR? Stage has broken it down into 4 individual chapters for you:
Chapters generated by Stage for commit 53ffa82 on Jul 15, 2026 6:35am UTC. |
Contributor
📝 WalkthroughWalkthroughThe terminal transport now uses shared RelaySocket retrying and token handling, exposes relay probe results for diagnosis, updates reconnect and lifecycle controls, and expands tests for retry thresholds, hidden-window behavior, sleep/wake recovery, output flushing, and disconnect handling. ChangesTerminal relay reconnection
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalTransport
participant createRelaySocket
participant primeRelayAffinity
participant RelaySocket
TerminalTransport->>createRelaySocket: create relay-backed transport
createRelaySocket->>primeRelayAffinity: probe relay affinity
primeRelayAffinity-->>createRelaySocket: return probe result
createRelaySocket->>RelaySocket: dial and retry connection
RelaySocket-->>TerminalTransport: deliver open, attached, data, close, or error events
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts (1)
530-539: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate parsed control messages before reading
type.The assertion does not validate runtime data. A valid payload such as
nullmakesmessage.typethrow instead of reaching the invalid-payload handling. Parse tounknownand use a type guard or schema before dispatching.As per coding guidelines, “Avoid
anyunless it is necessary; preserve type safety.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts` around lines 530 - 539, Update the message parsing in the terminal WebSocket handler to parse JSON as unknown and validate the result before accessing message.type. Add or reuse a type guard/schema for TerminalServerMessage, routing null and other malformed payloads through the existing invalid-payload handling while preserving dispatch for valid messages.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.test.ts`:
- Around line 423-436: Update the “surfaces the diagnosis for dial failures that
never open” test to create the transport via connect() without opening the
socket first, then invoke socket.dialFail() for all attempts before any
socket.open() call. Preserve the existing 10-failure threshold assertions and
avoid using connectAttached(), which opens and attaches the session.
In `@apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts`:
- Around line 513-514: Guard the WebSocket event handlers in the transport
around the message, close, and error listener callbacks so they return
immediately when the event’s socket is no longer the active _socket. Preserve
the existing disconnected state and prevent detached sockets from mutating logs
or terminal state after disconnect() or disposeTransport().
---
Outside diff comments:
In `@apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts`:
- Around line 530-539: Update the message parsing in the terminal WebSocket
handler to parse JSON as unknown and validate the result before accessing
message.type. Add or reuse a type guard/schema for TerminalServerMessage,
routing null and other malformed payloads through the existing invalid-payload
handling while preserving dispatch for valid messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0010ba4e-bdd1-4a1b-a92f-0897629838cf
📒 Files selected for processing (4)
apps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.tsapps/desktop/src/renderer/lib/terminal/terminal-ws-transport.test.tsapps/desktop/src/renderer/lib/terminal/terminal-ws-transport.tspackages/workspace-client/src/lib/relaySocket/relaySocket.ts
Contributor
Greptile SummaryThis PR migrates the desktop terminal WebSocket transport from a hand-rolled socket with a hard 10-attempt give-up to
Confidence Score: 3/5Safe to merge for the core reconnect fix; one latent issue with replay suppression on workspace switches needs a one-line fix before this is fully correct. The transport correctly resets _terminated, _diagnosisLogged, lastDiagnosis, and the write coalescer when pointed at a new endpoint, but skips _hasReceivedBytes. A user switching workspaces while a terminal is active would land on a blank terminal after reconnect because the server skips initial scrollback. The fix is a single-line addition; no existing test covers this path. apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts — the _hasReceivedBytes reset in the URL-change path of connect()
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts | Full migration to createRelaySocket; introduces forever-retry, per-dial token signing, wall-clock liveness watchdog, and retryCount-based diagnosis — but _hasReceivedBytes is not reset when the transport is pointed at a new endpoint, suppressing replay for the new session. |
| apps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.ts | Simplified retryConnect() to delegate to the new reconnect() export instead of manually resetting _reconnectAttempt; clean and correct. |
| packages/workspace-client/src/lib/relaySocket/relaySocket.ts | Adds additive onProbe hook to RelaySocketOptions; correctly sequenced before the 403 branch so _lastProbe is set before onAccessDenied reads it. |
| apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.test.ts | Comprehensive test suite covering forever-retry, retryCount threshold, dial-failure diagnosis, hidden-window gate, wall-clock watchdog, and write coalescing; FakeRelaySocket accurately models partysocket's synthetic-close behavior for dial failures. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant UI as "Terminal UI"
participant TR as "TerminalTransport"
participant RS as "createRelaySocket (partysocket)"
participant PA as "primeRelayAffinity"
participant WS as "WebSocket (relay/host)"
UI->>TR: connect(transport, terminal, wsUrl)
TR->>TR: stripToken(url), reset state fields
TR->>RS: createRelaySocket with buildUrl/getToken/onProbe/onAccessDenied
Note over RS: Socket created once, reused forever
loop Every dial attempt
RS->>PA: primeRelayAffinity(signedUrl)
PA-->>RS: RelayAffinityProbe or null
RS->>TR: onProbe(probe) sets _lastProbe
RS->>WS: WebSocket upgrade attempt
end
WS-->>TR: message type attached
TR->>UI: connectionState open
WS-->>TR: close with numeric code
TR->>TR: setConnectionState closed
TR->>TR: "maybeSurfaceDiagnosis checks retryCount >= 10"
Note over TR: Wall-clock watchdog fires every 5s
TR->>TR: "gap > 20s detected sleep/wake"
TR->>RS: socket.reconnect() immediate re-dial
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant UI as "Terminal UI"
participant TR as "TerminalTransport"
participant RS as "createRelaySocket (partysocket)"
participant PA as "primeRelayAffinity"
participant WS as "WebSocket (relay/host)"
UI->>TR: connect(transport, terminal, wsUrl)
TR->>TR: stripToken(url), reset state fields
TR->>RS: createRelaySocket with buildUrl/getToken/onProbe/onAccessDenied
Note over RS: Socket created once, reused forever
loop Every dial attempt
RS->>PA: primeRelayAffinity(signedUrl)
PA-->>RS: RelayAffinityProbe or null
RS->>TR: onProbe(probe) sets _lastProbe
RS->>WS: WebSocket upgrade attempt
end
WS-->>TR: message type attached
TR->>UI: connectionState open
WS-->>TR: close with numeric code
TR->>TR: setConnectionState closed
TR->>TR: "maybeSurfaceDiagnosis checks retryCount >= 10"
Note over TR: Wall-clock watchdog fires every 5s
TR->>TR: "gap > 20s detected sleep/wake"
TR->>RS: socket.reconnect() immediate re-dial
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts:416-457
**`_hasReceivedBytes` not reset on URL change, suppressing replay for new sessions**
When `connect()` is called with a different endpoint (new base URL), several fields are reset — `_terminated`, `_diagnosisLogged`, `lastDiagnosis`, `_writeCoalescer` — but `_hasReceivedBytes` is NOT. The `buildUrl` closure reads it live and appends `?replay=0` when true. If a previous session received bytes and the transport is then pointed at a new endpoint (e.g., via `registry.reconnect()` after a workspace switch), the server skips the initial scrollback for the new session and the user sees a blank terminal on connect.
The fix is to add `transport._hasReceivedBytes = false;` alongside the other resets in the non-idempotent path, before the early return for the existing-socket URL-change case.
### Issue 2 of 2
apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts:437-442
`_hasReceivedBytes` is not reset here when the transport is pointed at a new endpoint. The `buildUrl` closure reads it live and will append `?replay=0` on the very first dial to the new session, so the server skips initial scrollback and the user sees a blank terminal. Adding the reset alongside the other state resets fixes this.
```suggestion
transport.currentUrl = wsUrl;
transport._localToken = extractToken(wsUrl);
transport._terminal = terminal;
transport._terminated = false;
transport._diagnosisLogged = false;
transport.lastDiagnosis = null;
transport._hasReceivedBytes = false;
```
Reviews (1): Last reviewed commit: "fix(desktop): migrate terminal reconnect..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
416
to
+457
| export function connect( | ||
| transport: TerminalTransport, | ||
| terminal: XTerm, | ||
| wsUrl: string, | ||
| ) { | ||
| // Idempotent: skip if already connected/connecting to the same endpoint. | ||
| const isActive = | ||
| transport.connectionState === "open" || | ||
| transport.connectionState === "connecting"; | ||
| if (isActive && transport.currentUrl === wsUrl) return; | ||
|
|
||
| if (transport.socket) { | ||
| transport.socket.close(); | ||
| transport.socket = null; | ||
| const base = stripToken(wsUrl); | ||
|
|
||
| // Idempotent: a live socket already pointed at this endpoint just needs the | ||
| // latest token-bearing URL refreshed (the socket re-signs per dial anyway) — | ||
| // don't tear the connection down when only the rotating token changed. | ||
| if ( | ||
| transport._socket && | ||
| !transport._terminated && | ||
| transport.currentUrl && | ||
| stripToken(transport.currentUrl) === base | ||
| ) { | ||
| transport.currentUrl = wsUrl; | ||
| transport._localToken = extractToken(wsUrl); | ||
| return; | ||
| } | ||
|
|
||
| cancelReconnect(transport); | ||
| const epoch = ++transport._connectEpoch; | ||
| transport.currentUrl = wsUrl; | ||
| transport._localToken = extractToken(wsUrl); | ||
| transport._terminal = terminal; | ||
| // Recreate per connect so the coalescer always targets the current | ||
| // terminal; dispose flushes anything the previous socket left pending. | ||
| transport._terminated = false; | ||
| transport._diagnosisLogged = false; | ||
| transport.lastDiagnosis = null; | ||
| // Recreate per connect so the coalescer always targets the current terminal; | ||
| // dispose flushes anything the previous socket left pending. | ||
| transport._writeCoalescer?.dispose(); | ||
| transport._writeCoalescer = createWriteCoalescer((data) => | ||
| terminal.write(data), | ||
| ); | ||
| transport._terminated = false; | ||
| transport.lastDiagnosis = null; | ||
| setupLiveness(transport); | ||
| setConnectionState(transport, "connecting"); | ||
| const actualUrl = transport._hasReceivedBytes | ||
| ? appendQueryParam(wsUrl, "replay", "0") | ||
| : wsUrl; | ||
|
|
||
| const openSocket = (targetUrl: string) => { | ||
| // Bail if the transport raced into a different URL, was disconnected, or | ||
| // a newer connect() superseded this attempt while the pre-flight was in | ||
| // flight. | ||
| if ( | ||
| transport._connectEpoch !== epoch || | ||
| transport.currentUrl !== wsUrl || | ||
| transport.connectionState !== "connecting" | ||
| ) { | ||
| return; | ||
| } | ||
| let socket: WebSocket; | ||
| try { | ||
| socket = new WebSocket(targetUrl); | ||
| } catch (err) { | ||
|
|
||
| // Endpoint changed on an existing socket: re-point (buildUrl reads | ||
| // currentUrl live) and re-dial. | ||
| if (transport._socket) { | ||
| transport._socket.reconnect(); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
_hasReceivedBytes not reset on URL change, suppressing replay for new sessions
When connect() is called with a different endpoint (new base URL), several fields are reset — _terminated, _diagnosisLogged, lastDiagnosis, _writeCoalescer — but _hasReceivedBytes is NOT. The buildUrl closure reads it live and appends ?replay=0 when true. If a previous session received bytes and the transport is then pointed at a new endpoint (e.g., via registry.reconnect() after a workspace switch), the server skips the initial scrollback for the new session and the user sees a blank terminal on connect.
The fix is to add transport._hasReceivedBytes = false; alongside the other resets in the non-idempotent path, before the early return for the existing-socket URL-change case.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts
Line: 416-457
Comment:
**`_hasReceivedBytes` not reset on URL change, suppressing replay for new sessions**
When `connect()` is called with a different endpoint (new base URL), several fields are reset — `_terminated`, `_diagnosisLogged`, `lastDiagnosis`, `_writeCoalescer` — but `_hasReceivedBytes` is NOT. The `buildUrl` closure reads it live and appends `?replay=0` when true. If a previous session received bytes and the transport is then pointed at a new endpoint (e.g., via `registry.reconnect()` after a workspace switch), the server skips the initial scrollback for the new session and the user sees a blank terminal on connect.
The fix is to add `transport._hasReceivedBytes = false;` alongside the other resets in the non-idempotent path, before the early return for the existing-socket URL-change case.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
437
to
+442
| transport.currentUrl = wsUrl; | ||
| transport._localToken = extractToken(wsUrl); | ||
| transport._terminal = terminal; | ||
| // Recreate per connect so the coalescer always targets the current | ||
| // terminal; dispose flushes anything the previous socket left pending. | ||
| transport._terminated = false; | ||
| transport._diagnosisLogged = false; | ||
| transport.lastDiagnosis = null; |
Contributor
There was a problem hiding this comment.
_hasReceivedBytes is not reset here when the transport is pointed at a new endpoint. The buildUrl closure reads it live and will append ?replay=0 on the very first dial to the new session, so the server skips initial scrollback and the user sees a blank terminal. Adding the reset alongside the other state resets fixes this.
Suggested change
| transport.currentUrl = wsUrl; | |
| transport._localToken = extractToken(wsUrl); | |
| transport._terminal = terminal; | |
| // Recreate per connect so the coalescer always targets the current | |
| // terminal; dispose flushes anything the previous socket left pending. | |
| transport._terminated = false; | |
| transport._diagnosisLogged = false; | |
| transport.lastDiagnosis = null; | |
| transport.currentUrl = wsUrl; | |
| transport._localToken = extractToken(wsUrl); | |
| transport._terminal = terminal; | |
| transport._terminated = false; | |
| transport._diagnosisLogged = false; | |
| transport.lastDiagnosis = null; | |
| transport._hasReceivedBytes = false; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts
Line: 437-442
Comment:
`_hasReceivedBytes` is not reset here when the transport is pointed at a new endpoint. The `buildUrl` closure reads it live and will append `?replay=0` on the very first dial to the new session, so the server skips initial scrollback and the user sees a blank terminal. Adding the reset alongside the other state resets fixes this.
```suggestion
transport.currentUrl = wsUrl;
transport._localToken = extractToken(wsUrl);
transport._terminal = terminal;
transport._terminated = false;
transport._diagnosisLogged = false;
transport.lastDiagnosis = null;
transport._hasReceivedBytes = false;
```
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Architecture diagram
sequenceDiagram
participant Renderer as Desktop Renderer
participant Transport as Terminal Transport
participant RelaySocket as createRelaySocket
participant RelayHost as Relay / Host Service
participant Auth as ensureFreshJwt
participant Terminal as XTerm Terminal
Note over Renderer,Terminal: NEW: Forever-retry via shared partysocket
Renderer->>Transport: connect(url, terminal)
Transport->>Transport: createTransport() if new
Transport->>RelaySocket: NEW: createRelaySocket(options)
Note over RelaySocket: onProbe, onAccessDenied, etc.
loop Every reconnection attempt (forever)
RelaySocket->>Auth: buildUrl() → sign per-dial token
Auth-->>RelaySocket: fresh JWT
RelaySocket->>RelayHost: preflight _whoowns
RelayHost-->>RelaySocket: probe status (e.g., 200, 503, 401)
RelaySocket->>Transport: NEW: onProbe(probe) hook
Transport->>Transport: store _lastProbe
RelaySocket->>RelayHost: WebSocket upgrade (with token)
alt Upgrade succeeds
RelayHost-->>RelaySocket: OPEN
RelaySocket-->>Transport: open event
Transport->>Transport: set state to "connecting"
Transport->>RelaySocket: send {type:"attach", terminalId,…}
RelayHost-->>RelaySocket: {type:"attached", …}
RelaySocket-->>Transport: message event
Transport->>Transport: set state to "connected", clear diagnosis
Transport->>Terminal: write PTY output (coalesced)
Note over Transport: Wall-clock liveness watchdog starts
loop Every 5s
Transport->>Transport: check _lastLivenessTick
alt gap > 20s (sleep/wake)
Transport->>RelaySocket: CHANGED: forceReconnect() via socket.reconnect()
RelaySocket->>Auth: new token, new preflight
end
end
else Upgrade fails (dial failure)
RelayHost-->>RelaySocket: close / error
Note over RelaySocket: retryCount++ (partysocket)
RelaySocket-->>Transport: close event (code="close" on dial fail)
Transport->>Transport: CHANGED: evaluate retryCount, not close codes
alt retryCount >= 10 AND window NOT hidden
Transport->>Transport: CHANGED: surface diagnosis (host-offline/unauthorized/routing)
Transport->>Transport: log "Still retrying" once, capture PostHog event
else window hidden
Transport->>Transport: skip diagnosis (keep retrying silently)
end
RelaySocket->>RelayHost: next retry (exponential backoff, capped at 10s)
end
end
Note over Transport: Focus / visibility / online gates
rect rgb(240, 240, 240)
Note over Transport,RelaySocket: On window regain focus
Renderer->>Transport: visibilitychange / focus / online
Transport->>RelaySocket: forceReconnect() – resets retry budget
end
Note over Transport: PTY exit or fatal attach error
Transport->>Transport: set _terminated = true
Transport->>RelaySocket: close socket (no more retries)
Transport->>Transport: surface permanent diagnosis, log telemetry
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
bun's mock.module is process-global, so the partial `@superset/workspace-client /relay-socket` stub (createRelaySocket only) clobbered the real module for every later desktop test file, dropping setRelaySocketTelemetry and crashing unrelated tests with "export not found". Spread the real module and override only createRelaySocket, and drop the unnecessary auth-client/posthog mocks (getToken never runs with a faked socket; posthog.capture pre-init is a no-op). Full desktop suite: 2137 pass / 0 fail.
Address review feedback: - Restore the stale-socket guard (`if (transport._socket !== socket) return`) on the message/close/error handlers — dropped in the rewrite. Without it a late close after disconnect()/disposeTransport() flips "disconnected" back to "closed" and mutates logs on a torn-down transport. Regression-tested. - Epoch-guard onProbe in createRelaySocket so a slow preflight from a superseded dial can't publish a stale probe that misclassifies the current endpoint. - Make the "dial failures that never open" test start before open() so it actually exercises the never-opened path. - Add a regression test asserting the relay-socket mock preserves the module's other exports (setRelaySocketTelemetry), so a future partial mock.module stub fails fast instead of leaking suite-wide (the earlier CI break).
This was referenced Aug 11, 2026
1 of 4 tasks
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
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.onProbehook topackages/workspace-clientso 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
WebSocketwith 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. PostHogterminal_connect_failedconfirmed the pattern: 100% of these failures exhausted all attempts (host-offline bucket: 624 events / 31 users / 30 days).How It Works
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=0after first bytes, and terminated-on-exit/error.retryCount, not close-counting. partysocket delivers dial failures (host unreachable, upgrade rejected) as anerror+ 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 authoritativeretryCount(resets after a stable connection), surfaced after 10 consecutive failures — matching the events channel'srelaySocket.ts.onProbehook surfaces the existing_whoownspreflight result to the caller (previously only 403 leaked out viaonAccessDenied). No new network calls, no protocol change.Manual QA — verified live over CDP (dev desktop)
createRelaySocket→connectionState: "open"(new code confirmed live:_diagnosisLoggedpresent, old_reconnectAttemptgone)retryCountclimbs 10 → 11 → 12 (forever-retry, never self-closes)retryCount ≥ 10(was the silent-failure bug) and keeps retrying"open"in ~1.6s, diagnosis clearedTesting
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
retryCountover close-counting: robust to partysocket's synthetic-close/error double-fire on dial failures (same lesson asrelaySocket.ts). Caught by the live CDP run, not the unit tests — the fake initially dispatched numeric closes.Known Limitations / Follow-ups
apps/web) shares the give-up-at-12 shape but is an unused surface (desktop is the real one) — left for a parity pass.subscribeToSessionlacks the affinity preflight — separate, deferred.Risks / Rollout
@superset/workspace-client(confirmed: relay, api, host-service, electric-proxy, mobile, marketing, admin, docs = 0). No server code, endpoint, or protocol change._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.Summary by cubic
Migrated the desktop terminal to the shared
createRelaySocketwith forever-retry, fresh token per dial, and preflight affinity to fix sleep/wake reconnects and provide clear, thresholded offline diagnosis. Added areconnect()path and guarded late socket events so manual retries and teardown behave correctly.Bug Fixes
createRelaySocketfor indefinite retries and parity with events/web terminal.retryCountafter 10 consecutive failures; keeps retrying and logs once.replay=0after first bytes, and termination on PTY exit/fatal attach.reconnect()(used in the desktop registry) and guarded late events from detached sockets to prevent state/log flips after disconnect/teardown.createRelaySocketin@superset/workspace-client/relay-socketwhile preserving other exports; added regressions for never-open dial failures and teardown event handling.New Features
@superset/workspace-client/relay-socket: addedonProbeto expose relay_whoownspreflight results for better outage classification (host offline/unauthorized/routing), with an epoch guard to avoid publishing stale probes after URL swaps or reconnects.Written for commit 53ffa82. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Improvements
Tests