feat(step-failure-diagnostics): preserve typed session error diagnostics

openspec/changes/archive/2026-09-11-typed-session-errors/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-09-11

openspec/changes/archive/2026-09-11-typed-session-errors/design.md

@@ -0,0 +1,88 @@
+## Context
+
+OpenCode types its `session.error` payload as `ProviderAuthError | UnknownError | MessageOutputLengthError | MessageAbortedError | ApiError` (`@opencode-ai/sdk` 1.18.4, `EventSessionError`). `ApiError.data` carries `statusCode`, `isRetryable`, and the raw response; `ProviderAuthError.data` carries `providerID`. Convoy consumes the event in `describeSessionActivity` (`src/runner.ts`, `case "session.error"`), which returns `{ type: "error", error: formatEventError(properties.error) }` — a `string`. `formatEventError` prefers `message`, then `data.message`, then `name`/`type`, so every field other than the message is gone at the first hop.
+
+Downstream, `watchSession` keeps `lastSessionError: string | undefined`, echoes it as a `session error:` activity line, and finishes the attempt with `new Error(lastSessionError ?? "…without a completed response")` at both idle sites (the event `idle` case and the poll-loop fallback). The attempt boundary then does `if (isMessageAbortedError(result.error)) throw new SessionAbortedError(result.error)` else `throw new LoggedAttemptError(formatSdkError(result.error), { cause: result.error })`. Because the error is a plain `Error`, the abort check never matches for event-delivered aborts. The failure path continues to `writeAttemptLog`, `log.warn([phase] attempt N failed: …)`, `gateError = formatSdkError(error)`, and — after the gate resolves to abort or is `unavailable` headless — the phase `catch` calls `progress.phaseFailed(phase.name, formatSdkError(error))`, where `recordProgress` persists only `phaseEnded(name, "failed")`.
+
+Three places therefore see only a string: the gate, the attempt warning, and `metadata.json`. Everything else (`session.next.step.failed`, `tool.failed`, `retried`) is activity text and is out of scope.
+
+## Goals / Non-Goals
+
+**Goals**
+
+- Preserve the harness classification from the event to the attempt error, the gate text, the attempt warning, and the persisted phase record.
+- Keep every existing string identical when the harness supplies no status.
+- Fix the event-delivered abort so it becomes the existing `SessionAbortedError`.
+- One type for the classification, defined where both `runner.ts` and `metadata.ts` can import it without a new dependency direction.
+
+**Non-Goals**
+
+- Any automatic decision from the classification (retry, model fallback, exit codes) — see the capability's last requirement.
+- Typing `session.next.*` payloads (they are not in the SDK's `Event` union at 1.18.4).
+- Dashboard layout changes: the TUI keeps rendering `detail` strings.
+- Control-channel changes: `ControlProgress.phaseFailed` renders nothing and stays a no-op.
+
+## Decisions
+
+### D1: One `SessionErrorSignal` type, owned by `src/progress.ts`
+
+```ts
+export type SessionErrorSignal = {
+  name: string
+  message: string
+  statusCode?: number
+  isRetryable?: boolean
+  providerID?: string
+}
+```
+
+It lives next to the other progress payload types because `ProgressUI.phaseFailed` carries it and `metadata.ts` persists it; `runner.ts` already imports from `progress.ts`, `metadata.ts` must not import from `runner.ts`. `PhaseMetadata.error` reuses the type verbatim — no parallel `PhaseFailure` shape to keep in sync.
+
+`sessionErrorFromEvent(value: unknown): SessionErrorSignal` in `runner.ts` extracts `name` (falling back to `type`, then `"UnknownError"`), `message` with today's precedence (`message` → `data.message` → name), and the optional fields only when they are of the expected primitive type. `formatEventError(value)` becomes `sessionErrorFromEvent(value).message`, so the three activity-text call sites keep their output byte for byte.
+
+### D2: `SessionError` preserves the classification across the attempt boundary
+
+`SessionSignal`'s error variant becomes `{ type: "error"; error: SessionErrorSignal }`. `watchSession` stores the signal (`lastSessionError: SessionErrorSignal | undefined`), keeps the `session error: <message>` activity line, and finishes with `new SessionError(signal)` at both idle sites; the "went idle / never started" fallbacks stay plain `Error`s because nothing was classified.
+
+```ts
+export class SessionError extends Error {
+  constructor(readonly signal: SessionErrorSignal) {
+    super(signal.message)
+    this.name = "SessionError"
+  }
+}
+```
+
+`name` is pinned to `"SessionError"` and the SDK name lives only in `signal.name`; `isMessageAbortedError` inspects `signal.name` for a `SessionError` and falls back to the raw `name` for an event payload, so the event-delivered abort still turns into `SessionAbortedError`. Pinning `name` matters because the runner recognises its own sentinels by `name` (`isUserAbortError` matches `"UserAbortError"`): a provider that echoed such a name in a session error would otherwise impersonate an operator abort. The classification is not duplicated as fields on the error; `signal` is the single source of truth. `SessionError` deliberately does not extend `LoggedAttemptError`: the attempt boundary (`attemptFailureFor`) wraps it in `LoggedAttemptError` with `cause`, which keeps `writeAttemptLog` and every `instanceof LoggedAttemptError` decision exactly as they are. That wrapper only classifies OpenCode payloads (objects carrying a `name`); a Claude Code failure arrives as a plain string and stays an unclassified attempt failure.
+
+Alternative considered: `name = signal.name` so `isMessageAbortedError` needed no change. Rejected because it lets remote data choose which runner sentinel the error matches.
+
+Alternative considered: attach the signal as `cause` on a plain `Error`. Rejected because every consumer would have to unwrap `cause` to read a status.
+
+### D3: The classification is disclosed by `formatSdkError`
+
+`formatSdkError` gains one branch before the generic `instanceof Error` case:
+
+```ts
+if (error instanceof SessionError) return describeSessionError(error)
+```
+
+`describeSessionError` returns `message` alone when `statusCode` is undefined, otherwise `` `${message} (HTTP ${statusCode}, ${isRetryable ? "retryable" : "not retryable"})` ``. Because the attempt boundary builds the `LoggedAttemptError` message with `formatSdkError(result.error)`, the gate text (`gateError`), the `attempt N failed:` warning, and the phase `catch` all inherit the classified text through the wrapper's `message` without touching those call sites. The gate label and the `waitingFailure` status string are untouched.
+
+### D4: Persisting through an optional `failure` on `phaseFailed`
+
+`ProgressUI.phaseFailed(name: string, detail?: string, failure?: SessionErrorSignal)`. The parameter is additive: `noopProgress`, the TUI, `ControlProgress`, `trackRunStatus`, and hook/human callers compile unchanged. Only `recordProgress` reads it and calls `store.phaseEnded(name, "failed", failure)`; `MetadataStore.phaseEnded(name, status, failure?)` sets `entry.error = failure` when `status === "failed"` and `failure` is present, and never writes the key otherwise. `PhaseMetadata` gains `error?: SessionErrorSignal`.
+
+The phase `catch` computes the failure with `sessionErrorOf(error)`, which walks the `cause` chain (bounded) for a `SessionError` and stops at a `SessionAbortedError`. Two wrappers sit between the phase `catch` and the signal: the attempt boundary wraps the `SessionError` in a `LoggedAttemptError`, and an abort taken at the failure gate throws `UserAbortError` with that attempt error as `cause` — the gate only decides the failed attempt, it does not change why the phase failed, so the classification survives the operator's decision. A cancelled message (`SessionAbortedError`), hook failures, and deliverable-validation errors never carry a classification — matching the spec's "any other reason" scenario.
+
+Alternative considered: persist from `watchSession` directly through a store handle. Rejected: `watchSession` only sees `ProgressUI`, and the metadata store is deliberately reached through the `recordProgress` decorator.
+
+## Risks / Trade-offs
+
+- `test/reproduction.test.ts` asserts the source text `async phaseFailed(name, detail)` in `metadata.ts`; the assertion moves to the three-argument form. This is a test of a past regression, not behavior; the regression it guards (awaiting the store before forwarding) is preserved.
+- Event-delivered aborts change category from `LoggedAttemptError` to `SessionAbortedError`. This is the intended fix; any caller that distinguished the two only did so for prompt-returned aborts and now sees consistent behavior. Called out explicitly in the change description.
+- `metadata.json` readers that iterate phase keys see a new optional `error` object on failed phases. `runs`, `SUMMARY.md`, and the browser read named fields and ignore unknown ones.
+
+## Migration
+
+None. No configuration, CLI, or protocol change; existing `metadata.json` files remain valid (the field is optional).

openspec/changes/archive/2026-09-11-typed-session-errors/proposal.md

@@ -0,0 +1,31 @@
+## Why
+
+When OpenCode reports a `session.error` event, Convoy flattens it to a message string before anything can inspect it: `describeSessionActivity` returns `{ type: "error", error: formatEventError(...) }`, and `formatEventError` keeps only `message` (or `data.message`), discarding the SDK's error `name`, `data.statusCode`, `data.isRetryable`, and `data.providerID`. `watchSession` then finishes the attempt with `new Error(lastSessionError)`. By the time the failure gate shows "step failed — waiting for your decision", the attempt log is written, and `metadata.json` records the phase as `failed`, nobody can tell a retryable `429` from an expired provider credential or a truncated output — the operator opens the OpenCode session to find out, and a headless run leaves no structured trace at all.
+
+The same flattening hides a typed cancellation: `isMessageAbortedError` compares `error.name === "MessageAbortedError"`, but the plain `Error` built from the string never carries that name, so an abort delivered through `session.error` is reported as an ordinary attempt failure instead of a `SessionAbortedError`.
+
+## What Changes
+
+- Carry the SDK error's classification (`name`, `message`, `statusCode`, `isRetryable`, `providerID`) through the session signal as a `SessionErrorSignal`, and finish the attempt with a `SessionError` that preserves it. `formatEventError` keeps returning the exact same string for every other event.
+- Disclose the classification where the operator already looks: the failure gate text and the attempt warning read `<message> (HTTP 429, retryable)` when the SDK provides a status, and stay unchanged otherwise.
+- Persist the classification as `PhaseMetadata.error` when a phase ends `failed` because of a session error, through an optional `failure` argument on `ProgressUI.phaseFailed` that only `recordProgress` consumes.
+- Recognize an aborted message signalled through `session.error` as the existing typed cancellation (`SessionAbortedError`), since the preserved `name` now reaches `isMessageAbortedError`.
+- Introduce no automatic action: no retry, no model fallback, no exit-code change. The failure gate keeps waiting for the operator's decision; a headless run still fails the same way, only with a richer message and record.
+
+## Capabilities
+
+### New Capabilities
+
+- `step-failure-diagnostics`: session errors keep their harness classification through the signal, the failure gate, the attempt log, and the persisted phase metadata, without triggering any automatic decision.
+
+### Modified Capabilities
+
+<!-- None: existing gate, retry, and finalization behavior is unchanged. -->
+
+## Impact
+
+- `src/runner.ts` (`SessionSignal` error variant, `describeSessionActivity`, `formatEventError`, `watchSession`, `formatSdkError`, the phase `catch` that reports `phaseFailed`).
+- `src/progress.ts` (`SessionErrorSignal` type, optional `failure` on `phaseFailed`).
+- `src/metadata.ts` (`PhaseMetadata.error`, `phaseEnded` failure argument, `recordProgress` forwarding).
+- Tests under `test/` for the signal, the gate text, and the persisted metadata.
+- No CLI surface, control-channel protocol, pipeline configuration, or TUI layout change. `metadata.json` gains one optional field on failed phases.

openspec/changes/archive/2026-09-11-typed-session-errors/specs/step-failure-diagnostics/spec.md

@@ -0,0 +1,75 @@
+## Purpose
+
+Keep the harness's own classification of a failed session — error name, HTTP status, retryability, provider — visible to the operator and durable in the run record, so a failed step can be understood without reopening the OpenCode session and without Convoy deciding anything on the operator's behalf.
+
+## ADDED Requirements
+
+### Requirement: Session errors keep their harness classification
+
+When the harness emits a `session.error` event, Convoy SHALL carry the error's classification through its internal session signal instead of a flattened message: the error `name`, the human-readable `message`, and, when the harness provides them, the HTTP `statusCode`, the `isRetryable` flag, and the `providerID`. The message text derived for activity lines SHALL be identical to the text derived today, so existing log and dashboard output does not change for events that carry only a message.
+
+#### Scenario: A rate-limited provider response
+
+- **WHEN** the harness emits `session.error` with an `APIError` whose data carries `statusCode: 429` and `isRetryable: true`
+- **THEN** the session signal exposes `name: "APIError"`, the provider message, `statusCode: 429`, and `isRetryable: true`
+
+#### Scenario: A provider authentication failure
+
+- **WHEN** the harness emits `session.error` with a `ProviderAuthError` for provider `anthropic`
+- **THEN** the session signal exposes `name: "ProviderAuthError"`, the message, and `providerID: "anthropic"`, with no status or retryability claimed
+
+#### Scenario: An error without a message
+
+- **WHEN** the harness emits `session.error` with a `MessageOutputLengthError` whose data carries no message
+- **THEN** the session signal uses the error name as its message, exactly as the flattened text did before
+
+### Requirement: The failure gate and the attempt log disclose the classification
+
+When an attempt fails because of a classified session error, the failure gate's error text and the attempt warning SHALL append the classification to the message in the form `<message> (HTTP <status>, retryable)` or `<message> (HTTP <status>, not retryable)`. When the harness provided no status, the text SHALL be the message alone, unchanged from today. The gate label "step failed — waiting for your decision" and the gate's choices SHALL NOT change.
+
+#### Scenario: A retryable status reaches the gate
+
+- **WHEN** an attempt fails with a session error carrying `statusCode: 429` and `isRetryable: true`
+- **THEN** the gate error and the `attempt N failed:` warning read `<message> (HTTP 429, retryable)`
+
+#### Scenario: An unclassified error reaches the gate
+
+- **WHEN** an attempt fails with an `UnknownError` that carries only a message
+- **THEN** the gate error and the warning show the message with nothing appended
+
+### Requirement: Failed phase metadata records the classification
+
+When a phase ends `failed` because of a classified session error, the run's `metadata.json` SHALL record the classification on that phase as `error` with the same fields as the session signal. The failure gate's decision does not change the reason the phase failed: an abort chosen at the gate — or a run-wide shutdown while the gate waits — SHALL keep the failed attempt's classification. A phase that fails for any other reason — a cancelled message, hook failure, deliverable validation — SHALL NOT gain an `error` field, and successful or skipped phases SHALL be unaffected.
+
+#### Scenario: A step fails on a provider error and the operator aborts at the gate
+
+- **WHEN** a step's attempt fails with a `ProviderAuthError` and the operator chooses abort at the failure gate
+- **THEN** the run exits as it does today and the failed phase in `metadata.json` carries `error: { name: "ProviderAuthError", message, providerID }`
+
+#### Scenario: An operator cancels the message itself
+
+- **WHEN** the operator aborts the message (Esc) and then chooses abort at the failure gate
+- **THEN** the phase is recorded as `failed` without an `error` classification
+
+### Requirement: An aborted message signalled through the harness is a typed cancellation
+
+When the harness delivers `MessageAbortedError` through `session.error`, Convoy SHALL treat it as the same typed cancellation it already recognizes when the prompt call itself returns that error, so an Esc abort is never reported as an ordinary attempt failure.
+
+#### Scenario: Abort arrives as an event
+
+- **WHEN** the operator aborts the message and the harness reports it only through `session.error` with `name: "MessageAbortedError"`
+- **THEN** the attempt ends with the typed cancellation and the gate opens as it does for an abort returned by the prompt call
+
+### Requirement: Classification never triggers automatic action
+
+The classification SHALL be informational only. Convoy SHALL NOT retry an attempt, switch models, alter the gate's choices, or change the process exit code based on `statusCode` or `isRetryable`; a failed step still waits for the operator's decision, and a headless run still fails without one.
+
+#### Scenario: A retryable error in an interactive run
+
+- **WHEN** an attempt fails with `isRetryable: true` while a controller is attached
+- **THEN** the failure gate opens with the classified text and waits; no retry starts on its own
+
+#### Scenario: A retryable error in a headless run
+
+- **WHEN** an attempt fails with `isRetryable: true` and no controller is attached
+- **THEN** the run fails with the same exit code as before, with the classified text in the log and the record