perf(webclient,relay): fast first paint, resilient boot, relay liveness#864
Conversation
Web client (app.ade-app.dev):
- Fix rolldown chunk graph: entry no longer statically depends on the 4.2MB
Monaco chunk or vendor-graph (which had absorbed react/react-dom); login
screen entry graph drops from ~4.7MB raw JS + 646KB CSS to ~252KB, enforced
by a new post-build assertion + size budget (check-webclient-entry.mjs).
- _headers: immutable caching for hashed /assets/*, no-cache HTML, and fold in
the production CSP drift (Clerk avatar img-src hosts).
- webclient.html: inline CSS splash (no blank page while JS loads) and
preconnects to the Clerk issuer + account-directory origins.
- Boot: sign-in button and machine directory render immediately; IndexedDB
maintenance (account pruning, trust reset) no longer gates them, is batched
into single transactions, handles onblocked with a bounded timeout and a
retry affordance. Account-owned saved environments still never render
before pruning completes.
- Connection: inbound-silence watchdog (75s) + visibility-resume staleness
check; transport-open (8s) and hello (12s) deadlines replace the too-tight
4s combined timeout; DPoP/account-token prep overlaps socket dialing;
relay close codes map to accurate user-facing errors; pending commands
reject immediately on transport loss instead of waiting 65s.
Relay (tunnel-relay DO + brain client):
- Brain control socket gains native WS ping/pong liveness (30s/10s) — protocol
frames are edge-handled so a hibernated DO is not woken or billed; half-open
sockets after sleep/NAT rebind now recover in seconds instead of minutes.
- Single reconnect state machine: the 1s account poll no longer bypasses
jittered backoff (ends claim-POST storms).
- New {t:"reject"} control signaling: failed bridge validation/pipe/local
dials close the waiting client immediately (4507) instead of pinning a
tunnel slot until timeout; control.send failures close the client with 4501.
- Close codes and bounded reasons now survive both the DO pairing boundary
and the brain pipe<->local bridge (was: always 4000).
- Idle sweep alarm only runs while data sockets exist (stops ~8.6k needless
DO wakes/month per idle machine); early-frame buffer gains a 256KiB byte cap.
- Cached loopback bridge validation (<30s) moves off the tunnel-open critical
path with concurrent revalidation.
Directory:
- Worker bounds machine listing (ORDER BY last_seen_at DESC LIMIT 500) and
emits Server-Timing (auth/db) for measurable backend latency.
- Brain publisher publishes immediately (2s debounce) on relay-readiness
transitions instead of waiting for the next 30s tick, so a Mac that just
came online appears in the directory right away.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Blocker: remove the cached bridge-validation fast path — every relay
tunnel open now serially revalidates the loopback listener before the
authenticated local socket (with the private bridge-proof header) is
constructed, closing a 30s window where a swapped listener could receive
the bridge proof and buffered client hello. The probe is loopback (~ms),
so the latency cost is negligible.
- Sign-out gates relay work synchronously (live isAccountSignedIn check,
fail-closed) instead of waiting up to 1s for the lease poll.
- Superseded control sockets can no longer initiate tunnels (socket
identity rechecked in the message handler and across openTunnel awaits).
- envStore: IndexedDB versionchange clears the cached handle and
trust-reset promise so retry reopens instead of reusing a closed db.
- Entry checker requires >=1 local JS entry and rejects external scripts.
- Browser maps relay close 4507 (bridge rejected) to an accurate message.
- Prune result is an explicit {removedIds, environments} shape (removes
the hidden-property array hack and casts).
- Host control parser narrowed to {t:"open"} (JSON ping/pong dead since
native WS liveness); worker-side legacy ping support unchanged.
- OAuth-callback bootstrap poll slowed 16ms -> 150ms.
Gated for follow-up (documented, not applied): shared relay-protocol
constants module, connection lifecycle dedup, publisher scheduler
consolidation, manifest-based entry checker, large test-file splits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- iOS: classify new relay close codes (4506 pre-pipe overflow, 4507 bridge rejected) in SyncService/SyncRecoveryPolicy so they retry like transport faults instead of falling through generic handling; 16 targeted tests. - Docs: web-client, sync-and-multi-device, and ARCHITECTURE updated for the new boot sequence, caching headers, relay liveness/reject protocol, and event-driven directory publishing. - CLI/TUI parity: reviewed, no drift (no new actions or shared types). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
📝 WalkthroughWalkthroughThe PR adds directory timing instrumentation, relay-state publishing and tunnel protocol safeguards, transactional web-client storage, resilient sync connections, privacy-aware boot flows, build validation, and iOS socket close-error handling. ChangesDirectory timing instrumentation
Account machine relay publishing
Relay tunnel protocol
Desktop web client
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/webclient/public/_headers`:
- Around line 8-15: Update the _headers rules so the global /* block sets
Cache-Control: no-cache for all SPA fallback routes. Keep the /assets/* rule
after it with public, max-age=31536000, immutable so immutable assets override
the global directive, and remove the narrower / and /index.html rules.
In `@apps/desktop/src/renderer/webclient/shell/WebClientRoot.tsx`:
- Around line 448-455: Ensure the /account/callback bootstrap rejection path
clears accountProgressInterval before restoring the account state. Update the
async bootstrap flow and its catch handling so the interval is cleared whenever
bootstrap() rejects, while preserving the existing disposed guard and recovery
state updates in the catch block.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fa5edb57-cf3b-4858-91b6-13e78f0dcbae
⛔ Files ignored due to path filters (3)
docs/ARCHITECTURE.mdis excluded by!docs/**docs/features/sync-and-multi-device/README.mdis excluded by!docs/**docs/features/web-client/README.mdis excluded by!docs/**
📒 Files selected for processing (25)
apps/account-directory/src/directory.tsapps/account-directory/test/directory.test.tsapps/ade-cli/src/services/account/accountMachinePublisherService.test.tsapps/ade-cli/src/services/account/accountMachinePublisherService.tsapps/ade-cli/src/services/sync/syncTunnelClientService.test.tsapps/ade-cli/src/services/sync/syncTunnelClientService.tsapps/desktop/package.jsonapps/desktop/scripts/check-webclient-entry.mjsapps/desktop/src/renderer/webclient.htmlapps/desktop/src/renderer/webclient/public/_headersapps/desktop/src/renderer/webclient/shell/MachinePicker.tsxapps/desktop/src/renderer/webclient/shell/WebClientRoot.tsxapps/desktop/src/renderer/webclient/shell/__tests__/WebClientRoot.test.tsxapps/desktop/src/renderer/webclient/sync/__tests__/sync.test.tsapps/desktop/src/renderer/webclient/sync/client.tsapps/desktop/src/renderer/webclient/sync/connection.tsapps/desktop/src/renderer/webclient/sync/envStore.test.tsapps/desktop/src/renderer/webclient/sync/envStore.tsapps/desktop/vite.webclient.config.tsapps/ios/ADE/Services/SyncRecoveryPolicy.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADETests/SyncRecoveryPolicyTests.swiftapps/tunnel-relay/README.mdapps/tunnel-relay/src/tunnelDo.tsapps/tunnel-relay/test/relay.test.ts
💤 Files with no reviewable changes (1)
- apps/desktop/vite.webclient.config.ts
| /assets/* | ||
| Cache-Control: public, max-age=31536000, immutable | ||
|
|
||
| / | ||
| Cache-Control: no-cache | ||
|
|
||
| /index.html | ||
| Cache-Control: no-cache |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Apply no-cache to all SPA fallback routes.
Because this is a Single Page Application (SPA), deep links (e.g., /work or /pair) will route to and serve the index.html content via SPA fallback. However, Cloudflare Pages matches _headers rules based on the requested URL, not the file ultimately served. By restricting Cache-Control: no-cache to exactly / and /index.html, deep links will lack this directive. This can cause browsers to aggressively cache stale HTML on deep links, leading to blank screens when older referenced JavaScript chunks are removed in new deployments.
To fix this, apply Cache-Control: no-cache to the global /* block so it covers all HTML SPA routes, and allow the /assets/* block to override it for immutable, hashed assets. Cloudflare Pages naturally overrides headers when multiple rules match the same URL and define the same header, using a top-down approach where the last match wins.
🛠️ Proposed fix to ensure SPA fallback routes are not cached
/*
Content-Security-Policy: default-src 'self'; script-src 'self' 'wasm-unsafe-eval'; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob: https://img.clerk.com https://images.clerk.dev https://storage.googleapis.com/images.clerk.dev/; font-src 'self' data:; connect-src 'self' wss: https:; worker-src 'self' blob:; frame-ancestors 'none'; base-uri 'self'; form-action 'self'; object-src 'none'
X-Content-Type-Options: nosniff
Referrer-Policy: no-referrer
X-Frame-Options: DENY
Permissions-Policy: camera=(), microphone=(), geolocation=()
+ Cache-Control: no-cache
/assets/*
Cache-Control: public, max-age=31536000, immutable
-
-/
- Cache-Control: no-cache
-
-/index.html
- Cache-Control: no-cache📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /assets/* | |
| Cache-Control: public, max-age=31536000, immutable | |
| / | |
| Cache-Control: no-cache | |
| /index.html | |
| Cache-Control: no-cache | |
| /* | |
| Content-Security-Policy: default-src 'self'; script-src 'self' 'wasm-unsafe-eval'; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob: https://img.clerk.com https://images.clerk.dev https://storage.googleapis.com/images.clerk.dev/; font-src 'self' data:; connect-src 'self' wss: https:; worker-src 'self' blob:; frame-ancestors 'none'; base-uri 'self'; form-action 'self'; object-src 'none' | |
| X-Content-Type-Options: nosniff | |
| Referrer-Policy: no-referrer | |
| X-Frame-Options: DENY | |
| Permissions-Policy: camera=(), microphone=(), geolocation=() | |
| Cache-Control: no-cache | |
| /assets/* | |
| Cache-Control: public, max-age=31536000, immutable |
🤖 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/webclient/public/_headers` around lines 8 - 15,
Update the _headers rules so the global /* block sets Cache-Control: no-cache
for all SPA fallback routes. Keep the /assets/* rule after it with public,
max-age=31536000, immutable so immutable assets override the global directive,
and remove the narrower / and /index.html rules.
| })().catch(() => { | ||
| if (disposed) return; | ||
| const accountSnapshot = accountClient.getSnapshot(); | ||
| setAccount(accountSnapshot); | ||
| setDirectoryLoading(false); | ||
| setPhase({ kind: "machine-picker" }); | ||
| void applyAccountPrivacy(accountSnapshot); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Leaked polling interval when bootstrap() rejects.
On the /account/callback path, accountProgressInterval is started before await accountClient.bootstrap(). If bootstrap() rejects, the async IIFE throws before the clearInterval at Line 440-443, and this .catch handler never clears it. The 150ms poller then keeps firing until unmount, repeatedly calling setAccount/setDirectoryLoading(true)/setPhase/beginPrivacyLoad — which also fights the setDirectoryLoading(false) set here.
🛡️ Proposed fix
})().catch(() => {
if (disposed) return;
+ if (accountProgressInterval != null) {
+ window.clearInterval(accountProgressInterval);
+ accountProgressInterval = null;
+ }
const accountSnapshot = accountClient.getSnapshot();
setAccount(accountSnapshot);
setDirectoryLoading(false);
setPhase({ kind: "machine-picker" });
void applyAccountPrivacy(accountSnapshot);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| })().catch(() => { | |
| if (disposed) return; | |
| const accountSnapshot = accountClient.getSnapshot(); | |
| setAccount(accountSnapshot); | |
| setDirectoryLoading(false); | |
| setPhase({ kind: "machine-picker" }); | |
| void applyAccountPrivacy(accountSnapshot); | |
| }); | |
| })().catch(() => { | |
| if (disposed) return; | |
| if (accountProgressInterval != null) { | |
| window.clearInterval(accountProgressInterval); | |
| accountProgressInterval = null; | |
| } | |
| const accountSnapshot = accountClient.getSnapshot(); | |
| setAccount(accountSnapshot); | |
| setDirectoryLoading(false); | |
| setPhase({ kind: "machine-picker" }); | |
| void applyAccountPrivacy(accountSnapshot); | |
| }); |
🤖 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/webclient/shell/WebClientRoot.tsx` around lines 448
- 455, Ensure the /account/callback bootstrap rejection path clears
accountProgressInterval before restoring the account state. Update the async
bootstrap flow and its catch handling so the interval is cleared whenever
bootstrap() rejects, while preserving the existing disposed guard and recovery
state updates in the catch block.
…button-fd3c9bbe # Conflicts: # apps/ade-cli/src/services/account/accountMachinePublisherService.ts # apps/ade-cli/src/services/sync/syncTunnelClientService.test.ts # apps/ade-cli/src/services/sync/syncTunnelClientService.ts # apps/desktop/src/renderer/webclient/shell/WebClientRoot.tsx # apps/desktop/src/renderer/webclient/sync/connection.ts # apps/desktop/src/renderer/webclient/sync/envStore.ts # docs/features/sync-and-multi-device/README.md # docs/features/web-client/README.md
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…tion - _headers: Cache-Control: no-cache moves to the /* rule so SPA fallback deep links (/work, /open, /account/callback) get it too; the later /assets/* rule still overrides hashed assets to immutable. - WebClientRoot: the 150ms callback bootstrap poller is cleared in the rejection path as well, so a failed bootstrap can no longer leave it fighting the settled machine-picker state until unmount. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary by CodeRabbit
New Features
Improvements
Greptile Summary
This PR improves web client startup and sync reliability across the account directory, relay, CLI host, browser client, and iOS client. The main changes are:
Confidence Score: 5/5
Safe to merge with low risk.
The changed paths include targeted coverage for directory listing, relay behavior, CLI publishing, browser sync/storage flows, and iOS recovery policy. The reviewed code keeps account/session gating, bounded buffering, sanitized close propagation, and retry/backoff handling explicit.
No files require special attention.
What T-Rex did
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Browser as Web Client participant Directory as Account Directory participant Host as ADE CLI Host participant Relay as Tunnel Relay participant Local as Local Sync Listener Browser->>Directory: GET account machines Directory-->>Browser: recent machines with Server-Timing Host->>Directory: publish machine endpoints Host->>Relay: claim machine and open control socket Relay-->>Host: open request with connection id Host->>Local: validate loopback listener alt bridge valid Host->>Relay: open signed pipe socket Relay-->>Browser: pair client socket with host pipe Browser->>Local: sync request frames via relay Local-->>Browser: sync response frames via relay else bridge invalid or host unavailable Host-->>Relay: reject connection id and close code Relay-->>Browser: close client with application code end Host->>Relay: native WebSocket ping Relay-->>Host: native pong%%{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 Browser as Web Client participant Directory as Account Directory participant Host as ADE CLI Host participant Relay as Tunnel Relay participant Local as Local Sync Listener Browser->>Directory: GET account machines Directory-->>Browser: recent machines with Server-Timing Host->>Directory: publish machine endpoints Host->>Relay: claim machine and open control socket Relay-->>Host: open request with connection id Host->>Local: validate loopback listener alt bridge valid Host->>Relay: open signed pipe socket Relay-->>Browser: pair client socket with host pipe Browser->>Local: sync request frames via relay Local-->>Browser: sync response frames via relay else bridge invalid or host unavailable Host-->>Relay: reject connection id and close code Relay-->>Browser: close client with application code end Host->>Relay: native WebSocket ping Relay-->>Host: native pongReviews (3): Last reviewed commit: "fix(review): SPA-wide no-cache headers; ..." | Re-trigger Greptile