[codex] Structure Codex app-server request errors#3258
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 |
ApprovabilityVerdict: Approved Refactoring of error handling to use structured error types with typed operation fields and diagnostic metadata instead of free-form string details. Changes are mechanical with no business logic impact, limited to error classification and message formatting. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate dcb2646
There was a problem hiding this comment.
One convention finding on the new protocol-parse operation discriminator. Otherwise the error refactor aligns well with the conventions: structured attributes, messages derived from those attributes (not from cause), real causes preserved, and the multi-value operation on CodexAppServerRequestError is purely diagnostic.
Posted via Macroscope — Effect Service Conventions
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate 5cecba9
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate f8c426f
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate 798661f
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate 7a27805
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate 0d26774
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate 7821d64
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate 306576f
Summary
Validation
Note
Medium Risk
Touches JSON-RPC error shapes and termination behavior across the Codex client and server runtime; callers that matched on old
detailstrings or expected a transport error on stream end may need updates, though the change mainly reduces accidental secret leakage in logs.Overview
Restructures Codex app-server error handling so failures are classified by
operation(wire encode/decode, payload decode/encode, handler, transport, identifier generation) instead of ad hocdetailstrings.CodexAppServerRequestErrorgainsmethod,operation, schema summaries (issueCount,issueKinds,maximumPathDepth), and a retainedSchemaErrorcause; unexpected payloads on methods with no schema are reported via payload kind only.CodexAppServerProtocolParseErrorand transport errors use the same pattern, with newCodexAppServerIdentifierGenerationError(purpose-tagged UUID failures) andCodexAppServerInputStreamEndedError(clean stdin end, no synthetic cause).normalizeToRequestErroris removed in favor offromAppServerError/ static factories.Protocol and shared layers route decode/encode/handler failures through those factories; decode-failure logs emit safe operation/schema summaries without copying causes or wire bodies.
CodexSessionRuntimemapsturn/startschema failures viafromSchemaError, tags UUID generation by purpose, and logs full resume errors ascauseobjects.Focused tests cover secret non-leakage, handler cause chains, protocol logging, and Effect-based
openCodexThreadcases.Reviewed by Cursor Bugbot for commit 306576f. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Structure Codex app-server request errors with typed operations, diagnostics, and dedicated error classes
CodexAppServerProtocolParseError,CodexAppServerTransportError, andCodexAppServerRequestErrorwith typed operation enums and structured schema diagnostics (issue count, kinds, path depth).CodexAppServerIdentifierGenerationError(with purpose) andCodexAppServerInputStreamEndedErroras dedicated error types, replacing generic transport errors for these cases.decodeOptionalPayload,encodeOptionalPayload,runHandler, and protocol wire encode/decode functions in shared.ts and protocol.ts to emit the new structured errors.operation,method, and diagnostic fields, and explicitly omitcauseand raw payloads to avoid secret leakage.detailstrings or message text will break.Macroscope summarized 306576f.