[codex] Structure relay auth persistence errors - #3250
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 |
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: Re-link keeps stale linkedAt
- Added
createdAt: nowback to the onConflictDoUpdate set block so that re-linking an environment resets the timestamp exposed aslinkedAtin list/get responses.
- Added
Or push these changes by commenting:
@cursor push 0435e7e401
Preview (0435e7e401)
diff --git a/infra/relay/src/environments/EnvironmentLinks.ts b/infra/relay/src/environments/EnvironmentLinks.ts
--- a/infra/relay/src/environments/EnvironmentLinks.ts
+++ b/infra/relay/src/environments/EnvironmentLinks.ts
@@ -204,6 +204,7 @@
managedTunnelsEnabled: request.managedTunnelsEnabled,
createdByDeviceId: request.deviceId ?? null,
revokedAt: null,
+ createdAt: now,
updatedAt: now,
},
})You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit e0bace7. Configure here.
ApprovabilityVerdict: Approved This PR adds structured context (operation stage, identifiers) to persistence error classes for better debugging. No authentication logic or success paths are modified - only what metadata gets captured when errors occur. Tests verify sensitive data is excluded from errors. No code changes detected at You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>


Summary
Why
These services previously collapsed every persistence and crypto failure into a cause-only error. The resulting errors did not identify the affected user, environment, credential, DPoP operation, or failing stage, and broad outer mappings obscured where the failure occurred.
Validation
Note
Medium Risk
Touches authentication-adjacent persistence and error mapping paths; behavior for clients should stay the same, but mis-mapped errors could affect ops/debugging or leak context if schemas drift.
Overview
Relay DPoP replay, environment credentials, and environment links persistence failures now carry structured context—
operationorstage, plus safe identifiers such asenvironmentId,userId, and DPoPthumbprint/jti—instead of a barecause. Errors are built at each crypto/DB step via localizedEffect.mapError, and messages name the failing step; tokens, proofs, and public-key material stay out of payloads.Tests assert the new fields, prune/consume failure shapes, and that sensitive inputs are not retained on errors. HTTP uses
catchTagsfor environment credential auth persistence failures, and APNS delivery job error mapping inApi.tsis inlined (removingmapApnsDeliveryJobInternalError) without changing client-facing reasons.Reviewed by Cursor Bugbot for commit cda9aed. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Enrich relay auth and environment persistence errors with operation context and identifiers
DpopProofs,EnvironmentCredentials,EnvironmentLinks, and related modules to include operation stage, entity identifiers (e.g.environmentId,userId,thumbprint), and a more specific error message.Effect.mapErrorwrappers with stage-specific error mapping at each database operation, so errors carry precise context about where in the flow they occurred.operation/stagefields and confirm sensitive fields (e.g. tokens, public keys) are not retained in errors.mapApnsDeliveryJobInternalErrorhelper in Api.ts, inlining the mapping directly intopublishAgentActivity.Macroscope summarized cda9aed.