From 35d086213e4042363ca962abe26dd1372796bafa Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:40:05 -0600 Subject: [PATCH] fix(native): explain failed accepted results on the run (#19091) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run ends when the server projects the accepted result onto the run row. > - Operators read `errorCode` and `error` on a failed run to learn why it failed. > - Three native writers can set a run to `failed` without these fields. #19063 repairs the finalizer retry-exhaustion writer. > - The two result-projection writers still set `failed` with no cause when the provider failure is not classified. > - This pull request makes both projection writers record a cause when they project `failed`. > - The benefit is that, with #19063, no native writer can leave a failed run unexplained. ## Linked Issues or Issue Description Refs #19063. **What happened?** A staging soak found 11 failed native runs with `errorCode` null and `error` null. In each run the provider turn succeeded. Workspace copy-back then failed three times, and the finalizer retry-exhaustion path set the run to `failed`. #19063 fixes that writer. An audit of every `heartbeat_runs` status writer found two more writers with the same defect. `finalizeNativeRun` and `projectCommittedRun` project an accepted result with `runTerminalState: "failed"`. They write the error fields only when a classified provider failure exists. **Expected behavior** A run with status `failed` always has an `errorCode` and an `error`. A cause that is already recorded stays. **Steps to reproduce** 1. Accept a native result with `runTerminalState: "failed"` and no `turn.failed` event. 2. Call `finalizeNativeRun` with `projectRunStatus: true`, or project the committed run through `projectCommittedRun`. 3. Read the run. On master its status is `failed`, and its `errorCode` and `error` are null. ## What Changed - Add `explainFailedRun` in `server/src/services/native-runtime/native-run-failure-explanation.ts`. It builds SQL SET expressions. They keep a non-empty existing `errorCode` or `error`. Otherwise they write `native_run_failed` and a readable message. By default the expressions leave a run that is already `cancelled` unchanged, as the status writers do. - `finalizeNativeRun` uses the helper when it projects `failed`. The existing provider-failure and recovered-failure values remain the preferred causes. - `projectCommittedRun` uses the helper for a `failed` accepted result. This writer sets status unconditionally, so it does not use the cancelled branch. - Writer audit: all legacy and heartbeat callers of `setRunStatus*` that pass `failed` already set both fields. The other direct native writers also set both fields. ## Verification - Red, with the source change stashed: 3 new cases failed with `errorCode: null`. These were the finalizer projection, recovery projection with no cause, and recovery projection with an empty error. The control case with a recorded cause passed. - Green: `npx vitest run src/__tests__/native-run-projection-work.test.ts src/services/native-runtime/native-run-finalizer-telemetry.test.ts src/__tests__/native-status-arbiter-corpus.test.ts` (225 passed). - Wider run: all `server/src/__tests__/native-*.test.ts` and `server/src/services/native-runtime/*.test.ts` (7416 passed). Two failures come from the local environment. `plugin-runner-probe` needs the Rust runner source. `native-ingress-bootstrap-retry` needs the controller-host OpenCode proxy. - This change was not exercised on staging. It changes only fields that are written at terminal projection. ## Risks - Low risk. The change affects only rows that are projected to `failed`. A recorded cause is never replaced. Succeeded and cancelled projections are unchanged. - The staging failures need #19063 too. This PR alone does not change their writer. - The cause behind the staging copy-back failures is still unknown. The sandbox command reports the redacted `remote_workspace_operation_failed`, which also appears in a separate signature. This PR does not change that diagnostic. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edits, browser reads of staging run records). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Paperclip Co-authored-by: Claude Opus 5.5 --- .../native-run-projection-work.test.ts | 20 +++++++++++ .../native-run-failure-explanation.ts | 36 +++++++++++++++++++ .../native-runtime/native-run-finalizer.ts | 23 ++++++++---- .../native-runtime/native-run-projection.ts | 8 ++++- 4 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 server/src/services/native-runtime/native-run-failure-explanation.ts diff --git a/server/src/__tests__/native-run-projection-work.test.ts b/server/src/__tests__/native-run-projection-work.test.ts index 462f2d57ae5..678e66b0633 100644 --- a/server/src/__tests__/native-run-projection-work.test.ts +++ b/server/src/__tests__/native-run-projection-work.test.ts @@ -15,6 +15,7 @@ import { agentWakeupRequests, agents, companies, completionContracts, createDb, import { createDeliveryWorkCoordinator } from "../services/delivery-work-coordinator.js"; import { nativeRunProjectionWork } from "../services/native-runtime/native-run-projection-work.js"; import { projectCommittedRun } from "../services/native-runtime/native-run-projection.js"; +import { NATIVE_RUN_FAILED_UNEXPLAINED } from "../services/native-runtime/native-run-failure-explanation.js"; import { notifyNativeFinalizationWork } from "../services/native-finalization-notifications.js"; import { finalizeNativeRun, recordNativeFinalizationFailure } from "../services/native-runtime/native-run-finalizer.js"; import * as statusCommitter from "../services/native-runtime/status-decision-committer.js"; @@ -430,6 +431,25 @@ describe("committed native run projection lifecycle", () => { expect((await read(binding.runId)).executionStatusDeliveryId).toBe(settled.executionStatusDeliveryId); expect(emitAgentTaskRun).toHaveBeenCalledTimes(1); }); + it("explains a failed accepted result when the finalizer projects the run", async () => { + const binding = await accepted("failed"); + await finalizeNativeRun({ db, runId: binding.runId, workspaceFinalizeStatus: "succeeded", projectRunStatus: true }); + const run = await read(binding.runId); + expect(run).toMatchObject({ status: "failed", errorCode: NATIVE_RUN_FAILED_UNEXPLAINED.errorCode, error: NATIVE_RUN_FAILED_UNEXPLAINED.error }); + }); + it.each([ + ["no recorded cause", null, null, NATIVE_RUN_FAILED_UNEXPLAINED.errorCode, NATIVE_RUN_FAILED_UNEXPLAINED.error], + ["an empty error", "adapter_failed", " ", "adapter_failed", NATIVE_RUN_FAILED_UNEXPLAINED.error], + ["a recorded cause", "adapter_failed", "Adapter exited 1", "adapter_failed", "Adapter exited 1"], + ])("explains a failed accepted result in committed recovery projection with %s", async (_label, errorCode, error, expectedCode, expectedError) => { + const binding = await accepted("failed"); + await finalizeNativeRun({ db, runId: binding.runId, workspaceFinalizeStatus: "succeeded" }); + await db.update(heartbeatRuns).set({ errorCode, error }).where(eq(heartbeatRuns.id, binding.runId)); + const run = await read(binding.runId); + expect(run.status).toBe("running"); + await projectCommittedRun({ db, run, coordinator: await state(binding.runId) }); + expect(await read(binding.runId)).toMatchObject({ status: "failed", errorCode: expectedCode, error: expectedError }); + }); it("ignores routine company-only finalization hints and still repairs a committed run", async () => { await start(); await flush(); await quiet(); const previous = db.$client.options.debug; diff --git a/server/src/services/native-runtime/native-run-failure-explanation.ts b/server/src/services/native-runtime/native-run-failure-explanation.ts new file mode 100644 index 00000000000..73e7c0df69b --- /dev/null +++ b/server/src/services/native-runtime/native-run-failure-explanation.ts @@ -0,0 +1,36 @@ +import { sql, type SQL } from "drizzle-orm"; +import { heartbeatRuns } from "@paperclipai/db"; + +/** Used when a native run reaches `failed` and no writer recorded a more specific cause. */ +export const NATIVE_RUN_FAILED_UNEXPLAINED = { + errorCode: "native_run_failed", + error: "The native run ended in a failed state without a classified provider error. Inspect the run events for the last recorded step.", +} as const; + +export type NativeRunFailureExplanation = { errorCode: string; error: string }; + +/** + * A failed run must always say why. These SET expressions keep an explanation + * already on the row, and otherwise record `fallback`. A run that is already + * cancelled keeps its own fields, because the status writers preserve a + * concurrent cancellation. In an UPDATE, column references read the row before + * the update, which matches the status `case` expressions of the callers. + * + * `current` overrides the expression used for the existing value, for writers + * that already compute a projected value for that field. A writer that sets + * `failed` unconditionally passes `preserveCancelled: false`. + */ +export function explainFailedRun( + fallback: NativeRunFailureExplanation, + current: { errorCode?: SQL; error?: SQL; preserveCancelled?: boolean } = {}, +) { + const errorCode = current.errorCode ?? sql`${heartbeatRuns.errorCode}`; + const error = current.error ?? sql`${heartbeatRuns.error}`; + const explainedCode = sql`coalesce(nullif(btrim(${errorCode}), ''), ${fallback.errorCode})`; + const explainedError = sql`coalesce(nullif(btrim(${error}), ''), ${fallback.error})`; + if (current.preserveCancelled === false) return { errorCode: explainedCode, error: explainedError }; + return { + errorCode: sql`case when ${heartbeatRuns.status} = 'cancelled' then ${heartbeatRuns.errorCode} else ${explainedCode} end`, + error: sql`case when ${heartbeatRuns.status} = 'cancelled' then ${heartbeatRuns.error} else ${explainedError} end`, + }; +} diff --git a/server/src/services/native-runtime/native-run-finalizer.ts b/server/src/services/native-runtime/native-run-finalizer.ts index d9079d58b70..08baf07c3b9 100644 --- a/server/src/services/native-runtime/native-run-finalizer.ts +++ b/server/src/services/native-runtime/native-run-finalizer.ts @@ -26,6 +26,7 @@ import { hasPendingNativeChildCompletion } from "./native-child-completion-deliv import { settleExternalConversation } from "../slack-conversation-lifecycle.js"; import { executionFailureRetryCount } from "../execution-recovery-attempt.js"; import { readPersistedNativeFailureEvidence } from "./native-provider-failure-evidence.js"; +import { explainFailedRun, NATIVE_RUN_FAILED_UNEXPLAINED } from "./native-run-failure-explanation.js"; import { dismissAutomaticCompletionReviews } from "./automatic-completion-reviews.js"; import { findSupersededExternalReviewRequests } from "./superseded-external-review-requests.js"; import { getNativeReviewAssignment, readNativeReviewAssignmentContext } from "./native-review-participant.js"; @@ -1629,12 +1630,20 @@ export async function finalizeNativeRun(input: { || ${JSON.stringify(finalizationMetadata)}::jsonb || ${clearExecutionFailure ? sql`case when ${heartbeatRuns.status} = 'cancelled' then '{}'::jsonb else ${recoveredExecutionFailureMetadata()} end` : sql`'{}'::jsonb`}`; - const projectedError = sql`case when ${heartbeatRuns.status} = 'cancelled' then ${heartbeatRuns.error} - else ${providerFailure ? sql`coalesce(${heartbeatRuns.error}, ${providerFailure.errorMessage})` - : clearExecutionFailure ? sql`null` : heartbeatRuns.error} end`; - const projectedErrorCode = sql`case when ${heartbeatRuns.status} = 'cancelled' then ${heartbeatRuns.errorCode} - else ${providerFailure ? sql`coalesce(${heartbeatRuns.errorCode}, ${providerFailure.errorCode})` - : clearExecutionFailure ? sql`null` : heartbeatRuns.errorCode} end`; + const nextError = providerFailure ? sql`coalesce(${heartbeatRuns.error}, ${providerFailure.errorMessage})` + : clearExecutionFailure ? sql`null` : sql`${heartbeatRuns.error}`; + const nextErrorCode = providerFailure ? sql`coalesce(${heartbeatRuns.errorCode}, ${providerFailure.errorCode})` + : clearExecutionFailure ? sql`null` : sql`${heartbeatRuns.errorCode}`; + // A run projected to failed always records why, even when the provider + // reported a failed terminal without a classified failure. + const projectsFailedStatus = input.projectRunStatus && projectedStatus === "failed"; + const explained = projectsFailedStatus + ? explainFailedRun(NATIVE_RUN_FAILED_UNEXPLAINED, { error: nextError, errorCode: nextErrorCode }) + : null; + const projectedError = explained?.error ?? sql`case when ${heartbeatRuns.status} = 'cancelled' then ${heartbeatRuns.error} + else ${nextError} end`; + const projectedErrorCode = explained?.errorCode ?? sql`case when ${heartbeatRuns.status} = 'cancelled' then ${heartbeatRuns.errorCode} + else ${nextErrorCode} end`; const projectionChanged = or( sql`${heartbeatRuns.nativePhase} is distinct from ${finalizationPhase}`, sql`${heartbeatRuns.resultJson} is distinct from ${projectedResult}`, @@ -1659,7 +1668,7 @@ export async function finalizeNativeRun(input: { : {}), nativePhase: finalizationPhase, nativePhaseUpdatedAt: now, - ...(clearExecutionFailure || providerFailure ? { error: projectedError, errorCode: projectedErrorCode } : {}), + ...(clearExecutionFailure || providerFailure || projectsFailedStatus ? { error: projectedError, errorCode: projectedErrorCode } : {}), resultJson: projectedResult, updatedAt: now, }) diff --git a/server/src/services/native-runtime/native-run-projection.ts b/server/src/services/native-runtime/native-run-projection.ts index 706f80e7366..77dd186a30e 100644 --- a/server/src/services/native-runtime/native-run-projection.ts +++ b/server/src/services/native-runtime/native-run-projection.ts @@ -18,6 +18,7 @@ import { notifyChatRunMilestones } from "../chat-run-milestone-notifications.js" import { emitAgentTaskRun } from "../agent-task-run-telemetry.js"; import { isNewTerminalRunFailure, reportRunFailure } from "../run-failure-report.js"; import { nativeRunnerOwnershipNotHeldCondition } from "./native-runner-ownership.js"; +import { explainFailedRun, NATIVE_RUN_FAILED_UNEXPLAINED } from "./native-run-failure-explanation.js"; export function projectNativeTerminalRunStatus( @@ -192,7 +193,12 @@ export async function projectCommittedRun(input: { || ${recoveredExecutionFailureMetadata()} || jsonb_build_object('finalizationPhase', 'committed', 'failureCode', null, 'originalFailureCode', null, 'nextAttemptAt', null)`, } - : { resultJson: sql`coalesce(${heartbeatRuns.resultJson}, '{}'::jsonb) || jsonb_build_object('finalizationPhase', 'committed', 'failureCode', null, 'originalFailureCode', null, 'nextAttemptAt', null)` }), + : { + // A failed accepted result keeps any recorded cause, and otherwise + // names the unexplained failure. + ...(terminalState === "failed" ? explainFailedRun(NATIVE_RUN_FAILED_UNEXPLAINED, { preserveCancelled: false }) : {}), + resultJson: sql`coalesce(${heartbeatRuns.resultJson}, '{}'::jsonb) || jsonb_build_object('finalizationPhase', 'committed', 'failureCode', null, 'originalFailureCode', null, 'nextAttemptAt', null)`, + }), updatedAt: now, }) .where(needsRepair)