Repository navigation
fix(desktop): refresh relay JWT on WS reconnect and stop retrying definitive 403s #5628
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
8a51dde
fix(desktop): refresh relay JWT on WS reconnect and stop retrying def…
Kitenite b76609d
fix(relay): surface access denials on the _whoowns preflight
Kitenite 4ab51e6
fix(desktop): guard JWT refresh and terminal preflight against races
Kitenite ca76fc1
docs(plans): partysocket WS-client consolidation design (SUPER-1469)
Kitenite File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import { describe, expect, it } from "bun:test"; | ||
| import { decodeJwtExpiresAtMs } from "./jwt-expiry"; | ||
|
|
||
| function makeJwt(payload: object): string { | ||
| const encode = (obj: object) => | ||
| btoa(JSON.stringify(obj)) | ||
| .replace(/\+/g, "-") | ||
| .replace(/\//g, "_") | ||
| .replace(/=+$/, ""); | ||
| return `${encode({ alg: "EdDSA" })}.${encode(payload)}.sig`; | ||
| } | ||
|
|
||
| describe("decodeJwtExpiresAtMs", () => { | ||
| it("returns exp in epoch milliseconds", () => { | ||
| expect(decodeJwtExpiresAtMs(makeJwt({ exp: 1_800_000_000 }))).toBe( | ||
| 1_800_000_000_000, | ||
| ); | ||
| }); | ||
|
|
||
| it("returns null when exp is missing", () => { | ||
| expect(decodeJwtExpiresAtMs(makeJwt({ sub: "u1" }))).toBeNull(); | ||
| }); | ||
|
|
||
| it("returns null for malformed tokens", () => { | ||
| expect(decodeJwtExpiresAtMs("not-a-jwt")).toBeNull(); | ||
| expect(decodeJwtExpiresAtMs("a.%%%.c")).toBeNull(); | ||
| }); | ||
| }); |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| /** Reads the `exp` claim (as epoch ms) from a JWT without verifying it. */ | ||
| export function decodeJwtExpiresAtMs(token: string): number | null { | ||
| try { | ||
| const payloadPart = token.split(".")[1]; | ||
| if (!payloadPart) return null; | ||
| const payload = JSON.parse( | ||
| atob(payloadPart.replace(/-/g, "+").replace(/_/g, "/")), | ||
| ) as { exp?: unknown }; | ||
| return typeof payload.exp === "number" ? payload.exp * 1000 : null; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| # Consolidate reconnecting WebSocket clients on partysocket | ||
|
|
||
| Ticket: SUPER-1469 · Prereq: PR #5628 (merged — relay `_whoowns` now returns 403 on definitive denial) | ||
|
|
||
| ## Problem | ||
|
|
||
| Three hand-rolled reconnecting-WS clients re-solve backoff, buffering, and token freshness: | ||
|
|
||
| | Transport | Lines | Consumers | Token handling today | | ||
| |---|---|---|---| | ||
| | `packages/workspace-client/src/lib/eventBus.ts` | ~400 | 9 files | sync `getWsToken()` per attempt | | ||
| | `apps/web/.../WebTerminal/TerminalConnection.ts` | ~240 | 1 | async TTL-cached token per attempt (correct) | | ||
| | `apps/desktop/.../terminal-ws-transport.ts` | ~665 | via registry | fresh-token re-sign per attempt (fixed in #5628) | | ||
|
|
||
| The desktop copy's drift caused the July 2026 401 reconnect-loop bug (~25 users/day). One library removes the bug class instead of patching each copy. | ||
|
|
||
| ## Decision | ||
|
|
||
| Adopt `partysocket` (Cloudflare/PartyKit's maintained reconnecting-websocket fork; generic `WebSocket` export, no PartyKit coupling). Key primitive: `url` accepts `() => Promise<string>`, evaluated before **every** connection attempt — fresh token per dial for free. Also: `maxRetries`, delay bounds + grow factor, `connectionTimeout`, `minUptime`, `maxEnqueuedMessages`, `binaryType`, permanent `close()`. | ||
|
|
||
| ## Design: shared wrapper in `packages/workspace-client` | ||
|
|
||
| One `createRelaySocket(opts)` (~150 lines) wrapping partysocket's generic export: | ||
|
|
||
| - **URL provider**: async, calls the caller's token getter (desktop `ensureFreshJwt`, web `getAuthToken`) and signs the URL per attempt. | ||
| - **Preflight**: runs `primeRelayAffinity` (`_whoowns`) inside the provider before returning the URL — pins Fly edge affinity and surfaces the real HTTP status the WS API hides. | ||
| - **Fatal denial**: preflight **403** (only — 401 means expired token and self-heals via the provider) → permanent `close()` + `onAccessDenied` callback. Non-terminal consumers may instead pass `accessDeniedRetryMs` (eventBus: 5 min). | ||
| - **Bounded buffering**: `maxEnqueuedMessages` mapped from each transport's current cap. | ||
|
|
||
| ## Phases | ||
|
|
||
| 1. **Spike (blocking)**: partysocket's behavior when the async `url` provider *rejects* is undocumented — verify it schedules a retry (vs wedging/unhandled rejection) before building on it. Also confirm `event-target-polyfill` behaves in the Electron renderer. | ||
| 2. **eventBus** (~½–1d): replace `connect`/`scheduleReconnect` internals; `getEventBus` API and fs:watch resend unchanged → zero consumer changes. | ||
| 3. **Web TerminalConnection** (~½–1d): dedupe (not a fix — web already refreshes correctly); keep attach/replay semantics. | ||
| 4. **Desktop terminal transport**: deferred. Only ~200/665 lines are dial/backoff; the rest (sleep/wake watchdog, write coalescer, replay flags, epoch guards, telemetry) must be re-hung on partysocket's silent socket-swapping model — high regression risk on just-verified code for mostly aesthetic gain. Revisit when the transport needs touching anyway. | ||
|
|
||
| ## Risks / constraints | ||
|
|
||
| - partysocket swaps the underlying socket silently; code that compares socket identity (desktop transport does) needs restructuring — main reason phase 4 is deferred. | ||
| - React Native: cloudflare/partykit#401 (Hermes lacks `MessageEvent`) — check before any apps/mobile use. | ||
| - Relay semantics (post-#5628): 401 = expired/invalid JWT (retry with fresh token), 403 = definitive access denial (stop/slow-poll). The wrapper must never treat 401 as fatal. | ||
| - New WS consumers should use the wrapper from day one. |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.