fix(acp): surface JSON-RPC error data.details to users

[howie]

What problem does this solve?

When an ACP agent emits a JSON-RPC error such as {code: -32603, message: "Internal error"}, openab surfaces it as a single opaque "Internal Error (code: -32603)" — regardless of whether the actual cause is a deprecated model, an expired auth token, a missing npm peer dependency, or something else entirely.

The reason is that JsonRpcError deserializes only code and message, silently dropping the optional data field defined in JSON-RPC 2.0 §5.1. codex-acp (and other ACP adapters) put the actionable detail string in data.details, but openab discards it on the wire — so users see the same message for every cause and have no signal to recover.

Related: #547 documents one specific multi-org Codex case where this surfaces. That issue proposes a docs-only fix; this PR addresses the underlying transparency gap so all -32603 sources (model deprecation, peer-dep failures, auth, multi-org, etc.) automatically surface their upstream detail.

At a Glance

Before                                         After
------                                         -----
codex-acp error                                codex-acp error
{                                              {
  code: -32603,                                  code: -32603,
  message: "Internal error",                     message: "Internal error",
  data: {details: "model 'X' deprecated"}        data: {details: "model 'X' deprecated"}
}                                              }
        |                                              |
        v                                              v
openab JsonRpcError (drops data)               openab JsonRpcError {code, message, data}
        |                                              |
        v                                              v
format_coded_error(code, message)              format_coded_error(&err)
        |                                              |
        v                                              v
**Internal Error** (code: -32603)              **Internal Error** (code: -32603)
Internal error                                 Internal error
                                               model 'X' deprecated

Prior Art & Industry Research

OpenClaw — openclaw/acpx treats data.details as a first-class field. acpx normalizes ACP errors through dedicated modules (error-shapes.ts, error-normalization.ts, session-control-errors.ts) that:

• Carry data: unknown on every payload and walk it for typed hints (auth required, session not found, query closed).
• Distinguish causes that share -32603 via acp.data.details string matching.
• Surface the detail string in user-facing wrappers (e.g., formatSessionControlAcpSummary reads data?.details and renders ${details} (ACP ${code}, adapter reported "${message}")).

Hermes Agent — NousResearch/hermes-agent is the agent side and emits errors through the upstream acp Python SDK (acp_adapter/server.py). The SDK preserves data end-to-end, so client implementations that read it (acpx) see the agent's diagnostic context. The lesson: ACP's wire format already supports this — clients only need to deserialize the field.

JSON-RPC 2.0 spec — §5.1 defines data as an optional member that "MAY be omitted" but "SHOULD" carry "additional information about the error" when present. Dropping it is a conformance gap.

Proposed Solution

Minimal, surgical change in four files:

1. src/acp/protocol.rs — Add data: Option<serde_json::Value> to JsonRpcError (with #[serde(default)] for back-compat), plus a data_details() accessor that mirrors the codex-acp / acpx convention (data.details as a string).
2. src/error_display.rs — Change format_coded_error to take &JsonRpcError, append data.details when present, and de-duplicate when message already contains the same text (some adapters echo details into message for legacy clients).
3. src/adapter.rs — Update the single call site at the response-error handler.
4. src/acp/connection.rs — Add data: None to the synthetic "connection closed" error.

Tests cover: deserialize round-trip with and without data, non-string details (ignored), unknown data shapes (ignored), empty/whitespace details, and the de-duplication path. Existing format_coded_error tests are migrated to the new signature.

Why this approach?

• Minimal blast radius. One struct gains an optional field; one function signature changes; two call sites update. No new modules, no taxonomy.
• Backward compatible on the wire. #[serde(default)] means agents that don't emit data deserialize unchanged. Display impl preserves existing output when data.details is absent.
• Addresses all known -32603 causes at once. Today CLAUDE.md gotchas already document three distinct -32603 root causes (model deprecation, npx peer-dep failure, auth) — all of them look identical to users. After this change, each one carries its own actionable detail.
• Convention-aligned. Matches acpx's data.details string convention exactly, so future cross-tool diagnostics stay consistent.

Known limitations:

• Only data.details (string) is surfaced. acpx additionally recognises data.authRequired, data.methodId, data.methods for richer auth flows — out of scope here, can be added incrementally without breaking this change.
• The Discord/Slack message stays plain text; we don't try to format data as JSON when details is missing or non-string (silently ignored). This is conservative — avoids leaking implementation-defined payloads.

Alternatives Considered

1. Mirror acpx in full (error-normalization, error-shapes, NormalizedOutputError, retryable classification). Rejected: acpx is a CLI that exits with structured codes — openab is a chat adapter that streams text. The full taxonomy is value for CLI consumers but overhead here, and bundles unrelated concerns (retry policy, exit codes) into one PR.
2. Patch upstream codex-acp to echo data.details into message. Rejected: the wire format already supports data; the bug is openab dropping it. Fixing it client-side keeps the change in one repo and benefits every ACP adapter, not just codex-acp.
3. Render full data as JSON in user output. Rejected: leaks implementation detail and risks confusing users with adapter-internal fields. data.details is the documented convention.
4. Docs-only fix per fix(docs/codex): address Internal Error (-32603) in multi-org environments #547. Insufficient: documents one specific multi-org case but leaves the structural problem in place — every other -32603 cause still surfaces the same opaque label.

Validation

• cargo check — clean
• cargo test — 373 passed, 0 failed. 11 new tests cover:
  • acp::protocol: deserialize with/without data, non-string details, Display impl
  • error_display: append details, message-only fallback, dedup, unknown shapes, whitespace
• cargo clippy --all-targets on the changed files — clean. (2 pre-existing warnings in src/cron.rs exist on origin/main and are unrelated to this PR.)

Manual: not deployed to a live agent (would need a codex-acp instance hitting a known error); the format paths are exercised end-to-end by the new tests including the model-deprecation scenario from CLAUDE.md gotchas.

[github-actions]

⚠️ This PR is missing a Discord Discussion URL in the body.

All PRs must reference a prior Discord discussion to ensure community alignment before implementation.

Please edit the PR description to include a link like:

Discord Discussion URL: https://discord.com/channels/...

This PR will be automatically closed in 3 days if the link is not added.

[chaodu-agent]

Closing as duplicate of #885 which was merged yesterday and addresses the same issue (extracting error.data from JSON-RPC errors).