[codex] Extract infrastructure, telemetry, and test tooling#2994
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
|
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 |
|
🚀 Expo continuous deployment is ready!
|
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
ced8bdf to
22b4b24
Compare
Co-authored-by: codex <codex@users.noreply.github.com>
ApprovabilityVerdict: Needs human review This PR introduces new mobile telemetry/tracing infrastructure rather than being a pure extraction. An unresolved review comment raises concerns about the default tracesUrl fallback potentially enabling tracing unintentionally. New observability capabilities and infrastructure changes warrant human review. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Default trace URL enables tracing
- Changed the tracesUrl fallback in app.config.ts from a hardcoded Axiom URL to null, so omitting EXPO_PUBLIC_OTLP_TRACES_URL no longer silently enables OTLP export when only dataset and token are configured.
Or push these changes by commenting:
@cursor push d2151e6563
Preview (d2151e6563)
diff --git a/apps/mobile/app.config.ts b/apps/mobile/app.config.ts
--- a/apps/mobile/app.config.ts
+++ b/apps/mobile/app.config.ts
@@ -162,7 +162,7 @@
jwtTemplate: repoEnv.EXPO_PUBLIC_CLERK_JWT_TEMPLATE ?? null,
},
observability: {
- tracesUrl: repoEnv.EXPO_PUBLIC_OTLP_TRACES_URL ?? "https://api.axiom.co/v1/traces",
+ tracesUrl: repoEnv.EXPO_PUBLIC_OTLP_TRACES_URL ?? null,
tracesDataset: repoEnv.EXPO_PUBLIC_OTLP_TRACES_DATASET ?? null,
tracesToken: repoEnv.EXPO_PUBLIC_OTLP_TRACES_TOKEN ?? null,
},You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 08f375c. Configure here.
| observability: { | ||
| tracesUrl: repoEnv.EXPO_PUBLIC_OTLP_TRACES_URL ?? "https://api.axiom.co/v1/traces", | ||
| tracesDataset: repoEnv.EXPO_PUBLIC_OTLP_TRACES_DATASET ?? null, | ||
| tracesToken: repoEnv.EXPO_PUBLIC_OTLP_TRACES_TOKEN ?? null, |
There was a problem hiding this comment.
Default trace URL enables tracing
Medium Severity
observability.tracesUrl falls back to a hardcoded Axiom HTTPS endpoint whenever EXPO_PUBLIC_OTLP_TRACES_URL is unset, while dataset and token still come from env. hasMobileTracingPublicConfig then treats tracing as fully configured if only dataset and token are present, so omitting the URL no longer keeps OTLP export off.
Reviewed by Cursor Bugbot for commit 08f375c. Configure here.
|
Bugbot Autofix prepared a fix for the issue found in the latest run.
Or push these changes by commenting: Preview (c62f773017)diff --git a/apps/mobile/src/features/cloud/managedRelayState.ts b/apps/mobile/src/features/cloud/managedRelayState.ts
--- a/apps/mobile/src/features/cloud/managedRelayState.ts
+++ b/apps/mobile/src/features/cloud/managedRelayState.ts
@@ -15,7 +15,7 @@
import { AsyncResult, Atom } from "effect/unstable/reactivity";
import { useCallback } from "react";
-import { mobileRuntime } from "../../lib/runtime";
+import { mobileRuntime, mobileTracingLayerResolved } from "../../lib/runtime";
import { appAtomRegistry } from "../../state/atom-registry";
const managedRelayAtomRuntime = Atom.runtime(
@@ -24,7 +24,7 @@
mobileRuntime.contextEffect.pipe(
Effect.map((context) => Context.get(context, ManagedRelayClient)),
),
- ),
+ ).pipe(Layer.provideMerge(mobileTracingLayerResolved)),
);
export const managedRelayQueryManager = createManagedRelayQueryManager(managedRelayAtomRuntime);
diff --git a/apps/mobile/src/lib/runtime.ts b/apps/mobile/src/lib/runtime.ts
--- a/apps/mobile/src/lib/runtime.ts
+++ b/apps/mobile/src/lib/runtime.ts
@@ -14,10 +14,14 @@
const mobileHttpClientLayer = remoteHttpClientLayer(fetch);
+export const mobileTracingLayerResolved = mobileTracingLayer.pipe(
+ Layer.provide(mobileHttpClientLayer),
+);
+
export const mobileRuntime = ManagedRuntime.make(
mobileManagedRelayClientLayer(configuredRelayUrl()).pipe(
Layer.provideMerge(mobileCryptoLayer),
Layer.provideMerge(mobileHttpClientLayer),
- Layer.provide(mobileTracingLayer.pipe(Layer.provide(mobileHttpClientLayer))),
+ Layer.provide(mobileTracingLayerResolved),
),
);You can send follow-ups to the cloud agent here. |
Co-authored-by: codex <codex@users.noreply.github.com>
…g#2994) Co-authored-by: codex <codex@users.noreply.github.com>



Summary
Why
This is the base PR for the client connection architecture rewrite. Keeping infrastructure and telemetry setup here makes the stacked client PR reviewable as a client-runtime and connectivity change.
Validation
vp check(passes with pre-existing warnings frommain)vp run typecheckNote
Add mobile OTLP tracing, HTTP header redaction, and relay deploy observability config
resolveCloudPublicConfigin publicConfig.ts to a nested shape withclerk,relay, andobservabilitysections; adds URL validation enforcing HTTPShttpHeaderRedactionLayerin httpObservability.ts that redacts thedpopheader in traces; applied to relay, web, and client-side HTTP clients.envand fails if mobile tracing config is incomplete after deployno-manual-effect-runtime-in-testsoxlint rule to enforce use of@effect/vitestMacroscope summarized 8cffc1b.
Note
Medium Risk
Public ingest tokens ship in client config and root
.env, and relay deploy now hard-requires mobile tracing outputs; DPoP TTL and Hyperdrive pool changes affect live relay auth and DB connectivity.Overview
Adds optional mobile OTLP tracing wired into the shared mobile Effect runtime: public
observabilitysettings (HTTPS URL, dataset, ingest token) flow from Expo config through nestedCloudPublicConfig, and tracing only activates when all three values are present. Relay infra provisions a separate Axiom mobile ingest token, exports URL/dataset/token from the Alchemy stack, and relay deploy now reconciles those keys into the repo root.envalongsideT3CODE_RELAY_URL(deploy fails if stack output is incomplete).Introduces shared
httpHeaderRedactionLayer(addsdpopto redacted trace header names) on relay HTTP tracing, server/web observability, andremoteHttpClientLayer; relay HTTP span tests now expectauthorizationanddpopas<redacted>.Relay-side tweaks: parallel environment link + managed allocation loads in
EnvironmentConnectorand agent-activity replay; Hyperdrive origin connection limit 5→20; DPoP access token TTL 5→30 minutes; health handler drops extra DB timing log; CORS dropsx-t3-relay-auth-failurefrom exposed headers.Mobile callers move to
config.clerk.*/config.relay.url; managed-relay atoms usemobileRuntimeContextLayer. New Oxlint ruleno-manual-effect-runtime-in-tests(baselined legacy debt) is enabled invite.config.ts.Reviewed by Cursor Bugbot for commit 8cffc1b. Bugbot is set up for automated code reviews on this repo. Configure here.