fix(native): give run finalization its own retry budget (#20555)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs save an accepted result. Then a finalizer applies that
result to the issue.
> - After a control-plane restart, recovery can resume the same run.
Each resume uses one provider attempt (`coordinator.attempt`).
> - The finalizer used the same `coordinator.attempt` value as its own
retry budget. After three resumes, `attempt` was 3. So the first
transient finalization failure made the coordinator `terminal_failure`.
> - The result was then lost: the issue stayed `in_review` and no Retry
was available. This breaks the rule that a restart must not leave a task
stuck.
> - This pull request gives finalization its own retry count. Provider
attempts no longer use up finalization retries.
> - The benefit: a resumed run that finishes its turn also finishes its
task, and finalization stays bounded.

## Linked Issues or Issue Description

**What happened?**
In restart-chaos wave `w-20261110T044338-s20385`, task t03 (local,
Codex, three K1 SIGKILL restarts, `--db-pressure 4`), recovery resumed
run `d5d0e839` three times (`providerAttempt: 3`). The resumed turn
completed and the server logged `run.result.accepted`, and the run ended
`succeeded`. The first finalization hit `NativeStatusRaceError` three
times under row contention. `recordRetryableFailure` then compared
`coordinator.attempt` (3) with its limit of 3 and wrote
`terminal_failure` / `native_finalization_retry_exhausted`. The run's
`execution.nextAction` is "Reassess against the latest authoritative
issue status version.", which only the `status_cas_exhausted` branch
writes. The second finalize call 2 s later got
`native_finalization_terminal_failure` from `claimCoordinator`. The
issue stayed `in_review` with `canRetry=false`.

**Expected behavior**
A transient finalization failure must stay retryable. The finalizer must
apply the saved result without another provider turn. Finalization must
still have a limit.

**Steps to reproduce**
1. Resume a native run until its coordinator `attempt` is 3, for example
with three restarts.
2. Let the resumed run save an accepted result.
3. Make the first finalization fail with a retryable error.
4. Before this fix, the coordinator goes directly to `terminal_failure`.
The new test
`server/src/__tests__/native-finalization-retry-budget.test.ts`
reproduces this order.

**Paperclip version or commit**
The wave ran at e2b0290213. The bug is still on master at 902f376796. I
found no later merged PR that changes this check.

## What Changed

- `native-run-finalizer.ts`: `recordRetryableFailure` counts failed
finalizations of the saved result in
`heartbeat_runs.result_json.finalizationFailureCount`. It ends retries
after 3 failures. It no longer reads `coordinator.attempt`, which counts
provider attempts. The count goes on the run because each claim clears
the coordinator's `failure_detail`. Workspace-scope failures keep their
existing `workspaceFinalizeAttempt` budget.
- New regression test `native-finalization-retry-budget.test.ts`: a
coordinator saved at `attempt: 3` with no count stays
`retryable_failure` after the first failure. It goes to
`terminal_failure` only after the third failure.

## Verification

- `cd server && npx vitest run
src/__tests__/native-finalization-retry-budget.test.ts
src/__tests__/native-finalization-recovery.test.ts`: 37 tests pass.
- The new test fails on the unchanged finalizer (`terminal_failure`
received, `retryable_failure` expected). It passes with the fix.
- `pnpm verify` (selected tests).
- I did not run a live chaos replay. The evidence is the wave's run
events, its server log, and the code path.

## Risks

- Low risk. The only behavior change: runs that used provider attempts
now get their full 3 finalization retries.
- Previous-release story: rows that an older server saved have no
`finalizationFailureCount`. Their count starts at 0, so they get at most
3 more finalization tries, with no provider work. A coordinator that an
older server already set to `terminal_failure` stays terminal. This PR
migrates nothing.
- If another writer replaces `result_json` without spreading it, the
count can reset. That allows only a few more cheap retries, 30 s apart.
It cannot cause an unbounded loop unless the overwrite repeats.
- Not addressed here: when finalization really runs out of retries for a
succeeded run, the issue still rests `in_review` with a board recovery
card and no Retry. A safe Retry for that case must re-finalize the saved
result, not start a new provider run. That needs a separate change.

## Model Used

- Anthropic Claude, model ID `claude-opus-5-5`, used through Claude Code
with tool use (shell, code editing, test execution).

## 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 <noreply@anthropic.com>
This commit is contained in:
DottaandClaude authored and GitHub committed 2026-11-10 09:45:39 -06:00
1 parent bd30ef8510
commit c58356003f
3 files changed
+168 -2

No files matched your search

@@ -0,0 +1,132 @@
import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js";
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import { eq } from "drizzle-orm";
import {
agents,
companies,
completionContracts,
createDb,
heartbeatRuns,
issues,
nativeRunFinalizations,
nativeRunResults,
workspaceOperations,
} from "@paperclipai/db";
import {
CONTROL_PLANE_CONFORMANCE_OPEN,
CONTROL_PLANE_CONFORMANCE_RESULT,
CONTROL_PLANE_CONFORMANCE_TERMINAL,
} from "../vendor/paperclip-runner/testing.js";
import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
import { reconcileNativeFinalizations } from "../services/native-runtime/native-finalization-reconciler.js";
import { PaperclipControlPlanePort } from "../services/native-runtime/paperclip-control-plane-port.js";
const external = resolveExternalTestDatabaseUrl("PAPERCLIP_NATIVE_FINALIZATION_TEST_DATABASE_URL");
// Restart chaos w-20261110T044338-s20385/t03: three restarts resumed the run
// (provider attempt 3), the resumed turn's result was accepted, and the first
// transient finalization failure was judged against the provider attempt
// count. The coordinator went terminal at once and the issue rested in review
// with no Retry.
describe("native finalization retry budget after restart resumes", () => {
let temporary: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
let db: ReturnType<typeof createDb>;
const companyId = "72100000-0000-4000-8000-000000000001";
const agentId = "72100000-0000-4000-8000-000000000002";
const issueId = "72100000-0000-4000-8000-000000000003";
const contractId = "72100000-0000-4000-8000-000000000004";
const runId = "72100000-0000-4000-8000-000000000005";
beforeAll(async () => {
if (!external) temporary = await startEmbeddedPostgresTestDatabase("paperclip-native-retry-budget-");
db = createDb(external ?? temporary.connectionString);
await db.insert(companies).values({ id: companyId, name: "Native retry budget", issuePrefix: "NRB" });
await db.insert(agents).values({ id: agentId, companyId, name: "Budget agent", adapterType: "codex_local", status: "running" });
await db.insert(issues).values({
id: issueId, companyId, title: "Finalize after three resumes", status: "in_progress",
assigneeAgentId: agentId, workMode: "standard",
});
await db.insert(completionContracts).values({
id: contractId, companyId, issueId, revision: 1,
schemaVersion: "paperclip.completion-contract.v1", policyVersion: "phase6-v1", risk: "standard",
completionAuthority: "server_arbiter", incompleteCriteriaPolicy: "preserve_non_terminal",
contractJson: {
revision: "phase6-v1", objective: "Finalize after three resumes",
criteria: [{ id: "objective", requirement: "Finalization keeps its own budget" }],
},
canonicalSha256: "native-retry-budget-contract", createdByActorType: "system", createdByActorId: "test",
});
await db.insert(heartbeatRuns).values({
id: runId, companyId, agentId, status: "running", runtimeMode: "native",
runtimeModeReason: "persisted_before_kill_switch", nativeIssueId: issueId, nativeSessionId: runId,
runnerInstanceId: contractId, completionContractId: contractId,
completionContractSha256: "native-retry-budget-contract", contextSnapshot: { issueId },
});
const port = new PaperclipControlPlanePort(db, {
companyId, issueId, runId, agentId, sessionId: runId, completionContractId: contractId,
completionContractSha256: "native-retry-budget-contract", sourceInstanceId: contractId,
controlPlaneSourceInstanceId: "budget-control",
});
await port.openRun({
...CONTROL_PLANE_CONFORMANCE_OPEN,
identity: { companyId, issueId, runId, agentId, sessionId: runId },
sourceInstanceId: contractId,
});
await port.completeRun({
result: CONTROL_PLANE_CONFORMANCE_RESULT,
terminal: CONTROL_PLANE_CONFORMANCE_TERMINAL,
callerResultId: "budget-result",
});
// Each finalization attempt fails, as a status race under contention did.
const stored = await db.select().from(nativeRunResults).where(eq(nativeRunResults.runId, runId))
.limit(1).then((rows) => rows[0]!);
await db.update(nativeRunResults).set({
resultJson: {
...(stored.resultJson as Record<string, unknown>),
terminal: { ...CONTROL_PLANE_CONFORMANCE_TERMINAL, runTerminalState: "unknown" },
},
}).where(eq(nativeRunResults.id, stored.id));
await db.insert(workspaceOperations).values({
companyId, heartbeatRunId: runId, issueId, phase: "workspace_finalize", status: "succeeded",
});
// Saved as the previous release left it after three restart resumes: the
// provider attempt count is spent and no finalization count exists yet.
await db.update(nativeRunFinalizations).set({ attempt: 3 }).where(eq(nativeRunFinalizations.runId, runId));
}, 30_000);
afterAll(async () => {
await db?.$client.end({ timeout: 1 });
await temporary?.cleanup();
});
it("keeps a resumed run's first finalization failure retryable and still bounds finalization", async () => {
await expect(reconcileNativeFinalizations(db, [runId])).resolves.toEqual([
expect.objectContaining({ phase: "retryable_failure", failureCode: "native_finalization_invalid" }),
]);
await expect(db.select().from(issues).where(eq(issues.id, issueId))).resolves.toEqual([
expect.objectContaining({ status: "in_progress" }),
]);
await expect(db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId))).resolves.toEqual([
expect.objectContaining({
nativePhase: "retryable_failure",
resultJson: expect.objectContaining({ finalizationFailureCount: 1 }),
}),
]);
await db.update(nativeRunFinalizations).set({ nextAttemptAt: new Date(0) })
.where(eq(nativeRunFinalizations.runId, runId));
await expect(reconcileNativeFinalizations(db, [runId])).resolves.toEqual([
expect.objectContaining({ phase: "retryable_failure" }),
]);
await db.update(nativeRunFinalizations).set({ nextAttemptAt: new Date(0) })
.where(eq(nativeRunFinalizations.runId, runId));
await reconcileNativeFinalizations(db, [runId]);
await expect(db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, runId))).resolves.toEqual([
expect.objectContaining({ phase: "terminal_failure", failureCode: "native_finalization_retry_exhausted" }),
]);
await expect(db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId))).resolves.toEqual([
expect.objectContaining({ resultJson: expect.objectContaining({ finalizationFailureCount: 3 }) }),
]);
});
});
@@ -858,6 +858,9 @@ describeEmbeddedPostgres("native run finalizer / status decision committer — a
it("records the finalization error code when provider-scope retries fail an unsucceeded run", async () => { it("records the finalization error code when provider-scope retries fail an unsucceeded run", async () => {
const fixture = await seedNativeRun(); const fixture = await seedNativeRun();
// Two earlier finalizations of this result failed; this is the third.
await db.update(heartbeatRuns).set({ resultJson: { finalizationFailureCount: 2 } })
.where(eq(heartbeatRuns.id, fixture.runId));
await db.insert(nativeRunFinalizations).values({ await db.insert(nativeRunFinalizations).values({
runId: fixture.runId, companyId, issueId: fixture.issueId, phase: "observed", attempt: 3, runId: fixture.runId, companyId, issueId: fixture.issueId, phase: "observed", attempt: 3,
}); });
@@ -869,6 +872,20 @@ describeEmbeddedPostgres("native run finalizer / status decision committer — a
expect(run?.error).toBeTruthy(); expect(run?.error).toBeTruthy();
}); });
it("keeps a provider-succeeded run's first finalization failure retryable after three provider attempts", async () => {
const fixture = await seedNativeRun();
await db.update(heartbeatRuns).set({ resultJson: { prpRunTerminalState: "succeeded" } })
.where(eq(heartbeatRuns.id, fixture.runId));
await db.insert(nativeRunFinalizations).values({
runId: fixture.runId, companyId, issueId: fixture.issueId, phase: "observed", attempt: 3,
});
const failure = await recordNativeFinalizationFailure({
db, runId: fixture.runId, error: new Error("native_finalization_invalid"), projectRunStatus: true,
});
expect(failure).toMatchObject({ phase: "retryable_failure", failureCode: "native_finalization_invalid" });
await expect(readRunError(fixture.runId)).resolves.toEqual({ status: "running", errorCode: null, error: null });
});
it("fails a provider-succeeded run with an error code when workspace retries are exhausted", async () => { it("fails a provider-succeeded run with an error code when workspace retries are exhausted", async () => {
// The reported case: the provider turn succeeded, then the workspace // The reported case: the provider turn succeeded, then the workspace
// copy-back failed three times. Workspace scope fails the run regardless // copy-back failed three times. Workspace scope fails the run regardless
@@ -887,7 +904,7 @@ describeEmbeddedPostgres("native run finalizer / status decision committer — a
it("does not attach a finalization error to a provider-succeeded run when provider retries are exhausted", async () => { it("does not attach a finalization error to a provider-succeeded run when provider retries are exhausted", async () => {
const fixture = await seedNativeRun(); const fixture = await seedNativeRun();
await db.update(heartbeatRuns).set({ resultJson: { prpRunTerminalState: "succeeded" } }) await db.update(heartbeatRuns).set({ resultJson: { prpRunTerminalState: "succeeded", finalizationFailureCount: 2 } })
.where(eq(heartbeatRuns.id, fixture.runId)); .where(eq(heartbeatRuns.id, fixture.runId));
await db.insert(nativeRunFinalizations).values({ await db.insert(nativeRunFinalizations).values({
runId: fixture.runId, companyId, issueId: fixture.issueId, phase: "observed", attempt: 3, runId: fixture.runId, companyId, issueId: fixture.issueId, phase: "observed", attempt: 3,
@@ -326,6 +326,14 @@ function terminalFinalizationFailureMessage(failureCode: string, failureScope: "
: `Native run finalization failed (${failureCode}).`; : `Native run finalization failed (${failureCode}).`;
} }
const NATIVE_FINALIZATION_RETRY_LIMIT = 3;
/** Failed finalizations of this run's saved result. Rows saved before the count existed start at zero. */
function readFinalizationFailureCount(resultJson: unknown) {
const count = record(resultJson).finalizationFailureCount;
return typeof count === "number" && Number.isInteger(count) && count >= 0 ? count : 0;
}
async function recordRetryableFailure(input: { async function recordRetryableFailure(input: {
db: Db; db: Db;
run: typeof heartbeatRuns.$inferSelect; run: typeof heartbeatRuns.$inferSelect;
@@ -449,11 +457,19 @@ async function recordRetryableFailure(input: {
? priorFailureDetail.workspaceFinalizeAttempt ? priorFailureDetail.workspaceFinalizeAttempt
: 0) + 1 : 0) + 1
: null; : null;
// Finalization has its own budget. `coordinator.attempt` counts provider
// attempts, so a run resumed three times after restarts would otherwise
// turn its first transient finalization failure into a terminal one and
// strand an accepted result. The count lives on the run's result because
// every claim clears the coordinator's failure detail.
const finalizationFailureCount = input.failureScope === "workspace"
? null
: readFinalizationFailureCount(input.run.resultJson) + 1;
const exhausted = const exhausted =
input.permanent === true || input.permanent === true ||
(workspaceFinalizeAttempt !== null (workspaceFinalizeAttempt !== null
? workspaceFinalizeAttempt >= 3 ? workspaceFinalizeAttempt >= 3
: input.coordinator.attempt >= 3); : finalizationFailureCount! >= NATIVE_FINALIZATION_RETRY_LIMIT);
const phase = const phase =
supersededByNewerRun || exhausted supersededByNewerRun || exhausted
? ("terminal_failure" as const) ? ("terminal_failure" as const)
@@ -541,6 +557,7 @@ async function recordRetryableFailure(input: {
originalFailureCode: input.failureCode, originalFailureCode: input.failureCode,
// The latest attempt's redacted cause; cleared when it named none. // The latest attempt's redacted cause; cleared when it named none.
failureCause, failureCause,
...(finalizationFailureCount === null ? {} : { finalizationFailureCount }),
nextAttemptAt: supersededByNewerRun || exhausted ? null : nextAttemptAt.toISOString(), nextAttemptAt: supersededByNewerRun || exhausted ? null : nextAttemptAt.toISOString(),
})}::jsonb`, })}::jsonb`,
updatedAt: now, updatedAt: now,