[codex] Structure ACP transport errors

[juliusmarminge]

Summary

• attach transport operation and RPC method context while preserving original RPC, stream, process, and SchemaError causes
• keep messages stable instead of copying cause-derived diagnostics
• summarize schema failures with issue count, issue kinds, and maximum path depth; raw invalid payload values are never copied into direct fields or JSON-RPC data
• normalize core and extension notification SchemaErrors through one payload-safe structural mapper
• represent a clean ACP input-stream end as its own error rather than inventing a cause
• centralize reusable error transformations on the error classes

Validation

• vp test run packages/effect-acp/src/errors.test.ts packages/effect-acp/src/protocol.test.ts packages/effect-acp/src/agent.test.ts packages/effect-acp/src/client.test.ts (25 tests)
• vp check (0 errors; 20 pre-existing warnings)
• vp run typecheck

---

Note

Medium Risk
Error shapes and user-visible messages change across the ACP client/agent stack, which may break callers that match on message text; the changes are security-motivated (avoid leaking payload/cause details) and are covered by new and updated tests.

Overview
Restructures ACP transport, protocol parse, and request errors so public message strings are fixed templates with optional operation and method fields, while the original cause (RPC, stream, SchemaError, etc.) is preserved on the error object.

Schema validation failures no longer embed formatted issue text or raw invalid values in messages, logs, or JSON-RPC data. Instead they attach summarized diagnostics (issueCount, issueKinds, maximumPathDepth) via shared helpers on AcpProtocolParseError and AcpRequestError, with full details only on the SchemaError cause.

Protocol and RPC plumbing passes RPC method names into callRpc, maps handler failures through fromCoreHandlerError / fromExtensionHandlerError, logs decode failures with operation metadata only, uses stable RpcClient defect messages for termination and send failures, and treats a clean stdin end as AcpInputStreamEndedError instead of a synthetic transport error.

API cleanup: removes makeTerminal; the agent builds terminal handles as plain AcpTerminal-shaped objects. Adds errors.test.ts and extends protocol/client tests for the new error behavior.

^{Reviewed by Cursor Bugbot for commit 2b3948b. Bugbot is set up for automated code reviews on this repo. Configure here.}

Note

Structure ACP transport, request, and protocol parse errors with operation and method context

• Replaces free-form detail strings on AcpTransportError, AcpProtocolParseError, and AcpRequestError with structured operation and method fields, standardizing all user-facing error messages.
• Adds schemaIssueDiagnostics to aggregate schema issue counts, kinds, and path depth into error metadata without leaking raw payload values.
• Introduces AcpInputStreamEndedError as a distinct error type for end-of-input conditions, replacing ad-hoc AcpTransportError construction.
• Removes makeTerminal from terminal.ts and the normalizeToRequestError/toRpcClientError helpers from protocol.ts; callers must construct terminal objects directly.
• Risk: all public-facing error messages change, and callRpc now requires an explicit method string as its first argument — call sites in agent.ts and client.ts are updated accordingly.

^{Macroscope summarized 2b3948b.}

[coderabbitai]

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: d6cbc5df-e4c2-4611-aaf0-6851e78bbea4

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/audit-acp-errors

---

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

[macroscopeapp]

Approvability

Verdict: Approved

Refactoring of ACP transport error handling to use structured fields instead of free-form strings. No behavioral changes beyond error message formatting, with added benefit of preventing sensitive values from leaking into error messages. Comprehensive test coverage included.

^{You can customize Macroscope's approvability policy. Learn more.}

Dismissing prior approval to re-evaluate 8538a20

Dismissing prior approval to re-evaluate 90d09c7

Dismissing prior approval to re-evaluate 0009907

Dismissing prior approval to re-evaluate 62b4eb4

Dismissing prior approval to re-evaluate 625a431

Dismissing prior approval to re-evaluate 8c20ca7

Dismissing prior approval to re-evaluate e545938

[cursor]

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: Core handlers get extension errors
  • Replaced the misuse of fromHandlerError in runHandler with inline logic that produces core-appropriate error message ('ACP request handler failed') and operation tag ('handle-request') instead of the extension-specific wording.

Or push these changes by commenting:

@cursor push f1de956242

Preview (f1de956242)

diff --git a/packages/effect-acp/src/_internal/shared.ts b/packages/effect-acp/src/_internal/shared.ts
--- a/packages/effect-acp/src/_internal/shared.ts
+++ b/packages/effect-acp/src/_internal/shared.ts
@@ -35,9 +35,17 @@
     return yield* Effect.fail(AcpError.AcpRequestError.methodNotFound(method).toProtocolError());
   }
   return yield* handler(payload).pipe(
-    Effect.mapError((error) =>
-      AcpError.AcpRequestError.fromHandlerError(error, method).toProtocolError(),
-    ),
+    Effect.mapError((error) => {
+      const requestError =
+        error._tag === "AcpRequestError"
+          ? error
+          : AcpError.AcpRequestError.internalError(
+              `ACP request handler failed for method '${method}'`,
+              undefined,
+              { method, operation: "handle-request", cause: error },
+            );
+      return requestError.toProtocolError();
+    }),
   );
 });
 

diff --git a/packages/effect-acp/src/errors.test.ts b/packages/effect-acp/src/errors.test.ts
--- a/packages/effect-acp/src/errors.test.ts
+++ b/packages/effect-acp/src/errors.test.ts
@@ -96,7 +96,7 @@
 
       expect(error).toMatchObject({
         code: -32603,
-        message: "ACP extension request handler failed for method 'fs/read_text_file'",
+        message: "ACP request handler failed for method 'fs/read_text_file'",
       });
       expect(error.message).not.toContain(cause.message);
     });

diff --git a/packages/effect-acp/src/errors.ts b/packages/effect-acp/src/errors.ts
--- a/packages/effect-acp/src/errors.ts
+++ b/packages/effect-acp/src/errors.ts
@@ -6,6 +6,7 @@
 export const AcpRequestOperation = Schema.Literals([
   "decode-extension-request-payload",
   "handle-extension-request",
+  "handle-request",
 ]);
 export type AcpRequestOperation = typeof AcpRequestOperation.Type;

_{You can send follow-ups to the cloud agent here.}

^{Reviewed by Cursor Bugbot for commit e545938. Configure here.}