Skip to content

[codex] Correlate protocol request failures#3433

Merged
juliusmarminge merged 1 commit into
mainfrom
codex/protocol-request-error-correlation
Jun 20, 2026
Merged

[codex] Correlate protocol request failures#3433
juliusmarminge merged 1 commit into
mainfrom
codex/protocol-request-error-correlation

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • attach method, request ID, and operation diagnostics to ACP and Codex App Server request/response failures
  • retain complete encoded ACP failure causes while deriving stable protocol-facing messages
  • preserve pending-request method context and clean it up without widening catch semantics
  • centralize protocol error transformations on the error classes

Validation

  • vp test packages/effect-acp/src/errors.test.ts packages/effect-acp/src/protocol.test.ts packages/effect-codex-app-server/src/protocol.test.ts (26 tests)
  • vp check (0 errors; existing warnings only)
  • vp run typecheck
  • independent read-only review found no remaining actionable issues

Note

Medium Risk
Touches core JSON-RPC error mapping and pending-request lifecycle in two protocol packages; behavior is mostly additive diagnostics but changes error shapes and ACP extension error handling semantics.

Overview
ACP and Codex App Server now attach method, request ID, and operation (receive-response, encoding failures, etc.) when outbound encoding fails or when a client request gets an error response, so failures can be tied to the call that caused them.

Pending outbound requests keep method alongside the deferred (not just ID), and protocol layers use that when mapping JSON-RPC errors into AcpRequestError / CodexAppServerRequestError. ACP also adds focused factories for extension response failures, response encoding errors, and unsupported streaming chunks, keeps full exit cause on protocol errors where relevant, and tightens extension-handler error handling to AcpProtocolParseError instead of a blanket catch. Encode/decode failure logging can include requestId where available.

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

Note

Correlate protocol request failures with originating RPC method and request ID

  • Adds method and requestId metadata to AcpRequestError and CodexAppServerRequestError so that protocol-level failures can be traced back to the originating request.
  • Introduces AcpPendingRequest and CodexAppServerPendingRequest interfaces that store method alongside the deferred, enabling correlation when responses arrive.
  • New factory methods (fromProtocolError, fromExtensionResponseFailure, fromExtensionResponseEncodingError, unsupportedStreamingResponse) produce structured errors tagged with specific operation values (receive-response, encode-extension-response, receive-streaming-response).
  • Encoding failures in offerOutgoing and encodeWireMessage now propagate method and requestId into AcpProtocolParseError / CodexAppServerProtocolParseError.
  • Pending entries are now cleaned up via tapError on send failure rather than swallowing the error path.

Macroscope summarized 878b3a6.

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: de5691c3-fb17-466b-a77b-2a8c6b9247ae

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/protocol-request-error-correlation

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:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Adds optional diagnostic context (method, requestId, operation) to protocol error objects for better error correlation. Backwards-compatible schema additions with comprehensive test coverage and no behavioral changes.

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

@juliusmarminge juliusmarminge merged commit 6d8d995 into main Jun 20, 2026
16 checks passed
@juliusmarminge juliusmarminge deleted the codex/protocol-request-error-correlation branch June 20, 2026 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 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