mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 12:33:05 +01:00
fix(server): keep the root failure code when the execution deadline fails a run (#19028)
## 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
3a283fbc9b
commit
09f9d691cf
2 files changed
+51
No files matched your search
@@ -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);
|
||||
|
||||
@@ -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))
|
||||
|
||||
Reference in new issue
Block a user