Files
PaperClipAI/server/src/services/execution-recovery-attempt.test.ts
T
DottaandClaude Opus 5.5 a4a3e342ec fix(recovery): charge a restart attempt only when the provider accepted a turn and made no progress (#19948)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - A run has an execution recovery budget: three provider attempts for
native runs, and two failure retries for legacy runs
> - In the restart chaos campaign, control-plane restarts used up that
budget for runs that were healthy (RC-22, 22b, 22c, 40 and 60)
> - #19720 and #19747 fixed two variants with watermark heuristics, and
legacy interrupted preparation still spent the failure budget
> - The durable-ownership plan (section F, invariant 5) asks for one
rule: charge an attempt only when a recorded outcome proves that
provider work started and no progress followed
> - This pull request adds that rule and moves the native restart charge
and the legacy preparation retry onto it
> - The benefit is that a restart never spends budget for work the
provider never started, while real failure loops and crash loops stay
bounded

## Linked Issues or Issue Description

Refs #19720, #19747, #19856

**What happened?**
A control-plane restart charges a provider attempt by heuristics. The
first crash restart always charges. A later restart charges unless the
previous resume had no start evidence (#19747) or completed work
(#19720). A runner that spawned but whose provider never accepted a turn
counted as started, so a K6 crash loop could still spend the budget.
Legacy runs whose preparation a restart interrupted
(`providerWorkStarted: false`) spent `failureRetries`, the same counter
as real failures (`execution-recovery-attempt.ts`).

**Expected behavior**
An attempt is charged only when the provider accepted a turn
(`turn.accepted` or `turn.started`) and no accepted turn then completed
work. A startup before spawn, a runner that spawned but was never
admitted, and a `turn.start` the provider did not accept are not
charged. The generation-12 cap still bounds uncharged restarts.

**Steps to reproduce**
Start a native run at its last attempt. Kill the controller after the
runner spawns but before the provider accepts a turn. Before this
change, the next restart claim blocks the run with
`execution_recovery_budget_exhausted`.

## What Changed

- New `server/src/services/native-runtime/provider-attempt-outcome.ts`:
the one rule. `readProviderAttemptOutcome()` returns
`provider_not_started`, `provider_started_without_progress` or
`provider_progressed` from run events. `providerAttemptChargesBudget()`
charges only the second. `providerAttemptWindowStart()` starts the
attempt window at the latest `native.process_start_requested`, so
earlier work never counts for a later attempt.
`PROVIDER_ATTEMPT_CREDIT_GENERATION_LIMIT` (12) is the shared cap.
- `restartResumeKeepsAttempt()` (`native-restart-recovery.ts`) now
applies that rule to the attempt the restart interrupted, including the
first restart. It keeps the unstarted-resume carry from #19747, the
reattachment rule (an adopted provider counts as started), the drain
exemption from #19625, and the generation-12 cap. A history entry from
an older release without a recorded position still charges.
- Legacy runs: `interrupted_preparation` is now an uncharged retry lane
(`execution-recovery-attempt.ts`). It keeps its own counter, bounded at
the same cap (12), and never changes `failureRetries`. `retries.ts` uses
the longest transient delay after the second lane attempt.
- `doc/execution-semantics.md` states the rule.
- Accounting has its own "provider work started" readers (inventory
Summary finding 5). Phase 3 owns those; this PR exposes the rule for
them to adopt and does not change them.

## Verification

- `native-runner-restart-recovery.integration.test.ts` (embedded
Postgres, real `claimNativeRestartRecoveries`): 129 passed. New cases,
red on master:
- not charged at the last attempt for a claimed remote bootstrap, a
remote bootstrap retry, and a claimed unlaunched remote run;
- not charged when the runner spawned but was never admitted, and when
the provider did not accept `turn.start`;
- not charged on the first restart when the original attempt already
made progress;
- a normal retry that spawned but was never admitted is not charged, and
earlier work still does not count for it.
- Counter-cases, green on master and with the change: an accepted
`turn.accepted` or `turn.started` with no progress is charged; the
generation cap blocks at the last attempt; crash loops with and without
progress stay bounded; the #19720 and #19747 cases still pass.
- `execution-recovery-attempt.test.ts`: new lane tests (red on master).
- `workspace-recovery-preparation-dispatch.test.ts` (25 passed),
`heartbeat-process-recovery.test.ts` (interrupted-run cases),
`run-dispatch/adapters/postgres.test.ts` (interrupted cases).
- `server` typecheck (`scripts/typecheck-layers.mjs`, low-memory mode):
no errors.
- Not run: chaos waves (the coordinator schedules them on this host),
and the full server suite (CI covers it).

## Risks

- Behavior change: the first crash restart no longer charges when no
accepted turn is recorded. If a crash lands before the controller
projects the provider's `turn.accepted`, that attempt is not charged.
This fails toward more retries, and the generation-12 cap bounds it.
- A legacy preparation crash loop now gets up to 12 retries instead of
2. Each retry still passes the normal issue gates and keeps the
transient delay.
- The new queries use the `(run_id, seq)` index, once per restart claim.
Nothing new runs before `turn.start`.

> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with
tool use (shell, file edits, test execution). Extended reasoning
enabled.

## 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>
2026-11-07 21:20:05 -06:00

110 lines
7.5 KiB
TypeScript

import { describe, expect, it } from "vitest";
import { accountingForScheduledRetry, executionFailureRetryCount, executionRetryAttemptCount, INTERRUPTED_PREPARATION_RETRY_MAX_ATTEMPTS, INTERRUPTED_PREPARATION_RETRY_REASON } from "./execution-recovery-attempt.js";
import { RUNNER_REVISION_WAIT_RETRY_REASON } from "./runner-revision-mismatch.js";
import { RUNNER_REVISION_WAIT_RETRY_REASON as DISPATCH_RUNNER_REVISION_WAIT_RETRY_REASON } from "../modules/run-dispatch/index.js";
describe("failure attempts across resource waits", () => {
it("preserves prior failures through subscription waits without trusting unrelated context", () => {
expect(executionFailureRetryCount({ scheduledRetryReason: "ai_connection_busy", scheduledRetryAttempt: 12,
contextSnapshot: { failureRetriesBeforeAiConnectionWait: 1 } })).toBe(1);
expect(executionFailureRetryCount({ scheduledRetryReason: "transient_failure", scheduledRetryAttempt: 2,
contextSnapshot: { failureRetriesBeforeAiConnectionWait: 0 } })).toBe(2);
for (const count of [undefined, -1, 1.5, "0"]) {
expect(executionFailureRetryCount({ scheduledRetryReason: "ai_connection_busy", scheduledRetryAttempt: 4,
contextSnapshot: { failureRetriesBeforeAiConnectionWait: count } })).toBe(4);
}
});
it("preserves prior failures through repeated workspace waits", () => {
expect(executionFailureRetryCount({ scheduledRetryReason: "workspace_busy", scheduledRetryAttempt: 12,
contextSnapshot: { failureRetriesBeforeWorkspaceWait: 1 } })).toBe(1);
});
it("does not trust a caller-supplied count outside a server-created workspace retry", () => {
expect(executionFailureRetryCount({ scheduledRetryReason: "transient_failure", scheduledRetryAttempt: 2,
contextSnapshot: { failureRetriesBeforeWorkspaceWait: 0 } })).toBe(2);
});
it("keeps ambiguous historical counts without treating a historical productive count as failures", () => {
expect(executionFailureRetryCount({ scheduledRetryReason: "workspace_busy", scheduledRetryAttempt: 4 })).toBe(4);
expect(executionFailureRetryCount({ scheduledRetryReason: "max_turns_continuation", scheduledRetryAttempt: 4 })).toBe(0);
});
});
describe("persisted independent accounting", () => {
it.each(["max_turns_continuation", "issue_disposition_repair", "workspace_busy", "ai_connection_busy", "ai_connection_pool_wait", "runner_revision_wait"])("%s cannot erase prior infrastructure debits or spend more", scheduledRetryReason => {
expect(executionFailureRetryCount({ scheduledRetryReason, scheduledRetryAttempt: 20,
contextSnapshot: { executionRetryAccounting: { version: 1, failureRetries: 2, maxTurnContinuations: 1 } },
})).toBe(2);
});
it("never lowers the current failure count from a partial or stale ledger", () => {
for (const executionRetryAccounting of [
{ version: 1, failureRetries: 0, maxTurnContinuations: 1 },
{ version: 1, failureRetries: -1, maxTurnContinuations: 1 },
{ version: 1, failureRetries: "0", maxTurnContinuations: 1 },
{ version: 2, failureRetries: 0, maxTurnContinuations: 1 },
]) expect(executionFailureRetryCount({ scheduledRetryReason: "transient_failure", scheduledRetryAttempt: 2,
contextSnapshot: { executionRetryAccounting },
})).toBe(2);
});
});
describe("Runner update waits", () => {
it("uses the same retry reason as run dispatch", () => {
expect(RUNNER_REVISION_WAIT_RETRY_REASON).toBe(DISPATCH_RUNNER_REVISION_WAIT_RETRY_REASON);
});
const ledger = (failureRetries: number) => ({ executionRetryAccounting: { version: 1, failureRetries, maxTurnContinuations: 0 } });
it("never spend the failure retry budget", () => {
const waited = { scheduledRetryReason: "runner_revision_wait", scheduledRetryAttempt: 9, contextSnapshot: ledger(0) };
expect(executionRetryAttemptCount(waited, "runner_revision_wait")).toBe(9);
expect(accountingForScheduledRetry(waited, "runner_revision_wait", 10)).toMatchObject({ failureRetries: 0 });
expect(executionFailureRetryCount(waited)).toBe(0);
expect(executionRetryAttemptCount(waited, "transient_failure")).toBe(0);
});
it("keep failures recorded before the wait and are not shortened by them", () => {
const failed = { scheduledRetryReason: "transient_failure", scheduledRetryAttempt: 1, contextSnapshot: ledger(1) };
expect(executionRetryAttemptCount(failed, "runner_revision_wait")).toBe(0);
const waited = { scheduledRetryReason: "runner_revision_wait", scheduledRetryAttempt: 3,
contextSnapshot: { ...ledger(1), executionRetryAccounting: accountingForScheduledRetry(failed, "runner_revision_wait", 3) } };
expect(executionFailureRetryCount(waited)).toBe(1);
expect(executionRetryAttemptCount(waited, "transient_failure")).toBe(1);
});
});
// Section F: a preparation interrupted before provider work started spent no
// provider work, so it never charges the failure budget. Its own lane counts
// its retries, up to the same cap as native controller generations.
describe("interrupted preparation retries", () => {
const accounting = (failureRetries: number) => ({ version: 1, failureRetries, maxTurnContinuations: 0 });
it("keeps the failure count when scheduling an interrupted-preparation retry", () => {
const source = { scheduledRetryReason: "transient_failure", scheduledRetryAttempt: 1,
contextSnapshot: { executionRetryAccounting: accounting(1) } };
expect(executionRetryAttemptCount(source, INTERRUPTED_PREPARATION_RETRY_REASON)).toBe(0);
expect(accountingForScheduledRetry(source, INTERRUPTED_PREPARATION_RETRY_REASON, 1))
.toEqual({ ...accounting(1), interruptedPreparations: 1 });
});
it("counts consecutive interrupted preparations in their own lane, never as failures", () => {
const retry = { scheduledRetryReason: INTERRUPTED_PREPARATION_RETRY_REASON, scheduledRetryAttempt: 5,
contextSnapshot: { executionRetryAccounting: accounting(0) } };
expect(executionRetryAttemptCount(retry, INTERRUPTED_PREPARATION_RETRY_REASON)).toBe(5);
expect(executionFailureRetryCount(retry)).toBe(0);
expect(accountingForScheduledRetry(retry, INTERRUPTED_PREPARATION_RETRY_REASON, 6))
.toEqual({ ...accounting(0), interruptedPreparations: 6 });
expect(INTERRUPTED_PREPARATION_RETRY_MAX_ATTEMPTS).toBe(12);
});
it("keeps the interrupted-preparation count across a wait retry in between", () => {
const prepared = accountingForScheduledRetry({ scheduledRetryReason: INTERRUPTED_PREPARATION_RETRY_REASON,
scheduledRetryAttempt: 4, contextSnapshot: { executionRetryAccounting: accounting(0) } },
INTERRUPTED_PREPARATION_RETRY_REASON, 5);
expect(prepared).toEqual({ ...accounting(0), interruptedPreparations: 5 });
// A workspace wait in between carries the chain's accounting forward.
const waited = { scheduledRetryReason: "workspace_busy", scheduledRetryAttempt: 1,
contextSnapshot: { executionRetryAccounting: accountingForScheduledRetry(
{ contextSnapshot: { executionRetryAccounting: prepared } }, "workspace_busy", 1) } };
expect(executionRetryAttemptCount(waited, INTERRUPTED_PREPARATION_RETRY_REASON)).toBe(5);
});
it("still charges a real failure after interrupted preparations", () => {
const retry = { scheduledRetryReason: INTERRUPTED_PREPARATION_RETRY_REASON, scheduledRetryAttempt: 3,
contextSnapshot: { executionRetryAccounting: accounting(1) } };
expect(executionRetryAttemptCount(retry, "transient_failure")).toBe(1);
expect(accountingForScheduledRetry(retry, "transient_failure", 2).failureRetries).toBe(2);
});
});