Skip to content

[codex] Preserve client connection error causes - #3242

Merged
juliusmarminge merged 10 commits into
codex/redact-dpop-request-targetfrom
codex/audit-relay-errors-wave1
Jun 20, 2026
Merged

[codex] Preserve client connection error causes#3242
juliusmarminge merged 10 commits into
codex/redact-dpop-request-targetfrom
codex/audit-relay-errors-wave1

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • model remote fetch, invalid-response, and timeout failures with structured request context and messages that do not stringify their causes
  • retain the immediate relay, remote authorization, DPoP, and RPC failure as cause when translating into connection classifications
  • move relay HTTP failure mapping onto ManagedRelayRequestFailedError, use tag-directed retry handling, and inline a no-value proof-error field wrapper
  • add focused coverage for classification metadata and complete cause chains

The undeclared-status conversion is intentionally left for the SSH follow-up because its current instanceof consumer is being changed by #3206.

Validation

  • vp check
  • vp run typecheck
  • vp test packages/client-runtime/src/connection/errors.test.ts packages/client-runtime/src/authorization/remote.test.ts packages/client-runtime/src/authorization/layer.test.ts packages/client-runtime/src/rpc/session.test.ts packages/client-runtime/src/relay/managedRelay.test.ts packages/client-runtime/src/connection/onboarding.test.ts

Note

Medium Risk
Touches authentication, connection resolution, and relay/remote HTTP error paths across desktop, mobile, web, and server; behavior changes for error shape and logging are broad but covered by focused tests.

Overview
This PR tightens client connection and relay error handling so failures keep a inspectable cause chain while logs and user-facing messages stop embedding full URLs, credentials, or raw config entries.

Connection layer: ConnectionBlockedError and ConnectionTransientError gain optional cause plus structured fields (connectionId, expectedEnvironmentId, actualEnvironmentId). Relay and remote-environment mappers attach the original error instead of dropping it. Environment mismatch and missing profile/credential paths build ConnectionBlockedError inline with those fields.

Remote HTTP / auth: RemoteEnvironmentAuthFetchError, timeout, invalid JSON, and undeclared-status errors become Schema.TaggedErrorClass types with hostname / protocol / URL length diagnostics; messages no longer stringify transport details or embed request URLs. Desktop SSH uses isRemoteEnvironmentAuthUndeclaredStatusError instead of instanceof.

Managed relay: Invalid relay URLs and DPoP failures use getUrlDiagnostics / redactDpopRequestTarget; proof errors distinguish key-load vs creation via fromTarget. HTTP failures map through ManagedRelayRequestFailedError.fromHttpRequest; DPoP token retry uses catchTags. Offline health-check logs emit endpoint diagnostics, not raw httpBaseUrl.

Server keybindings: KeybindingsConfigError is renamed and modeled with operation + cause (replacing free-form detail). Runtime issues use stable messages; warnings log validation metadata (stage, field presence, cause counts) without full entries or secrets. Startup and watcher failures log structured cause attributes.

Tests cover redaction, cause retention, and stable decode errors across the touched paths.

Reviewed by Cursor Bugbot for commit 9b8b72c. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Preserve error causes and redact sensitive URLs in client connection errors

  • Extends ConnectionBlockedError and ConnectionTransientError with optional cause, connectionId, expectedEnvironmentId, and actualEnvironmentId fields so callers can inspect the original error chain.
  • Replaces generic session/snapshot error classes in managedRelayState.ts with structured union types (ManagedRelayTokenReadError, ManagedRelayTokenUnavailableError, ManagedRelaySessionUnavailableError, and snapshot-specific variants) that preserve account IDs and original causes.
  • Replaces raw URL fields in ManagedRelayUrlInvalidError, ManagedRelayDpopProofCreationError, and related classes with redacted diagnostics (input length, protocol, hostname) via getUrlDiagnostics; DPoP errors use new fromTarget static factories.
  • Rewrites RemoteEnvironmentAuthFetchError, RemoteEnvironmentAuthTimeoutError, RemoteEnvironmentAuthInvalidJsonError, and RemoteEnvironmentAuthUndeclaredStatusError in rpc/http.ts as Schema.TaggedErrorClass with URL diagnostics and fromRequestUrl factories; messages no longer include raw URLs or cause strings.
  • Updates keybindings.ts to emit structured KeybindingsConfigError (renamed from KeybindingsConfigParseError) with an operation field and sanitized log attributes that omit raw entries and secret values.
  • Behavioral Change: error message strings across relay, remote auth, and keybindings change to reference redacted URL metadata (host, character count) rather than full URLs.

Macroscope summarized 9b8b72c.

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c7525ea8-dece-49c8-be7e-3e7197e4a3ed

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/audit-relay-errors-wave1

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Jun 20, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Error handling infrastructure refactor that preserves error cause chains for debugging while redacting sensitive URL credentials from logs and error messages. Changes are mechanical with security benefits and extensive test coverage.

You can customize Macroscope's approvability policy. Learn more.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
@macroscopeapp
macroscopeapp Bot dismissed their stale review June 20, 2026 14:14

Dismissing prior approval to re-evaluate dc28b4d

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
@macroscopeapp
macroscopeapp Bot dismissed their stale review June 20, 2026 14:49

Dismissing prior approval to re-evaluate ead817e

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Load-key stage wrong message
    • Made the message getter in ManagedRelayDpopProofCreationError branch on this.stage, returning a key-load message for 'load-key' and the existing proof-creation message for 'create-proof'.

Create PR

Or push these changes by commenting:

@cursor push 04e79537b9
Preview (04e79537b9)
diff --git a/packages/client-runtime/src/relay/managedRelay.ts b/packages/client-runtime/src/relay/managedRelay.ts
--- a/packages/client-runtime/src/relay/managedRelay.ts
+++ b/packages/client-runtime/src/relay/managedRelay.ts
@@ -71,7 +71,9 @@
   },
 ) {
   override get message(): string {
-    return `Could not create the relay DPoP proof for ${this.method} ${this.url}.`;
+    return this.stage === "load-key"
+      ? `Could not load the relay DPoP key for ${this.method} ${this.url}.`
+      : `Could not create the relay DPoP proof for ${this.method} ${this.url}.`;
   }
 }

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 59f9a97. Configure here.

Comment thread packages/client-runtime/src/relay/managedRelay.ts Outdated
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
juliusmarminge and others added 7 commits June 20, 2026 09:31
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>
@juliusmarminge
juliusmarminge force-pushed the codex/audit-relay-errors-wave1 branch from 1e542ee to 62192e0 Compare June 20, 2026 16:57
@juliusmarminge
juliusmarminge changed the base branch from main to codex/redact-dpop-request-target June 20, 2026 16:57
@macroscopeapp
macroscopeapp Bot dismissed their stale review June 20, 2026 16:57

Dismissing prior approval to re-evaluate 62192e0

@github-actions github-actions Bot added size:XL 500-999 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
@macroscopeapp
macroscopeapp Bot dismissed their stale review June 20, 2026 17:26

Dismissing prior approval to re-evaluate 6ea4fd3

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
juliusmarminge and others added 2 commits June 20, 2026 11:30
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
@macroscopeapp
macroscopeapp Bot dismissed their stale review June 20, 2026 18:30

Dismissing prior approval to re-evaluate 9b8b72c

@juliusmarminge
juliusmarminge merged commit 666c114 into codex/redact-dpop-request-target Jun 20, 2026
16 checks passed
@juliusmarminge
juliusmarminge deleted the codex/audit-relay-errors-wave1 branch June 20, 2026 18:39
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL 500-999 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant