feat(cloud): Add managed relay tunnels and APN service#2837
feat(cloud): Add managed relay tunnels and APN service#2837juliusmarminge wants to merge 8 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
554a1d2 to
8480c92
Compare
65c36c0 to
a7ed828
Compare
8480c92 to
e3ab348
Compare
a7ed828 to
b868fee
Compare
e3ab348 to
436b1b9
Compare
b868fee to
589e2ed
Compare
436b1b9 to
d20a8ce
Compare
63a525d to
8027af0
Compare
6c0e54d to
f15e2ba
Compare
8027af0 to
1a912f6
Compare
f15e2ba to
71e0186
Compare
1a912f6 to
90bf2b3
Compare
71e0186 to
e721336
Compare
e63e3f4 to
ba9802d
Compare
22e103a to
60b7d8d
Compare
ba9802d to
8789910
Compare
60b7d8d to
ee4ec05
Compare
8789910 to
f7ac694
Compare
c06b655 to
4c50731
Compare
4c50731 to
7deb0c4
Compare
bf992b4 to
d374900
Compare
a668bac to
a42a3f5
Compare
597e56d to
f9c9f4d
Compare
a42a3f5 to
2627b33
Compare
There was a problem hiding this comment.
🟡 Medium
The cache invalidation logic at line 290 only checks expectedMetadata, which omits the environment variables embedded by writeDevelopmentLauncherScript. When the metadata matches, the function returns early at line 293 without regenerating the launcher script, even if VITE_DEV_SERVER_URL or T3CODE_PORT has changed. This causes the app to use stale environment variables from a previous run — for example, attempting to connect to a Vite dev server port that is no longer running.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/desktop/scripts/electron-launcher.mjs around line 277:
The cache invalidation logic at line 290 only checks `expectedMetadata`, which omits the environment variables embedded by `writeDevelopmentLauncherScript`. When the metadata matches, the function returns early at line 293 without regenerating the launcher script, even if `VITE_DEV_SERVER_URL` or `T3CODE_PORT` has changed. This causes the app to use stale environment variables from a previous run — for example, attempting to connect to a Vite dev server port that is no longer running.
Evidence trail:
apps/desktop/scripts/electron-launcher.mjs lines 277-294 (expectedMetadata definition and early return), lines 97-130 (writeDevelopmentLauncherScript embedding env vars into the launcher script), lines 300-302 (writeDevelopmentLauncherScript only called when cache is invalidated)
Co-authored-by: codex <codex@users.noreply.github.com>
| const existingDnsRecordId = yield* HttpClientRequest.make("GET")( | ||
| "/zones/${cf.zoneId}/dns_records", | ||
| ).pipe( |
There was a problem hiding this comment.
🟠 High environments/ManagedEndpointProvider.ts:254
The URL at line 255 uses double quotes with ${cf.zoneId}, so the literal string /zones/${cf.zoneId}/dns_records is sent to the API instead of the actual zone ID. Line 269-270 correctly use backticks for template literals. Consider changing to backticks to interpolate the zone ID.
- const existingDnsRecordId = yield* HttpClientRequest.make("GET")(
- "/zones/${cf.zoneId}/dns_records",
+ const existingDnsRecordId = yield* HttpClientRequest.make("GET")(
+ `/zones/${cf.zoneId}/dns_records`,🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file infra/relay/src/environments/ManagedEndpointProvider.ts around lines 254-256:
The URL at line 255 uses double quotes with `${cf.zoneId}`, so the literal string `/zones/${cf.zoneId}/dns_records` is sent to the API instead of the actual zone ID. Line 269-270 correctly use backticks for template literals. Consider changing to backticks to interpolate the zone ID.
Evidence trail:
infra/relay/src/environments/ManagedEndpointProvider.ts lines 254-270 at REVIEWED_COMMIT: line 255 uses double quotes ("/zones/${cf.zoneId}/dns_records") while lines 269-270 use backticks (`/zones/${cf.zoneId}/dns_records/${id}` and `/zones/${cf.zoneId}/dns_records`).
| Effect.provide(runtimeLayer), | ||
| ), |
There was a problem hiding this comment.
🟡 Medium src/worker.ts:249
The queue message handler and cron handler create spans using Stream.withSpan and Effect.withSpan, but they only provide runtimeLayer which does not include the tracer. This means spans from background jobs are silently dropped instead of being exported to Axiom, despite the explicit instrumentation. Consider providing relayTraceLayer to these handlers, similar to the HTTP fetch handler at line 270.
Effect.provide(runtimeLayer),
+ Effect.provide(relayTraceLayer),🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file infra/relay/src/worker.ts around lines 249-250:
The queue message handler and cron handler create spans using `Stream.withSpan` and `Effect.withSpan`, but they only provide `runtimeLayer` which does not include the tracer. This means spans from background jobs are silently dropped instead of being exported to Axiom, despite the explicit instrumentation. Consider providing `relayTraceLayer` to these handlers, similar to the HTTP fetch handler at line 270.
Evidence trail:
infra/relay/src/worker.ts lines 170-176 (relayTraceLayer definition with OtlpTracer), lines 178-213 (runtimeLayer definition without tracer), line 242 (Stream.withSpan in queue handler), line 246 (Effect.withSpan in queue handler), line 249 (Effect.provide(runtimeLayer) - no tracer), line 256 (Effect.withSpan in cron handler), line 257 (Effect.provide(runtimeLayer) - no tracer), line 270 (HTTP handler uses relayTraceLayer). infra/relay/src/observability.ts lines 53-72 (makeRelayTraceLayer creates OtlpTracer.layer for Axiom export).
| managedEndpointBaseDomain: yield* managedEndpointZoneId, | ||
| cloudflareZoneId: yield* managedEndpointZoneId, |
There was a problem hiding this comment.
🟠 High src/worker.ts:162
Both managedEndpointBaseDomain and cloudflareZoneId are set to managedEndpointZoneId (the zone ID), but managedEndpointBaseDomain semantically requires the domain name (e.g., "ineededadomain.com"). The ManagedEndpointZone provides zoneName which contains the correct value, but it is never extracted or used. This will cause managed endpoint domain construction to use an invalid zone ID string instead of the actual domain name.
- managedEndpointBaseDomain: yield* managedEndpointZoneId,
+ managedEndpointBaseDomain: yield* managedEndpointZone.zoneName,🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file infra/relay/src/worker.ts around lines 162-163:
Both `managedEndpointBaseDomain` and `cloudflareZoneId` are set to `managedEndpointZoneId` (the zone ID), but `managedEndpointBaseDomain` semantically requires the domain name (e.g., "ineededadomain.com"). The `ManagedEndpointZone` provides `zoneName` which contains the correct value, but it is never extracted or used. This will cause managed endpoint domain construction to use an invalid zone ID string instead of the actual domain name.
Evidence trail:
infra/relay/src/managedEndpointStack.ts:9-29 (ManagedEndpointZone returns zoneId, zoneName, dnsToken; zoneName = zone.name = 'ineededadomain.com')
infra/relay/src/worker.ts:140 (only zoneId extracted, zoneName ignored)
infra/relay/src/worker.ts:162-163 (both managedEndpointBaseDomain and cloudflareZoneId assigned from managedEndpointZoneId)
infra/relay/src/environments/ManagedEndpointProvider.ts:131 (baseDomain: settings.managedEndpointBaseDomain)
infra/relay/src/environments/ManagedEndpointProvider.ts:149 (baseDomain used in hostname construction: `...${baseDomain}`)
Stack
codex/collection-performance-refactors->main(merged)t3code/mobile-remote-connect->maincodex/relay-managed-tunnels-auth-infra->t3code/mobile-remote-connectSummary
This stacked draft PR adds the relay-managed tunnel and cloud authentication work on top of the mobile remote-runtime PR. General collection/performance rewrites from #2854 and the TypeScript/Effect tooling base are now on
main.effect/Cryptowhile retaining Expo-compatible implementationsValidation
bun fmtbun lint(passes with 8 existing web warnings)bun lint:mobilebun typecheckcd infra/relay && bun run test(103passed,5skipped)cd apps/mobile && bun run test(135passed)cd apps/web && bun run test(1005passed)cd apps/server && bun run test(1075passed,4skipped)Rebase Note
General collection/performance rewrites from #2854 are now merged into
main; mobile command metadata, pairing-URL redaction, and shared-runtime Crypto cleanup remain in #2013. This PR retains the managed-relay changes to the mobile connection contract and runtime above those inherited lower-layer changes.