[codex] Share safe URL diagnostics#3403
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 |
ApprovabilityVerdict: Needs human review Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
Dismissing prior approval to re-evaluate ef9ed1e
Dismissing prior approval to re-evaluate e445dc2
Dismissing prior approval to re-evaluate 9e3a040
Dismissing prior approval to re-evaluate c41bc6c
Dismissing prior approval to re-evaluate 6dabb2f
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Catalog errors mislabel failure stage
- Added resolvePersistenceStage() helper that derives the correct stage from the underlying ConnectionStorageOperationError, replacing the hardcoded 'read'/'write' stage values in catalog operations across both web and mobile storage files.
Or push these changes by commenting:
@cursor push b41df4da87
Preview (b41df4da87)
diff --git a/apps/mobile/src/connection/storage.ts b/apps/mobile/src/connection/storage.ts
--- a/apps/mobile/src/connection/storage.ts
+++ b/apps/mobile/src/connection/storage.ts
@@ -7,6 +7,7 @@
removeConnectionFromCatalog,
removeCatalogValue,
replaceCatalogValue,
+ resolvePersistenceStage,
} from "@t3tools/client-runtime/platform";
import { TokenStore } from "@t3tools/client-runtime/authorization";
import {
@@ -212,7 +213,7 @@
(cause) =>
new ConnectionPersistenceError({
operation: "list-targets",
- stage: "read",
+ stage: resolvePersistenceStage(cause, "read"),
resource: "connection-catalog",
cause,
}),
@@ -228,7 +229,7 @@
(cause) =>
new ConnectionPersistenceError({
operation: "register-connection",
- stage: "write",
+ stage: resolvePersistenceStage(cause, "write"),
resource: "connection-catalog",
environmentId: registration.target.environmentId,
cause,
@@ -243,7 +244,7 @@
(cause) =>
new ConnectionPersistenceError({
operation: "remove-connection",
- stage: "write",
+ stage: resolvePersistenceStage(cause, "write"),
resource: "connection-catalog",
environmentId: target.environmentId,
cause,
diff --git a/apps/web/src/connection/storage.ts b/apps/web/src/connection/storage.ts
--- a/apps/web/src/connection/storage.ts
+++ b/apps/web/src/connection/storage.ts
@@ -10,6 +10,7 @@
removeCatalogValue,
removeConnectionFromCatalog,
replaceCatalogValue,
+ resolvePersistenceStage,
} from "@t3tools/client-runtime/platform";
import { TokenStore } from "@t3tools/client-runtime/authorization";
import {
@@ -402,7 +403,7 @@
(cause) =>
new ConnectionPersistenceError({
operation: "list-targets",
- stage: "read",
+ stage: resolvePersistenceStage(cause, "read"),
resource: "connection-catalog",
cause,
}),
@@ -418,7 +419,7 @@
(cause) =>
new ConnectionPersistenceError({
operation: "register-connection",
- stage: "write",
+ stage: resolvePersistenceStage(cause, "write"),
resource: "connection-catalog",
environmentId: registration.target.environmentId,
cause,
@@ -433,7 +434,7 @@
(cause) =>
new ConnectionPersistenceError({
operation: "remove-connection",
- stage: "write",
+ stage: resolvePersistenceStage(cause, "write"),
resource: "connection-catalog",
environmentId: target.environmentId,
cause,
diff --git a/packages/client-runtime/src/platform/persistence.ts b/packages/client-runtime/src/platform/persistence.ts
--- a/packages/client-runtime/src/platform/persistence.ts
+++ b/packages/client-runtime/src/platform/persistence.ts
@@ -10,6 +10,7 @@
import * as Schema from "effect/Schema";
import type { ConnectionRegistration } from "../connection/catalog.ts";
+import type { ConnectionStorageOperation } from "../connection/model.ts";
import type { ConnectionTarget } from "../connection/model.ts";
export class ConnectionPersistenceError extends Schema.TaggedErrorClass<ConnectionPersistenceError>()(
@@ -48,6 +49,40 @@
}
}
+const storageOperationToStage: Record<
+ ConnectionStorageOperation,
+ ConnectionPersistenceError["stage"]
+> = {
+ open: "read",
+ read: "read",
+ load: "read",
+ decode: "decode",
+ migrate: "decode",
+ encode: "encode",
+ write: "write",
+ save: "write",
+ remove: "remove",
+ delete: "remove",
+};
+
+export function resolvePersistenceStage(
+ error: { readonly cause?: unknown },
+ fallback: ConnectionPersistenceError["stage"],
+): ConnectionPersistenceError["stage"] {
+ const inner = error.cause;
+ if (
+ inner != null &&
+ typeof inner === "object" &&
+ "_tag" in inner &&
+ inner._tag === "ConnectionStorageOperationError" &&
+ "operation" in inner
+ ) {
+ const operation = (inner as { operation: ConnectionStorageOperation }).operation;
+ return storageOperationToStage[operation] ?? fallback;
+ }
+ return fallback;
+}
+
export class ConnectionTargetStore extends Context.Service<
ConnectionTargetStore,
{You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit b87e64a. Configure here.
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>
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>
b87e64a to
3167aeb
Compare
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>


Summary
Error context often needs enough URL information to correlate failures without retaining secrets. Several clients had independently added local URL parsers, increasing the chance that credentials, paths, query parameters, fragments, or signed tokens would leak into logs and telemetry.
This shared foundation adds two deliberately separate helpers:
getUrlDiagnosticsfrom the explicit@t3tools/shared/urlDiagnosticssubpath returns only{ inputLength, protocol?, hostname? }. Invalid input returns length only; it never exposes path, userinfo, query, or fragment data.redactDpopRequestTargetremains in@t3tools/shared/dpopfor the narrower DPoP use case, where scheme/host/port/path are part of the request-target diagnostics while credentials/query/fragment are removed.Focused tests cover sensitive valid URLs and invalid input for both policies.
Validation
vp test run packages/shared/src/urlDiagnostics.test.ts packages/shared/src/dpop.test.ts(10 tests)vp check(passes with 20 pre-existing warnings)vp run typecheckNote
Redact sensitive URL data from errors and logs across client, server, and relay
getUrlDiagnosticsandredactDpopRequestTargetutilities inpackages/sharedthat extract safe fields (input length, protocol, hostname) from URLs without exposing credentials, paths, or query strings.Data.TaggedErrorerror classes withSchema.TaggedErrorClassacross the codebase, adding structured fields (URL diagnostics, operation stage, cause) and stablemessagegetters that avoid embedding raw URLs or cause messages.AgentAwarenessRelay,RelayEnvironmentDiscovery,ManagedRelayClient, andlinkEnvironmentare updated to spread redacted diagnostic attributes instead of logging raw URL strings or fullCause.prettyoutput.ConnectionBlockedError,ConnectionTransientError, andConnectionPersistenceErrorare extended with structured storage operation context, cause fields, and afromStorageFailurefactory used by web, mobile, and desktop storage backends.fromRequestUrl,fromTarget,fromEndpoint,fromStorageFailure); direct construction orinstanceofchecks at call sites must be updated to use the new factories andSchema.istype guards.Macroscope summarized a2c3536.
Note
Medium Risk
Wide cross-cutting change to error tags, fields, and messages (including DPoP and connection storage); callers that match on old shapes need updates, but runtime paths are mostly observability and failure reporting.
Overview
Rolls out shared
getUrlDiagnosticsandredactDpopRequestTargetso failures can be correlated without logging credentials, signed paths, query tokens, or full URLs.Error model: Ad-hoc strings and
Data.TaggedErrorare replaced withSchema.TaggedErrorClasstypes that carry operation/stage fields, a typedcause, and genericmessagetext. Raw URLs are dropped from error objects and log attributes in favor of length, protocol, hostname, hashes, or redacted DPoPrequestTargetvalues.Notable behavior shifts: Desktop
ElectronShell.openExternal/copyTextnow surface typed failures instead of swallowing them asfalse; window handlers log viaEffect.ignore. Mobile connection/catalog/shell persistence, legacy migration, DPoP, image prefetch, and composer image paths use the new storage/persistence errors. Server keybindings, OTLP trace proxy, relay activity publishing, and web browser targets / cloud linking follow the same logging and error-shape rules.Tests assert secrets stay out of messages and structured fields while causes and safe context are preserved.
Reviewed by Cursor Bugbot for commit a2c3536. Bugbot is set up for automated code reviews on this repo. Configure here.