Skip to content

Clean up cloud secret error mappings#3402

Closed
juliusmarminge wants to merge 4 commits into
mainfrom
codex/cleanup-cloud-secret-errors
Closed

Clean up cloud secret error mappings#3402
juliusmarminge wants to merge 4 commits into
mainfrom
codex/cleanup-cloud-secret-errors

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • remove valueless constructor/mapping wrappers from cloud credential and environment-key paths
  • use Effect.catchTags for tagged timeout and secret persistence failures
  • preserve exact underlying causes at each Cloud CLI error boundary
  • cover all Cloud CLI public error mappings with a test service layer

Validation

  • vp test apps/server/src/auth/ServerSecretStore.test.ts apps/server/src/cloud/environmentKeys.test.ts apps/server/src/cloud/CliTokenManager.test.ts (14 tests)
  • vp check (0 errors; existing warnings only)
  • vp run typecheck

Note

Medium Risk
Touches authentication, secret persistence, and cloud link/reconcile flows with narrower error handling and changed client-visible relay failure text; regressions could mis-map failures or hide useful diagnostics despite preserved internal causes.

Overview
This PR restructures server-side error handling across authentication, on-disk secrets, Cloud CLI credentials, and T3 Connect relay calls. Tagged errors now carry actionable context (secret names/paths, scopes, session IDs, OAuth stages, DPoP proof metadata) instead of generic resource strings or opaque wrappers.

ServerSecretStore splits directory vs per-secret failures, adds operation on persist errors, narrows each method’s failure union, maps directory init failures explicitly, and uses Effect.catchTags for concurrent-create recovery so only SecretStorePersistError with AlreadyExists retries.

EnvironmentAuth adds fields on scope/forbidden/internal errors, drops serverAuthCredentialReason / isServerAuthInvalidRequestError-style helpers, types each service method with specific error unions, and routes HTTP/WebSocket auth through catchEnvironmentAuthenticationErrors plus per-handler catchTags in auth/http.ts, http.ts, and ws.ts.

Cloud paths remove CliTokenManager’s wrapError in favor of stage-scoped CloudCli* errors with preserved causes; CloudRelayRequestError classifies relay HTTP failures by operation/phase and exposes stable messages that do not echo upstream transport/body text. DPoP replay mapping now includes proof/replay context.

Tests assert the new fields and boundaries (including relay redaction and CLI credential stages).

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

Note

Add structured context fields to cloud secret store and auth error classes

  • Replaces generic resource string fields on secret store error classes with specific secretName, secretPath, and operation fields; renames SecretStoreTemporaryPathError to SecretStoreTemporaryPathGenerationError and adds new SecretStoreDirectoryCreateError/SecretStoreDirectorySecureError classes.
  • Adds structured context (subject, scopes, session IDs, DPoP proof identifiers, credential kind) to auth error classes in EnvironmentAuth.ts and narrows per-method error union types across the EnvironmentAuth.Service interface.
  • Introduces CloudRelayConfigurationError and CloudRelayRequestError in cloud/http.ts to replace generic internal server errors for relay URL and HTTP client failures, with phase and response status annotations.
  • Expands CloudCliTokenManager error classes with stage, secret name, token endpoint, and callback address fields; error contracts for get, getExisting, hasCredential, and clear are now stage-specific.
  • Replaces broad catchIf error handlers across auth, cloud, and WebSocket routes with Effect.catchTags targeting specific error tags, using a new catchEnvironmentAuthenticationErrors helper.
  • Risk: callers that previously matched on SecretStoreError, isServerAuthCredentialError, or isServerAuthInternalError union helpers (which are removed) must now use tag-based matching.

Macroscope summarized dfc32cd.

juliusmarminge and others added 4 commits June 20, 2026 02:03
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>
@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: 3475486b-68d9-4b46-85d3-bc08b4564ed8

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/cleanup-cloud-secret-errors

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

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Jun 20, 2026
@juliusmarminge juliusmarminge force-pushed the codex/cleanup-cloud-secret-errors branch from 6b4c998 to dfc32cd Compare June 20, 2026 15:36
@juliusmarminge

Copy link
Copy Markdown
Member Author

Closing as fully superseded by the existing stack: #3243 already removes the environment-key wrappers and adopts catchTags for secret persistence, while #3249 already removes the Cloud CLI wrapError helper, adopts catchTags for timeout handling, preserves richer structured causes, and includes focused tests. Rebasing onto #3249 produced an empty patch, so a separate PR would add no value.

@github-actions github-actions Bot added size:XXL 1,000+ changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Jun 20, 2026
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

Changes to error handling patterns in auth/cloud code paths, including narrowing which error types are caught. Auth-related code changes warrant human review regardless of apparent simplicity.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ 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