From 09f9d691cf193abe68c140990972a87dd252e0ce Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 4 Nov 2026 11:51:36 -0800 Subject: [PATCH] fix(server): keep the root failure code when the execution deadline fails a run (#19028) 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. > - The execution-control watchdog fails a run when its control deadline passes and no owner finished it. > - The watchdog replaces the run's error code with `execution_finalization_deadline_exceeded`. > - The run-failure report reads the root cause from `resultJson.originalFailureCode`. The watchdog did not write that field. > - So every watchdog failure goes to Sentry with the same fingerprint, whatever its first failure was. > - This pull request keeps the earlier failure code as `originalFailureCode`, as the native finalizer already does. > - The benefit is that Sentry groups watchdog failures by their real first failure, so each cause can be found and fixed. ## Linked Issues or Issue Description **What happened?** One hosted Sentry group collects all `execution_finalization_deadline_exceeded` run failures. It has 17 events from many drivers (Codex app server, OpenCode server, ACPX runtime). In one recent event, the run first failed with `native_execution_ownership_unverified`. The watchdog then failed it at the deadline. The recovery action evidence kept `originalFailureCode: native_execution_ownership_unverified`, but the run row and the Sentry event did not. The event had no `root_error_code` tag, and its fingerprint was only `[execution_finalization_deadline_exceeded, paperclip_runner]`. **Expected behavior** When the watchdog replaces an earlier, more specific error code, the run keeps that code as `resultJson.originalFailureCode`. The run-failure report then sends `root_error_code` and adds it to the fingerprint (`server/src/sentry.ts`). **Steps to reproduce** 1. Create a running run with `errorCode: native_execution_ownership_unverified` and an expired `executionControlDeadlineAt`. 2. Run `reconcileAbandonedExecutionControl`. 3. The run fails with `execution_finalization_deadline_exceeded`, and the earlier code is lost. The captured event has no `diagnostics.execution.originalFailureCode`. **Paperclip version or commit** `master` at `89703c3ff7de`. Refs #14110, a similar fix for the finalization sweep. A search found no other open PR for this path. Draft #18672 changes legacy deferred wakes, which is a different path. ## What Changed - `execution-control-reconciliation.ts`: when the watchdog fails a run, it writes the earlier code (`coordinator.failureCode ?? run.errorCode`) to `resultJson.originalFailureCode`. This is the same value that it already writes to the recovery action evidence. - The watchdog keeps the other `resultJson` fields. - The watchdog does not replace a root cause that is already recorded. - The watchdog does not record the deadline code as its own root cause. - Two regression tests in `execution-control-reconciliation.test.ts`. ## Verification - The new test "keeps the run's prior failure code as the reported root cause" fails on unchanged `master`. The run's `resultJson` has no `originalFailureCode`. With the fix, the test passes. - `pnpm exec vitest run src/services/execution-control-reconciliation.test.ts src/__tests__/execution-control-work.test.ts src/__tests__/run-failure-sentry.test.ts src/__tests__/sentry.test.ts`: 111 passed, 5 skipped. - `pnpm exec vitest run src/services/task-retry.test.ts src/services/heartbeat/recovery.test.ts src/services/native-runtime/native-safe-replacement.test.ts src/services/workspace-recovery.test.ts`: 437 passed. These suites use this watchdog cause. - `node scripts/typecheck-layers.mjs` in `server/`: passed. - Not run locally: the full `pnpm -r typecheck` (the sandbox has no Rust toolchain for the runner crate), `pnpm test:run` and `pnpm build`. CI runs these. ## Risks - Low risk. The change adds one `resultJson` key only when the watchdog moves a run to `failed`, and only when a different earlier code exists. - Other readers of `resultJson.originalFailureCode` (`native-run-projection.ts`, `idle-sleep-safety.ts`) only check committed or succeeded runs. A watchdog failure is neither, so their results do not change. - `isPendingNativeRetry` reads only `nativePhase`, so reporting is not suppressed. - Sentry groups will split by root cause after deployment. New events will go to new groups, and the existing group will get fewer events. This is the intended change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in a Paperclip agent sandbox. ## 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: Claude Opus 5.5 --- .../execution-control-reconciliation.test.ts | 43 +++++++++++++++++++ .../execution-control-reconciliation.ts | 8 ++++ 2 files changed, 51 insertions(+) diff --git a/server/src/services/execution-control-reconciliation.test.ts b/server/src/services/execution-control-reconciliation.test.ts index 1cd52611df5..d53a2870965 100644 --- a/server/src/services/execution-control-reconciliation.test.ts +++ b/server/src/services/execution-control-reconciliation.test.ts @@ -149,6 +149,49 @@ describeEmbeddedPostgres("reconcileAbandonedExecutionControl reports a genuine f }); }); + it("keeps the run's prior failure code as the reported root cause", async () => { + const { runId } = await seedAbandonedRunFixture(); + await db.update(heartbeatRuns).set({ + errorCode: "native_execution_ownership_unverified", + resultJson: { workspaceRestoreFailure: "restore_failed" }, + }).where(eq(heartbeatRuns.id, runId)); + const captureCallsBefore = mockCaptureRunFailure.mock.calls.length; + + await reconcileAbandonedExecutionControl(db); + + const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId)); + expect(run).toMatchObject({ + status: "failed", + errorCode: "execution_finalization_deadline_exceeded", + resultJson: { workspaceRestoreFailure: "restore_failed", originalFailureCode: "native_execution_ownership_unverified" }, + }); + await vi.waitFor(() => { + expect(mockCaptureRunFailure.mock.calls.slice(captureCallsBefore)).toHaveLength(1); + }, { timeout: 5_000 }); + expect(mockCaptureRunFailure.mock.calls.slice(captureCallsBefore)[0]?.[0]).toMatchObject({ + runId, + errorCode: "execution_finalization_deadline_exceeded", + diagnostics: { execution: { originalFailureCode: "native_execution_ownership_unverified" } }, + }); + }); + + it("does not replace an earlier recorded root cause or record the deadline as its own cause", async () => { + const earlier = await seedAbandonedRunFixture(); + await db.update(heartbeatRuns).set({ + errorCode: "native_execution_ownership_unverified", + resultJson: { originalFailureCode: "native_provider_session_failed" }, + }).where(eq(heartbeatRuns.id, earlier.runId)); + const repeated = await seedAbandonedRunFixture(); + await db.update(heartbeatRuns).set({ errorCode: "execution_finalization_deadline_exceeded" }) + .where(eq(heartbeatRuns.id, repeated.runId)); + + await reconcileAbandonedExecutionControl(db); + + const runs = await db.select().from(heartbeatRuns).where(inArray(heartbeatRuns.id, [earlier.runId, repeated.runId])); + expect(runs.find(run => run.id === earlier.runId)?.resultJson).toEqual({ originalFailureCode: "native_provider_session_failed" }); + expect(runs.find(run => run.id === repeated.runId)?.resultJson ?? {}).not.toHaveProperty("originalFailureCode"); + }); + it("leaves a run owned by a live legacy controller to that controller", async () => { const { runId } = await seedAbandonedRunFixture(); const leaseExpiresAt = new Date(Date.now() + 45_000); diff --git a/server/src/services/execution-control-reconciliation.ts b/server/src/services/execution-control-reconciliation.ts index 0133f215d27..3e4c5a080cd 100644 --- a/server/src/services/execution-control-reconciliation.ts +++ b/server/src/services/execution-control-reconciliation.ts @@ -219,6 +219,11 @@ export async function reconcileAbandonedExecutionControl( const cause = "execution_finalization_deadline_exceeded"; const nextAction = "Inspect the failed run and verify its provider has stopped. Reconcile any uncertain external action before explicitly continuing this task."; + // The deadline replaces the run's error code. Keep the earlier + // diagnosis as its root cause, as the native finalizer does. + const priorFailureCode = coordinator?.failureCode ?? run.errorCode; + const recordsOriginalFailure = Boolean(priorFailureCode) && priorFailureCode !== cause && + typeof run.resultJson?.originalFailureCode !== "string"; if (coordinator) await tx .update(nativeRunFinalizations) @@ -256,6 +261,9 @@ export async function reconcileAbandonedExecutionControl( errorCode: cause, error: nextAction, nextAction, + ...(recordsOriginalFailure + ? { resultJson: { ...run.resultJson, originalFailureCode: priorFailureCode } } + : {}), updatedAt: now, }) .where(eq(heartbeatRuns.id, run.id))