diff --git a/openspec/changes/archive/2026-09-11-typed-session-errors/.openspec.yaml b/openspec/changes/archive/2026-09-11-typed-session-errors/.openspec.yaml new file mode 100644 index 0000000..515eaae --- /dev/null +++ b/openspec/changes/archive/2026-09-11-typed-session-errors/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-11 diff --git a/openspec/changes/archive/2026-09-11-typed-session-errors/design.md b/openspec/changes/archive/2026-09-11-typed-session-errors/design.md new file mode 100644 index 0000000..2198fbb --- /dev/null +++ b/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: ` 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). diff --git a/openspec/changes/archive/2026-09-11-typed-session-errors/proposal.md b/openspec/changes/archive/2026-09-11-typed-session-errors/proposal.md new file mode 100644 index 0000000..bf7d2c7 --- /dev/null +++ b/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 ` (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 + + + +## 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. diff --git a/openspec/changes/archive/2026-09-11-typed-session-errors/specs/step-failure-diagnostics/spec.md b/openspec/changes/archive/2026-09-11-typed-session-errors/specs/step-failure-diagnostics/spec.md new file mode 100644 index 0000000..2272054 --- /dev/null +++ b/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 ` (HTTP , retryable)` or ` (HTTP , 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 ` (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 diff --git a/openspec/changes/archive/2026-09-11-typed-session-errors/tasks.md b/openspec/changes/archive/2026-09-11-typed-session-errors/tasks.md new file mode 100644 index 0000000..a8f414e --- /dev/null +++ b/openspec/changes/archive/2026-09-11-typed-session-errors/tasks.md @@ -0,0 +1,24 @@ +## 1. Typed session error signal + +- [x] 1.1 In `src/progress.ts`, add and export `SessionErrorSignal` (design D1) and widen `ProgressUI.phaseFailed` to `(name, detail?, failure?: SessionErrorSignal)`. Verification: `bun run typecheck` passes with every existing `phaseFailed` implementation unchanged. +- [x] 1.2 In `src/runner.ts`, add `sessionErrorFromEvent(value: unknown): SessionErrorSignal` (exported for tests), make `formatEventError` return its `message`, and change the `SessionSignal` error variant to carry the signal from `describeSessionActivity`'s `session.error` case. Verification: `test/runner.test.ts` cases for `APIError` 429 retryable, `ProviderAuthError` with `providerID`, `UnknownError`, `MessageOutputLengthError` without message (name as message), and a non-object payload; a snapshot of `formatEventError` output for each payload equals the pre-change string. +- [x] 1.3 In `src/runner.ts`, add `SessionError` (design D2), store the signal in `watchSession`'s `lastSessionError`, keep the `session error: ` activity line, and finish with `new SessionError(signal)` at both idle sites. Verification: a `watchSession` test with a fake client yielding `session.error` (429) then `session.idle` rejects with a `SessionError` whose `signal.statusCode`/`signal.isRetryable` match; a Claude Code failure string stays an unclassified `LoggedAttemptError`; a session error named like a runner sentinel never satisfies `isUserAbortError`; a `MessageAbortedError` event yields an error for which `isMessageAbortedError` is true and the attempt boundary throws `SessionAbortedError`. + +## 2. Failure gate and attempt log + +- [x] 2.1 In `src/runner.ts`, add `describeSessionError` and the `SessionError` branch at the top of `formatSdkError` (design D3). Verification: unit tests — `SessionError` with `statusCode: 429, isRetryable: true` → `" (HTTP 429, retryable)"`; with `isRetryable: false` → `"… not retryable)"`; without `statusCode` → `""`; a plain `Error` and an SDK-shaped object format as before. +- [x] 2.2 Confirm through the existing attempt-boundary path that `LoggedAttemptError.message`, `gateError`, and the `attempt N failed:` warning carry the classified text without editing those lines. Verification: a runner test that drives one failed attempt with a 429 `SessionError` asserts the `phaseRunning`/warning text and the gate `error` field. + +## 3. Persist the classification + +- [x] 3.1 In `src/metadata.ts`, add `PhaseMetadata.error?: SessionErrorSignal`, extend `MetadataStore.phaseEnded(name, status, failure?)` to set `error` only when `status === "failed"` and `failure` is given, and forward the third argument from `recordProgress.phaseFailed(name, detail, failure)` (design D4). Verification: `test/metadata.test.ts` — `phaseEnded("build", "failed", signal)` persists `error` in `metadata.json`; `phaseEnded("build", "failed")` leaves the key absent; `recordProgress` forwards `failure` to the store and the wrapped UI; `test/reproduction.test.ts` source assertion updated to the three-argument signature. +- [x] 3.2 In the phase `catch` of `src/runner.ts` (the `progress.phaseFailed(phase.name, formatSdkError(error))` site), pass `sessionErrorOf(error)`, which walks the `cause` chain; the failure gate's abort rethrows `UserAbortError` with the attempt error as `cause`. Verification: runner test asserting `phaseFailed` receives the signal for a `LoggedAttemptError` wrapping a `SessionError`, for an abort answered at the failure gate, and `undefined` for a cancelled message. + +## 4. Tests and coverage + +- [x] 4.1 Run `bun run typecheck` and `bun test`; all pass. Verification: `bun run test:coverage` stays at or above the threshold enforced by `.github/workflows/verify.yml`. + +## 5. Verify + +- [x] 5.1 `openspec validate typed-session-errors --strict`. +- [x] 5.2 Headless smoke: run a one-step pipeline with `--no-tui` against a provider with an invalid API key. Verification: the log shows `attempt 1 failed: ` with the provider classification, the exit code is unchanged from before this change, and after aborting at the failure gate the run's `metadata.json` carries the classification on the failed phase (an invalid Anthropic key arrives from the SDK as `APIError` with `statusCode: 401, isRetryable: false`). diff --git a/openspec/specs/step-failure-diagnostics/spec.md b/openspec/specs/step-failure-diagnostics/spec.md new file mode 100644 index 0000000..c37dcf5 --- /dev/null +++ b/openspec/specs/step-failure-diagnostics/spec.md @@ -0,0 +1,76 @@ +# step-failure-diagnostics Specification + +## 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. + +## 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 ` (HTTP , retryable)` or ` (HTTP , 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 ` (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 diff --git a/src/metadata.ts b/src/metadata.ts index f3bf72b..273e988 100644 --- a/src/metadata.ts +++ b/src/metadata.ts @@ -10,6 +10,7 @@ import type { ProgressPhase, ProgressPhaseSnapshot, ProgressStepUsage, + SessionErrorSignal, ProgressTokens, ProgressUI, ProgressUsage, @@ -48,6 +49,8 @@ export type PhaseMetadata = { repositoryBaseline?: RepoSnapshot advisor?: AdvisorPhaseAggregate advisorEvents?: AdvisorEvent[] + /** Harness-provided classification for failures caused by a session error. */ + error?: SessionErrorSignal } /** The stage a goal cycle's durable record says the run is in. */ @@ -166,7 +169,7 @@ export type RunMetadataStore = { phaseAdvisorEvent(name: string, event: AdvisorEvent): void repositoryBaseline(name: string): RepoSnapshot | undefined phaseRepositoryBaseline(name: string, baseline: RepoSnapshot): Promise - phaseEnded(name: string, status: "completed" | "skipped" | "failed", detail?: string): Promise + phaseEnded(name: string, status: "completed" | "skipped" | "failed", detail?: string, failure?: SessionErrorSignal): Promise controlState(): RunControlState setControlState(state: RunControlState): Promise flush(): Promise @@ -399,6 +402,7 @@ export async function openRunMetadata( async phaseStarted(name) { const entry = phase(name) entry.status = "running" + delete entry.error entry.startedAt ??= Date.now() await persist({ throwOnError: true }) }, @@ -432,9 +436,11 @@ export async function openRunMetadata( phase(name).repositoryBaseline = baseline await persist({ throwOnError: true }) }, - async phaseEnded(name, status, detail) { + async phaseEnded(name, status, detail, failure) { const entry = phase(name) entry.status = status + if (status === "failed" && failure) entry.error = failure + else delete entry.error entry.endedAt = Date.now() if (detail) entry.detail = detail if (entry.startedAt !== undefined) entry.durationMs = entry.endedAt - entry.startedAt @@ -533,9 +539,9 @@ export function recordProgress(progress: ProgressUI, store: RunMetadataStore): P await store.phaseEnded(name, "skipped").catch((error) => log.warn(`couldn't persist phase-skipped metadata: ${String(error)}`)) progress.phaseSkipped(name) }, - async phaseFailed(name, detail) { - await store.phaseEnded(name, "failed", detail).catch((error) => log.warn(`couldn't persist phase-failed metadata: ${String(error)}`)) - progress.phaseFailed(name, detail) + async phaseFailed(name, detail, failure) { + await store.phaseEnded(name, "failed", detail, failure).catch((error) => log.warn(`couldn't persist phase-failed metadata: ${String(error)}`)) + progress.phaseFailed(name, detail, failure) }, phaseRestored: (name, snapshot) => progress.phaseRestored(name, snapshot), message: (message) => progress.message(message), diff --git a/src/progress.ts b/src/progress.ts index 8141015..1f8a01b 100644 --- a/src/progress.ts +++ b/src/progress.ts @@ -108,6 +108,15 @@ export type ProgressDiffSummary = { deletions: number } +/** Harness classification retained for a session error without changing activity text. */ +export type SessionErrorSignal = { + name: string + message: string + statusCode?: number + isRetryable?: boolean + providerID?: string +} + export type PermissionReply = "once" | "always" | "reject" /** @@ -356,7 +365,7 @@ export type ProgressUI = { phaseDiff(name: string, summary: ProgressDiffSummary): void phaseCompleted(name: string, detail?: string): void phaseSkipped(name: string): void - phaseFailed(name: string, detail?: string): void + phaseFailed(name: string, detail?: string, failure?: SessionErrorSignal): void /** Replays a phase finished in a previous run (--resume) with its real duration, cost, and session. */ phaseRestored(name: string, snapshot: ProgressPhaseSnapshot): void /** When present, the UI resolves permission prompts itself (no terminal fallback). */ diff --git a/src/run-status.ts b/src/run-status.ts index ff3e18b..0a73e56 100644 --- a/src/run-status.ts +++ b/src/run-status.ts @@ -488,10 +488,10 @@ export function trackRunStatus(progress: ProgressUI, tracker: RunStatusTracker): tracker.phaseEnded(name, "skipped") progress.phaseSkipped(name) }, - phaseFailed(name, detail) { + phaseFailed(name, detail, failure) { endHumanWait(name) tracker.phaseEnded(name, "failed") - progress.phaseFailed(name, detail) + progress.phaseFailed(name, detail, failure) }, phaseRestored(name, snapshot) { tracker.phaseEnded(name, snapshot.status) diff --git a/src/runner.ts b/src/runner.ts index ee9acb9..5fd0d91 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -57,6 +57,7 @@ import { type ProgressUsage, type RunControlState, type RunOutcome, + type SessionErrorSignal, } from "./progress" import { discoverProjectContextFiles } from "./project-context" import { createStepRunnerImpl, stepRunnerFor, stepRunnerModel, type StepRunnerId, type StepRunnerImpl } from "./step-runners" @@ -74,8 +75,8 @@ export type ActiveSession = { } export class UserAbortError extends Error { - constructor(message = "aborted by user") { - super(message) + constructor(message = "aborted by user", options?: ErrorOptions) { + super(message, options) this.name = "UserAbortError" } } @@ -1511,7 +1512,7 @@ async function runPhase( ) progress.phaseCompleted(phase.name, "report saved and commit checked") } catch (error) { - progress.phaseFailed(phase.name, formatSdkError(error)) + progress.phaseFailed(phase.name, formatSdkError(error), sessionErrorOf(error)) throw error } } @@ -1845,6 +1846,11 @@ export async function runPhaseUntilResolved( }, runner: phase.runner, runDir: workspace.dir, + }).catch((gateFailure: unknown) => { + // The gate only decides the failed attempt; an abort taken there does + // not change why the phase failed, so the attempt error travels on as + // the cause and the phase record keeps its session classification. + throw isUserAbortError(gateFailure) ? new UserAbortError(gateFailure.message, { cause: error }) : gateFailure }) if (outcome === "continue") { // The failed attempt's chat text is gone; the rescued deliverable can @@ -2093,10 +2099,7 @@ async function runPhaseAttempt( text: result.assistantText, }) - if (result.error) { - if (isMessageAbortedError(result.error)) throw new SessionAbortedError(result.error) - throw new LoggedAttemptError(formatSdkError(result.error), { cause: result.error }) - } + if (result.error) throw attemptFailureFor(result.error) return result.assistantText } @@ -2492,6 +2495,7 @@ export async function promptPhase( if (!input.shutdown.aborted && !isUserAbortError(error)) { await abortSessionQuietly(client, session.data.id, input.targetDir, input.phase.name) } + if (isMessageAbortedError(error)) throw new SessionAbortedError(error) throw error } finally { // The report/advisor handles must outlive the idle/failed prompt: the human @@ -2775,7 +2779,7 @@ type SessionSignal = | { type: "todos"; todos: ProgressTodo[]; message: string } | { type: "diff"; summary: ProgressDiffSummary } | { type: "idle" } - | { type: "error"; error: string } + | { type: "error"; error: SessionErrorSignal } const sessionPollMs = 30_000 const maxConsecutivePollFailures = 10 @@ -2841,7 +2845,7 @@ export function watchSession( let settled = false let sawWork = false let idlePollsWithoutResult = 0 - let lastSessionError: string | undefined + let lastSessionError: SessionErrorSignal | undefined let verifying: Promise | undefined let resolveResult!: (value: SessionResult) => void @@ -2904,6 +2908,10 @@ export function watchSession( const turn = anchor === -1 ? assistant : assistant.slice(anchor + 1) const last = turn[turn.length - 1] if (!last || (!last.info.time.completed && !last.info.error)) return false + if (last.info.error && lastSessionError) { + finish({ error: new SessionError(lastSessionError) }) + return true + } finish({ value: { info: last.info, @@ -2943,13 +2951,13 @@ export function watchSession( return case "error": lastSessionError = signal.error - input.progress.phaseActivity(input.phaseName, `session error: ${signal.error}`, "error") + input.progress.phaseActivity(input.phaseName, `session error: ${signal.error.message}`, "error") await verifyCompletion() return case "idle": input.progress.phaseActivity(input.phaseName, "session idle; collecting results", "info") if (!(await verifyCompletion()) && sawWork) { - finish({ error: new Error(lastSessionError ?? "session went idle without a completed response") }) + finish({ error: lastSessionError ? new SessionError(lastSessionError) : new Error("session went idle without a completed response") }) } return } @@ -3002,7 +3010,7 @@ export function watchSession( idlePollsWithoutResult++ const limit = sawWork ? 2 : 4 if (idlePollsWithoutResult >= limit) { - finish({ error: new Error(lastSessionError ?? `session ${sawWork ? "went idle" : "never started"} without a completed response`) }) + finish({ error: lastSessionError ? new SessionError(lastSessionError) : new Error(`session ${sawWork ? "went idle" : "never started"} without a completed response`) }) return } } else { @@ -3186,7 +3194,7 @@ export function describeSessionActivity(payload: unknown, state: ActivityState): case "session.diff": return { type: "diff", summary: diffSummaryFromEvent(properties.diff) } case "session.error": - return { type: "error", error: formatEventError(properties.error) } + return { type: "error", error: sessionErrorFromEvent(properties.error) } default: if (type.startsWith("session.next.")) return activity("info", type.replace(/^session\.next\./, "")) return undefined @@ -3659,13 +3667,24 @@ function describeToolContent(value: unknown) { return "done" } -function formatEventError(value: unknown) { - if (!value || typeof value !== "object") return String(value ?? "unknown error") - const message = (value as { message?: unknown }).message - if (typeof message === "string") return message - const data = (value as { data?: unknown }).data - if (data && typeof data === "object" && typeof (data as { message?: unknown }).message === "string") return (data as { message: string }).message - return String((value as { name?: unknown; type?: unknown }).name ?? (value as { type?: unknown }).type ?? "unknown error") +export function sessionErrorFromEvent(value: unknown): SessionErrorSignal { + if (!value || typeof value !== "object") return { name: "UnknownError", message: String(value ?? "unknown error") } + const error = value as { name?: unknown; type?: unknown; message?: unknown; data?: unknown } + const data = error.data && typeof error.data === "object" ? (error.data as Record) : undefined + const suppliedName = typeof error.name === "string" ? error.name : typeof error.type === "string" ? error.type : undefined + const name = suppliedName ?? "UnknownError" + const message = typeof error.message === "string" ? error.message : typeof data?.message === "string" ? data.message : suppliedName ?? "unknown error" + return { + name, + message, + ...(typeof data?.statusCode === "number" ? { statusCode: data.statusCode } : {}), + ...(typeof data?.isRetryable === "boolean" ? { isRetryable: data.isRetryable } : {}), + ...(typeof data?.providerID === "string" ? { providerID: data.providerID } : {}), + } +} + +export function formatEventError(value: unknown) { + return sessionErrorFromEvent(value).message } function pickString(values: Record, keys: string[]) { @@ -3933,18 +3952,66 @@ class LoggedAttemptError extends Error { } } +/** + * An OpenCode session error with its original machine-readable classification. + * The SDK name lives in `signal.name` only: `Error.name` stays fixed so a + * provider payload can never impersonate a harness sentinel such as + * `UserAbortError`, which is recognised by name. + */ +export class SessionError extends Error { + constructor(readonly signal: SessionErrorSignal) { + super(signal.message) + this.name = "SessionError" + } +} + /** Typed cancellation returned when Esc aborts an OpenCode message. */ export class SessionAbortedError extends LoggedAttemptError { - constructor(error: { name: "MessageAbortedError"; data?: { message?: string } }) { - super(error.data?.message || "OpenCode session message aborted", { cause: error }) + constructor(error: { name: "MessageAbortedError"; data?: { message?: string } } | SessionError) { + super(error instanceof SessionError ? error.message : error.data?.message || "OpenCode session message aborted", { cause: error }) this.name = "SessionAbortedError" } } -export function isMessageAbortedError(error: unknown): error is { name: "MessageAbortedError"; data?: { message?: string } } { +export function isMessageAbortedError(error: unknown): error is { name: "MessageAbortedError"; data?: { message?: string } } | SessionError { + if (error instanceof SessionError) return error.signal.name === "MessageAbortedError" return Boolean(error && typeof error === "object" && "name" in error && error.name === "MessageAbortedError") } +/** + * The session classification behind a phase failure, found through the `cause` + * chain: the attempt boundary and the failure gate each add one wrapper. A + * cancelled message stays a cancellation wherever it sits in the chain. + */ +export function sessionErrorOf(error: unknown): SessionErrorSignal | undefined { + for (let current = error, depth = 0; current instanceof Error && depth < maxCauseDepth; current = current.cause, depth++) { + if (current instanceof SessionAbortedError) return undefined + if (current instanceof SessionError) return current.signal + } + return undefined +} + +const maxCauseDepth = 8 + +/** + * The attempt-boundary failure for a raw SDK error carried by the terminal + * assistant message. It gets the same classification as an event-delivered + * `session.error`, so the gate text and the phase metadata do not depend on + * which of the two representations reached the harness first. + */ +export function attemptFailureFor(error: unknown): LoggedAttemptError { + if (isMessageAbortedError(error)) return new SessionAbortedError(error) + // Only an OpenCode session error carries a classification. A Claude Code + // failure arrives as a plain string and stays an ordinary attempt failure. + if (!isSessionErrorPayload(error)) return new LoggedAttemptError(formatSdkError(error), { cause: error }) + const sessionError = new SessionError(sessionErrorFromEvent(error)) + return new LoggedAttemptError(describeSessionError(sessionError), { cause: sessionError }) +} + +function isSessionErrorPayload(value: unknown): value is { name: string } { + return Boolean(value && typeof value === "object" && "name" in value && typeof value.name === "string") +} + export function extractAssistantText(parts: readonly Part[]) { return parts .filter((part): part is Part & { type: "text"; text: string } => part.type === "text") @@ -4005,7 +4072,8 @@ async function exists(path: string) { } } -function formatSdkError(error: unknown): string { +export function formatSdkError(error: unknown): string { + if (error instanceof SessionError) return describeSessionError(error) if (error instanceof Error) return error.message if (typeof error === "object" && error && "data" in error) { const data = (error as { data?: unknown }).data @@ -4014,3 +4082,9 @@ function formatSdkError(error: unknown): string { if (typeof error === "object" && error && "name" in error) return String((error as { name?: unknown }).name) return String(error) } + +export function describeSessionError(error: SessionError): string { + const { statusCode, isRetryable } = error.signal + if (statusCode === undefined) return error.message + return `${error.message} (HTTP ${statusCode}, ${isRetryable ? "retryable" : "not retryable"})` +} diff --git a/test/metadata.test.ts b/test/metadata.test.ts index 7b787ee..21681c1 100644 --- a/test/metadata.test.ts +++ b/test/metadata.test.ts @@ -7,7 +7,7 @@ import { readRunMetadata, openRunMetadata, recordProgress, type RunMetadataStore import type { RepoSnapshot } from "../src/git" import type { Pipeline, AgentStep, HumanStep } from "../src/types" import type { Workspace } from "../src/workspace" -import type { ProgressUI, GoalLoopView } from "../src/progress" +import type { ProgressUI, GoalLoopView, SessionErrorSignal } from "../src/progress" import type { AdvisorEvent } from "../src/advisor-events" function validAgentStep(name: string): AgentStep { @@ -386,6 +386,34 @@ describe("openRunMetadata", () => { } }) + test("persists a classified session error only while a phase is failed", async () => { + const { dir, ws, cleanup } = await withDir("failure-signal") + const failure: SessionErrorSignal = { name: "ProviderAuthError", message: "expired key", providerID: "anthropic" } + const store = await openRunMetadata(ws, "/target", validPipeline([validAgentStep("design"), validAgentStep("code")])) + try { + await store.phaseEnded("design", "failed", undefined, failure) + await store.phaseEnded("code", "failed") + await store.flush() + const metadata = (await readRunMetadata(`${dir}/metadata.json`))! + expect(metadata.phases.design?.error).toEqual(failure) + expect(metadata.phases.code?.error).toBeUndefined() + + await store.phaseStarted("design") + await store.flush() + const restarted = (await readRunMetadata(`${dir}/metadata.json`))! + expect(restarted.phases.design?.status).toBe("running") + expect(restarted.phases.design?.error).toBeUndefined() + + await store.phaseEnded("design", "failed", undefined, failure) + await store.phaseEnded("design", "completed") + await store.flush() + const recovered = (await readRunMetadata(`${dir}/metadata.json`))! + expect(recovered.phases.design?.error).toBeUndefined() + } finally { + await cleanup() + } + }) + test("serverStarted and serverStopped", async () => { const { dir, ws, cleanup } = await withDir("srv") const store = await openRunMetadata(ws, "/target", validPipeline([validAgentStep("design")])) @@ -1042,6 +1070,30 @@ describe("recordProgress", () => { expect(storeCalls).toContain("phaseEnded(test, failed)") }) + test("forwards failure classifications to metadata without changing the UI contract", async () => { + const calls: string[] = [] + const failures: SessionErrorSignal[] = [] + const uiFailures: SessionErrorSignal[] = [] + const fakeUI = makeFakeUI(calls) + fakeUI.phaseFailed = (name, _detail, failure) => { + calls.push(`phaseFailed(${name})`) + if (failure) uiFailures.push(failure) + } + const mockStore = makeMockStore([]) + mockStore.phaseEnded = (_name, _status, _detail, failure) => { + if (failure) failures.push(failure) + return Promise.resolve() + } + const recorder = recordProgress(fakeUI, mockStore) + const failure = { name: "APIError", message: "rate limited", statusCode: 429, isRetryable: true } + + await recorder.phaseFailed("test", "rate limited", failure) + + expect(failures).toEqual([failure]) + expect(uiFailures).toEqual([failure]) + expect(calls).toContain("phaseFailed(test)") + }) + test("forwards suspend, resume, stop, message", async () => { const calls: string[] = [] const fakeUI = makeFakeUI(calls) diff --git a/test/reproduction.test.ts b/test/reproduction.test.ts index 698b48d..1a8dfee 100644 --- a/test/reproduction.test.ts +++ b/test/reproduction.test.ts @@ -244,7 +244,7 @@ describe("HN-002: metadata lifecycle methods", () => { expect(phaseStartedAwait).not.toBeNull() // phaseEnded now uses `await persist({ throwOnError: true })` - const phaseEndedAwait = source.match(/async phaseEnded[\s\S]{0,300}await persist\(\{ throwOnError: true \}\)/) + const phaseEndedAwait = source.match(/async phaseEnded[\s\S]{0,400}await persist\(\{ throwOnError: true \}\)/) expect(phaseEndedAwait).not.toBeNull() // serverStopped now uses `await persist({ throwOnError: true })` @@ -276,7 +276,7 @@ describe("HN-002: metadata lifecycle methods", () => { // The interface should now declare Promise so callers must await expect(source).toContain("serverStopped(): Promise") expect(source).toContain("phaseStarted(name: string): Promise") - expect(source).toContain('phaseEnded(name: string, status: "completed" | "skipped" | "failed", detail?: string): Promise') + expect(source).toContain('phaseEnded(name: string, status: "completed" | "skipped" | "failed", detail?: string, failure?: SessionErrorSignal): Promise') }) test("direct call sites in runner.ts await the store methods (HN-002 fix)", async () => { @@ -307,8 +307,8 @@ describe("HN-002: metadata lifecycle methods", () => { expect(source).toContain("async phaseSkipped(name)") expect(source).toContain('await store.phaseEnded(name, "skipped").catch(') // phaseFailed callback should be async and await store.phaseEnded with .catch() - expect(source).toContain("async phaseFailed(name, detail)") - expect(source).toContain('await store.phaseEnded(name, "failed", detail).catch(') + expect(source).toContain("async phaseFailed(name, detail, failure)") + expect(source).toContain('await store.phaseEnded(name, "failed", detail, failure).catch(') }) }) diff --git a/test/run-status.test.ts b/test/run-status.test.ts index 1eaccd5..f53c40b 100644 --- a/test/run-status.test.ts +++ b/test/run-status.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test" -import { noopProgress, type ProgressPhase, type ProgressUI, type RunStatus } from "../src/progress" +import { noopProgress, type ProgressPhase, type ProgressUI, type RunStatus, type SessionErrorSignal } from "../src/progress" import { planBatches } from "../src/runner" import { formatTerminalTitle, @@ -456,23 +456,29 @@ describe("trackRunStatus", () => { test("forwards every lifecycle call to the wrapped UI unchanged", () => { const calls: string[] = [] + let forwardedFailure: SessionErrorSignal | undefined const progress: ProgressUI = { ...noopProgress, phaseStarted: (name) => calls.push(`started:${name}`), phaseCompleted: (name) => calls.push(`completed:${name}`), - phaseFailed: (name) => calls.push(`failed:${name}`), + phaseFailed: (name, _detail, failure) => { + calls.push(`failed:${name}`) + forwardedFailure = failure + }, phaseSkipped: (name) => calls.push(`skipped:${name}`), stop: () => calls.push("stop"), } const wrapped = trackRunStatus(progress, new RunStatusTracker({ phases: [agentPhase("plan")], identity })) + const failure = { name: "APIError", message: "rate limited", statusCode: 429, isRetryable: true } wrapped.phaseStarted("plan") wrapped.phaseCompleted("plan") - wrapped.phaseFailed("plan") + wrapped.phaseFailed("plan", "rate limited", failure) wrapped.phaseSkipped("plan") wrapped.stop() expect(calls).toEqual(["started:plan", "completed:plan", "failed:plan", "skipped:plan", "stop"]) + expect(forwardedFailure).toEqual(failure) }) test("stop() marks the run stopped before the wrapped UI tears its renderer down", () => { diff --git a/test/runner.test.ts b/test/runner.test.ts index 019bde7..5948322 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -12,6 +12,7 @@ import { RunShutdown, RunControl, SessionAbortedError, + SessionError, UserAbortError, PhaseGroupError, waitForPhaseGate, @@ -36,6 +37,9 @@ import { applyReportCheckpoint, resolveDeliverableCandidate, runPhaseUntilResolved, + attemptFailureFor, + sessionErrorFromEvent, + sessionErrorOf, restorePhaseFromPreviousRun, selectInterruptedPhase, shouldSkip, @@ -43,6 +47,8 @@ import { watchSession, withReadOnlyRepositoryBoundary, softBudgetNudgeText, + formatEventError, + formatSdkError, type ActiveSession, } from "../src/runner" import type { AgentStep, DeliverableContract, HumanStep, Pipeline, Step } from "../src/types" @@ -220,6 +226,125 @@ describe("runner helpers", () => { expect(wrapped.cause).toBe(error) }) + test("preserves session-error classifications while retaining legacy message formatting", () => { + expect(sessionErrorFromEvent({ name: "APIError", message: "rate limited", data: { statusCode: 429, isRetryable: true } })).toEqual({ + name: "APIError", + message: "rate limited", + statusCode: 429, + isRetryable: true, + }) + expect(sessionErrorFromEvent({ name: "ProviderAuthError", data: { message: "expired key", providerID: "anthropic" } })).toEqual({ + name: "ProviderAuthError", + message: "expired key", + providerID: "anthropic", + }) + expect(sessionErrorFromEvent({ name: "MessageOutputLengthError", data: {} })).toEqual({ + name: "MessageOutputLengthError", + message: "MessageOutputLengthError", + }) + expect(sessionErrorFromEvent(undefined)).toEqual({ name: "UnknownError", message: "unknown error" }) + }) + + test("keeps legacy event-error text for every message fallback", () => { + const cases: Array<[unknown, string]> = [ + [{ message: "direct message", data: { message: "nested message" } }, "direct message"], + [{ data: { message: "nested message" } }, "nested message"], + [{ name: "MessageOutputLengthError", data: {} }, "MessageOutputLengthError"], + [{ type: "UnknownError" }, "UnknownError"], + [{}, "unknown error"], + [[], "unknown error"], + [undefined, "unknown error"], + ] + + for (const [payload, expected] of cases) expect(formatEventError(payload)).toBe(expected) + }) + + test("retains only valid primitive classifications from a session error", () => { + expect(sessionErrorFromEvent({ name: "APIError", data: { statusCode: "429", isRetryable: 1, providerID: 42 } })).toEqual({ + name: "APIError", + message: "APIError", + }) + }) + + test("emits classified session-error signals while activity text remains unchanged", () => { + const signal = describeSessionActivity( + { type: "session.error", properties: { error: { name: "APIError", data: { message: "rate limited", statusCode: 429, isRetryable: true } } } }, + newActivityState(), + ) + + expect(signal).toEqual({ + type: "error", + error: { name: "APIError", message: "rate limited", statusCode: 429, isRetryable: true }, + }) + expect(describeSessionActivity({ type: "session.next.step.failed", properties: { error: { data: { message: "rate limited" } } } }, newActivityState())).toEqual({ + type: "activity", + kind: "error", + message: "step failed: rate limited", + }) + }) + + test("formats typed session errors for failure surfaces without changing unclassified errors", () => { + expect(formatSdkError(new SessionError({ name: "APIError", message: "rate limited", statusCode: 429, isRetryable: true }))).toBe( + "rate limited (HTTP 429, retryable)", + ) + expect(formatSdkError(new SessionError({ name: "APIError", message: "bad request", statusCode: 400, isRetryable: false }))).toBe( + "bad request (HTTP 400, not retryable)", + ) + expect(formatSdkError(new SessionError({ name: "UnknownError", message: "plain failure" }))).toBe("plain failure") + expect(formatSdkError(new Error("unchanged"))).toBe("unchanged") + }) + + test("turns event-delivered message aborts into typed cancellations", () => { + const error = new SessionError({ name: "MessageAbortedError", message: "stopped" }) + expect(isMessageAbortedError(error)).toBeTrue() + expect(new SessionAbortedError(error).cause).toBe(error) + }) + + test("finds a session classification directly or through the attempt wrapper", () => { + const failure = new SessionError({ name: "ProviderAuthError", message: "expired key", providerID: "anthropic" }) + expect(sessionErrorOf(failure)).toEqual(failure.signal) + expect(sessionErrorOf(new Error("attempt failed", { cause: failure }))).toEqual(failure.signal) + expect(sessionErrorOf(new SessionAbortedError(failure))).toBeUndefined() + expect(sessionErrorOf(new Error("operator abort"))).toBeUndefined() + // An abort answered at the failure gate wraps the attempt error one level deeper. + const gateAbort = new UserAbortError("aborted from phase gate", { cause: new Error("attempt failed", { cause: failure }) }) + expect(sessionErrorOf(gateAbort)).toEqual(failure.signal) + // A cancelled message stays a cancellation however deep it sits. + expect(sessionErrorOf(new UserAbortError("aborted from phase gate", { cause: new SessionAbortedError(failure) }))).toBeUndefined() + }) + + test("classifies a message-level session error at the attempt boundary like an event-delivered one", () => { + // The terminal assistant message can carry the raw SDK error without a + // matching `session.error` event (the completion poll can win the race). + const rateLimited = attemptFailureFor({ name: "APIError", data: { message: "rate limited", statusCode: 429, isRetryable: true } }) + expect(rateLimited.message).toBe("rate limited (HTTP 429, retryable)") + expect(rateLimited.cause).toBeInstanceOf(SessionError) + expect(sessionErrorOf(rateLimited)).toEqual({ name: "APIError", message: "rate limited", statusCode: 429, isRetryable: true }) + + const unknown = attemptFailureFor({ name: "UnknownError", data: { message: "plain failure" } }) + expect(unknown.message).toBe("plain failure") + expect(sessionErrorOf(unknown)).toEqual({ name: "UnknownError", message: "plain failure" }) + + const aborted = attemptFailureFor({ name: "MessageAbortedError", data: { message: "stopped" } }) + expect(aborted).toBeInstanceOf(SessionAbortedError) + expect(sessionErrorOf(aborted)).toBeUndefined() + }) + + test("a Claude Code failure string stays an unclassified attempt failure", () => { + const failure = attemptFailureFor("claude exited with error_max_turns") + expect(failure.message).toBe("claude exited with error_max_turns") + expect(failure.cause).toBe("claude exited with error_max_turns") + expect(sessionErrorOf(failure)).toBeUndefined() + }) + + test("a session error never impersonates the operator-abort sentinel", () => { + const forged = new SessionError({ name: "UserAbortError", message: "provider said so" }) + expect(forged.name).toBe("SessionError") + expect(isUserAbortError(forged)).toBeFalse() + expect(isMessageAbortedError(new SessionError({ name: "MessageAbortedError", message: "stopped" }))).toBeTrue() + expect(isMessageAbortedError(new SessionError({ name: "APIError", message: "rate limited", statusCode: 429 }))).toBeFalse() + }) + test("parses provider/model values", () => { expect(parseModel("anthropic/claude-sonnet-4-6")).toEqual({ providerID: "anthropic", @@ -619,7 +744,7 @@ describe("run phase gate", () => { sessionRef!.id = "ses_failed" const handle = reports.begin("ses_failed", phase, phase.deliverableContract, qualityDimensionWeights) await handle.write({ markdown: "# Survived the failure" }) - throw new Error("provider temporarily unavailable") + throw new SessionError({ name: "APIError", message: "provider temporarily unavailable", statusCode: 429, isRetryable: true }) }, restorePhaseBaseline: async () => { restores++ @@ -636,6 +761,7 @@ describe("run phase gate", () => { expect(restores).toBe(0) expect(prompts[0]?.kind).toBe("failure") expect(prompts[0]?.canRetry).toBe(true) + expect(prompts[0]?.error).toBe("provider temporarily unavailable (HTTP 429, retryable)") }) test("a loop-guard trip reaches the decision gate instead of being swallowed", async () => { @@ -786,6 +912,49 @@ describe("run phase gate", () => { } }) + test("answering abort at a failure gate keeps the failed attempt's session classification", async () => { + const workspace = await retryWorkspace() + const progress: ProgressUI = { + ...noopProgress, + askHumanReview: (info) => { + expect(info.kind).toBe("failure") + expect(info.error).toBe("API key is invalid.") + return Promise.resolve("abort") + }, + } + const shutdown = trackedShutdown() + const rejected = { name: "ProviderAuthError", data: { message: "API key is invalid.", providerID: "anthropic" } } + + try { + const failure = await runPhaseUntilResolved( + {} as never, + workspace, + agentStep("implementer"), + "/repo", + prepared, + undefined, + progress, + shutdown, + createGitLock(), + { serverUrl: "http://127.0.0.1:1" }, + { + runPhaseAttempt: async () => { + throw attemptFailureFor(rejected) + }, + restorePhaseBaseline: async () => {}, + }, + ).then( + () => undefined, + (error: unknown) => error, + ) + expect(failure).toBeInstanceOf(UserAbortError) + expect(sessionErrorOf(failure)).toEqual({ name: "ProviderAuthError", message: "API key is invalid.", providerID: "anthropic" }) + expect(shutdown.aborted).toBe(true) + } finally { + shutdown.dispose() + } + }) + test("a max-steps trip fails instead of continuing when no dashboard or TTY can answer the budget gate", async () => { const workspace = await retryWorkspace() const trip = { @@ -2545,6 +2714,63 @@ describe("watchSession turn scoping", () => { expect(result.lastAssistantParts).toHaveLength(1) }) + test("rejects idle event-delivered failures with their session classification", async () => { + async function* stream() { + yield { type: "session.next.prompted", properties: { sessionID: "ses_1" } } + yield { type: "session.error", properties: { sessionID: "ses_1", error: { name: "APIError", data: { message: "rate limited", statusCode: 429, isRetryable: true } } } } + yield { type: "session.idle", properties: { sessionID: "ses_1" } } + await new Promise(() => {}) + } + const client = { + event: { subscribe: async () => ({ stream: stream() }) }, + session: { + messages: async () => ({ data: [] }), + status: async () => ({ data: {} }), + }, + } as never + const watcher = watchSession(client, { + directory: "/repo", + phaseName: "build", + sessionID: "ses_1", + progress: noopProgress, + signal: new AbortController().signal, + }) + + try { + await expect(watcher.result).rejects.toMatchObject({ name: "SessionError", signal: { name: "APIError", statusCode: 429, isRetryable: true } }) + } finally { + await watcher.stop() + } + }) + + test("prefers an event classification over the terminal assistant error", async () => { + async function* stream() { + yield { type: "session.error", properties: { sessionID: "ses_1", error: { name: "APIError", data: { message: "rate limited", statusCode: 429, isRetryable: true } } } } + await new Promise(() => {}) + } + const terminal = assistantMessage("msg_1", 0, "") + const client = { + event: { subscribe: async () => ({ stream: stream() }) }, + session: { + messages: async () => ({ data: [{ ...terminal, info: { ...terminal.info, error: { name: "APIError", data: { message: "rate limited" } } } }] }), + status: async () => ({ data: {} }), + }, + } as never + const watcher = watchSession(client, { + directory: "/repo", + phaseName: "build", + sessionID: "ses_1", + progress: noopProgress, + signal: new AbortController().signal, + }) + + try { + await expect(watcher.result).rejects.toMatchObject({ name: "SessionError", signal: { name: "APIError", statusCode: 429, isRetryable: true } }) + } finally { + await watcher.stop() + } + }) + test("aborts the session when the loop guard sees the same tool call over and over", async () => { const aborted: string[] = [] const activities: string[] = [] @@ -2695,6 +2921,45 @@ describe("loopGuard seam regressions", () => { expect(result.info.id).toBe("msg_1") }) + test("promptPhase turns an event-delivered message abort into a typed cancellation", async () => { + const started = deferred() + async function* stream() { + await started.promise + yield { type: "session.next.prompted", properties: { sessionID: "ses_1" } } + yield { type: "session.error", properties: { sessionID: "ses_1", error: { name: "MessageAbortedError", data: { message: "stopped" } } } } + yield { type: "session.idle", properties: { sessionID: "ses_1" } } + await new Promise(() => {}) + } + const client = { + event: { subscribe: async () => ({ stream: stream() }) }, + session: { + create: async () => ({ data: { id: "ses_1" } }), + promptAsync: async () => { + started.resolve() + return {} + }, + messages: async () => ({ data: [] }), + status: async () => ({ data: {} }), + abort: async () => ({}), + }, + } as never + + await expect( + promptPhase(client, { + phase: agentStep("implementer"), + workspace: { dir: "/run", runID: "test-run" } as Workspace, + targetDir: "/repo", + prompt: "do the thing", + model: { providerID: "openai", modelID: "gpt-5.5" }, + attachments: [], + progress: noopProgress, + shutdown: trackedShutdown(), + attempt: 1, + loopGuardConfig: resolveLoopGuard({}), + }), + ).rejects.toBeInstanceOf(SessionAbortedError) + }) + test("queues the soft nudge through v2 for a session created and prompted through v1", async () => { const started = deferred() const queued: unknown[] = []