mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 22:13:59 +01:00
## 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 <noreply@paperclip.ing> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
37 lines
1.9 KiB
TypeScript
37 lines
1.9 KiB
TypeScript
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`,
|
|
};
|
|
}
|