Repository navigation
fix(relay): resolve the flagged relay in every consumer - #6340
Merged
Merged
Conversation
runNow (the manual 'run automation now' trigger) hardcoded env.RELAY_URL, the same gap as scheduled dispatch — a host on an overridden relay was unreachable. Extract resolveUserRelayUrl and route relayEndpoint, runNow, and dispatch through it. Claude-Session: https://claude.ai/code/session_01A2JifsfA63nWthegPwNt8G
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change centralizes per-user relay URL resolution across server, mobile, and CLI relay consumers. Mobile clients prime and cache the resolved URL. CLI host-target resolution becomes asynchronous. ChangesPer-user relay URL routing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AuthenticatedLayout
participant primeRelayUrl
participant host_relayEndpoint
participant resolveUserRelayUrl
participant PostHog
AuthenticatedLayout->>primeRelayUrl: prime relay URL after session exists
primeRelayUrl->>host_relayEndpoint: request relay endpoint
host_relayEndpoint->>resolveUserRelayUrl: resolve URL for user
resolveUserRelayUrl->>PostHog: retrieve relay-url-override
PostHog-->>resolveUserRelayUrl: override or unavailable payload
resolveUserRelayUrl-->>host_relayEndpoint: resolved relay URL
host_relayEndpoint-->>primeRelayUrl: relay URL
primeRelayUrl-->>AuthenticatedLayout: cache normalized URL
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/trpc/src/lib/relay-url.ts`:
- Around line 23-24: Update the relay override handling around payload.url to
trim whitespace before validating and returning it, so blank or padded-only
values fall back to env.RELAY_URL while nonblank values return their normalized
content. Add regression coverage for blank and whitespace-padded overrides.
🪄 Autofix
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: 9a05f91c-bea6-414d-bba0-42cb12c9353b
📒 Files selected for processing (3)
packages/trpc/src/lib/relay-url.tspackages/trpc/src/router/automation/automation.tspackages/trpc/src/router/host/host.ts
Comment on lines
+23
to
+24
| const override = payload?.url; | ||
| if (typeof override === "string" && override.length > 0) return override; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject blank relay overrides.
override.length > 0 accepts values such as " ". This returns an unusable relay value instead of falling back to env.RELAY_URL. Trim the value before checking it and add a regression test for blank and padded values.
Proposed normalization
const override = payload?.url;
-if (typeof override === "string" && override.length > 0) return override;
+if (typeof override === "string") {
+ const normalizedOverride = override.trim();
+ if (normalizedOverride.length > 0) return normalizedOverride;
+}📝 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.
Suggested change
| const override = payload?.url; | |
| if (typeof override === "string" && override.length > 0) return override; | |
| const override = payload?.url; | |
| if (typeof override === "string") { | |
| const normalizedOverride = override.trim(); | |
| if (normalizedOverride.length > 0) return normalizedOverride; | |
| } |
🤖 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 `@packages/trpc/src/lib/relay-url.ts` around lines 23 - 24, Update the relay
override handling around payload.url to trim whitespace before validating and
returning it, so blank or padded-only values fall back to env.RELAY_URL while
nonblank values return their normalized content. Add regression coverage for
blank and whitespace-padded overrides.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/trpc/src/lib/relay-url.ts">
<violation number="1" location="packages/trpc/src/lib/relay-url.ts:17">
P2: Relay resolution remains duplicated in `apps/api`, so validation and fallback behavior can drift despite this PR’s shared-resolver goal; export this helper and replace `apps/api/src/lib/relay-url.ts` callers with it.</violation>
<violation number="2" location="packages/trpc/src/lib/relay-url.ts:24">
P2: A whitespace-only override value (e.g. " ") would pass the `override.length > 0` check and be returned as the relay URL, even though it's not a usable value. Trim the string before checking its length so blank/padded overrides correctly fall back to `env.RELAY_URL`.</violation>
<violation number="3" location="packages/trpc/src/lib/relay-url.ts:25">
P2: PostHog failures silently route overridden hosts to the default relay, with no signal to diagnose the resulting run-now/dispatch failures; log the caught error before falling back.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as Client (Desktop/CLI)
participant Web as Web App
participant Trpc as TRPC Router
participant Resolver as resolveUserRelayUrl()
participant Posthog as PostHog
participant Dispatch as dispatchAutomation()
participant Host as User's Host
Note over Client,Host: Centralized relay resolution via resolveUserRelayUrl()
%% relayEndpoint flow
Web->>Trpc: query relayEndpoint()
Trpc->>Resolver: resolveUserRelayUrl(userId)
Resolver->>Posthog: getFeatureFlagPayload(RELAY_URL_OVERRIDE, userId)
alt Flag payload provides valid override URL
Posthog-->>Resolver: { url: "https://custom-relay.example.com" }
Resolver-->>Trpc: custom relay URL
else Flag off / Posthog unavailable / empty url
Posthog-->>Resolver: null or error
Resolver-->>Trpc: env.RELAY_URL (default)
end
Trpc-->>Web: { url: resolvedRelayUrl }
Note over Web,Host: Host registers with resolved relay
%% automation.runNow flow
Client->>Trpc: runNow(automationId)
Trpc->>Resolver: resolveUserRelayUrl(automation.ownerUserId)
Resolver->>Posthog: getFeatureFlagPayload(RELAY_URL_OVERRIDE, ownerUserId)
alt Flag payload provides valid override URL
Posthog-->>Resolver: { url: "https://custom-relay.example.com" }
Resolver-->>Trpc: custom relay URL
else Flag off / Posthog unavailable / empty url
Posthog-->>Resolver: null or error
Resolver-->>Trpc: env.RELAY_URL (default)
end
Trpc->>Dispatch: dispatchAutomation({ automation, scheduledFor, relayUrl })
Dispatch->>Host: Execute automation via resolved relayUrl
%% dispatch flow (scheduled/direct)
Note over Dispatch,Host: Dispatch also uses resolver for scheduled automations
Dispatch->>Resolver: resolveUserRelayUrl(ownerUserId)
Resolver-->>Dispatch: resolved relay URL
Dispatch->>Host: Send automation event via resolved relay
Note over Client,Host: All three paths now converge on the same relay resolution
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * resolve through here — a hardcoded default relay silently misses a host | ||
| * that has moved. | ||
| */ | ||
| export async function resolveUserRelayUrl(userId: string): Promise<string> { |
Contributor
There was a problem hiding this comment.
P2: Relay resolution remains duplicated in apps/api, so validation and fallback behavior can drift despite this PR’s shared-resolver goal; export this helper and replace apps/api/src/lib/relay-url.ts callers with it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/trpc/src/lib/relay-url.ts, line 17:
<comment>Relay resolution remains duplicated in `apps/api`, so validation and fallback behavior can drift despite this PR’s shared-resolver goal; export this helper and replace `apps/api/src/lib/relay-url.ts` callers with it.</comment>
<file context>
@@ -0,0 +1,29 @@
+ * resolve through here — a hardcoded default relay silently misses a host
+ * that has moved.
+ */
+export async function resolveUserRelayUrl(userId: string): Promise<string> {
+ try {
+ const payload = (await posthog.getFeatureFlagPayload(
</file context>
| )) as RelayUrlPayload | null | undefined; | ||
| const override = payload?.url; | ||
| if (typeof override === "string" && override.length > 0) return override; | ||
| } catch { |
Contributor
There was a problem hiding this comment.
P2: PostHog failures silently route overridden hosts to the default relay, with no signal to diagnose the resulting run-now/dispatch failures; log the caught error before falling back.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/trpc/src/lib/relay-url.ts, line 25:
<comment>PostHog failures silently route overridden hosts to the default relay, with no signal to diagnose the resulting run-now/dispatch failures; log the caught error before falling back.</comment>
<file context>
@@ -0,0 +1,29 @@
+ )) as RelayUrlPayload | null | undefined;
+ const override = payload?.url;
+ if (typeof override === "string" && override.length > 0) return override;
+ } catch {
+ // Fall through to the default relay.
+ }
</file context>
| userId, | ||
| )) as RelayUrlPayload | null | undefined; | ||
| const override = payload?.url; | ||
| if (typeof override === "string" && override.length > 0) return override; |
Contributor
There was a problem hiding this comment.
P2: A whitespace-only override value (e.g. " ") would pass the override.length > 0 check and be returned as the relay URL, even though it's not a usable value. Trim the string before checking its length so blank/padded overrides correctly fall back to env.RELAY_URL.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/trpc/src/lib/relay-url.ts, line 24:
<comment>A whitespace-only override value (e.g. " ") would pass the `override.length > 0` check and be returned as the relay URL, even though it's not a usable value. Trim the string before checking its length so blank/padded overrides correctly fall back to `env.RELAY_URL`.</comment>
<file context>
@@ -0,0 +1,29 @@
+ userId,
+ )) as RelayUrlPayload | null | undefined;
+ const override = payload?.url;
+ if (typeof override === "string" && override.length > 0) return override;
+ } catch {
+ // Fall through to the default relay.
</file context>
…mers Close the last three relay-resolution gaps found in the audit: - CLI: resolveHostTarget built remote-host URLs from env.RELAY_URL, so every command targeting a remote host (workspaces/terminals/projects/ agents --host X) missed a host on an overridden relay. Thread the api client through and resolve getRelayUrl for the remote branch. - web: allowlist the relay2 origin in the CSP connect-src (the runtime relay comes from the flag while the header is built at compile time), mirroring the desktop CSP. - mobile: resolve host.relayEndpoint once after sign-in and cache it; the URL builders read the cache with the Expo env as fallback. All become unnecessary once relay2 answers on relay.superset.sh, but until the cutover a host on relay2 must be reachable from every client. Claude-Session: https://claude.ai/code/session_01A2JifsfA63nWthegPwNt8G
saddlepaddle
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/mobile/app/`(authenticated)/_layout.tsx:
- Around line 31-34: Update the authenticated layout’s relay initialization
around primeRelayUrl so relay-dependent descendant clients and host-service
queries are not created or evaluated until priming completes. Expose and
propagate a completion state or shared promise through the relevant hooks,
including useChatThread, useHostTerminalAgents, and createHostClient, and ensure
memoized clients are rebuilt after the resolved relay URL is available instead
of retaining EXPO_PUBLIC_RELAY_URL.
In `@apps/mobile/lib/host/client.ts`:
- Around line 23-31: Scope relayOverride to the current authenticated session:
update primeRelayUrl to clear it when the lookup returns no URL or errors, and
prevent late responses from an earlier session from assigning a URL after
sign-in. Also clear relayOverride during the sign-out flow, preserving the
environment fallback when no valid session-specific override exists.
In `@packages/cli/src/commands/automations/update/command.ts`:
- Line 67: Move the existing setEnabled block in the update command to execute
only after resolveAutomationTarget completes successfully, including workspace,
project, and relay validation. Preserve the current target-update behavior, and
use a single server mutation when enabled-state and target changes need to be
applied atomically.
In `@packages/cli/src/commands/workspaces/create/command.ts`:
- Around line 112-116: Move the --project/--name validation in the workspace
creation command before the await resolveHostTarget call, so missing --name is
rejected before any API-backed host resolution; preserve the existing validation
message and subsequent resolver behavior for valid inputs.
In `@packages/cli/src/lib/host-target/resolveHostTarget.ts`:
- Around line 79-86: Update the remote-client setup in resolveHostTarget to
create an AbortController with a bounded timeout before constructing
httpBatchLink, and pass its signal through the link’s fetch options so
getRelayUrl and the relay endpoint lookup cannot stall indefinitely. Preserve
the existing fallback behavior and clear the timeout when the request completes.
🪄 Autofix
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: 092aa493-5110-42b7-a896-258702c14082
📒 Files selected for processing (26)
apps/mobile/app/(authenticated)/_layout.tsxapps/mobile/lib/host-service/client.tsapps/mobile/lib/host/client.tsapps/mobile/lib/trpc/host-client.tsapps/web/next.config.tspackages/cli/src/commands/agents/create/command.tspackages/cli/src/commands/agents/list/command.tspackages/cli/src/commands/automations/create/command.tspackages/cli/src/commands/automations/resolveAutomationTarget.tspackages/cli/src/commands/automations/update/command.tspackages/cli/src/commands/projects/create/command.tspackages/cli/src/commands/projects/list/command.tspackages/cli/src/commands/projects/setup/command.tspackages/cli/src/commands/terminals/close/command.tspackages/cli/src/commands/terminals/create/command.tspackages/cli/src/commands/terminals/list/command.tspackages/cli/src/commands/terminals/read/command.tspackages/cli/src/commands/terminals/send/command.tspackages/cli/src/commands/workspaces/create/command.tspackages/cli/src/commands/workspaces/delete/command.tspackages/cli/src/commands/workspaces/get/command.tspackages/cli/src/commands/workspaces/list/command.tspackages/cli/src/commands/workspaces/open/command.tspackages/cli/src/commands/workspaces/update/command.tspackages/cli/src/lib/host-target/resolveHostTarget.tspackages/cli/src/lib/host-workspaces/workspacesOnHost.ts
Comment on lines
+31
to
+34
| // Resolve which relay this user's host is on, once inside the authed area. | ||
| useEffect(() => { | ||
| void primeRelayUrl(); | ||
| }, []); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'primeRelayUrl|getRelayUrl|buildRelayHostUrl|getHostRelayTrpcUrl|create.*Host' apps/mobileRepository: superset-sh/superset
Length of output: 37271
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- authenticated layout ---'
sed -n '1,90p' 'apps/mobile/app/(authenticated)/_layout.tsx'
printf '%s\n' '--- relay client ---'
sed -n '1,120p' apps/mobile/lib/host/client.ts
printf '%s\n' '--- host service client ---'
sed -n '1,110p' apps/mobile/lib/host-service/client.ts
printf '%s\n' '--- host tRPC client ---'
sed -n '1,90p' apps/mobile/lib/trpc/host-client.ts
printf '%s\n' '--- chat hook ---'
sed -n '80,125p' 'apps/mobile/screens/(authenticated)/workspace/[id]/chat/[sessionId]/hooks/useChatThread/useChatThread.ts'
printf '%s\n' '--- terminal agents hook ---'
sed -n '25,60p' 'apps/mobile/screens/(authenticated)/(home)/home/hooks/useHostTerminalAgents/useHostTerminalAgents.ts'
printf '%s\n' '--- host workspace hook ---'
sed -n '50,100p' apps/mobile/hooks/useHostWorkspaces/useHostWorkspaces.tsRepository: superset-sh/superset
Length of output: 15451
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
files = [
Path("apps/mobile/app/(authenticated)/_layout.tsx"),
Path("apps/mobile/lib/host/client.ts"),
Path("apps/mobile/lib/host-service/client.ts"),
Path("apps/mobile/lib/trpc/host-client.ts"),
Path("apps/mobile/screens/(authenticated)/workspace/[id]/chat/[sessionId]/hooks/useChatThread/useChatThread.ts"),
Path("apps/mobile/screens/(authenticated)/(home)/home/hooks/useHostTerminalAgents/useHostTerminalAgents.ts"),
Path("apps/mobile/hooks/useHostWorkspaces/useHostWorkspaces.ts"),
]
for path in files:
text = path.read_text()
print(f"{path}:")
for i, line in enumerate(text.splitlines(), 1):
if re.search(r"primeRelayUrl|getRelayUrl|buildRelayHostUrl|getHostRelayTrpcUrl|createHostClient|useMemo|useQuery|useQueries|useEffect", line):
print(f" {i}: {line.strip()}")
PYRepository: superset-sh/superset
Length of output: 2471
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
layout = Path("apps/mobile/app/(authenticated)/_layout.tsx").read_text()
relay = Path("apps/mobile/lib/host/client.ts").read_text()
chat = Path("apps/mobile/screens/(authenticated)/workspace/[id]/chat/[sessionId]/hooks/useChatThread/useChatThread.ts").read_text()
terminal = Path("apps/mobile/screens/(authenticated)/(home)/home/hooks/useHostTerminalAgents/useHostTerminalAgents.ts").read_text()
workspaces = Path("apps/mobile/hooks/useHostWorkspaces/useHostWorkspaces.ts").read_text()
checks = {
"priming starts in passive effect": bool(re.search(r"useEffect\s*\(\s*\(\)\s*=>\s*\{\s*void primeRelayUrl\(\)", layout, re.S)),
"relay override is module state without React notification": bool(re.search(r"let relayOverride: string \| null = null;", relay))
and "setRelayOverride" not in relay
and "useState" not in relay,
"relay getter falls back before override": bool(re.search(r"relayOverride \?\? env\.EXPO_PUBLIC_RELAY_URL", relay)),
"chat client created in render-time useMemo": bool(re.search(r"const client = useMemo\(\(\) => \{.*?createHostClient", chat, re.S)),
"chat client memo depends only on host identity": bool(re.search(r"\},\s*\[organizationId, hostId\]\);", chat)),
"terminal client created in render-time useMemo": bool(re.search(r"const client = useMemo\(\(\) => \{.*?createHostClient", terminal, re.S)),
"host-service URL built before query": bool(re.search(r"const hostUrl = .*?buildRelayHostUrl.*?const query = useQuery", workspaces, re.S)),
}
for name, result in checks.items():
print(f"{'PASS' if result else 'FAIL'}: {name}")
print("\nRelevant call-site facts:")
print("chat createHostClient precedes snapshot useQuery in source:",
chat.index("createHostClient") < chat.index("const snapshotQuery"))
print("host-service buildRelayHostUrl precedes useQuery in source:",
workspaces.index("buildRelayHostUrl") < workspaces.index("const query"))
print("primeRelayUrl call is not awaited by the effect:",
"void primeRelayUrl();" in layout)
PY
printf '%s\n' '--- direct consumers of the affected client constructors ---'
rg -n 'useChatThread|useHostTerminalAgents|useHostWorkspaces|createHostClient|getHostServiceClientByUrl' \
'apps/mobile/screens' 'apps/mobile/hooks' 'apps/mobile/lib'Repository: superset-sh/superset
Length of output: 11358
Wait for relay priming before creating host clients.
useEffect starts primeRelayUrl() after descendant hooks render. useChatThread and useHostTerminalAgents create clients in useMemo, while host-service hooks build URLs during render. These calls can use EXPO_PUBLIC_RELAY_URL.
relayOverride does not trigger a React render. A memoized createHostClient can therefore retain the fallback URL. Gate relay-dependent clients and queries on priming completion, or await a shared priming promise.
🤖 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/mobile/app/`(authenticated)/_layout.tsx around lines 31 - 34, Update the
authenticated layout’s relay initialization around primeRelayUrl so
relay-dependent descendant clients and host-service queries are not created or
evaluated until priming completes. Expose and propagate a completion state or
shared promise through the relevant hooks, including useChatThread,
useHostTerminalAgents, and createHostClient, and ensure memoized clients are
rebuilt after the resolved relay URL is available instead of retaining
EXPO_PUBLIC_RELAY_URL.
Comment on lines
+23
to
+31
| let relayOverride: string | null = null; | ||
|
|
||
| export async function primeRelayUrl(): Promise<void> { | ||
| try { | ||
| const endpoint = await apiClient.host.relayEndpoint.query(); | ||
| if (endpoint?.url) relayOverride = endpoint.url.replace(/\/$/, ""); | ||
| } catch { | ||
| // Keep the env fallback. | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'primeRelayUrl|relayOverride|signOut|logout|useSession' apps/mobileRepository: superset-sh/superset
Length of output: 24033
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- host client ---'
cat -n apps/mobile/lib/host/client.ts
printf '%s\n' '--- authentication client ---'
cat -n apps/mobile/lib/auth/client.ts
printf '%s\n' '--- authenticated layout ---'
cat -n 'apps/mobile/app/(authenticated)/_layout.tsx'
printf '%s\n' '--- sign-out hook ---'
cat -n apps/mobile/hooks/useSignOut/useSignOut.ts
printf '%s\n' '--- sign-in and session transition call sites ---'
rg -n -C 8 'signIn\(|primeRelayUrl|router\.(replace|push)|activeOrganizationId' apps/mobile --glob '*.{ts,tsx}'Repository: superset-sh/superset
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- sign-in implementations and call sites ---'
rg -n -l 'signIn' apps/mobile --glob '*.{ts,tsx}' | while IFS= read -r file; do
printf '\n--- %s ---\n' "$file"
rg -n -C 6 'signIn' "$file"
done
printf '%s\n' '--- auth route/session guards ---'
rg -n -C 8 'useSession|isPending|Redirect|authenticated|auth\)' apps/mobile/app apps/mobile/screens/RootLayout --glob '*.{ts,tsx}' \
| rg -v 'file-tree.stories|useSession\.ts' \
| head -n 240
printf '%s\n' '--- relay endpoint definitions and client transport usage ---'
rg -n -C 8 'relayEndpoint|relay-url-override|EXPO_PUBLIC_RELAY_URL|getRelayUrl' apps packages --glob '*.{ts,tsx,js}' --glob '!**/node_modules/**'Repository: superset-sh/superset
Length of output: 50376
Scope relayOverride to the authenticated session.
relayOverride persists across sign-out, while primeRelayUrl() runs once per authenticated-layout mount. If the next lookup fails or an older request resolves after sign-in, getRelayUrl() can route the new user's host requests through the previous user's relay. Clear the override on sign-out and on missing/error responses, or key it by the authenticated user. Ignore late responses from previous sessions.
🤖 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/mobile/lib/host/client.ts` around lines 23 - 31, Scope relayOverride to
the current authenticated session: update primeRelayUrl to clear it when the
lookup returns no URL or errors, and prevent late responses from an earlier
session from assigning a URL after sign-in. Also clear relayOverride during the
sign-out flow, preserving the environment fallback when no valid
session-specific override exists.
| target = await resolveAutomationTarget({ | ||
| organizationId, | ||
| userJwt: ctx.bearer, | ||
| api: ctx.api, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Resolve the target before calling setEnabled.
When --enabled is combined with --workspace or --project, setEnabled runs before resolveAutomationTarget. The target resolver can fail during workspace, project, or relay validation. The command can then exit with the enabled state changed but without applying the requested target update. Move the existing setEnabled block below target resolution. Use one server mutation if both changes must be atomic.
🤖 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 `@packages/cli/src/commands/automations/update/command.ts` at line 67, Move the
existing setEnabled block in the update command to execute only after
resolveAutomationTarget completes successfully, including workspace, project,
and relay validation. Preserve the current target-update behavior, and use a
single server mutation when enabled-state and target changes need to be applied
atomically.
Comment on lines
+112
to
+116
| const target = await resolveHostTarget({ | ||
| requestedHostId: hostId, | ||
| organizationId, | ||
| userJwt: ctx.bearer, | ||
| api: ctx.api, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate --name before resolving the host.
When --project is set without --name, the check at Line 119 runs only after this awaited resolver. Remote resolution performs an API-backed relay lookup. Invalid input can therefore make an unnecessary external request and can report a relay/API error instead of --name is required when --project is set. Move the name check before Line 112.
Suggested ordering
+ if (!isSession && !options.name) {
+ throw new CLIError("--name is required when --project is set");
+ }
+
const target = await resolveHostTarget({
requestedHostId: hostId,
organizationId,
userJwt: ctx.bearer,
api: ctx.api,
});
- if (!isSession && !options.name) {
- throw new CLIError("--name is required when --project is set");
- }📝 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.
Suggested change
| const target = await resolveHostTarget({ | |
| requestedHostId: hostId, | |
| organizationId, | |
| userJwt: ctx.bearer, | |
| api: ctx.api, | |
| if (!isSession && !options.name) { | |
| throw new CLIError("--name is required when --project is set"); | |
| } | |
| const target = await resolveHostTarget({ | |
| requestedHostId: hostId, | |
| organizationId, | |
| userJwt: ctx.bearer, | |
| api: ctx.api, | |
| }); |
🤖 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 `@packages/cli/src/commands/workspaces/create/command.ts` around lines 112 -
116, Move the --project/--name validation in the workspace creation command
before the await resolveHostTarget call, so missing --name is rejected before
any API-backed host resolution; preserve the existing validation message and
subsequent resolver behavior for valid inputs.
Comment on lines
+79
to
+86
| const relayUrl = await getRelayUrl(options.api); | ||
| return { | ||
| kind: "remote", | ||
| hostId: targetHostId, | ||
| client: createTRPCClient<HostServiceRouter>({ | ||
| links: [ | ||
| httpBatchLink({ | ||
| url: `${env.RELAY_URL}/hosts/${routingKey}/trpc`, | ||
| url: `${relayUrl}/hosts/${routingKey}/trpc`, |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 6 'relayEndpoint|timeout|httpBatchLink|fetch' \
packages/cli/src/lib packages/cli/src/commandsRepository: superset-sh/superset
Length of output: 22253
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- api-client.ts ---'
cat -n packages/cli/src/lib/api-client.ts
printf '%s\n' '--- relay-url.ts ---'
cat -n packages/cli/src/lib/host/relay-url.ts
printf '%s\n' '--- resolveHostTarget.ts ---'
cat -n packages/cli/src/lib/host-target/resolveHostTarget.ts
printf '%s\n' '--- package manifests and lockfile references ---'
rg -n -C 3 '"`@trpc/`(client|server)"|`@trpc/client`|httpBatchLink' \
package.json packages/*/package.json bun.lockb bun.lock 2>/dev/null || trueRepository: superset-sh/superset
Length of output: 18008
🌐 Web query:
@trpc/client 11.16.0 httpBatchLink timeout fetch option default timeout
💡 Result:
The @trpc/client package, including version 11.16.0, does not have a built-in default timeout for httpBatchLink [1][2][3]. Because tRPC relies on the underlying fetch implementation, and the standard Web Fetch API does not support timeouts by default, requests made via httpBatchLink will not automatically time out [4]. To implement a timeout, you can provide a custom fetch function to the httpBatchLink options that utilizes the AbortController API [4]. Example implementation: const fetchWithTimeout = async (url: RequestInfo, options: RequestInit, timeout = 10000) => { const controller = new AbortController; const id = setTimeout( => controller.abort, timeout); try { return await fetch(url, {...options, signal: controller.signal }); } finally { clearTimeout(id); } }; // Usage in tRPC client httpBatchLink({ url: 'https://your-api.com/trpc', fetch: fetchWithTimeout, }) [4]
Citations:
- 1: https://trpc.io/docs/client/links/httpBatchLink
- 2: https://github.com/trpc/trpc/blob/main/www/docs/client/links/httpBatchLink.md
- 3: https://github.com/trpc/trpc/blob/next/packages/client/src/links/httpBatchLink.ts
- 4: How to raise error when tRPC server is down. trpc/trpc#6774
Bound the relay endpoint lookup.
createApiClient provides no timeout or custom fetch to httpBatchLink. If api.host.relayEndpoint.query() stalls, getRelayUrl() never reaches its fallback and remote commands remain blocked. Add an AbortController timeout before creating the remote client.
🤖 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 `@packages/cli/src/lib/host-target/resolveHostTarget.ts` around lines 79 - 86,
Update the remote-client setup in resolveHostTarget to create an AbortController
with a bounded timeout before constructing httpBatchLink, and pass its signal
through the link’s fetch options so getRelayUrl and the relay endpoint lookup
cannot stall indefinitely. Preserve the existing fallback behavior and clear the
timeout when the request completes.
Contributor
There was a problem hiding this comment.
5 issues found across 26 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/mobile/lib/host/client.ts">
<violation number="1" location="apps/mobile/lib/host/client.ts:28">
P1: Signing out then signing in as another user can keep the prior account's relay when endpoint resolution fails, routing all host calls to the wrong relay; clear `relayOverride` on session change/sign-out before resolving the new endpoint.</violation>
<violation number="2" location="apps/mobile/lib/host/client.ts:29">
P2: Relay endpoint failures silently fall back with no diagnostic, making misrouted/unreachable hosts hard to investigate; log the rejected error with relay-resolution context before retaining fallback.</violation>
</file>
<file name="apps/mobile/app/(authenticated)/_layout.tsx">
<violation number="1" location="apps/mobile/app/(authenticated)/_layout.tsx:33">
P1: First host requests can still use default relay because children render before asynchronous relay resolution completes. Gate host-consuming children on relay readiness (or make relay resolution part of each request) so overridden-relay hosts never issue the initial request to the fallback.</violation>
</file>
<file name="packages/cli/src/commands/automations/update/command.ts">
<violation number="1" location="packages/cli/src/commands/automations/update/command.ts:67">
P2: resolveAutomationTarget now performs an async relay/API lookup (via the newly threaded api parameter). If setEnabled runs before this call and the target resolution subsequently fails validating the workspace, project, or relay, the automation's enabled state will already have changed even though the requested target update was never applied, leaving the command in a partially-applied state. Consider resolving the target before toggling enabled, or wrap both changes in a single mutation.</violation>
</file>
<file name="packages/cli/src/lib/host-target/resolveHostTarget.ts">
<violation number="1" location="packages/cli/src/lib/host-target/resolveHostTarget.ts:79">
P2: The relay endpoint lookup added here (`getRelayUrl(options.api)`) has no timeout or abort mechanism. If the underlying `api.host.relayEndpoint.query()` call stalls (e.g. network issue), the CLI command will hang indefinitely instead of falling back to the default relay URL. Consider adding a request timeout/AbortController around this lookup.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| export async function primeRelayUrl(): Promise<void> { | ||
| try { | ||
| const endpoint = await apiClient.host.relayEndpoint.query(); | ||
| if (endpoint?.url) relayOverride = endpoint.url.replace(/\/$/, ""); |
Contributor
There was a problem hiding this comment.
P1: Signing out then signing in as another user can keep the prior account's relay when endpoint resolution fails, routing all host calls to the wrong relay; clear relayOverride on session change/sign-out before resolving the new endpoint.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/lib/host/client.ts, line 28:
<comment>Signing out then signing in as another user can keep the prior account's relay when endpoint resolution fails, routing all host calls to the wrong relay; clear `relayOverride` on session change/sign-out before resolving the new endpoint.</comment>
<file context>
@@ -10,13 +10,29 @@ import type {
+export async function primeRelayUrl(): Promise<void> {
+ try {
+ const endpoint = await apiClient.host.relayEndpoint.query();
+ if (endpoint?.url) relayOverride = endpoint.url.replace(/\/$/, "");
+ } catch {
+ // Keep the env fallback.
</file context>
|
|
||
| // Resolve which relay this user's host is on, once inside the authed area. | ||
| useEffect(() => { | ||
| void primeRelayUrl(); |
Contributor
There was a problem hiding this comment.
P1: First host requests can still use default relay because children render before asynchronous relay resolution completes. Gate host-consuming children on relay readiness (or make relay resolution part of each request) so overridden-relay hosts never issue the initial request to the fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/app/(authenticated)/_layout.tsx, line 33:
<comment>First host requests can still use default relay because children render before asynchronous relay resolution completes. Gate host-consuming children on relay readiness (or make relay resolution part of each request) so overridden-relay hosts never issue the initial request to the fallback.</comment>
<file context>
@@ -26,6 +28,11 @@ const glassHeaderOptions = {
+ // Resolve which relay this user's host is on, once inside the authed area.
+ useEffect(() => {
+ void primeRelayUrl();
+ }, []);
+
</file context>
Comment on lines
+29
to
+31
| } catch { | ||
| // Keep the env fallback. | ||
| } |
Contributor
There was a problem hiding this comment.
P2: Relay endpoint failures silently fall back with no diagnostic, making misrouted/unreachable hosts hard to investigate; log the rejected error with relay-resolution context before retaining fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/lib/host/client.ts, line 29:
<comment>Relay endpoint failures silently fall back with no diagnostic, making misrouted/unreachable hosts hard to investigate; log the rejected error with relay-resolution context before retaining fallback.</comment>
<file context>
@@ -10,13 +10,29 @@ import type {
+ try {
+ const endpoint = await apiClient.host.relayEndpoint.query();
+ if (endpoint?.url) relayOverride = endpoint.url.replace(/\/$/, "");
+ } catch {
+ // Keep the env fallback.
+ }
</file context>
Suggested change
| } catch { | |
| // Keep the env fallback. | |
| } | |
| } catch (error) { | |
| console.warn( | |
| "[host/relay] Failed to resolve relay endpoint; using Expo fallback:", | |
| error, | |
| ); | |
| } |
| target = await resolveAutomationTarget({ | ||
| organizationId, | ||
| userJwt: ctx.bearer, | ||
| api: ctx.api, |
Contributor
There was a problem hiding this comment.
P2: resolveAutomationTarget now performs an async relay/API lookup (via the newly threaded api parameter). If setEnabled runs before this call and the target resolution subsequently fails validating the workspace, project, or relay, the automation's enabled state will already have changed even though the requested target update was never applied, leaving the command in a partially-applied state. Consider resolving the target before toggling enabled, or wrap both changes in a single mutation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/commands/automations/update/command.ts, line 67:
<comment>resolveAutomationTarget now performs an async relay/API lookup (via the newly threaded api parameter). If setEnabled runs before this call and the target resolution subsequently fails validating the workspace, project, or relay, the automation's enabled state will already have changed even though the requested target update was never applied, leaving the command in a partially-applied state. Consider resolving the target before toggling enabled, or wrap both changes in a single mutation.</comment>
<file context>
@@ -64,6 +64,7 @@ export default command({
target = await resolveAutomationTarget({
organizationId,
userJwt: ctx.bearer,
+ api: ctx.api,
hostId: options.host ?? undefined,
workspaceId: options.workspace ?? undefined,
</file context>
| } | ||
|
|
||
| const routingKey = buildHostRoutingKey(options.organizationId, targetHostId); | ||
| const relayUrl = await getRelayUrl(options.api); |
Contributor
There was a problem hiding this comment.
P2: The relay endpoint lookup added here (getRelayUrl(options.api)) has no timeout or abort mechanism. If the underlying api.host.relayEndpoint.query() call stalls (e.g. network issue), the CLI command will hang indefinitely instead of falling back to the default relay URL. Consider adding a request timeout/AbortController around this lookup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/lib/host-target/resolveHostTarget.ts, line 79:
<comment>The relay endpoint lookup added here (`getRelayUrl(options.api)`) has no timeout or abort mechanism. If the underlying `api.host.relayEndpoint.query()` call stalls (e.g. network issue), the CLI command will hang indefinitely instead of falling back to the default relay URL. Consider adding a request timeout/AbortController around this lookup.</comment>
<file context>
@@ -73,13 +76,14 @@ export function resolveHostTarget(
}
const routingKey = buildHostRoutingKey(options.organizationId, targetHostId);
+ const relayUrl = await getRelayUrl(options.api);
return {
kind: "remote",
</file context>
Match the layout's useDevicePresence idiom instead of an inline fire-and-forget useEffect; also gate the prime on an established session. Claude-Session: https://claude.ai/code/session_01A2JifsfA63nWthegPwNt8G
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/mobile/hooks/usePrimeRelayUrl/usePrimeRelayUrl.ts`:
- Around line 13-15: Update the useEffect around primeRelayUrl so it clears the
module-level relayOverride when session is null and associates each in-flight
request with the session that started it. Commit the resolved relay URL only if
the effect is still active and that session remains current, preventing stale
responses from overwriting the latest session’s value.
🪄 Autofix
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: 7fdd7c3c-7c39-46fe-a2ff-e4192f2f17fc
📒 Files selected for processing (3)
apps/mobile/app/(authenticated)/_layout.tsxapps/mobile/hooks/usePrimeRelayUrl/index.tsapps/mobile/hooks/usePrimeRelayUrl/usePrimeRelayUrl.ts
Comment on lines
+13
to
+15
| useEffect(() => { | ||
| if (session) void primeRelayUrl(); | ||
| }, [session]); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make relay priming session-scoped and latest-wins.
When session changes while primeRelayUrl() is in flight, this effect starts another request without cancellation or session identity. A late response from the previous session can overwrite the module-level relayOverride. When the session becomes null, the old override also remains cached. The next user can then build host URLs with the previous user's relay. Clear the cache on sign-out and commit only the result for the active session.
🤖 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/mobile/hooks/usePrimeRelayUrl/usePrimeRelayUrl.ts` around lines 13 - 15,
Update the useEffect around primeRelayUrl so it clears the module-level
relayOverride when session is null and associates each in-flight request with
the session that started it. Commit the resolved relay URL only if the effect is
still active and that session remains current, preventing stale responses from
overwriting the latest session’s value.
…tegy (D-20/D-21) Claude-Session: https://claude.ai/code/session_01A2JifsfA63nWthegPwNt8G
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
plans/20260804-1815-relay2-durable-objects-canonical.md (1)
44-44: 🩺 Stability & Availability | 🔵 TrivialVerify readiness after deployment.
Removing the fixed-window health gate avoids transient failures, but the replacement described at Line 44 checks only deployment completion. Add bounded polling with a timeout, or document a separate promotion check that verifies
/healthreturnsproto: 2before rollout.🤖 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 `@plans/20260804-1815-relay2-durable-objects-canonical.md` at line 44, Update the Milestone 1 CI deploy entry to include a bounded post-deployment readiness check: poll the relay2 `/health` endpoint until it reports `proto: 2`, with a defined timeout and failure behavior. Alternatively, document a separate promotion check that performs this verification before rollout, while preserving tolerance for the initial propagation delay.
🤖 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 `@plans/20260804-1815-relay2-durable-objects-canonical.md`:
- Line 46: Update the Milestone 4 description to replace the hyphenated term
“identify-race” with “identity race,” leaving the surrounding wording unchanged.
- Around line 112-116: Update Decision D-21 to state the actual shared resolver
contract: paths reaching a user’s host must use the shared or API-served
resolver, such as resolveUserRelayUrl or relayEndpoint, which applies the relay
override and falls back to env.RELAY_URL. Remove the blanket requirement that
every path read the flag directly, while preserving the listed affected paths
and hostname-cutover rationale.
- Around line 112-116: Update Milestone 5’s hostname-cutover procedure to follow
D-20’s wait-for-drain sequence: leave Fly serving relay.superset.sh, allow
pre-v2 clients to drain through auto-update until v1 traffic is near zero, then
repoint the hostname to relay2. Remove the instruction to mount apps/relay-do
for compatibility, and retain that compatibility layer only as an explicitly
approved fallback if Fly must be retired sooner.
- Line 33: Expand the D-16 account-rename item in the relay migration plan to
document pre-rename checks for the API-served endpoint, desktop and web CSP
allowlists, and client fallback behavior using the new hostname. Retain the
existing cutover rollback checks.
---
Nitpick comments:
In `@plans/20260804-1815-relay2-durable-objects-canonical.md`:
- Line 44: Update the Milestone 1 CI deploy entry to include a bounded
post-deployment readiness check: poll the relay2 `/health` endpoint until it
reports `proto: 2`, with a defined timeout and failure behavior. Alternatively,
document a separate promotion check that performs this verification before
rollout, while preserving tolerance for the initial propagation delay.
🪄 Autofix
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: 95781058-c0a2-4ad4-b5b6-7136159ff369
📒 Files selected for processing (1)
plans/20260804-1815-relay2-durable-objects-canonical.md
| ## Open Questions | ||
|
|
||
| - Client-side predictive echo (mosh-style local echo in the terminal renderer) is the biggest lever for perceived typing latency and is intentionally **out of scope** here; does it get its own plan immediately after, or wait for relay2 to land? → Decision Log placeholder D-13. | ||
| - Cloudflare account rename off `avi-6ac` to a Superset-branded account (D-16) before `relay.superset.sh` and more CSPs bake the workers.dev host in — still not done; do it before the M5 cutover. |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f -e ts -e tsx -e js -e json -e md -e yml -e yaml . \
| xargs -r rg -n -C 2 \
'superset-relay2\.avi-6ac\.workers\.dev|workers\.dev|Content-Security-Policy|relayEndpoint'Repository: superset-sh/superset
Length of output: 23604
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- plan decisions and milestones ---'
sed -n '112,125p;200,220p' plans/20260804-1815-relay2-durable-objects-canonical.md
printf '%s\n' '--- resolver and fallback implementations ---'
sed -n '1,70p' packages/host-service/src/tunnel/connect.ts
sed -n '1,45p' packages/cli/src/lib/host/relay-url.ts
sed -n '1,45p' apps/mobile/lib/host/client.ts
sed -n '1,80p' packages/trpc/src/router/host/host.ts
printf '%s\n' '--- CSP declarations ---'
sed -n '40,70p' apps/web/next.config.ts
rg -n -C 3 'superset-relay2\.avi-6ac\.workers\.dev|relay\.superset\.sh|Content-Security-Policy' apps/desktop packages apps/web apps/mobile --glob '!**/node_modules/**' || true
printf '%s\n' '--- deterministic plan consistency check ---'
python3 - <<'PY'
from pathlib import Path
p = Path("plans/20260804-1815-relay2-durable-objects-canonical.md")
text = p.read_text()
checks = {
"D-16 requires rename before Milestone 4 CSP entry": "do this before Milestone 4's CSP entry" in text,
"Milestone 4 is marked shipped": "- [x] (2026-08-10) Milestone 4 SHIPPED" in text,
"D-16 is still listed as not done": "still not done; do it before the M5 cutover" in text,
"desktop CSP contains old workers.dev hostname": "https://superset-relay2.avi-6ac.workers.dev" in text,
"Milestone 5 has a CNAME rollback": "repoint the CNAME back" in text,
"Milestone 5 checks stale desktop build": "stale desktop build (pre-CSP-change) works" in text,
"Milestone 5 checks both protocols": "e2e probe against `https://relay.superset.sh` in both protocols" in text,
}
for label, result in checks.items():
print(f"{label}: {result}")
PYRepository: superset-sh/superset
Length of output: 27992
Document the account-rename migration checks.
D-16 remains unresolved, although Milestone 4 already shipped CSP entries for superset-relay2.avi-6ac.workers.dev. Before the rename, record checks for the API-served endpoint, desktop and web CSP allowlists, and client fallback behavior against the new hostname. Keep the existing cutover rollback checks.
🤖 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 `@plans/20260804-1815-relay2-durable-objects-canonical.md` at line 33, Expand
the D-16 account-rename item in the relay migration plan to document pre-rename
checks for the API-served endpoint, desktop and web CSP allowlists, and client
fallback behavior using the new hostname. Retain the existing cutover rollback
checks.
| - [ ] Milestone 5: rollout, decommission `apps/relay` + Fly fleet + Upstash, rename `relay2` → `relay`. | ||
| - [x] (2026-08-05) Milestone 1 CI deploy: `deploy-relay2` job added to `deploy-production.yml` (mirrors electric-proxy; same CLOUDFLARE secrets). relay2 auto-deploys on every push to main. The loud health-gate variant was dropped — a fresh Worker serves 500s for ~1–2 min while propagating, so a fixed-window gate would flake; `wrangler deploy` already fails the job loudly on a bad upload. | ||
| - [x] (2026-08-10) Milestone 3 RESOLVED as not-needed for terminals: the terminal already has app-layer resumable catch-up (`host-service/src/terminal/terminal.ts` retained ring + `?seq=<epoch>:<n>` anchor; renderer `terminal-ws-transport.ts` tracks the anchor and reconnects with it). It is transport-agnostic, so it resumes gap-free through relay2's byte splice — verified by a live `wrangler deploy` mid-session: host reconnected on v2 in ~15s, zero `session-gone`. A relay-layer DO buffer (original D-7) would duplicate this; not building it. The event bus is the only stream without app-layer resume, but a dropped event just triggers a refetch — not worth a DO buffer. | ||
| - [x] (2026-08-10) Milestone 4 SHIPPED: v2 client + API-served relay config in stable desktop 1.20.2. `host.relayEndpoint` (jwtProcedure) is the single server-side resolver — the host-service asks for it after authenticating, killing the identify-race that stranded hosts on the wrong relay. Desktop renderer/main, CLI, web, and mobile all read it. Root cause of the rollout pain reproduced under controlled conditions (old code took the fallback relay, new code resolved the flag). Relay-consumer gap sweep merged: automation dispatch (#6337), trpc runNow + shared `resolveUserRelayUrl` + CLI resolveHostTarget + web CSP + mobile (#6340). Web CSP allowlists the relay2 origin. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace identify-race with identity race.
The sentence describes a race in host identity selection. identity race is the correct term.
🧰 Tools
🪛 LanguageTool
[grammar] ~46-~46: Ensure spelling is correct
Context: ...or it after authenticating, killing the identify-race that stranded hosts on the wrong re...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 `@plans/20260804-1815-relay2-durable-objects-canonical.md` at line 46, Update
the Milestone 4 description to replace the hyphenated term “identify-race” with
“identity race,” leaving the surrounding wording unchanged.
Source: Linters/SAST tools
Contributor
There was a problem hiding this comment.
3 issues found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/mobile/app/(authenticated)/_layout.tsx">
<violation number="1" location="apps/mobile/app/(authenticated)/_layout.tsx:29">
P3: Minor: dependency on the whole session object can re-run primeRelayUrl() whenever the session refetches and produces a new reference, re-issuing the relayEndpoint.query() network call even though the relay should only be resolved once after sign-in. Consider depending on session existence (e.g. const isAuthed = Boolean(session); useEffect(..., [isAuthed])) so priming fires exactly once when authenticated.</violation>
</file>
<file name="plans/20260804-1815-relay2-durable-objects-canonical.md">
<violation number="1" location="plans/20260804-1815-relay2-durable-objects-canonical.md:48">
P3: The plan contradicts itself about the cutover strategy: the Milestone 5 progress line and revision note call it a 'wait-for-drain' cutover, while the D-20 decision it points to says the opposite — 'accept-the-churn' and 'we do not need to carefully wait for full drain.' At cutover time these are opposite operational instructions (wait vs. don't wait). Align the labels with D-20's actual guidance, e.g. 'accept-the-churn' instead of 'wait-for-drain', so the milestone framing doesn't mislead the operator.</violation>
<violation number="2" location="plans/20260804-1815-relay2-durable-objects-canonical.md:112">
P2: D-20 now declares the v1 compatibility layer will 'never' be built and supersedes D-12, but the Milestone 5 Plan of Work section still directs an operator to 'mount the v1-protocol compatibility layer' (D-12) at cutover, and the 2026-08-04 revision note still says the prototype is repurposed as that layer. An executor following the un-updated Milestone 5 steps would rebuild code the new decision explicitly cancels. Reconcile the Milestone 5 section (drop the compat-layer mount and D-12 references) with the D-20 decision so the operational instructions match the shipped intent.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| Date/Author: 2026-08-05 / Claude session, reviewed direction from Satya's "no debt" push. | ||
| - Decision (D-19): v1-era control messages (`drain`, `hello`) are removed from tunnel v2. DO restarts drop sockets and partysocket reconnects on ~1s backoff — deploy drain choreography was a Fly-ism; version telemetry can ride the control URL later if ever needed. `apps/relay-do` deleted from the tree (git history + PR #6165 retain it; the M5 cutover compat layer will be rebuilt from history if pursued). | ||
| Date/Author: 2026-08-05 / Satya + Claude session. | ||
| - Decision (D-20): No v1 compatibility layer in relay2 — ever. The M5 all-users cutover is **ops-only, accept-the-churn**: rename the CF account (D-16), set up the Cloudflare-for-SaaS custom hostname (D-14), repoint `relay.superset.sh` → relay2, and delete the `relay-url-override` flag so the default relay IS relay2. No code. Satya (2026-08-10): relay usage is low enough that minor churn at cutover is fine — we do not need to carefully wait for full drain. The bounded, self-healing churn is: hosts still on a pre-v2 build (≤1.20.1) were pointed at `relay.superset.sh` (Fly) and now hit relay2, which they can't speak, so they show offline until they auto-update to ≥1.20.2. v2 hosts just reconnect from the workers.dev URL to `relay.superset.sh` (same protocol). Supersedes D-12 (compat layer) entirely; the ~350-line mux-in-DO port from `apps/relay/src/tunnel.ts` is never built. |
Contributor
There was a problem hiding this comment.
P2: D-20 now declares the v1 compatibility layer will 'never' be built and supersedes D-12, but the Milestone 5 Plan of Work section still directs an operator to 'mount the v1-protocol compatibility layer' (D-12) at cutover, and the 2026-08-04 revision note still says the prototype is repurposed as that layer. An executor following the un-updated Milestone 5 steps would rebuild code the new decision explicitly cancels. Reconcile the Milestone 5 section (drop the compat-layer mount and D-12 references) with the D-20 decision so the operational instructions match the shipped intent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plans/20260804-1815-relay2-durable-objects-canonical.md, line 112:
<comment>D-20 now declares the v1 compatibility layer will 'never' be built and supersedes D-12, but the Milestone 5 Plan of Work section still directs an operator to 'mount the v1-protocol compatibility layer' (D-12) at cutover, and the 2026-08-04 revision note still says the prototype is repurposed as that layer. An executor following the un-updated Milestone 5 steps would rebuild code the new decision explicitly cancels. Reconcile the Milestone 5 section (drop the compat-layer mount and D-12 references) with the D-20 decision so the operational instructions match the shipped intent.</comment>
<file context>
@@ -106,6 +109,11 @@ A working prototype already validated the platform (PR #6165, `apps/relay-do`):
Date/Author: 2026-08-05 / Claude session, reviewed direction from Satya's "no debt" push.
- Decision (D-19): v1-era control messages (`drain`, `hello`) are removed from tunnel v2. DO restarts drop sockets and partysocket reconnects on ~1s backoff — deploy drain choreography was a Fly-ism; version telemetry can ride the control URL later if ever needed. `apps/relay-do` deleted from the tree (git history + PR #6165 retain it; the M5 cutover compat layer will be rebuilt from history if pursued).
Date/Author: 2026-08-05 / Satya + Claude session.
+- Decision (D-20): No v1 compatibility layer in relay2 — ever. The M5 all-users cutover is **ops-only, accept-the-churn**: rename the CF account (D-16), set up the Cloudflare-for-SaaS custom hostname (D-14), repoint `relay.superset.sh` → relay2, and delete the `relay-url-override` flag so the default relay IS relay2. No code. Satya (2026-08-10): relay usage is low enough that minor churn at cutover is fine — we do not need to carefully wait for full drain. The bounded, self-healing churn is: hosts still on a pre-v2 build (≤1.20.1) were pointed at `relay.superset.sh` (Fly) and now hit relay2, which they can't speak, so they show offline until they auto-update to ≥1.20.2. v2 hosts just reconnect from the workers.dev URL to `relay.superset.sh` (same protocol). Supersedes D-12 (compat layer) entirely; the ~350-line mux-in-DO port from `apps/relay/src/tunnel.ts` is never built.
+ Rationale: 1.20.2 stable already speaks v2 and auto-update carries everyone there; the residual breakage is a handful of stale hosts self-healing on update, against low relay usage. A delicate DO state machine to prevent that is the wrong trade.
+ Date/Author: 2026-08-10 / Satya + Claude session.
</file context>
|
|
||
| export default function AuthenticatedLayout() { | ||
| useDevicePresence(); | ||
| usePrimeRelayUrl(); |
Contributor
There was a problem hiding this comment.
P3: Minor: dependency on the whole session object can re-run primeRelayUrl() whenever the session refetches and produces a new reference, re-issuing the relayEndpoint.query() network call even though the relay should only be resolved once after sign-in. Consider depending on session existence (e.g. const isAuthed = Boolean(session); useEffect(..., [isAuthed])) so priming fires exactly once when authenticated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/app/(authenticated)/_layout.tsx, line 29:
<comment>Minor: dependency on the whole session object can re-run primeRelayUrl() whenever the session refetches and produces a new reference, re-issuing the relayEndpoint.query() network call even though the relay should only be resolved once after sign-in. Consider depending on session existence (e.g. const isAuthed = Boolean(session); useEffect(..., [isAuthed])) so priming fires exactly once when authenticated.</comment>
<file context>
@@ -27,11 +26,7 @@ const glassHeaderOptions = {
- useEffect(() => {
- void primeRelayUrl();
- }, []);
+ usePrimeRelayUrl();
return (
</file context>
| - [x] (2026-08-10) Milestone 3 RESOLVED as not-needed for terminals: the terminal already has app-layer resumable catch-up (`host-service/src/terminal/terminal.ts` retained ring + `?seq=<epoch>:<n>` anchor; renderer `terminal-ws-transport.ts` tracks the anchor and reconnects with it). It is transport-agnostic, so it resumes gap-free through relay2's byte splice — verified by a live `wrangler deploy` mid-session: host reconnected on v2 in ~15s, zero `session-gone`. A relay-layer DO buffer (original D-7) would duplicate this; not building it. The event bus is the only stream without app-layer resume, but a dropped event just triggers a refetch — not worth a DO buffer. | ||
| - [x] (2026-08-10) Milestone 4 SHIPPED: v2 client + API-served relay config in stable desktop 1.20.2. `host.relayEndpoint` (jwtProcedure) is the single server-side resolver — the host-service asks for it after authenticating, killing the identify-race that stranded hosts on the wrong relay. Desktop renderer/main, CLI, web, and mobile all read it. Root cause of the rollout pain reproduced under controlled conditions (old code took the fallback relay, new code resolved the flag). Relay-consumer gap sweep merged: automation dispatch (#6337), trpc runNow + shared `resolveUserRelayUrl` + CLI resolveHostTarget + web CSP + mobile (#6340). Web CSP allowlists the relay2 origin. | ||
| - [ ] Milestone 4 remaining: internal team fully ported (flag = `@superset.sh`; Satya on relay2 and verified via live traffic; Kiet/Avi move on their next canary restart). Web CSP allowlist and all the per-consumer flag plumbing are cutover-temporary (see D-20). | ||
| - [ ] Milestone 5: rollout to all users + decommission `apps/relay` + Fly fleet + Upstash + rename `relay2` → `relay`. Strategy is the hostname cutover with **wait-for-drain**, not a v1 compat layer (D-20). |
Contributor
There was a problem hiding this comment.
P3: The plan contradicts itself about the cutover strategy: the Milestone 5 progress line and revision note call it a 'wait-for-drain' cutover, while the D-20 decision it points to says the opposite — 'accept-the-churn' and 'we do not need to carefully wait for full drain.' At cutover time these are opposite operational instructions (wait vs. don't wait). Align the labels with D-20's actual guidance, e.g. 'accept-the-churn' instead of 'wait-for-drain', so the milestone framing doesn't mislead the operator.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plans/20260804-1815-relay2-durable-objects-canonical.md, line 48:
<comment>The plan contradicts itself about the cutover strategy: the Milestone 5 progress line and revision note call it a 'wait-for-drain' cutover, while the D-20 decision it points to says the opposite — 'accept-the-churn' and 'we do not need to carefully wait for full drain.' At cutover time these are opposite operational instructions (wait vs. don't wait). Align the labels with D-20's actual guidance, e.g. 'accept-the-churn' instead of 'wait-for-drain', so the milestone framing doesn't mislead the operator.</comment>
<file context>
@@ -40,9 +41,11 @@ A working prototype already validated the platform (PR #6165, `apps/relay-do`):
+- [x] (2026-08-10) Milestone 3 RESOLVED as not-needed for terminals: the terminal already has app-layer resumable catch-up (`host-service/src/terminal/terminal.ts` retained ring + `?seq=<epoch>:<n>` anchor; renderer `terminal-ws-transport.ts` tracks the anchor and reconnects with it). It is transport-agnostic, so it resumes gap-free through relay2's byte splice — verified by a live `wrangler deploy` mid-session: host reconnected on v2 in ~15s, zero `session-gone`. A relay-layer DO buffer (original D-7) would duplicate this; not building it. The event bus is the only stream without app-layer resume, but a dropped event just triggers a refetch — not worth a DO buffer.
+- [x] (2026-08-10) Milestone 4 SHIPPED: v2 client + API-served relay config in stable desktop 1.20.2. `host.relayEndpoint` (jwtProcedure) is the single server-side resolver — the host-service asks for it after authenticating, killing the identify-race that stranded hosts on the wrong relay. Desktop renderer/main, CLI, web, and mobile all read it. Root cause of the rollout pain reproduced under controlled conditions (old code took the fallback relay, new code resolved the flag). Relay-consumer gap sweep merged: automation dispatch (#6337), trpc runNow + shared `resolveUserRelayUrl` + CLI resolveHostTarget + web CSP + mobile (#6340). Web CSP allowlists the relay2 origin.
+- [ ] Milestone 4 remaining: internal team fully ported (flag = `@superset.sh`; Satya on relay2 and verified via live traffic; Kiet/Avi move on their next canary restart). Web CSP allowlist and all the per-consumer flag plumbing are cutover-temporary (see D-20).
+- [ ] Milestone 5: rollout to all users + decommission `apps/relay` + Fly fleet + Upstash + rename `relay2` → `relay`. Strategy is the hostname cutover with **wait-for-drain**, not a v1 compat layer (D-20).
## Surprises & Discoveries
</file context>
Suggested change
| - [ ] Milestone 5: rollout to all users + decommission `apps/relay` + Fly fleet + Upstash + rename `relay2` → `relay`. Strategy is the hostname cutover with **wait-for-drain**, not a v1 compat layer (D-20). | |
| - [ ] Milestone 5: rollout to all users + decommission `apps/relay` + Fly fleet + Upstash + rename `relay2` → `relay`. Strategy is the hostname cutover with **accept-the-churn**, not a v1 compat layer (D-20). |
This was referenced Aug 12, 2026
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes every relay-resolution gap found in the audit — the class of bug behind "automations can't reach my machine": when a host moves to an overridden relay (
relay-url-override), every path that reaches it must resolve the same flag, or it silently dials the default relay and fails.Server (tRPC)
resolveUserRelayUrl(userId); routehost.relayEndpoint,automation.runNow, and scheduled dispatch through it.runNow(the manual "Run now" button) had the same hardcoded-env.RELAY_URLbug as scheduled dispatch (fixed separately in fix(api): resolve owner's relay when dispatching automations #6337).CLI
resolveHostTargetbuilt remote-host URLs fromenv.RELAY_URL, so every command targeting a remote host (workspaces/terminals/projects/agents --host X) missed a host on an overridden relay. Made it async, threaded the api client through all ~17 call sites, and resolvegetRelayUrlin the remote branch only (local targets skip it).Web
connect-src. The runtime relay comes from the flag while the header is built at compile time, so a flag-pointed relay was blocked — same fix already applied to the desktop CSP.Mobile
host.relayEndpointresolved once after sign-in and cached; the URL builders read the cache with the Expo env as fallback. (iOS-only, not actively used, but no longer silently pinned to the default relay.)Note
All of these become unnecessary once relay2 answers on
relay.superset.sh(the hostname cutover), since everyenv.RELAY_URLbecomes correct automatically. Until then, a host on relay2 has to be reachable from every client — which is what this PR ensures.Verified: full typecheck clean across trpc/cli/web/mobile (excluding 3 pre-existing cross-package stale-build errors unrelated to this change),
bun run lintexits 0.Summary by CodeRabbit