[codex] Structure cloud HTTP boundary errors#3246
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: Approved Refactors HTTP error handling from string messages to structured reason codes with backwards compatibility. Changes are mechanical, add secret-leakage prevention, and include comprehensive tests for the new error structure. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
47c0424 to
8090110
Compare
6c12fe1 to
1c4bd6e
Compare
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate fe10a0b
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: Structured cause dropped from logs
- Added
{ cause }back to theEffect.logErrorcall infailEnvironmentCloudInternalErrorso the structured cause chain is preserved in server logs, matching the pattern used byfailEnvironmentInternalinauth/http.ts.
- Added
Or push these changes by commenting:
@cursor push a6684f6aaa
Preview (a6684f6aaa)
diff --git a/apps/server/src/cloud/http.ts b/apps/server/src/cloud/http.ts
--- a/apps/server/src/cloud/http.ts
+++ b/apps/server/src/cloud/http.ts
@@ -201,7 +201,7 @@
const failEnvironmentCloudInternalError =
(message: string) =>
(cause: unknown): Effect.Effect<never, EnvironmentHttpInternalServerError> =>
- Effect.logError(message).pipe(
+ Effect.logError(message, { cause }).pipe(
Effect.flatMap(() => Effect.fail(new EnvironmentHttpInternalServerError({ message, cause }))),
);You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit fe10a0b. Configure here.
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate f785979
050fbe6
into
codex/server-secret-store-errors


Stacked on #3243.
Summary
Validation
Note
Medium Risk
Touches authentication, cloud linking, health/mint, and relay credential flows across contracts and the environment server; behavior changes are mostly error shape and redaction, but mis-mapped catch tags could surface wrong client messages.
Overview
Cloud HTTP errors move from ad-hoc
messagestrings to reason/operation-coded contract types (EnvironmentHttpBadRequestReason,EnvironmentHttpUnauthorizedReason,EnvironmentHttpInternalOperation, relay operation/phase, conflict reasons). Constructors derive stable user-facing text; wire payloads keep optional fields so message-only legacy bodies still decode during rolling deploys.Relay outbound calls gain
CloudRelayRequestError(phase classification, URL diagnostics without secrets) andCloudRelayConfigurationErrorfor badT3CODE_RELAY_URL.relayClientRequestmaps every HTTP client failure throughfromClientFailureinstead of stringifying causes into 500s.failEnvironmentCloudInternalErrornow takes structured operation context, logscauseTagonly (not full cause), and returnsEnvironmentHttpInternalServerErrorwith internalcauseretained but omitted from encoded JSON (tests assert this).Cloud handlers switch from broad
catchIf/ generic secret-store catches toEffect.catchTagswith per-failure operation labels. Unauthorized CLI linking usesreason: cloud_cli_authorization_required. Validation and conflict paths use machine reasons instead of inline messages.Tests cover redaction, legacy decode, reconcile link auth, and mobile link-proof error propagation.
Reviewed by Cursor Bugbot for commit f785979. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Structure cloud HTTP boundary errors with typed reason codes and operation diagnostics
reasonfields to 400, 401, and 409 error payloads (e.g.cloud_cli_authorization_required,invalid_relay_url) and structuredoperation/relayOperation/relayPhasefields to 500 payloads in environmentHttp.ts.CloudRelayRequestErrorin http.ts to wrap relay client failures with phase diagnostics (encode-request,send-request,check-response-status,decode-response) and sanitized messages that don't leak sensitive URL parts or upstream details.failEnvironmentCloudInternalErrorto log acauseTaginstead of the raw cause, and replaces broadcatchIfcatches withEffect.catchTagsthroughout all cloud HTTP handlers.consumeCloudReplayGuardsnow only swallowsSecretStorePersistErrorwith already-exists semantics; other secret store errors now propagate as failures instead of being silently ignored.Macroscope summarized f785979.