Remote access via Cloudflare relay + polished CLI auth UX#4
Merged
Conversation
Auth gate now redirects unauthenticated users to /sign-in (LOCAL_MODE bypasses). Daemon validates Bearer JWTs via @clerk/backend on all routes except /health; skips validation when CLERK_SECRET_KEY is unset. Sign-out navigates to /sign-in, auth screens use theme tokens. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CF Worker + Durable Object per user. Daemon and web/mobile clients connect via WebSocket, relay routes messages between them. Clerk JWT verification on all WS upgrades. Heartbeat via DO alarm, daemon replacement on reconnect, client fan-out for multi-device support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Daemon connects to the Cloudflare relay when AIMUX_RELAY_URL and AIMUX_RELAY_TOKEN env vars are set. Relay forwards client requests through the WS; daemon routes them via the same logic as direct HTTP. Exponential backoff reconnect (1s–30s), clean shutdown on daemon stop. Refactored daemon routing into public routeRequest() for reuse. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
App-side RelayTransport class connects to the relay as a client, wraps HTTP requests as relay messages, and tracks daemon online status. API layer seamlessly switches between direct HTTP and relay based on connection state. Daemon gains /proxy/:host/:port/* route to forward metadata server requests through the tunnel. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Unauthenticated users see a landing page with sign-up/sign-in CTAs and feature highlights instead of jumping straight to sign-in. Production wrangler config adds relay.aimux.com custom domain. Deployment guide covers Clerk, relay, web app, daemon, and native. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Relay mints long-lived (90d) HS256 daemon tokens via POST /cli/issue-token after verifying a Clerk session JWT. /daemon/connect now accepts both relay-signed daemon tokens and Clerk JWTs, dispatching by token shape. CLI gains ~/.aimux/auth.json credential storage (0600) with a remoteEnabled flag for runtime toggling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`aimux login` spins up a localhost callback server, opens the browser to the web app's /cli-auth page, and captures a minted daemon token — no token pasting. The cli-auth page issues the token from the relay using the user's Clerk session and redirects to the callback. Adds `aimux logout` and `aimux whoami`. Verified end-to-end against local relay. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Daemon now reads relay config from stored credentials (env-var override retained) and connects only when remote access is enabled. RelayClient tracks connection status (connecting/connected/reconnecting/auth_failed) and surfaces it via /relay/status. New `aimux remote enable|disable| status` commands toggle the relay at runtime without restarting the daemon; `aimux daemon status` now shows relay state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
RelayTransport now connects on mount when EXPO_PUBLIC_AIMUX_RELAY_URL is set, syncs status into a Jotai atom, and registers with the API layer so requests route through the tunnel. New RelayIndicator pill in the TopBar shows live connection state (Remote / Connecting / Host offline), hidden entirely for local-only deployments. Verified end-to-end: browser → relay → daemon project list over the WS tunnel. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5 tasks
added 9 commits
May 21, 2026 15:31
# Conflicts: # app/components/AuthMenu.tsx
# Conflicts: # app/lib/api.ts
base64UrlDecodeToString restored - and _ but never re-added the = padding that base64UrlEncode strips. Workers' atob is strict about length being a multiple of 4 — JWT payloads with non-trivial sizes routinely produced 3-mod-4 segments and atob would throw, breaking verifyDaemonToken and isDaemonToken on the affected tokens. Re-pad before atob.
The Durable Object was broadcasting every daemon "response" frame to all connected client sockets. With multiple clients on the same user (desktop + mobile, or two browser tabs) that leaked each client's response bodies to the other and confused request/response correlation. Track an in-flight requestId → client-socket map on each inbound request, route the matching response to that one socket on the way back, and clear entries on client close or after a TTL so a dropped response can't pin memory forever.
/proxy previously accepted any host/port from the remote client, turning the daemon into an SSRF gadget for whatever network the user's machine could reach. Restrict targets to a small allowlist (127.0.0.1, ::1, localhost) and reject everything else with 403. Add a bounded outbound timeout (10s) on the proxied requestJson call; return 504 when it trips so the relay client can fail fast instead of holding a daemon-side handler open. Drop the HTTP-level Clerk auth wrapper from handle(): the daemon binds to 127.0.0.1 only, and remote app requests come in over the relay (which verifies Clerk/HS256 before forwarding) and call routeRequest() directly in-process. The HTTP wrapper only ever saw local CLI calls and broke `aimux remote enable/disable/status` whenever CLERK_SECRET_KEY was set.
cli-auth accepted any ?callback= URL and redirected to it with the newly minted daemon token in the query string. A signed-in user clicking a crafted link could send their long-lived token to an attacker-controlled host. Validate the callback up front: scheme must be http, hostname must be 127.0.0.1 / localhost / [::1], path must be /callback. If it doesn't match, refuse outright — don't even attempt to mint a token, don't redirect anywhere — and show the user why.
Both docs/deployment.md and .env.example still described AIMUX_RELAY_TOKEN as a Clerk session JWT pasted into the user's environment. In the current architecture the daemon token is produced by \`aimux login\`, signed HS256 by the relay, and stored at ~/.aimux/auth.json with ~90d TTL. Operators reading the old text would be misled into trying short-lived Clerk JWTs. Describe the login flow and call out that AIMUX_RELAY_TOKEN is only useful as a manual override of the stored credential.
Adds a small routing-level test suite covering the cases the recent hardening touched: - /relay/status returns status: off when no relay is configured - /proxy rejects non-loopback hosts with 403 and never reaches requestJson - /proxy forwards loopback requests with timeoutMs set - /proxy returns 504 when the proxied target times out Uses the existing requestJson mock and the AimuxDaemon class directly, matching the pattern in the rest of the file.
added 4 commits
May 21, 2026 18:13
URL.hostname strips the brackets from IPv6 literals, so a callback like http://[::1]:port/callback parses to hostname "::1", not "[::1]". The previous allowlist would refuse the only valid IPv6 loopback shape.
The pending-request TTL only ran when the daemon sent a response. If the daemon hung or stopped responding mid-request, entries accumulated and the waiting client sat idle until its own transport timeout. Run the sweep from alarm() too, and emit a 504 response to the waiting client when an entry expires so it can fail fast. Keep the alarm armed as long as pendingRequests is non-empty so a single in-flight request after the last socket left still gets cleaned up on schedule.
POST /cli/issue-token already 500s if the secret is missing, but the daemon-token verification path on /daemon/connect would have called verifyDaemonToken with an empty key and either thrown (surfacing as a misleading 401 "Invalid token") or accepted whatever crypto the empty secret produced. Treat unset secret as a deployment misconfiguration — return 500 with a clear message, distinct from a genuinely invalid token.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (19)
✅ Files skipped from review due to trivial changes (4)
📝 WalkthroughWalkthroughThis PR implements a complete relay system enabling remote WebSocket access to a local daemon. It adds a Cloudflare Worker relay service with per-user Durable Object request routing, integrates a browser-based CLI login flow with token issuance, and extends the daemon with relay connectivity, proxy forwarding, and remote access commands. ChangesRelay Remote Access Implementation
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Comment on lines
+20
to
+28
| function openBrowser(url: string): void { | ||
| const cmd = platform() === "darwin" ? "open" : platform() === "win32" ? "cmd" : "xdg-open"; | ||
| const args = platform() === "win32" ? ["/c", "start", "", url] : [url]; | ||
| try { | ||
| const child = spawn(cmd, args, { stdio: "ignore", detached: true }); | ||
| child.unref(); | ||
| } catch { | ||
| // Browser open is best-effort; the URL is also printed for manual open. | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed in 64141dd — added child.on("error", () => {}) listener so async ENOENT from spawn doesn't crash with an unhandled error event.
Comment on lines
+27
to
+41
| # Set the Clerk secret key (verifies app-side session JWTs) | ||
| wrangler secret put CLERK_SECRET_KEY | ||
| # Paste your sk_live_... key | ||
|
|
||
| # Set the daemon-token signing key (HS256 secret used to mint + verify | ||
| # long-lived daemon tokens from `aimux login`). Use a strong random | ||
| # value — anything that compromises this lets an attacker forge tokens. | ||
| # e.g. `openssl rand -base64 48` or `head -c 48 /dev/urandom | base64` | ||
| wrangler secret put RELAY_TOKEN_SECRET | ||
|
|
||
| # Deploy (dev) | ||
| wrangler deploy | ||
|
|
||
| # Deploy (production with custom domain) | ||
| wrangler deploy --env production |
Owner
Author
There was a problem hiding this comment.
Fixed in 64141dd — added wrangler secret put CLI_TOKEN_ALLOWED_ORIGINS to the deployment steps with a note on the expected format (comma-separated origins).
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
- login-flow.ts: add error listener to spawned child process so async ENOENT doesn't crash with unhandled error - docs/deployment.md: add CLI_TOKEN_ALLOWED_ORIGINS to wrangler secrets setup — required for /cli/issue-token CORS to work Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 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 `@app/app/`(main)/_layout.tsx:
- Around line 36-49: During the cleanup for the RelayTransport effect,
explicitly set the relayStatusAtom to the disconnected state because unsub()
removes the status listener before transport.disconnect() can emit
"disconnected"; update the cleanup returned by the effect that uses
RelayTransport (references: RelayTransport, relayStatusAtom, unsub,
transport.disconnect, setApiRelay) to call unsub(), then transport.disconnect(),
then store.set(relayStatusAtom, "disconnected") (and finally setApiRelay(null))
so the UI reflects the final disconnected state immediately.
In `@app/components/TopBar.tsx`:
- Around line 13-15: TopBar currently always renders a <View className="mr-3">
wrapper around RelayIndicator which reserves horizontal space even when
RelayIndicator returns null; change the rendering so the wrapper is only emitted
when RelayIndicator actually renders (e.g., conditionally render the View by
checking RelayIndicator's presence or move the "mr-3" spacing into
RelayIndicator itself), targeting the RelayIndicator usage in TopBar.tsx so the
gap before AuthMenu disappears for local-only setups.
In `@app/lib/api.ts`:
- Around line 56-75: Several API methods still call localhost directly and/or
check wsConnected instead of using the relay path; update them to use the relay
routing functions and to gate routing on isConnected. For each remote-only flow
listed (getAgentHistory, getAgentOutput, sendAgentInput, getPlan, spawnAgent,
stopAgent, listThreads, and any other daemon/service wrappers around lines
referenced) replace the direct-localhost calls with callDaemonViaRelay or
callServiceViaRelay as appropriate, and change the guard from wsConnected (or
the old predicate) to isConnected so the code falls back correctly when the
relay is not available; ensure you pass the same method/path/body semantics into
callDaemonViaRelay and construct proxyPath for callServiceViaRelay using the
existing ServiceEndpoint shape.
In `@docs/deployment.md`:
- Line 63: Two fenced code blocks in the deployment docs are missing language
identifiers; add appropriate language tags to the blocks containing the
environment variables and the ASCII diagram (e.g., change the env block around
"EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY=pk_live_..." to use ```dotenv and change the
diagram block starting with "User's machine Cloud
User's phone/laptop" to use ```text) so markdownlint passes and readability
improves.
In `@relay/src/auth.ts`:
- Around line 8-9: verifyWsToken currently returns payload.sub but verifyToken
from `@clerk/backend` returns { data, errors } instead of throwing; update
verifyWsToken to inspect the result (the object returned by verifyToken) and: if
result.errors is present, throw an error including the errors; if
result.data?.sub is missing or not a string, throw an error indicating an
invalid/missing subject; only return result.data.sub when it exists and is a
string. Reference verifyWsToken and verifyToken to locate and fix the logic.
In `@relay/src/index.ts`:
- Around line 131-169: The current flow reads token only from the query string,
which exposes long‑lived daemon tokens; update the auth extraction so that for
daemon connections you first accept and validate a bearer token from
Authorization (and/or Sec-WebSocket-Protocol for WS handshakes) before falling
back to the query param only for browser short‑lived tokens; keep the existing
isDaemonToken, verifyDaemonToken, verifyWsToken, and secret checks
(RELAY_TOKEN_SECRET/CLERK_SECRET_KEY) but change the initial token acquisition
to prefer request.headers.get("authorization") (and parse Sec‑WebSocket‑Protocol
when present) for daemon validation, then only use url.searchParams.get("token")
for non‑daemon/browser cases, and continue deleting the token from doUrl before
proxying.
In `@relay/src/relay-object.ts`:
- Around line 81-87: The current logic stores in-flight requests keyed only by
parsed.id in pendingRequests, which allows different sockets to collide; change
this so pendingRequests is scoped per socket or uses a composite key (e.g.,
socketId + parsed.id) or reject duplicate parsed.ids from the same socket;
specifically update the code around the parsed.type === "request" branch where
pendingRequests.set(parsed.id, ...) (and related lookup/cleanup) to either (1)
generate a relayRequestId and store mapping relayRequestId -> { client: ws,
originalId: parsed.id, expiresAt: ... } and forward the relayRequestId to the
daemon, or (2) use a composite key like `${ws.id}:${parsed.id}` when setting and
resolving entries; ensure the daemonWs forwarding and response handling logic
uses the same key mapping and that PENDING_REQUEST_TTL_MS cleanup still applies.
- Around line 28-37: Before overwriting this.daemonWs in the role === "daemon"
branch, capture the old socket and immediately fail any pending requests
associated with that socket so they don't linger until TTL expiry; specifically,
in the block that currently calls this.send(...) and oldWs.close(...), call the
same failure path used by webSocketClose/removeSocket (or introduce a helper
like failPendingRequestsForSocket(socket)) to iterate pendingRequests and emit
errors for requests sent to the displaced daemon socket, then proceed to close
and replace this.daemonWs and broadcast; alternatively, update
removeSocket/webSocketClose to detect and fail requests for sockets no longer
referenced as this.daemonWs by using the socket identity rather than relying on
equality with this.daemonWs.
- Around line 10-14: Rehydrate in-memory socket state on Durable Object wake by
calling this.ctx.getWebSockets() (e.g., in the RelayObject constructor or at
start of fetch) and rebuild daemonWs, clientSockets, and per-client pending
request mappings from socket tags/metadata instead of relying solely on
ephemeral fields; change pendingRequests from a single Map keyed by parsed.id to
a Map<WebSocket, Map<string, {client: WebSocket; expiresAt: number}>> (or a
composite key that includes a stable client identifier) so request ids like "rN"
cannot collide across tabs, update code paths that add/remove pending entries
(e.g., in fetch(), removeSocket(), and webSocketClose()) to use the per-socket
map, and when replacing daemonWs ensure you first call removeSocket(oldDaemonWs)
or drain/forward its pending entries to avoid leaving in-flight requests
stranded before setting this.daemonWs = newWs.
In `@src/daemon.ts`:
- Around line 601-608: The handle() method is dropping URL query params by
passing only url.pathname into routeRequest(); update handle to preserve the
full path+query when forwarding by passing url.pathname + url.search (or
url.pathname + (url.search || "")) to routeRequest (keep using the same readJson
logic and IncomingMessage/ServerResponse flow), so routeRequest receives the
original request path including query string instead of losing GET params.
- Around line 283-287: The code currently sets enabled from creds first so
environment overrides (AIMUX_RELAY_URL/AIMUX_RELAY_TOKEN) are ignored; update
the enabled calculation so if either AIMUX_RELAY_URL or AIMUX_RELAY_TOKEN is
present then enabled = true, otherwise fall back to creds.remoteEnabled when
creds exists (and false if neither exists). Change the enabled assignment near
loadCredentials()/creds/relayUrl/relayToken to check process.env.AIMUX_RELAY_URL
|| process.env.AIMUX_RELAY_TOKEN first, then use creds?.remoteEnabled as the
fallback.
In `@src/login-flow.ts`:
- Around line 88-96: The saveCredentials call in the login callback can throw
and crash the CLI; wrap the saveCredentials(...) invocation in a try/catch
inside the same callback (the code that currently calls resolve({ userId })),
and on error log the failure (using the existing logger or console) and call
reject(err) (or reject(new Error(...))) so the login Promise fails gracefully
instead of letting the write exception bubble and terminate the process; keep
resolve({ userId }) on success.
In `@src/main.ts`:
- Around line 1406-1451: Change the "remote status" and "remote disable" flows
so they do not call ensureDaemonRunning() (which can spawn the daemon); instead
use a non-starting check and fall back to local credential state. For the status
command, remove await ensureDaemonRunning() and call requestDaemonJson only if a
new helper like isDaemonRunning() returns true (or call a non-starting variant
of requestDaemonJson), otherwise report relay info from loadCredentials() as the
existing catch does. For the disable command, avoid calling
ensureDaemonRunning(): if the daemon is running call
requestDaemonJson("/relay/disable", { method: "POST" }), but if it is not
running update the local credential state (e.g., set creds.remoteEnabled = false
and persist) and print the disabled message without starting the daemon; update
references to ensureDaemonRunning, requestDaemonJson, loadCredentials, and the
remote disable/status command handlers accordingly.
🪄 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 Plus
Run ID: aa97295c-5ec9-41ca-ba54-5967e897d032
⛔ Files ignored due to path filters (1)
relay/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (35)
.env.example.gitignoreapp/.env.exampleapp/app/(main)/_layout.tsxapp/app/_layout.tsxapp/app/cli-auth.tsxapp/app/index.tsxapp/app/landing.tsxapp/app/sign-in.tsxapp/app/sign-up.tsxapp/components/AuthMenu.tsxapp/components/RelayIndicator.tsxapp/components/TopBar.tsxapp/lib/api.tsapp/lib/env.tsapp/lib/envContract.jsapp/lib/envRuntime.tsapp/lib/relay-transport.tsapp/stores/relay.tsdocs/deployment.mdrelay/package.jsonrelay/src/auth.tsrelay/src/daemon-token.tsrelay/src/index.tsrelay/src/relay-object.tsrelay/src/types.tsrelay/tsconfig.jsonrelay/wrangler.tomlsrc/credentials.tssrc/daemon.test.tssrc/daemon.tssrc/login-flow.tssrc/main.tssrc/paths.tssrc/relay-client.ts
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Comment on lines
+606
to
+608
| const url = new URL(req.url ?? "/", `http://${DAEMON_HOST}:${DAEMON_PORT}`); | ||
| const body = req.method === "POST" ? await readJson(req) : undefined; | ||
| const result = await this.routeRequest(req.method ?? "GET", url.pathname, body); |
Comment on lines
+67
to
+75
| async function callServiceViaRelay<T>( | ||
| endpoint: ServiceEndpoint, | ||
| method: string, | ||
| path: string, | ||
| body?: unknown, | ||
| ): Promise<T> { | ||
| const proxyPath = `/proxy/${endpoint.host}/${endpoint.port}${path}`; | ||
| return callDaemonViaRelay<T>(method, proxyPath, body); | ||
| } |
Comment on lines
754
to
758
| const repoRoot = mkdtempSync(join(tmpdir(), "aimux-session-migrate-")); | ||
| const targetRoot = mkdtempSync(join(tmpdir(), "aimux-session-migrate-target-")); | ||
| execFileSync("git", ["init"], { cwd: repoRoot, stdio: "ignore" }); | ||
| gitInit(repoRoot); | ||
| execFileSync("git", ["init"], { cwd: targetRoot, stdio: "ignore" }); | ||
| await initPaths(repoRoot); |
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (6)
relay/src/auth.ts (1)
8-9:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winHandle
verifyTokenreturn value correctly (returns{ data, errors }not payload).
@clerk/backend'sverifyTokenreturnsPromise<JwtReturnType<JwtPayload, TokenVerificationError>>, which resolves to{ data, errors }rather than a direct payload. The current code treats the result as a payload object, causingpayload.subto be undefined and allowing invalid tokens through.🔒 Proposed fix
export async function verifyWsToken(token: string, env: Env): Promise<string> { if (!env.CLERK_SECRET_KEY) { throw new Error("CLERK_SECRET_KEY not configured"); } - const payload = await verifyToken(token, { secretKey: env.CLERK_SECRET_KEY }); - return payload.sub; + const result = await verifyToken(token, { secretKey: env.CLERK_SECRET_KEY }); + if (result.errors) { + throw new Error(`Token verification failed: ${result.errors[0]?.message || 'Unknown error'}`); + } + if (!result.data?.sub || typeof result.data.sub !== 'string') { + throw new Error("Invalid token: missing or invalid sub claim"); + } + return result.data.sub; }🤖 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 `@relay/src/auth.ts` around lines 8 - 9, The code incorrectly treats verifyToken(...) as returning a payload; update the call to destructure its result (const { data, errors } = await verifyToken(token, { secretKey: env.CLERK_SECRET_KEY })) then validate: if errors exist or data is falsy, throw or return an authentication failure; otherwise return data.sub. Apply this in the function that calls verifyToken in relay/src/auth.ts, and include a clear runtime check to avoid undefined sub values.docs/deployment.md (1)
69-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifiers to fenced code blocks.
These blocks are missing language tags and will keep MD040 warnings active.
Suggested patch
-``` +```dotenv EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY=pk_live_... EXPO_PUBLIC_AIMUX_RELAY_URL=wss://relay.aimux.com-
+text
User's machine Cloud User's phone/laptop
┌──────────┐ ┌──────────────┐ ┌──────────────┐
│ aimux │───WS tunnel───│ Cloudflare │───WS tunnel───│ aimux app │
@@
Daemon: HS256 minted by relay App: Clerk session JWTAlso applies to: 121-133
🤖 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 `@docs/deployment.md` around lines 69 - 72, Update the fenced code blocks in the documentation to include language identifiers so MD040 warnings are resolved: change the first environment variables block to use a dotenv tag (e.g., replace ``` with ```dotenv) and the ASCII diagram block to use a text tag (e.g., replace ``` with ```text); apply the same change for the second occurrence noted around the other block (lines referenced in the review) so all untagged triple-backtick blocks in the docs/deployment.md snippet have explicit language identifiers.relay/src/index.ts (1)
131-134:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid URL query auth for daemon WebSocket tokens.
Daemon tokens are long-lived credentials; keeping them in
?token=risks leakage via logs/traces. PreferAuthorization/Sec-WebSocket-Protocolfor/daemon/connect, with query fallback only where required.Suggested patch
- const token = url.searchParams.get("token"); + const authHeader = request.headers.get("authorization"); + const bearer = authHeader?.startsWith("Bearer ") ? authHeader.slice(7) : null; + const wsProtocol = request.headers.get("sec-websocket-protocol") ?? ""; + const protoToken = + wsProtocol + .split(",") + .map((p) => p.trim()) + .find((p) => p.startsWith("bearer."))?.slice("bearer.".length) ?? null; + + const token = + url.pathname === "/daemon/connect" + ? (bearer ?? protoToken ?? url.searchParams.get("token")) + : url.searchParams.get("token");🤖 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 `@relay/src/index.ts` around lines 131 - 134, The current handler extracts daemon credentials from the URL query (const token = url.searchParams.get("token")) which can leak long-lived tokens; change the auth flow in the /daemon/connect handling to first read the token from the Authorization header (Bearer) and then from Sec-WebSocket-Protocol before falling back to url.searchParams.get("token") only if absolutely necessary, update the auth-check branch that returns the corsResponse to use the new token source, and ensure you do not log or include the raw token in any error messages; look for the token variable and the corsResponse return path to modify.relay/src/relay-object.ts (3)
28-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFail displaced-daemon in-flight requests immediately.
When replacing daemon socket, pending requests for the old daemon can linger until TTL instead of failing fast.
Suggested patch
if (role === "daemon") { if (this.daemonWs) { + for (const [id, entry] of this.pendingRequests) { + try { + entry.client.send( + JSON.stringify({ + id, + type: "response", + status: 502, + body: { ok: false, error: "Daemon connection replaced" }, + }), + ); + } catch {} + } + this.pendingRequests.clear(); try { this.send(this.daemonWs, { type: "error", message: "Replaced by new daemon connection" }); this.daemonWs.close(1000, "Replaced"); } catch {} }Also applies to: 156-177
🤖 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 `@relay/src/relay-object.ts` around lines 28 - 37, When replacing the daemon WebSocket (daemonWs) you must immediately fail any in-flight requests that were targeted at the old daemon instead of letting them linger; locate the code in the block that handles role === "daemon" where daemonWs is replaced, and after sending the "Replaced by new daemon connection" message and closing the old socket, iterate your in-flight request tracking structure (e.g., pendingDaemonRequests, requestsMap, or similar) and invoke the corresponding error/resolution callbacks with a clear error (e.g., "daemon replaced") and clear any TTL/timeouts for each entry; ensure the new daemonWs is set and the new connected message is sent (send(server, { type: "connected", role: "daemon" })) and that broadcastToClients({ type: "daemon_status", online: true }) remains unchanged.
70-87:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftScope in-flight request keys per socket (or relay-generated id).
Using a single global key
parsed.idallows collisions between concurrent clients and can route responses to the wrong requester.🤖 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 `@relay/src/relay-object.ts` around lines 70 - 87, The current logic uses parsed.id as a global key causing collisions; instead generate and use a relay-scoped id when forwarding requests and store pendingRequests under that id tied to the originating socket. In the branch where !isDaemon and parsed.type === "request" (symbols: parsed, ws, daemonWs, PENDING_REQUEST_TTL_MS, pendingRequests), create a unique relayId (e.g., incrementing counter or UUID), set parsed.id = relayId before sending to the daemon, and store pendingRequests.set(relayId, { client: ws, expiresAt: Date.now() + PENDING_REQUEST_TTL_MS }). In the isDaemon && parsed.type === "response" branch (symbols: isDaemon, parsed, sweepExpiredPending, pendingRequests), look up pendingRequests by the relayId in parsed.id, forward the response to the stored client, and delete the pendingRequests entry; keep sweepExpiredPending and TTL logic unchanged.
10-14:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRehydrate socket state after Durable Object wake.
daemonWs/clientSocketsare never rebuilt from live sockets, so post-hibernation routing can degrade to false “daemon not connected” until reconnect.Suggested direction
export class RelayObject extends DurableObject<Env> { private daemonWs: WebSocket | null = null; private clientSockets = new Set<WebSocket>(); private pendingRequests = new Map<string, { client: WebSocket; expiresAt: number }>(); + + constructor(ctx: DurableObjectState, env: Env) { + super(ctx, env); + for (const ws of this.ctx.getWebSockets()) { + const tags = this.ctx.getTags(ws); + if (tags.includes("daemon")) this.daemonWs = ws; + else this.clientSockets.add(ws); + } + }Also applies to: 15-47
🤖 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 `@relay/src/relay-object.ts` around lines 10 - 14, RelayObject currently loses in-memory socket references after hibernation; persist minimal socket metadata when opening/closing sockets (e.g., socket ids, role: daemon|client, pending request ids with expiresAt) and, on Durable Object wake (in the RelayObject constructor or on first fetch), read that metadata from this.state.storage and rehydrate daemonWs, clientSockets, and pendingRequests by resolving persisted socket ids to actual live WebSocket objects via a module-level registry or weak map that tracks live WebSocket instances; ensure socket open/close handlers update both the in-memory sets/maps (daemonWs, clientSockets, pendingRequests) and the persisted metadata so routing remains correct after wake.
🧹 Nitpick comments (1)
.gitignore (1)
11-11: 💤 Low valueRemove duplicate
.enventry.Line 11 duplicates the
.enventry already present at line 6. While harmless, removing it improves maintainability.🤖 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 @.gitignore at line 11, Remove the duplicate ".env" entry from the .gitignore file: keep a single ".env" line (the one already present) and delete the redundant second ".env" line to avoid duplication and improve maintainability.
🤖 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 `@app/app/cli-auth.tsx`:
- Around line 88-91: The POST to `${base}/cli/issue-token` uses a plain fetch
that can hang; wrap it with an AbortController and a timeout to abort the
request after a short duration (e.g. 10s) and clear the timer on completion. In
the function containing the fetch (the block where `const res = await
fetch(`${base}/cli/issue-token`, { method: "POST", headers: { Authorization:
\`Bearer ${token}\` }, });` is declared) create an AbortController, pass its
signal to fetch, start a setTimeout that calls controller.abort(), and on
success or error clear the timeout; catch the abort (check for
DOMException/AbortError) and transition the component state out of `issuing`
into an error/timeout state so the UI won't remain stuck. Ensure the controller
is cleaned up to avoid leaks.
- Around line 70-73: When LOCAL_MODE is true the function currently sets
setError and setPhase and returns without notifying the CLI's localhost
callback, causing CLI commands to hang; update the LOCAL_MODE branch so that if
the parsed callback URL (e.g., callbackUrl or localhost callback variable used
elsewhere in this file) is present you POST a notification to it (await a
fetch/POST of JSON like {status:"error", message: "This deployment runs in local
mode — no remote login needed."}) before returning, then call setError and
setPhase as now; reference LOCAL_MODE, setError, setPhase and the
callbackUrl/localhost callback variable to find where to send the notification.
In `@relay/src/index.ts`:
- Around line 136-158: Enforce endpoint-specific token types: when handling the
connect route, check the request path/route and if the endpoint is
'/daemon/connect' require isDaemonToken(token), ensure env.RELAY_TOKEN_SECRET is
set, and call verifyDaemonToken(token, env.RELAY_TOKEN_SECRET!) to set userId;
if the endpoint is '/client/connect' require !isDaemonToken(token), ensure
env.CLERK_SECRET_KEY is set, and call verifyWsToken(token, env) to set userId;
if the token shape does not match the endpoint role return a 401 (or appropriate
error) rather than falling back to the other verifier; update the pre-checks
that currently gate secrets (env.RELAY_TOKEN_SECRET / env.CLERK_SECRET_KEY) to
be conditional on the endpoint so missing secrets produce 500 only for the
endpoint that needs them.
In `@relay/src/types.ts`:
- Around line 22-27: The RelayResponse interface requires a body but some HTTP
responses may be empty; update the interface RelayResponse to make body optional
(change body: unknown to body?: unknown) and then audit references to
RelayResponse (e.g., any functions expecting response.body) to handle undefined
safely (add guards or defaults where needed).
In `@src/credentials.ts`:
- Around line 38-40: The current empty catch around chmodSync(path, 0o600)
silently ignores failures; change it to surface and handle the error by catching
the exception, logging or including the original error and the target path, and
then rethrowing or returning a failure so callers know permission hardening
failed; specifically update the try/catch around chmodSync to catch (err) and
either throw a new Error(`chmod ${path} failed: ${err}`) or call the existing
logger with the error and propagate the failure to the caller.
In `@src/multiplexer/session-launch.test.ts`:
- Line 757: Replace the raw execFileSync("git", ["init"], { cwd: targetRoot,
stdio: "ignore" }) call with the existing gitInit helper to ensure both repos
are initialized with the same sanitized environment; specifically, locate the
initialization that uses targetRoot and call gitInit(targetRoot) (matching the
earlier gitInit(repoRoot) usage) and remove the direct execFileSync invocation
so both repositories use the helper.
---
Duplicate comments:
In `@docs/deployment.md`:
- Around line 69-72: Update the fenced code blocks in the documentation to
include language identifiers so MD040 warnings are resolved: change the first
environment variables block to use a dotenv tag (e.g., replace ``` with
```dotenv) and the ASCII diagram block to use a text tag (e.g., replace ``` with
```text); apply the same change for the second occurrence noted around the other
block (lines referenced in the review) so all untagged triple-backtick blocks in
the docs/deployment.md snippet have explicit language identifiers.
In `@relay/src/auth.ts`:
- Around line 8-9: The code incorrectly treats verifyToken(...) as returning a
payload; update the call to destructure its result (const { data, errors } =
await verifyToken(token, { secretKey: env.CLERK_SECRET_KEY })) then validate: if
errors exist or data is falsy, throw or return an authentication failure;
otherwise return data.sub. Apply this in the function that calls verifyToken in
relay/src/auth.ts, and include a clear runtime check to avoid undefined sub
values.
In `@relay/src/index.ts`:
- Around line 131-134: The current handler extracts daemon credentials from the
URL query (const token = url.searchParams.get("token")) which can leak
long-lived tokens; change the auth flow in the /daemon/connect handling to first
read the token from the Authorization header (Bearer) and then from
Sec-WebSocket-Protocol before falling back to url.searchParams.get("token") only
if absolutely necessary, update the auth-check branch that returns the
corsResponse to use the new token source, and ensure you do not log or include
the raw token in any error messages; look for the token variable and the
corsResponse return path to modify.
In `@relay/src/relay-object.ts`:
- Around line 28-37: When replacing the daemon WebSocket (daemonWs) you must
immediately fail any in-flight requests that were targeted at the old daemon
instead of letting them linger; locate the code in the block that handles role
=== "daemon" where daemonWs is replaced, and after sending the "Replaced by new
daemon connection" message and closing the old socket, iterate your in-flight
request tracking structure (e.g., pendingDaemonRequests, requestsMap, or
similar) and invoke the corresponding error/resolution callbacks with a clear
error (e.g., "daemon replaced") and clear any TTL/timeouts for each entry;
ensure the new daemonWs is set and the new connected message is sent
(send(server, { type: "connected", role: "daemon" })) and that
broadcastToClients({ type: "daemon_status", online: true }) remains unchanged.
- Around line 70-87: The current logic uses parsed.id as a global key causing
collisions; instead generate and use a relay-scoped id when forwarding requests
and store pendingRequests under that id tied to the originating socket. In the
branch where !isDaemon and parsed.type === "request" (symbols: parsed, ws,
daemonWs, PENDING_REQUEST_TTL_MS, pendingRequests), create a unique relayId
(e.g., incrementing counter or UUID), set parsed.id = relayId before sending to
the daemon, and store pendingRequests.set(relayId, { client: ws, expiresAt:
Date.now() + PENDING_REQUEST_TTL_MS }). In the isDaemon && parsed.type ===
"response" branch (symbols: isDaemon, parsed, sweepExpiredPending,
pendingRequests), look up pendingRequests by the relayId in parsed.id, forward
the response to the stored client, and delete the pendingRequests entry; keep
sweepExpiredPending and TTL logic unchanged.
- Around line 10-14: RelayObject currently loses in-memory socket references
after hibernation; persist minimal socket metadata when opening/closing sockets
(e.g., socket ids, role: daemon|client, pending request ids with expiresAt) and,
on Durable Object wake (in the RelayObject constructor or on first fetch), read
that metadata from this.state.storage and rehydrate daemonWs, clientSockets, and
pendingRequests by resolving persisted socket ids to actual live WebSocket
objects via a module-level registry or weak map that tracks live WebSocket
instances; ensure socket open/close handlers update both the in-memory sets/maps
(daemonWs, clientSockets, pendingRequests) and the persisted metadata so routing
remains correct after wake.
---
Nitpick comments:
In @.gitignore:
- Line 11: Remove the duplicate ".env" entry from the .gitignore file: keep a
single ".env" line (the one already present) and delete the redundant second
".env" line to avoid duplication and improve maintainability.
🪄 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 Plus
Run ID: 24abed91-6e42-47bb-818b-ef20c01d9d06
⛔ Files ignored due to path filters (1)
relay/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
.env.example.gitignoreapp/.env.exampleapp/app/(main)/_layout.tsxapp/app/_layout.tsxapp/app/cli-auth.tsxapp/app/index.tsxapp/app/landing.tsxapp/app/sign-in.tsxapp/app/sign-up.tsxapp/components/AuthMenu.tsxapp/components/RelayIndicator.tsxapp/components/TopBar.tsxapp/lib/api.tsapp/lib/env.tsapp/lib/envContract.jsapp/lib/envRuntime.tsapp/lib/relay-transport.tsapp/stores/relay.tsdocs/deployment.mdrelay/package.jsonrelay/src/auth.tsrelay/src/daemon-token.tsrelay/src/index.tsrelay/src/relay-object.tsrelay/src/types.tsrelay/tsconfig.jsonrelay/wrangler.tomlsrc/credentials.tssrc/daemon.test.tssrc/daemon.tssrc/login-flow.tssrc/main.tssrc/multiplexer/session-launch.test.tssrc/paths.tssrc/relay-client.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
relay/src/relay-object.ts (3)
81-87:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftRequest ID collision across client sessions.
pendingRequestsis keyed byparsed.idalone. If two app sessions for the same user generate the same incrementing ID (r1,r2, etc.), the later request overwrites the earlier entry, causing misrouted or lost responses.This matches the prior review feedback that was not addressed.
🤖 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 `@relay/src/relay-object.ts` around lines 81 - 87, pendingRequests is keyed only by parsed.id which causes collisions across different client sessions; change the key used in this.pendingRequests.set (and the corresponding lookup when handling responses) to a composite key that includes a client/session-specific identifier (for example combine parsed.id with a unique per-connection id on the ws object or generate a UUID per request), so in the branch where parsed.type === "request" and when responses are matched you use the same compositeKey instead of parsed.id alone; update any code that reads pendingRequests (response handling logic) to compute the same composite key for lookup and preserve existing metadata like expiresAt and client.
28-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPending requests for displaced daemon not failed on replacement.
When a new daemon connects, the old daemon's socket is closed but
pendingRequestsentries targeting it are not failed. They'll wait until TTL expiry (60s) instead of failing immediately with a 502.This matches the prior review feedback that was not addressed.
🤖 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 `@relay/src/relay-object.ts` around lines 28 - 37, When replacing the daemon connection in the block that handles role === "daemon" (where daemonWs is swapped), immediately fail any pendingRequests that target the old socket instead of leaving them to TTL expiry: iterate the pendingRequests map/array, find entries whose target/ws reference equals the old daemonWs, remove them from pendingRequests, send each requester a failure response (HTTP 502 style message) via send (or the same response path used for timeouts), and clean up any associated timers/resolve/reject handlers; do this before assigning this.daemonWs = server and broadcasting the new daemon status so no request waits the 60s TTL.
10-14:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftDurable Object state not rehydrated after hibernation wake.
After DO hibernation,
daemonWs,clientSockets, andpendingRequestsreset to initial values while WebSocket connections may still be alive viathis.ctx.getWebSockets(). This causes existing clients to receive "Daemon not connected" errors until the daemon reconnects.This matches the prior review feedback that was not addressed.
🤖 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 `@relay/src/relay-object.ts` around lines 10 - 14, The Durable Object loses in-memory handles after hibernation because daemonWs, clientSockets and pendingRequests are only in-memory; fix by persisting minimal metadata and rehydrating from this.ctx.getWebSockets() and state storage on wake/initialization: when accepting a WebSocket, store a small role/id record in DurableObject state (e.g., mark sockets as "daemon" vs "client" and persist pendingRequests entries with expiresAt), and in RelayObject initialization (constructor or first fetch) call this.ctx.getWebSockets() to iterate current live sockets and reassign the daemonWs reference and refill clientSockets Set from those live socket objects, then reload pendingRequests from this.state.storage and prune expired entries so the DO correctly reconnects clients to the existing daemon socket after hibernation.src/main.ts (2)
1406-1416:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
remote statusspawns daemon unnecessarily for a read-only query.The
ensureDaemonRunning()call at line 1410 will start a daemon if none is running, which is surprising for a status check. The existing catch block already falls back to credential state, so the daemon spawn should be conditional.This matches the prior review feedback that was not fully addressed.
Suggested fix
.action(async (opts: { json?: boolean }) => { const creds = loadCredentials(); let relay: unknown = { status: "off" }; - try { - await ensureDaemonRunning(); - const result = await requestDaemonJson("/relay/status"); - relay = result.relay; - } catch { - // Daemon not running — fall back to credential state. + if (loadDaemonInfo()) { + try { + const result = await requestDaemonJson("/relay/status"); + relay = result.relay; + } catch { + // Daemon not reachable — fall back to credential state. + } }🤖 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 `@src/main.ts` around lines 1406 - 1416, The handler currently calls ensureDaemonRunning() which can start the daemon for a read-only status query; instead, remove or avoid calling ensureDaemonRunning() unconditionally and try the non-spawning request first: call requestDaemonJson("/relay/status") inside the try and only fall back to credential state on failure (or only call ensureDaemonRunning() if you have an explicit start flag). Update the .action handler to attempt requestDaemonJson(...) directly (using ensureDaemonRunning only when intentionally starting the daemon) and keep the existing catch block to use credential state when the daemon is not available.
1445-1452:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
remote disablespawns daemon unnecessarily.Calling
ensureDaemonRunning()before/relay/disablewill start a daemon just to disable remote access on it. If the daemon isn't running, updating the local credential file (settingremoteEnabled: false) achieves the same goal without side effects.This matches the prior review feedback that was not fully addressed.
Suggested fix
+import { clearCredentials, loadCredentials, setRemoteEnabled } from "./credentials.js";remoteCmd .command("disable") .description("Disable remote access and disconnect from the relay") .action(async () => { - await ensureDaemonRunning(); - await requestDaemonJson("/relay/disable", { method: "POST" }); + if (loadDaemonInfo()) { + try { + await requestDaemonJson("/relay/disable", { method: "POST" }); + } catch { + // Daemon not reachable — update local state only. + } + } + setRemoteEnabled(false); console.log("✓ Remote access disabled. Daemon disconnected from relay."); });🤖 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 `@src/main.ts` around lines 1445 - 1452, The current disable command always calls ensureDaemonRunning() which can start the daemon unnecessarily; change remoteCmd.command("disable") so it first checks whether the daemon is active (use the existing isDaemonRunning() or equivalent status check) and only call requestDaemonJson("/relay/disable", { method: "POST" }) if the daemon is running; if the daemon is not running, do not call ensureDaemonRunning() or requestDaemonJson—instead directly update the local credential/config file to set remoteEnabled: false (use the project's existing credential write/update helper) and print the same success message. Ensure you remove or guard the ensureDaemonRunning() call in this command handler and keep console output consistent.src/login-flow.ts (1)
87-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard credential persistence failures in the callback handler.
saveCredentials()at line 87 can throw on file system errors, which would crash the process instead of rejecting the login promise gracefully.This matches the prior review feedback that was not addressed.
Proposed fix
- saveCredentials({ - version: 1, - relayUrl, - token, - userId, - createdAt: new Date().toISOString(), - remoteEnabled: true, - }); - resolve({ userId }); + try { + saveCredentials({ + version: 1, + relayUrl, + token, + userId, + createdAt: new Date().toISOString(), + remoteEnabled: true, + }); + resolve({ userId }); + } catch (err) { + reject(err instanceof Error ? err : new Error(String(err))); + }🤖 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 `@src/login-flow.ts` around lines 87 - 95, saveCredentials(...) can throw and will crash the process; wrap the call in a try/catch inside the login promise callback and on failure log the error and call the promise's reject handler instead of letting the exception propagate. Specifically, around saveCredentials({ version: 1, relayUrl, token, userId, ... }) catch any error, use processLogger.error(...) (or the available logger) to record details, then call reject(err) (or otherwise reject the enclosing login promise) so the handler returns a rejected promise rather than crashing; leave the existing resolve({ userId }) only in the success path.
🧹 Nitpick comments (2)
.gitignore (1)
11-11: 💤 Low valueDuplicate
.enventry.The
.envpattern is already present on line 6. Consider removing this duplicate to keep the file clean.🤖 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 @.gitignore at line 11, Remove the duplicate `.env` entry in the .gitignore file so the pattern appears only once; locate the second `.env` line and delete it to keep the file clean and avoid redundant entries.relay/tsconfig.json (1)
13-14: ⚡ Quick winConflicting TypeScript compiler options.
Setting both
"declaration": trueand"noEmit": trueis contradictory. WhennoEmitis enabled, TypeScript skips all output including declaration files, sodeclarationhas no effect. If the goal is type-checking only, remove"declaration": truefor clarity.🔧 Suggested fix
"outDir": "dist", "rootDir": "src", - "declaration": true, "noEmit": true🤖 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 `@relay/tsconfig.json` around lines 13 - 14, The tsconfig contains conflicting compiler options: "declaration": true and "noEmit": true; update the configuration so they are not contradictory by either removing "declaration" if you only want type-checking (keep "noEmit": true) or disabling "noEmit" if you need declaration output (keep "declaration": true). Locate the entries "declaration" and "noEmit" in the tsconfig JSON and make the appropriate change so only the intended behavior remains.
🤖 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 `@relay/package.json`:
- Around line 11-18: The `@clerk/backend` dependency is pinned to a vulnerable
version "3.0.1" which falls into HIGH-severity ranges; update the dependency
entry for "`@clerk/backend`" to at least ">=3.2.14" (e.g., "3.2.14" or a caret
range "^3.2.14") in package.json to include both the Authz bypass and SSRF
patches, then run your package manager install and retest; also re-run the
vulnerability scan for "`@cloudflare/workers-types`", "typescript", and "wrangler"
to confirm no further advisories are present.
---
Duplicate comments:
In `@relay/src/relay-object.ts`:
- Around line 81-87: pendingRequests is keyed only by parsed.id which causes
collisions across different client sessions; change the key used in
this.pendingRequests.set (and the corresponding lookup when handling responses)
to a composite key that includes a client/session-specific identifier (for
example combine parsed.id with a unique per-connection id on the ws object or
generate a UUID per request), so in the branch where parsed.type === "request"
and when responses are matched you use the same compositeKey instead of
parsed.id alone; update any code that reads pendingRequests (response handling
logic) to compute the same composite key for lookup and preserve existing
metadata like expiresAt and client.
- Around line 28-37: When replacing the daemon connection in the block that
handles role === "daemon" (where daemonWs is swapped), immediately fail any
pendingRequests that target the old socket instead of leaving them to TTL
expiry: iterate the pendingRequests map/array, find entries whose target/ws
reference equals the old daemonWs, remove them from pendingRequests, send each
requester a failure response (HTTP 502 style message) via send (or the same
response path used for timeouts), and clean up any associated
timers/resolve/reject handlers; do this before assigning this.daemonWs = server
and broadcasting the new daemon status so no request waits the 60s TTL.
- Around line 10-14: The Durable Object loses in-memory handles after
hibernation because daemonWs, clientSockets and pendingRequests are only
in-memory; fix by persisting minimal metadata and rehydrating from
this.ctx.getWebSockets() and state storage on wake/initialization: when
accepting a WebSocket, store a small role/id record in DurableObject state
(e.g., mark sockets as "daemon" vs "client" and persist pendingRequests entries
with expiresAt), and in RelayObject initialization (constructor or first fetch)
call this.ctx.getWebSockets() to iterate current live sockets and reassign the
daemonWs reference and refill clientSockets Set from those live socket objects,
then reload pendingRequests from this.state.storage and prune expired entries so
the DO correctly reconnects clients to the existing daemon socket after
hibernation.
In `@src/login-flow.ts`:
- Around line 87-95: saveCredentials(...) can throw and will crash the process;
wrap the call in a try/catch inside the login promise callback and on failure
log the error and call the promise's reject handler instead of letting the
exception propagate. Specifically, around saveCredentials({ version: 1,
relayUrl, token, userId, ... }) catch any error, use processLogger.error(...)
(or the available logger) to record details, then call reject(err) (or otherwise
reject the enclosing login promise) so the handler returns a rejected promise
rather than crashing; leave the existing resolve({ userId }) only in the success
path.
In `@src/main.ts`:
- Around line 1406-1416: The handler currently calls ensureDaemonRunning() which
can start the daemon for a read-only status query; instead, remove or avoid
calling ensureDaemonRunning() unconditionally and try the non-spawning request
first: call requestDaemonJson("/relay/status") inside the try and only fall back
to credential state on failure (or only call ensureDaemonRunning() if you have
an explicit start flag). Update the .action handler to attempt
requestDaemonJson(...) directly (using ensureDaemonRunning only when
intentionally starting the daemon) and keep the existing catch block to use
credential state when the daemon is not available.
- Around line 1445-1452: The current disable command always calls
ensureDaemonRunning() which can start the daemon unnecessarily; change
remoteCmd.command("disable") so it first checks whether the daemon is active
(use the existing isDaemonRunning() or equivalent status check) and only call
requestDaemonJson("/relay/disable", { method: "POST" }) if the daemon is
running; if the daemon is not running, do not call ensureDaemonRunning() or
requestDaemonJson—instead directly update the local credential/config file to
set remoteEnabled: false (use the project's existing credential write/update
helper) and print the same success message. Ensure you remove or guard the
ensureDaemonRunning() call in this command handler and keep console output
consistent.
---
Nitpick comments:
In @.gitignore:
- Line 11: Remove the duplicate `.env` entry in the .gitignore file so the
pattern appears only once; locate the second `.env` line and delete it to keep
the file clean and avoid redundant entries.
In `@relay/tsconfig.json`:
- Around line 13-14: The tsconfig contains conflicting compiler options:
"declaration": true and "noEmit": true; update the configuration so they are not
contradictory by either removing "declaration" if you only want type-checking
(keep "noEmit": true) or disabling "noEmit" if you need declaration output (keep
"declaration": true). Locate the entries "declaration" and "noEmit" in the
tsconfig JSON and make the appropriate change so only the intended behavior
remains.
🪄 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 Plus
Run ID: fd5365dd-1269-4ce2-b318-bcb7f51e9621
⛔ Files ignored due to path filters (1)
relay/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
.env.example.gitignoreapp/.env.exampleapp/app/(main)/_layout.tsxapp/app/_layout.tsxapp/app/cli-auth.tsxapp/app/index.tsxapp/app/landing.tsxapp/app/sign-in.tsxapp/app/sign-up.tsxapp/components/AuthMenu.tsxapp/components/RelayIndicator.tsxapp/components/TopBar.tsxapp/lib/api.tsapp/lib/env.tsapp/lib/envContract.jsapp/lib/envRuntime.tsapp/lib/relay-transport.tsapp/stores/relay.tsdocs/deployment.mdrelay/package.jsonrelay/src/auth.tsrelay/src/daemon-token.tsrelay/src/index.tsrelay/src/relay-object.tsrelay/src/types.tsrelay/tsconfig.jsonrelay/wrangler.tomlsrc/credentials.tssrc/daemon.test.tssrc/daemon.tssrc/login-flow.tssrc/main.tssrc/multiplexer/session-launch.test.tssrc/paths.tssrc/relay-client.ts
added 2 commits
May 21, 2026 23:23
…te-access # Conflicts: # app/lib/api.ts
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This was referenced May 22, 2026
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
Stacked on #3. Adds remote agent control: a Cloudflare Workers + Durable Objects relay brokers an authenticated WebSocket tunnel between a logged-in app session and the user's local daemon.
Architecture
relay/— Cloudflare Worker. Verifies Clerk JWT on app connect; for daemons, accepts long-lived HS256 tokens it mints itself. Durable Object holds the WS pair and proxies app↔daemon traffic + per-project metadata-server requests via a/proxy/:host/:port/*route@clerk/clerk-expo); daemon Bearer-token validation via@clerk/backendwhenCLERK_SECRET_KEYis setsrc/relay-client.ts) — dials the WS tunnel with the stored token, exposes a status streamapp/lib/relay-transport.ts) — wraps the same fetch/SSE surface as the local HTTP client so the UI works identically over local HTTP or remote WSPolished CLI UX
aimux login— browser-based auth flow (localhost callback server, opens${WEB_APP_URL}/cli-auth, no token pasting)aimux logout/aimux whoamiaimux remote enable | disable | status— toggle the relay without restarting the daemonaimux daemon statusnow reports relay stateWhy daemon tokens are HS256-signed by the relay (not Clerk JWTs)
Clerk JWTs are ~60s. The daemon is long-lived. After browser-based
aimux login, the relay verifies the user's Clerk session and mints a 90d HS256 daemon token (relay/src/daemon-token.ts). The daemon then connects to the relay using that token — decoupled from Clerk's TTL.Test plan
yarn verifypasses 566/566 (both standalone and under pre-push)aimux loginbrowser flow drives token issuance and stores credentialsReview base
Base branch is
feat/rn-gui-base(PR #3). This PR's diff shows only the remote-access additions on top of the GUI rebuild — Copilot can review it independently.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
aimux login,aimux logout,aimux whoami, andaimux remotefor managing remote accessDocumentation