diff --git a/packages/opencode/src/provider/error.ts b/packages/opencode/src/provider/error.ts index e45b9167b..b2f2d4aaf 100644 --- a/packages/opencode/src/provider/error.ts +++ b/packages/opencode/src/provider/error.ts @@ -22,6 +22,47 @@ export const ProviderFailureKind = z.enum([ ]) export type ProviderFailureKind = z.infer +// The provider-API-rejection kinds: a provider returned an API-level error (an +// HTTP response was received, or a typed provider error body arrived) rather +// than the connection dropping. transport_disconnect and decompression are +// stream/transport failures and are deliberately left out. run-incident routes +// these to its provider_api_error terminal cause, and the renderer passes their +// real message through instead of a generic connection-lost interruption string. +// +// This tuple is the single source of truth: the type is derived from it and the +// runtime check reads it directly, so adding a ProviderFailureKind forces an +// explicit decision here and the two can never drift. `satisfies` keeps every +// entry a valid ProviderFailureKind. +const PROVIDER_API_ERROR_KINDS = [ + "auth", + "rate_limit", + "quota_exhausted", + "server_overload", + "invalid_request", + "unknown", +] as const satisfies readonly ProviderFailureKind[] +export type ProviderApiErrorKind = (typeof PROVIDER_API_ERROR_KINDS)[number] + +function isProviderApiErrorKind(kind: ProviderFailureKind | undefined): kind is ProviderApiErrorKind { + return kind !== undefined && (PROVIDER_API_ERROR_KINDS as readonly ProviderFailureKind[]).includes(kind) +} + +// Whether a parsed APIError is a provider API rejection (an HTTP response was +// received, or a typed provider error body arrived) rather than a wrapped +// connection failure. The explicit kinds are only ever assigned from a status +// code or a typed error body, so they always qualify. "unknown" is the +// catch-all and must be gated on API evidence (a status code or a response +// body): without it, a wrapped network error — e.g. an APICallError with no +// HTTP response, or a DNS failure the stream classifier did not recognize — +// would be misclassified as a provider API error instead of a transport drop. +export function isProviderApiError< + T extends { kind?: ProviderFailureKind; statusCode?: number; hasResponseBody?: boolean }, +>(input: T): input is T & { kind: ProviderApiErrorKind } { + if (!isProviderApiErrorKind(input.kind)) return false + if (input.kind === "unknown") return input.statusCode !== undefined || input.hasResponseBody === true + return true +} + function apiCallErrorKind(statusCode: number | undefined, code: string | undefined): ProviderFailureKind { if (code === "insufficient_quota" || code === "usage_not_included") return "quota_exhausted" if (code === "invalid_prompt") return "invalid_request" diff --git a/packages/opencode/src/session/processor.ts b/packages/opencode/src/session/processor.ts index 94f9521f7..f9eedc410 100644 --- a/packages/opencode/src/session/processor.ts +++ b/packages/opencode/src/session/processor.ts @@ -19,6 +19,7 @@ import { SessionDiagnostics } from "./diagnostics" import { classifyToolFailure } from "./tool-failure" import type { Provider } from "@/provider" import { ProviderTransform } from "@/provider" +import { isProviderApiError, type ProviderFailureKind } from "@/provider/error" import { ExternalResult } from "@/tool/external-result" import { errorMessage } from "@/util/error" import { Log } from "@opencode-ai/core/util/log" @@ -212,6 +213,12 @@ function retryTimeoutPolicyFor( }) } +// Appended whenever a side effect may already have run, so the user verifies +// external state before resending. Kept separate from the "Connection lost." +// framing so a provider rejection (billing / auth) can reuse the bare hint +// without being mislabelled as a dropped connection. +const SIDE_EFFECT_SAFETY_HINT = "Please check whether the last operation completed before resending." + function recoveryInterruptionMessage(recovery: NonNullable["recovery"] | undefined) { switch (recovery?.reason) { case "no_visible_output_or_tool_execution": @@ -224,7 +231,7 @@ function recoveryInterruptionMessage(recovery: NonNullable["recovery"] | undefined, + providerMessage?: string, +): string | undefined { + if (providerApiRejection) { + const reason = recovery?.reason + if (reason === "provider_api_error") return undefined + if ( + providerMessage && + (reason === "tool_execution_started" || + reason === "unsafe_side_effect_started" || + reason === "side_effect_facts_incomplete") + ) { + return `${providerMessage} ${SIDE_EFFECT_SAFETY_HINT}` + } + } + return recoveryInterruptionMessage(recovery) +} + type PendingLoopAction = { loopAction: "block" | "stop" tool: string @@ -1728,18 +1776,40 @@ export const layer: Layer.Layer< if (phase) { return { retryable: true, - message: "Connection timed out", - watchdog: { phase }, + message: "Connection timed out" as string | undefined, + watchdog: { phase } as { phase: "connect" | "silent_stream" | "unknown" } | undefined, + providerFailure: undefined as + | { kind: ProviderFailureKind; code?: string; statusCode?: number; hasResponseBody?: boolean } + | undefined, + providerMessage: undefined as string | undefined, } } + // Parse once for the retry decision; surface providerFailure plus the + // HTTP evidence so the recorder routes a provider API rejection to + // provider_api_error instead of re-parsing or defaulting it to a + // transport disconnect. The provider's own message rides along too so + // the halt path renders it without a second parse that could drift + // from this classification. const parsed = parse(error) + const apiError = MessageV2.APIError.isInstance(parsed) ? parsed : undefined + const providerFailure = apiError?.data.providerFailure + ? { + kind: apiError.data.providerFailure.kind, + code: apiError.data.providerFailure.code, + statusCode: apiError.data.statusCode, + hasResponseBody: + typeof apiError.data.responseBody === "string" && apiError.data.responseBody.length > 0, + } + : undefined + const providerMessage = apiError?.data.message const classification = SessionRetry.classifyRetry(parsed) - if (!classification) return { retryable: false } + if (!classification) + return { retryable: false, message: undefined, watchdog: undefined, providerFailure, providerMessage } if (SessionRetry.retryAction(classification) === "stop") { ctx.terminalClassification = classification - return { retryable: false } + return { retryable: false, message: undefined, watchdog: undefined, providerFailure, providerMessage } } - return { retryable: true, message: classification.raw } + return { retryable: true, message: classification.raw, watchdog: undefined, providerFailure, providerMessage } } const removeReasoningForAttempt = Effect.fn("SessionProcessor.removeReasoningForAttempt")(function* ( @@ -1883,6 +1953,7 @@ export const layer: Layer.Layer< evidence: retrySignal.watchdog ? ["watchdog_fired", "iterator_error"] : ["iterator_error"], watchdog: retrySignal.watchdog, retryable: retrySignal.retryable, + providerFailure: retrySignal.providerFailure, }) const retryDecision = buildModelRetryDecision({ technicalRetryability: retrySignal.retryable @@ -1997,9 +2068,16 @@ export const layer: Layer.Layer< break } + // Pick the halt message (see haltInterruptionMessage): a terminal + // provider API rejection with no side-effect risk keeps its own + // actionable text; a provider rejection after a side effect keeps both + // that text and the safety hint; transport drops keep the recovery + // string. Lifecycle-close and user-cancel halts above keep their own + // authoritative interruption messages. + const providerApiRejection = !!retrySignal.providerFailure && isProviderApiError(retrySignal.providerFailure) yield* halt(result.error, attemptID, { recordFailure: false, - interruptionMessage: recoveryInterruptionMessage(decision), + interruptionMessage: haltInterruptionMessage(providerApiRejection, decision, retrySignal.providerMessage), }) break } diff --git a/packages/opencode/src/session/run-incident/derive.ts b/packages/opencode/src/session/run-incident/derive.ts index c3e7bbfa4..f1e2c7228 100644 --- a/packages/opencode/src/session/run-incident/derive.ts +++ b/packages/opencode/src/session/run-incident/derive.ts @@ -240,6 +240,20 @@ function phaseFor(input: { } } +export function providerApiCause(input: { + kind: Extract["subcategory"] + retryable?: boolean + error?: SafeErrorFingerprint +}): Extract { + return { + category: "provider_api_error", + subcategory: input.kind, + retryable: input.retryable, + error: input.error, + confidence: "high", + } +} + export function transportCause(input: { error?: SafeErrorFingerprint providerProgressSeen: boolean diff --git a/packages/opencode/src/session/run-incident/index.ts b/packages/opencode/src/session/run-incident/index.ts index 8bc8a6a4c..536684ab8 100644 --- a/packages/opencode/src/session/run-incident/index.ts +++ b/packages/opencode/src/session/run-incident/index.ts @@ -1,4 +1,8 @@ -import { deriveIncident as deriveRunIncident, transportCause as providerTransportCause } from "./derive" +import { + deriveIncident as deriveRunIncident, + providerApiCause as deriveProviderApiCause, + transportCause as providerTransportCause, +} from "./derive" import { recoveryFor as deriveRecovery } from "./policy" import { plainSummary as derivePlainSummary, userSummary as deriveUserSummary } from "./presentation" import { sanitizeIncident as sanitizeRunIncident, sanitizeLifecycleRequest } from "./sanitize" @@ -31,6 +35,7 @@ export namespace RunIncident { export const SCHEMA_VERSION = VERSION export const derive = deriveRunIncident export const transportCause = providerTransportCause + export const providerApiCause = deriveProviderApiCause export const recoveryFor = deriveRecovery export const evaluateReplaySafety = deriveReplaySafety export const userSummary = deriveUserSummary diff --git a/packages/opencode/src/session/run-incident/policy.ts b/packages/opencode/src/session/run-incident/policy.ts index 677a1a91d..2ebd533be 100644 --- a/packages/opencode/src/session/run-incident/policy.ts +++ b/packages/opencode/src/session/run-incident/policy.ts @@ -17,9 +17,16 @@ export function recoveryFor(input: { const terminalFacts = input.terminalFacts ?? input.facts const noToolActivity = !terminalFacts.tool_input_started && !terminalFacts.tool_call_materialized && !terminalFacts.tool_execution_started - const retryableTransport = + // A failure whose retryability says auto-retry is worth attempting, subject to + // the side-effect-safety gates below. Includes a retryable provider API error + // (rate_limit / server_overload) alongside transport drops and watchdog + // timeouts — they all replay the same way when no visible output or tool side + // effect has happened yet. + const retryableProviderFailure = input.retryable === true && - (input.cause.category === "provider_transport_disconnect" || input.cause.category === "watchdog_timeout") + (input.cause.category === "provider_transport_disconnect" || + input.cause.category === "watchdog_timeout" || + input.cause.category === "provider_api_error") if (input.cause.category === "user_cancel") { return { ...base, recommendation: "do_not_retry", confidence: "high", reason: "user_cancel" } } @@ -31,12 +38,35 @@ export function recoveryFor(input: { reason: "local_lifecycle_close", } } + if (input.cause.category === "provider_api_error" && input.retryable !== true) { + // A terminal provider API rejection (auth / quota_exhausted / invalid_request, + // or unknown retryability) cannot be fixed by retrying and is not a connection + // drop, so always stop (do_not_retry). Retryable provider API errors + // (rate_limit / server_overload) fall through to the auto-retry tree via + // retryableProviderFailure. + // + // With no side-effect risk we keep the reason out of the connection-lost set + // ("provider_api_error"), so the renderer shows the real provider message + // verbatim. But if a tool already ran, an unsafe side effect started, or + // side-effect facts are incomplete, we surface that side-effect reason instead + // (still do_not_retry): the renderer then keeps the "check external state" + // safety hint alongside the provider message, so a user who fixes their + // balance/auth and resends does not silently re-run a side-effecting operation. + const reason = terminalFacts.unsafe_side_effect_started + ? "unsafe_side_effect_started" + : terminalFacts.tool_execution_started + ? "tool_execution_started" + : !terminalFacts.side_effect_facts_complete + ? "side_effect_facts_incomplete" + : "provider_api_error" + return { ...base, recommendation: "do_not_retry", confidence: "high", reason } + } if ( canAutoRetryBeforeFirstProviderProgress({ cause: input.cause, facts: input.facts, terminalFacts, - retryableTransport, + retryableProviderFailure, }) ) { return { @@ -56,7 +86,7 @@ export function recoveryFor(input: { } } if ( - retryableTransport && + retryableProviderFailure && noToolActivity && !isBeforeFirstProviderProgressCause(input.cause) && terminalFacts.reasoning_output_started && @@ -81,7 +111,7 @@ export function recoveryFor(input: { } } if ( - retryableTransport && + retryableProviderFailure && noToolActivity && terminalFacts.reasoning_output_started && !terminalFacts.text_output_started @@ -150,7 +180,7 @@ export function recoveryFor(input: { if (input.facts.lifecycle_close_seen) { return { ...base, recommendation: "do_not_retry", confidence: "high", reason: "local_lifecycle_close" } } - if (retryableTransport) { + if (retryableProviderFailure) { return { ...base, recommendation: "auto_retry", @@ -166,9 +196,9 @@ function canAutoRetryBeforeFirstProviderProgress(input: { cause: TerminalCause facts: IncidentFacts terminalFacts: IncidentFacts - retryableTransport: boolean + retryableProviderFailure: boolean }) { - if (!input.retryableTransport) return false + if (!input.retryableProviderFailure) return false if (input.facts.user_cancel_seen || input.facts.lifecycle_close_seen) return false if (!isBeforeFirstProviderProgressCause(input.cause)) return false if (input.terminalFacts.provider_progress_seen) return false diff --git a/packages/opencode/src/session/run-incident/presentation.ts b/packages/opencode/src/session/run-incident/presentation.ts index f67c4b03d..559c3dd19 100644 --- a/packages/opencode/src/session/run-incident/presentation.ts +++ b/packages/opencode/src/session/run-incident/presentation.ts @@ -22,6 +22,8 @@ export function plainSummary(input: { cause: TerminalCause; facts: IncidentFacts if (input.cause.category === "local_lifecycle_close") { return "The active run was interrupted by a local lifecycle close." } + if (input.cause.category === "provider_api_error") + return "The provider rejected the request before the response completed." if (input.cause.category === "user_cancel") return "The run was cancelled by the user." if (input.cause.category === "watchdog_timeout") return "The run stopped after PawWork waited too long for provider progress." @@ -39,7 +41,11 @@ function actionKey(recovery: RecoveryDecision) { function severity(cause: TerminalCause) { if (cause.category === "user_cancel") return "info" as const - if (cause.category === "unknown_interruption" || cause.category === "crash_or_restart_incomplete") + if ( + cause.category === "provider_api_error" || + cause.category === "unknown_interruption" || + cause.category === "crash_or_restart_incomplete" + ) return "error" as const return "warning" as const } diff --git a/packages/opencode/src/session/run-incident/types.ts b/packages/opencode/src/session/run-incident/types.ts index d5480ec6e..74007eb5b 100644 --- a/packages/opencode/src/session/run-incident/types.ts +++ b/packages/opencode/src/session/run-incident/types.ts @@ -10,8 +10,11 @@ import type { ToolEffectKind, } from "../run-observability/types" import type { LifecycleRequest } from "../lifecycle-provenance" +import type { ProviderApiErrorKind } from "@/provider/error" -export const RUN_INCIDENT_SCHEMA_VERSION = 1 +// v2: added the provider_api_error terminal cause (a provider returned an +// API-level rejection rather than the connection dropping). +export const RUN_INCIDENT_SCHEMA_VERSION = 2 export type Confidence = "low" | "medium" | "high" @@ -63,6 +66,13 @@ export type TerminalCause = error?: SafeErrorFingerprint confidence: Confidence } + | { + category: "provider_api_error" + subcategory: ProviderApiErrorKind + retryable?: boolean + error?: SafeErrorFingerprint + confidence: Confidence + } | { category: "local_lifecycle_close" subcategory: LifecycleKind | "unknown_lifecycle_close" @@ -187,6 +197,7 @@ export type RecoveryDecision = { | "side_effect_facts_incomplete" | "local_lifecycle_close" | "user_cancel" + | "provider_api_error" | "unknown" auto_retry?: { max_attempts: number; backoff_ms: number; attempted_at?: number } user_action?: { kind: "continue" | "resume" | "retry" | "confirm_continue" | "dismiss"; idempotency_key: string } diff --git a/packages/opencode/src/session/run-observability/recorder.ts b/packages/opencode/src/session/run-observability/recorder.ts index a74f74bda..17dd0324c 100644 --- a/packages/opencode/src/session/run-observability/recorder.ts +++ b/packages/opencode/src/session/run-observability/recorder.ts @@ -17,6 +17,7 @@ import { import { safeErrorFingerprint } from "./sanitize" import { RunIncident } from "../run-incident" import { cloneRequest, type LifecycleRequest } from "../lifecycle-provenance" +import { isProviderApiError, type ProviderApiErrorKind } from "@/provider/error" type AttemptMutable = AttemptSummary & { lastMonotonicMs: number } @@ -30,6 +31,15 @@ type Failure = attemptID?: AttemptID retryable?: boolean } + | { + type: "provider_api" + at: number + monotonicMs: number + error: unknown + kind: ProviderApiErrorKind + retryable?: boolean + attemptID?: AttemptID + } | { type: "setup"; at: number; monotonicMs: number; error: unknown } | { type: "scope_closed" @@ -150,7 +160,7 @@ export function createRecorder(input: RecorderInput): Recorder { parentMessageID: input.parentMessageID, createdAt: input.createdAt, completedAt, - retryable: failure?.type === "transport" ? failure.retryable : undefined, + retryable: failure?.type === "transport" || failure?.type === "provider_api" ? failure.retryable : undefined, evidence: options?.includeRecoveredTerminal ? evidence : terminalEvidence(), unsafeSideEffectKinds: unsafeKinds, sideEffectFactsComplete, @@ -204,6 +214,36 @@ export function createRecorder(input: RecorderInput): Recorder { }) rememberEvent(next.monotonicMs) } + const recordProviderApiFailureEvidence = (next: { + attemptID?: AttemptID + at: number + monotonicMs: number + error: unknown + kind: ProviderApiErrorKind + retryable?: boolean + }) => { + const error = safeErrorFingerprint(next.error) + failure ??= { + type: "provider_api", + at: next.at, + monotonicMs: next.monotonicMs, + error: next.error, + kind: next.kind, + retryable: next.retryable, + attemptID: next.attemptID, + } + appendEvidence({ + monotonic_ms: next.monotonicMs, + source: "provider_stream", + attempt_id: next.attemptID, + event_type: "provider_api_error", + terminal_candidate: true, + confidence: "high", + error, + cause: RunIncident.providerApiCause({ kind: next.kind, retryable: next.retryable, error }), + }) + rememberEvent(next.monotonicMs) + } const recordWatchdogFailureEvidence = (next: { attemptID?: AttemptID at: number @@ -528,6 +568,11 @@ export function createRecorder(input: RecorderInput): Recorder { recordAttemptFailureAndDeriveRecovery(next) { if (next.watchdog) { recordWatchdogFailureEvidence({ ...next, phase: next.watchdog.phase }) + } else if (next.providerFailure && isProviderApiError(next.providerFailure)) { + // A provider returned an API-level rejection (an HTTP response, or a typed + // provider error body) — not a connection drop. Route it to its own + // terminal cause so it is not mislabeled a transport disconnect. + recordProviderApiFailureEvidence({ ...next, kind: next.providerFailure.kind }) } else { recordTransportFailureEvidence(next) } @@ -675,7 +720,7 @@ export function createRecorder(input: RecorderInput): Recorder { reasoningOutputStarted: terminalAttempt?.reasoning_output_started ?? incident?.facts.reasoning_output_started ?? false, toolExecutionStarted: terminalAttempt?.tool_execution_started ?? toolExecutionStarted, unsafeSideEffectStarted: terminalAttempt?.unsafe_side_effect_started ?? unsafeSideEffectStarted, - retryable: failure?.type === "transport" ? failure.retryable : undefined, + retryable: failure?.type === "transport" || failure?.type === "provider_api" ? failure.retryable : undefined, }) const completedAt = final.completedAt const failureMonotonicMs = failure?.monotonicMs @@ -750,12 +795,15 @@ function classify(failure: Failure | undefined): Classification { return "local_instance_dispose" return failure.lifecycleActionID ? "known_lifecycle_close" : "unknown_scope_close" } + if (failure.type === "provider_api") return "provider_api_error" if (failure.type === "transport") return "external_stream_disconnect" return "unknown_failure" } function classificationForIncident(cause: RunIncident.TerminalCause): Classification { switch (cause.category) { + case "provider_api_error": + return "provider_api_error" case "provider_transport_disconnect": case "watchdog_timeout": return "external_stream_disconnect" @@ -780,6 +828,7 @@ function classificationForIncident(cause: RunIncident.TerminalCause): Classifica } function summarySuffixForIncident(cause: RunIncident.TerminalCause, input: { providerProgressSeen: boolean }) { + if (cause.category === "provider_api_error") return cause.subcategory if (cause.category === "watchdog_timeout") return "watchdog_timeout" if (cause.category === "provider_transport_disconnect") { if (!input.providerProgressSeen) return "transport_failure" @@ -799,6 +848,7 @@ function summarySuffixForIncident(cause: RunIncident.TerminalCause, input: { pro } function summarySuffix(input: { failure: Failure | undefined; providerProgressSeen: boolean }) { + if (input.failure?.type === "provider_api") return input.failure.kind if (input.failure?.type === "transport") { const error = safeErrorFingerprint(input.failure.error) if (input.providerProgressSeen && error.cause_code === "UND_ERR_SOCKET") return "provider_progress_socket_closed" @@ -903,6 +953,21 @@ function retrySafetyFor(input: { reason: "no_visible_output_or_tool_execution", } } + if (input.classification === "provider_api_error") { + // Same side-effect-safety axis as #1118: a terminal provider rejection + // (retryable=false) is a hard stop; a retryable one that reached finalize + // exhausted its budget but auto-retrying it would still be side-effect-safe + // here (no visible output, no tool execution — those are gated above). + if (input.retryable === false) { + return { ...base, recommendation: "do_not_auto_retry", confidence: "high", reason: "provider_terminal_failure" } + } + return { + ...base, + recommendation: "candidate_safe_auto_retry", + confidence: "medium", + reason: "no_visible_output_or_tool_execution", + } + } if (input.classification === "known_lifecycle_close" || input.classification === "unknown_scope_close") { return { ...base, diff --git a/packages/opencode/src/session/run-observability/types.ts b/packages/opencode/src/session/run-observability/types.ts index 28cc6ec54..92ed3b065 100644 --- a/packages/opencode/src/session/run-observability/types.ts +++ b/packages/opencode/src/session/run-observability/types.ts @@ -2,8 +2,11 @@ import { MessageID, SessionID } from "../schema" import z from "zod" import type { RunIncident } from "../run-incident" import type { LifecycleRequest } from "../lifecycle-provenance" +import type { ProviderFailureKind } from "@/provider/error" -export const SCHEMA_VERSION = 1 +// v2: added the provider_api_error classification (a provider returned an +// API-level rejection rather than the stream disconnecting). +export const SCHEMA_VERSION = 2 export const RunID = z.string().brand<"RunID">() export type RunID = z.infer @@ -14,6 +17,7 @@ export type AttemptID = z.infer export const Classification = z.enum([ "success", "external_stream_disconnect", + "provider_api_error", "local_instance_reload", "local_instance_dispose", "known_lifecycle_close", @@ -248,6 +252,12 @@ export type Recorder = { evidence?: string[] watchdog?: { phase: "connect" | "silent_stream" | "unknown" } retryable?: boolean + /** The parsed providerFailure (slice ①) plus the HTTP evidence the recorder + * needs to route a real provider API rejection to provider_api_error instead + * of defaulting it to a transport disconnect. statusCode/hasResponseBody gate + * the catch-all "unknown" kind so a wrapped connection failure is not + * mislabeled. Absent for watchdog timeouts and raw transport drops. */ + providerFailure?: { kind: ProviderFailureKind; code?: string; statusCode?: number; hasResponseBody?: boolean } }): RunIncident.Recovery recordRecoveryDecision(input: { attemptID?: AttemptID diff --git a/packages/opencode/test/session/export.test.ts b/packages/opencode/test/session/export.test.ts index e475e455c..e5e016825 100644 --- a/packages/opencode/test/session/export.test.ts +++ b/packages/opencode/test/session/export.test.ts @@ -855,7 +855,7 @@ describe("Export.session", () => { const userID = MessageID.make("msg_run_obs_user") const assistantID = MessageID.make("msg_run_obs_assistant") const summary: RunObservability.Summary = { - schema_version: 1, + schema_version: 2, run_id: RunObservability.RunID.make("run_export"), trace_id: assistantID, session_id: root.id, @@ -915,7 +915,7 @@ describe("Export.session", () => { } as MessageV2.Assistant) const result = await AppRuntime.runPromise(Export.session(root.id)) - expect(result.diagnostics.run_observability_schema_version).toBe(1) + expect(result.diagnostics.run_observability_schema_version).toBe(2) expect(result.diagnostics.run_observability).toEqual([summary]) } finally { await SessionNs.remove(root.id) @@ -1972,7 +1972,7 @@ describe("redactPart", () => { test("sanitizeSnapshot preserves safe run observability error fingerprints", () => { const summary: RunObservability.Summary = { - schema_version: 1, + schema_version: 2, run_id: RunObservability.RunID.make("run_sanitize"), trace_id: MessageID.make("msg_sanitize"), session_id: SessionID.make("ses_sanitize"), @@ -2020,7 +2020,7 @@ describe("redactPart", () => { model_refs: {}, stats: { session_count: 1, message_count: 1, part_count: 0, omitted_attachment_count: 0 }, }, - diagnostics: { run_observability_schema_version: 1, run_observability: [summary] }, + diagnostics: { run_observability_schema_version: 2, run_observability: [summary] }, session: { info: { id: SessionID.make("ses_sanitize"), @@ -2147,7 +2147,7 @@ describe("redactPart", () => { diagnostics_complete: true, } const summary = { - schema_version: 1, + schema_version: 2, run_id: RunObservability.RunID.make("run_sanitize_incident"), trace_id: MessageID.make("msg_sanitize_incident"), session_id: SessionID.make("ses_sanitize_incident"), @@ -2194,7 +2194,7 @@ describe("redactPart", () => { model_refs: {}, stats: { session_count: 1, message_count: 1, part_count: 0, omitted_attachment_count: 0 }, }, - diagnostics: { run_observability_schema_version: 1, run_observability: [summary] }, + diagnostics: { run_observability_schema_version: 2, run_observability: [summary] }, session: { info: { id: SessionID.make("ses_sanitize_incident"), @@ -2210,7 +2210,7 @@ describe("redactPart", () => { }, }) - expect(sanitized.diagnostics.run_incident_schema_version).toBe(1) + expect(sanitized.diagnostics.run_incident_schema_version).toBe(2) expect(sanitized.diagnostics.run_incidents?.[0]?.terminal_cause.category).toBe("provider_transport_disconnect") expect(sanitized.diagnostics.incident_chains?.[0]).toMatchObject({ incident_id: "incident:msg_sanitize", @@ -2356,7 +2356,7 @@ describe("redactPart", () => { model_refs: {}, stats: { session_count: 1, message_count: 0, part_count: 0, omitted_attachment_count: 0 }, }, - diagnostics: { run_observability_schema_version: 1, run_observability: [summary] }, + diagnostics: { run_observability_schema_version: 2, run_observability: [summary] }, session: { info: { id: SessionID.make("ses_side_effect_snapshot_sanitize"), @@ -2453,7 +2453,7 @@ describe("redactPart", () => { model_refs: {}, stats: { session_count: 1, message_count: 0, part_count: 0, omitted_attachment_count: 0 }, }, - diagnostics: { run_observability_schema_version: 1, run_observability: [summary] }, + diagnostics: { run_observability_schema_version: 2, run_observability: [summary] }, session: { info: { id: SessionID.make("ses_recovered_incident_export"), @@ -2542,7 +2542,7 @@ describe("redactPart", () => { model_refs: {}, stats: { session_count: 1, message_count: 0, part_count: 0, omitted_attachment_count: 0 }, }, - diagnostics: { run_observability_schema_version: 1, run_observability: [summary] }, + diagnostics: { run_observability_schema_version: 2, run_observability: [summary] }, session: { info: { id: SessionID.make("ses_generated_chain_sanitize"), diff --git a/packages/opencode/test/session/processor-effect.test.ts b/packages/opencode/test/session/processor-effect.test.ts index a1b232926..d2713e7ae 100644 --- a/packages/opencode/test/session/processor-effect.test.ts +++ b/packages/opencode/test/session/processor-effect.test.ts @@ -1463,6 +1463,60 @@ it.live("session.processor effect tests do not retry unknown json errors", () => ), ) +it.live("surfaces a terminal provider API error's real message instead of a connection-lost interruption", () => + provideTmpdirServer( + ({ dir, llm }) => + Effect.gen(function* () { + const { processors, session, provider } = yield* boot() + + // DeepSeek direct: account in arrears returns 402 Insufficient Balance. + // This must surface as the real provider error, not "Connection lost". + yield* llm.error(402, { + error: { message: "Insufficient Balance", code: "invalid_request_error", type: "unknown_error" }, + }) + + const chat = yield* session.create({}) + const parent = yield* user(chat.id, "spend") + const msg = yield* assistant(chat.id, parent.id, path.resolve(dir)) + const mdl = yield* provider.getModel(ref.providerID, ref.modelID) + const handle = yield* processors.create({ + safeRecoveryDelay: FAST_SAFE_RECOVERY_DELAY, + assistantMessage: msg, + sessionID: chat.id, + model: mdl, + }) + + const value = yield* handle.process({ + user: { + id: parent.id, + sessionID: chat.id, + role: "user", + time: parent.time, + agent: parent.agent, + model: { providerID: ref.providerID, modelID: ref.modelID }, + } satisfies MessageV2.User, + sessionID: chat.id, + model: mdl, + agent: agent(), + system: [], + messages: [{ role: "user", content: "spend" }], + tools: {}, + }) + + expect(value).toBe("stop") + const error = handle.message.error + expect(error?.name).toBe("APIError") + // The real provider failure is classified and its body is preserved … + const data = error && "data" in error ? (error.data as Record) : undefined + expect((data?.providerFailure as { kind?: string } | undefined)?.kind).toBeDefined() + expect(String(data?.responseBody ?? "")).toContain("Insufficient Balance") + // … and the message is NOT overwritten with the generic connection-lost text. + expect(String(data?.message ?? "")).not.toContain("Connection lost") + }), + { git: true, config: (url) => providerCfg(url) }, + ), +) + it.live("session.processor effect tests retry recognized structured json errors", () => provideTmpdirServer( ({ dir, llm }) => diff --git a/packages/opencode/test/session/processor-interruption-message.test.ts b/packages/opencode/test/session/processor-interruption-message.test.ts new file mode 100644 index 000000000..797fcd716 --- /dev/null +++ b/packages/opencode/test/session/processor-interruption-message.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, test } from "bun:test" +import { haltInterruptionMessage } from "../../src/session/processor" +import type { RunObservability } from "../../src/session/run-observability" + +type Recovery = NonNullable["recovery"] + +// haltInterruptionMessage only reads recovery.reason; a minimal stub is enough. +function recovery(reason: NonNullable["reason"]): Recovery { + return { reason } as unknown as Recovery +} + +// Substring of the side-effect safety hint that must not be swallowed. +const SAFETY_HINT = "check whether the last operation completed" +const SIDE_EFFECT_REASONS = [ + "tool_execution_started", + "unsafe_side_effect_started", + "side_effect_facts_incomplete", +] as const + +describe("haltInterruptionMessage", () => { + test("a terminal provider API rejection (no side-effect risk) keeps its own message", () => { + // reason "provider_api_error" only marks a terminal rejection with no side + // effect (e.g. a 402 "Insufficient Balance" before any tool ran); its real + // text must show, so return undefined and let halt() leave it in place — even + // if a provider message is supplied. + expect(haltInterruptionMessage(true, recovery("provider_api_error"))).toBeUndefined() + expect(haltInterruptionMessage(true, recovery("provider_api_error"), "Insufficient Balance")).toBeUndefined() + }) + + // The P1 fix: a provider rejection that lands after a side effect (a terminal + // rejection after a tool ran, OR a retryable rate_limit / server_overload that + // exhausted retries) keeps BOTH the provider's real reason and the safety hint, + // so a user who fixes balance/auth still checks external state before resending. + for (const reason of SIDE_EFFECT_REASONS) { + test(`a provider rejection with reason "${reason}" + provider message keeps both reason and hint`, () => { + const message = haltInterruptionMessage(true, recovery(reason), "Insufficient Balance") + expect(message).toContain("Insufficient Balance") + expect(message).toContain(SAFETY_HINT) + // The bare hint is used, not the "Connection lost." framing that would + // mislabel a billing/auth rejection. + expect(message).not.toContain("Connection lost") + }) + + test(`a provider rejection with reason "${reason}" but no provider message still keeps the hint`, () => { + // Fallback when no provider text is available: the recovery string still + // carries the hint (the safety warning is never dropped). + expect(haltInterruptionMessage(true, recovery(reason))).toContain(SAFETY_HINT) + }) + } + + test("a non-provider failure uses the recovery message verbatim", () => { + expect(haltInterruptionMessage(false, recovery("no_visible_output_or_tool_execution"))).toContain("Connection lost") + }) + + test("the provider_api_error reason has no override even without the rejection flag", () => { + // Belt-and-suspenders: recoveryInterruptionMessage's switch also returns + // undefined for "provider_api_error", so the two never disagree. + expect(haltInterruptionMessage(false, recovery("provider_api_error"))).toBeUndefined() + }) + + test("undefined recovery yields no override", () => { + expect(haltInterruptionMessage(true, undefined)).toBeUndefined() + expect(haltInterruptionMessage(true, undefined, "Insufficient Balance")).toBeUndefined() + expect(haltInterruptionMessage(false, undefined)).toBeUndefined() + }) +}) diff --git a/packages/opencode/test/session/run-observability.test.ts b/packages/opencode/test/session/run-observability.test.ts index dc2849881..0d2e65587 100644 --- a/packages/opencode/test/session/run-observability.test.ts +++ b/packages/opencode/test/session/run-observability.test.ts @@ -3,6 +3,7 @@ import { LLM } from "../../src/session/llm" import { MessageID, SessionID } from "../../src/session/schema" import { RunIncident } from "../../src/session/run-incident" import { RunObservability } from "../../src/session/run-observability" +import { haltInterruptionMessage } from "../../src/session/processor" const beforeProgressCause = { category: "provider_transport_disconnect", @@ -734,6 +735,208 @@ describe("RunObservability", () => { }) }) + test("classifies a terminal provider API rejection as provider_api_error, not a transport disconnect", () => { + const recorder = RunObservability.createRecorder({ + runID: RunObservability.RunID.make("run_provider_api_terminal"), + traceID: MessageID.make("msg_provider_api_terminal"), + sessionID: SessionID.make("ses_provider_api_terminal"), + messageID: MessageID.make("msg_provider_api_terminal"), + providerID: "deepseek", + modelID: "deepseek-chat", + createdAt: 10, + monotonicStartMs: 100, + }) + + const attempt = recorder.beginAttempt({ attemptIndex: 1, at: 11, monotonicMs: 110 }) + const recovery = recorder.recordAttemptFailureAndDeriveRecovery({ + attemptID: attempt.attemptID, + at: 12, + monotonicMs: 120, + error: { name: "APIError", message: "402 Insufficient Balance" }, + evidence: ["iterator_error"], + retryable: false, + providerFailure: { kind: "quota_exhausted", code: "invalid_request_error" }, + }) + + expect(recovery.recommendation).toBe("do_not_retry") + expect(recovery.reason).toBe("provider_api_error") + + const summary = recorder.finalize({ completedAt: 13, monotonicMs: 130 }) + expect(summary.classification).toBe("provider_api_error") + expect(String(summary.summary_key)).toBe("provider_api_error.quota_exhausted") + expect(summary.retry_safety).toEqual({ + recommendation: "do_not_auto_retry", + confidence: "high", + reason: "provider_terminal_failure", + safety_scope: "user_visible_and_tool_side_effects", + }) + expect(summary.incident?.terminal_cause).toMatchObject({ + category: "provider_api_error", + subcategory: "quota_exhausted", + retryable: false, + }) + expect(summary.incident?.user_summary).toMatchObject({ + title_key: "run_incident.provider_api_error", + body_key: "run_incident.provider_api_error.quota_exhausted", + severity: "error", + }) + }) + + test("treats a retryable provider API error that exhausted its budget as side-effect-safe to retry", () => { + const recorder = RunObservability.createRecorder({ + runID: RunObservability.RunID.make("run_provider_api_retryable"), + traceID: MessageID.make("msg_provider_api_retryable"), + sessionID: SessionID.make("ses_provider_api_retryable"), + messageID: MessageID.make("msg_provider_api_retryable"), + providerID: "openai", + modelID: "gpt-5.5", + createdAt: 10, + monotonicStartMs: 100, + }) + + const attempt = recorder.beginAttempt({ attemptIndex: 1, at: 11, monotonicMs: 110 }) + recorder.recordAttemptFailureAndDeriveRecovery({ + attemptID: attempt.attemptID, + at: 12, + monotonicMs: 120, + error: { name: "APIError", message: "429 Too Many Requests" }, + evidence: ["iterator_error"], + retryable: true, + providerFailure: { kind: "rate_limit" }, + }) + + const summary = recorder.finalize({ completedAt: 13, monotonicMs: 130 }) + expect(summary.classification).toBe("provider_api_error") + expect(String(summary.summary_key)).toBe("provider_api_error.rate_limit") + expect(summary.retry_safety).toEqual({ + recommendation: "candidate_safe_auto_retry", + confidence: "medium", + reason: "no_visible_output_or_tool_execution", + safety_scope: "user_visible_and_tool_side_effects", + }) + }) + + test("keeps a transport_disconnect providerFailure on the transport path, not provider_api_error", () => { + const recorder = RunObservability.createRecorder({ + runID: RunObservability.RunID.make("run_provider_api_transport"), + traceID: MessageID.make("msg_provider_api_transport"), + sessionID: SessionID.make("ses_provider_api_transport"), + messageID: MessageID.make("msg_provider_api_transport"), + providerID: "openai", + modelID: "gpt-5.5", + createdAt: 10, + monotonicStartMs: 100, + }) + + const attempt = recorder.beginAttempt({ attemptIndex: 1, at: 11, monotonicMs: 110 }) + recorder.recordAttemptFailureAndDeriveRecovery({ + attemptID: attempt.attemptID, + at: 12, + monotonicMs: 120, + error: { name: "APIError", message: "Connection interrupted", code: "ECONNRESET" }, + evidence: ["iterator_error"], + retryable: true, + providerFailure: { kind: "transport_disconnect", code: "ECONNRESET" }, + }) + + const summary = recorder.finalize({ completedAt: 13, monotonicMs: 130 }) + expect(summary.classification).toBe("external_stream_disconnect") + expect(summary.incident?.terminal_cause.category).toBe("provider_transport_disconnect") + }) + + test("keeps a decompression providerFailure on the transport path, not provider_api_error", () => { + const recorder = RunObservability.createRecorder({ + runID: RunObservability.RunID.make("run_provider_api_decompression"), + traceID: MessageID.make("msg_provider_api_decompression"), + sessionID: SessionID.make("ses_provider_api_decompression"), + messageID: MessageID.make("msg_provider_api_decompression"), + providerID: "openai", + modelID: "gpt-5.5", + createdAt: 10, + monotonicStartMs: 100, + }) + + const attempt = recorder.beginAttempt({ attemptIndex: 1, at: 11, monotonicMs: 110 }) + recorder.recordAttemptFailureAndDeriveRecovery({ + attemptID: attempt.attemptID, + at: 12, + monotonicMs: 120, + error: { name: "APIError", message: "Response decompression failed", code: "ZlibError" }, + evidence: ["iterator_error"], + retryable: true, + providerFailure: { kind: "decompression", code: "ZlibError" }, + }) + + const summary = recorder.finalize({ completedAt: 13, monotonicMs: 130 }) + expect(summary.classification).toBe("external_stream_disconnect") + expect(summary.incident?.terminal_cause.category).toBe("provider_transport_disconnect") + }) + + test("keeps an unknown-kind failure with no HTTP evidence on the transport path", () => { + const recorder = RunObservability.createRecorder({ + runID: RunObservability.RunID.make("run_unknown_no_evidence"), + traceID: MessageID.make("msg_unknown_no_evidence"), + sessionID: SessionID.make("ses_unknown_no_evidence"), + messageID: MessageID.make("msg_unknown_no_evidence"), + providerID: "openai", + modelID: "gpt-5.5", + createdAt: 10, + monotonicStartMs: 100, + }) + + const attempt = recorder.beginAttempt({ attemptIndex: 1, at: 11, monotonicMs: 110 }) + recorder.recordAttemptFailureAndDeriveRecovery({ + attemptID: attempt.attemptID, + at: 12, + monotonicMs: 120, + // A wrapped network failure (e.g. DNS ENOTFOUND) the stream classifier did + // not recognize: kind "unknown" with no status code and no response body. + error: { name: "APIError", message: "fetch failed", code: "ENOTFOUND" }, + evidence: ["iterator_error"], + retryable: true, + providerFailure: { kind: "unknown", code: "ENOTFOUND" }, + }) + + const summary = recorder.finalize({ completedAt: 13, monotonicMs: 130 }) + expect(summary.classification).toBe("external_stream_disconnect") + expect(summary.incident?.terminal_cause.category).toBe("provider_transport_disconnect") + }) + + test("routes an unknown-kind failure WITH an HTTP status to provider_api_error", () => { + const recorder = RunObservability.createRecorder({ + runID: RunObservability.RunID.make("run_unknown_with_status"), + traceID: MessageID.make("msg_unknown_with_status"), + sessionID: SessionID.make("ses_unknown_with_status"), + messageID: MessageID.make("msg_unknown_with_status"), + providerID: "deepseek", + modelID: "deepseek-chat", + createdAt: 10, + monotonicStartMs: 100, + }) + + const attempt = recorder.beginAttempt({ attemptIndex: 1, at: 11, monotonicMs: 110 }) + recorder.recordAttemptFailureAndDeriveRecovery({ + attemptID: attempt.attemptID, + at: 12, + monotonicMs: 120, + // 402 Insufficient Balance before PR1 classifies it: kind "unknown" but a + // real HTTP response (status 402) was received, so it is a provider API error. + error: { name: "APIError", message: "402 Insufficient Balance" }, + evidence: ["iterator_error"], + retryable: false, + providerFailure: { kind: "unknown", statusCode: 402, hasResponseBody: true }, + }) + + const summary = recorder.finalize({ completedAt: 13, monotonicMs: 130 }) + expect(summary.classification).toBe("provider_api_error") + expect(String(summary.summary_key)).toBe("provider_api_error.unknown") + expect(summary.incident?.terminal_cause).toMatchObject({ + category: "provider_api_error", + subcategory: "unknown", + retryable: false, + }) + }) + test("derives provider transport incident during partial tool input instead of tool failure", () => { const recorder = RunObservability.createRecorder({ runID: RunObservability.RunID.make("run_partial_tool_input_disconnect"), @@ -1626,6 +1829,106 @@ describe("RunObservability", () => { }) }) + describe("recoveryFor — terminal provider API rejection", () => { + const terminalProviderCause = { + category: "provider_api_error", + subcategory: "quota_exhausted", + confidence: "high", + } satisfies RunIncident.TerminalCause + + // retryable=false → a terminal (do-not-retry) provider rejection, e.g. a 402 + // "Insufficient Balance". It always stops; the reason reflects side-effect risk. + function terminalProviderRecovery(overrides: Partial) { + return RunIncident.recoveryFor({ cause: terminalProviderCause, facts: beforeProgressFacts(overrides), retryable: false }) + } + + test("with no side-effect risk it stays a pure provider passthrough", () => { + // No tool ran, no unsafe side effect, side-effect facts complete: keep + // reason "provider_api_error" so the renderer shows the real provider text. + expect( + terminalProviderRecovery({ + tool_execution_started: false, + unsafe_side_effect_started: false, + side_effect_facts_complete: true, + }), + ).toMatchObject({ recommendation: "do_not_retry", reason: "provider_api_error" }) + }) + + test("after a tool ran it surfaces the tool_execution_started safety reason (still do_not_retry)", () => { + expect(terminalProviderRecovery({ tool_execution_started: true, side_effect_facts_complete: true })).toMatchObject({ + recommendation: "do_not_retry", + reason: "tool_execution_started", + }) + }) + + test("after an unsafe side effect it surfaces the unsafe_side_effect_started safety reason (precedence over tool)", () => { + expect( + terminalProviderRecovery({ + unsafe_side_effect_started: true, + tool_execution_started: true, + side_effect_facts_complete: true, + }), + ).toMatchObject({ recommendation: "do_not_retry", reason: "unsafe_side_effect_started" }) + }) + + test("with incomplete side-effect facts it surfaces the side_effect_facts_incomplete safety reason", () => { + expect(terminalProviderRecovery({ side_effect_facts_complete: false })).toMatchObject({ + recommendation: "do_not_retry", + reason: "side_effect_facts_incomplete", + }) + }) + }) + + test("a terminal provider rejection after a tool ran keeps both the provider reason and the safety hint (end-to-end)", () => { + // Record a real tool execution, then drive recordAttemptFailureAndDeriveRecovery + // with a terminal quota_exhausted (retryable=false). The derived recovery must + // carry the side-effect reason (not the bare "provider_api_error" passthrough), + // and the final halt message must keep BOTH the provider reason and the + // "check external state" hint — so a user who tops up their balance and resends + // does not silently re-run the tool's side effect. + const recorder = RunObservability.createRecorder({ + runID: RunObservability.RunID.make("run_terminal_provider_after_tool"), + traceID: MessageID.make("msg_terminal_provider_after_tool"), + sessionID: SessionID.make("ses_terminal_provider_after_tool"), + messageID: MessageID.make("msg_terminal_provider_after_tool"), + providerID: "deepseek", + modelID: "deepseek-chat", + createdAt: 10, + monotonicStartMs: 100, + }) + const attempt = recorder.beginAttempt({ attemptIndex: 1, at: 11, monotonicMs: 110 }) + recorder.recordToolCallMaterialized({ + attemptID: attempt.attemptID, + at: 12, + monotonicMs: 120, + toolName: RunObservability.safeToolName("grep"), + effect: RunObservability.toolEffect("grep"), + }) + recorder.recordToolExecutionStarted({ + attemptID: attempt.attemptID, + at: 13, + monotonicMs: 130, + toolName: RunObservability.safeToolName("grep"), + effect: RunObservability.toolEffect("grep"), + }) + const recovery = recorder.recordAttemptFailureAndDeriveRecovery({ + attemptID: attempt.attemptID, + at: 14, + monotonicMs: 140, + error: { name: "AI_APICallError", message: "Insufficient Balance" }, + retryable: false, + providerFailure: { kind: "quota_exhausted", statusCode: 402, hasResponseBody: true }, + }) + + // Side-effect reason, not the pure "provider_api_error" passthrough; still do_not_retry. + expect(recovery).toMatchObject({ recommendation: "do_not_retry", reason: "tool_execution_started" }) + + const message = haltInterruptionMessage(true, recovery, "Insufficient Balance") + expect(message).toContain("Insufficient Balance") + expect(message).toContain("check whether the last operation completed") + expect(message).not.toContain("Connection lost") + }) + test("transport failure after tool input end is not classified as text generation", () => { const recorder = RunObservability.createRecorder({ runID: RunObservability.RunID.make("run_after_tool_input_end"),