Skip to content

fix(server): a provider's internal error reports what actually failed - #16

Merged
leonardoxr merged 1 commit into
main-xavierfrom
fix/acp-internal-error-details
Aug 19, 2026
Merged

fix(server): a provider's internal error reports what actually failed#16
leonardoxr merged 1 commit into
main-xavierfrom
fix/acp-internal-error-details

Conversation

@leonardoxr

Copy link
Copy Markdown
Owner

Problem

A failed omp turn reported this, and nothing else:

Oh My Pi prompt request failed.
Error: Internal error
    at decodeJsonError (…/Schema-Ch40_dAF.mjs:25144:111)
    at decodeDefect (…)
    …

-32603 "Internal error" is what an ACP agent answers for any unclassified handler throw, with the thrown error's own message tucked into data:

// @oh-my-pi/pi-utils/src/acp/transport.ts
RequestError.internalError({ details: error instanceof Error ? error.message : String(error) })

AcpRequestError keeps that payload, but message returns errorMessage alone and mapAcpToAdapterError reported exactly that. The one useful sentence was on the wire and never made it to the user — and it is not recoverable elsewhere either: the native ACP logs record shapes only (summarizePayload), and the child's stderr is drained.

Fix

The ACP error mapper folds the payload into the reported detail — data.details, a plain string payload, or compact JSON — deduped when the message already contains it, and capped at 600 chars so a runaway payload cannot land whole in the timeline. All three ACP adapters share this mapper, and formatFailureDetail already prefers this detail over a pretty-printed cause.

- Provider adapter request failed (omp) for session/prompt: Internal error
+ Provider adapter request failed (omp) for session/prompt: Internal error: ACP session closed before queued prompt could run

Verification

  • acp-mock-agent.ts gains T3_ACP_PROMPT_ERROR_DETAILS, so a test reproduces the exact wire shape (-32603 + data.details).
  • New OmpAdapter test asserts the sentence reaches turn.completed.errorMessagefails on main-xavier with the assertion above.
  • Three mapper unit tests: details surfaced, no duplication when the message already carries them, runaway payload capped.
  • AcpAdapterSupport + OmpAdapter + GrokAdapter + CursorAdapter: 76 tests green. tsgo clean; lint/format clean.

Known gap, not addressed here: if a provider crashes instead of answering, its stderr is still discarded (packages/effect-acp/src/_internal/stdio.ts drains it), so we get only a process-exit error. omp writes its own log file, which is why I left the transport alone.

Built with Claude Opus 5 on Oh My Pi.

An ACP agent answers any unclassified handler throw with the standard
-32603 "Internal error" and puts the thrown error's own message in `data`.
T3 reported `message` alone, so a failed turn read "Internal error" followed
by the decoder's JavaScript stack — nothing an operator can act on, and the
one useful sentence was already on the wire.

The ACP error mapper now folds that payload into the reported detail
(`data.details`, a plain string payload, or compact JSON), deduped against the
message and capped so a runaway payload cannot land whole in the timeline. All
three ACP adapters share the mapper, and the failure activity already prefers
this detail over a pretty-printed cause.

Built with Claude Opus 5 on Oh My Pi.
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M labels Aug 19, 2026
@leonardoxr
leonardoxr merged commit 2241539 into main-xavier Aug 19, 2026
5 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 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