mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(native): a failed terminal followed by an active restart continuation is not a permanent failure (#20149)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a restart loses a native run's active turn, the run records that turn as failed with `provider_turn_lost_on_restore` and continues on a same-conversation continuation turn > - Restart recovery blocks a run as `provider_checkpoint_permanently_failed` when its checkpoint carries a failed run terminal and no pending restart interruption > - While a continuation is running, the checkpoint keeps the lost turn's failed run terminal and names the continuation as the active turn. The pending-interruption check returns nothing once a turn is active > - So a second restart during the continuation read the run as permanently failed, blocked replay, and needed the board > - This pull request treats that failed terminal as the restart loss it records whenever an active turn follows a restart-lost terminal turn > - The benefit is that runs survive repeated restarts during their continuations, including M1 parents and their subtasks ## Linked Issues or Issue Description Refs #20148 **What happened?** Local restart-chaos M1 K4/K6 rerun w-20261109T003126-s9701 (master8e3b64a257), t05 (claude M1 parent): 1. Run `e1e37dab` lost turns to restarts at 00:41:32 and 00:47:53. Each time it continued, and the third continuation `turn_lab_92b03dc7…` was active. 2. At 00:48:29 the next restart blocked it: `native_restart_recovery_blocked` / `provider_checkpoint_permanently_failed`, with replay blocked. 3. Its saved checkpoint held `terminal.runTerminalState: "failed"` (from the lost turn), `activeTurnId` set to the continuation, and a last terminal turn fingerprinted `provider_turn_lost_on_restore`. Its codex/ACPX subtask's run `f9b10d06` was blocked in the same pass with the same shape, and its continuation had already proposed a semantic result. Both stayed blocked. The parent's Retry was held on the blocked child. **Expected behavior** A failed run terminal that records a restart loss, followed by an active continuation turn, is not a permanent failure. Recovery claims `resume_dead_runner`. The runtime then reconciles the active turn, and a held result settles a lost turn as completed. A real provider failure followed by a later turn still blocks. **Steps to reproduce** 1. Checkpoint a run whose last terminal turn is a restart loss, whose `terminal` is failed, and whose `activeTurnId` is a later continuation (optionally with a semantic result). 2. Claim restart recovery. Before this change it blocks with `provider_checkpoint_permanently_failed`. **Paperclip version or commit**8e3b64a257(wave). Branch rebased on current master. ## What Changed - `contracts/native-session-backend.ts` (runner): the new `nativeRestartContinuationInFlight` is true when all of these hold: - the checkpoint has an active turn; - its last terminal turn is a different turn; - that turn's fingerprint is a failed, result-less, recoverable `provider_turn_lost_on_restore`. - `native-session-runtime.ts`: recovery clears that stale failed terminal for a continuation in flight, as it already does for a checkpointed interruption. Before, it threw `provider_checkpoint_failed_terminal` before it reconciled the active turn. - `native-restart-recovery.ts` (server): `checkpointFailed` now also requires that no restart continuation be in flight. It uses the same contract function through the vendor boundary. - `native-session-runtime.test.ts`: an end-to-end recovery test. A checkpoint with the stale failed terminal and an active continuation resolves on the continuation's result without starting a new turn. - `native-runner-restart-recovery.integration.test.ts`: - a codex case of the stale-terminal shape as saved in production; - an ACPX case with a held result; - a held case where an active turn follows a real provider failure (`model_not_found`). ## Verification - Red/green: on master, the stale-terminal case is blocked with `provider_checkpoint_permanently_failed`. With the change it claims `resume_dead_runner`. The real-failure case stays blocked on both. - Red/green (runtime): without the runtime change, the end-to-end test throws `provider_checkpoint_failed_terminal`. With it, the run completes. - `vitest run` over the restart-recovery and failed-workspace-recovery test files in `server/src/services/native-runtime`: 5 files, 305 tests passed (14 skipped). - `vitest run` over the runner restart, recovery, continuation and terminal test files: 9 files, 385 tests passed. - `PAPERCLIP_TYPECHECK_LOW_MEMORY=1 pnpm typecheck` (`server`) and `pnpm run typecheck:typescript` (`packages/paperclip-runner`): passed. ## Risks - Low risk. The exception applies only when the recorded failure is itself a recoverable restart loss and a later turn is in flight. Every other recovery gate is unchanged: the provider-attempt budget, controller takeover and process proofs. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use (shell, file edits, 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 Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
0502cc9fac
commit
cedf335c98
6 files changed
+94
-7
No files matched your search
@@ -118,6 +118,20 @@ export function nativeRestartInterruptedTurnId(snapshot: Pick<PersistedNativeSes
|
||||
} catch { return null; }
|
||||
}
|
||||
|
||||
export function nativeRestartContinuationInFlight(snapshot: Pick<PersistedNativeSession,
|
||||
"activeTurnId" | "terminalTurns"
|
||||
>): boolean {
|
||||
const activeTurnId = snapshot.activeTurnId;
|
||||
if (typeof activeTurnId !== "string" || !activeTurnId) return false;
|
||||
const last = Array.isArray(snapshot.terminalTurns) ? snapshot.terminalTurns.at(-1) : null;
|
||||
if (!last || typeof last.fingerprint !== "string" || !last.turnId || last.turnId === activeTurnId) return false;
|
||||
try {
|
||||
const fingerprint = JSON.parse(last.fingerprint);
|
||||
return fingerprint?.terminalState === "failed" && fingerprint.result === null &&
|
||||
isNativeRestartInterruption(fingerprint.error);
|
||||
} catch { return false; }
|
||||
}
|
||||
|
||||
/**
|
||||
* Cancellation is a synchronous authority transition followed by passive
|
||||
* provider cleanup. Once `cancel` returns, the provider session must no longer
|
||||
|
||||
@@ -22,7 +22,7 @@ import {
|
||||
NativeSessionCloseUnrecoverableError,
|
||||
NativeSessionCleanupQuarantinedError,
|
||||
NativeSessionProtocolIntegrityError,
|
||||
nativeRestartInterruptedTurnId,
|
||||
nativeRestartContinuationInFlight, nativeRestartInterruptedTurnId,
|
||||
} from "./contracts/native-session-backend.js";
|
||||
import { PROVIDER_ATTACH_REFUSED, PROVIDER_SIDE_EFFECT_PENDING, PROVIDER_STATE_INCOMPATIBLE } from "./contracts/harness-driver.js";
|
||||
import type {
|
||||
@@ -642,6 +642,53 @@ describe("executeNativeSession recovery", () => {
|
||||
expect(events.filter(event => event.eventType === "turn.failed").map(event => event.turnId)).toEqual(["turn-unrecorded-continuation"]);
|
||||
});
|
||||
|
||||
it("recovers a restart continuation whose checkpoint still carries the lost turn's failed terminal", async () => {
|
||||
const failure = { code: "provider_turn_lost_on_restore", recoverable: true };
|
||||
const checkpoint: PersistedNativeSession = {
|
||||
backendKind: "runner", driverKind: "codex_app_server", sessionId: "thread-recovery", identity,
|
||||
providerSessionId: "provider-recovery", cursor: "0", activeTurnId: "turn-continuation",
|
||||
terminal: { schema: "paperclip.prp.terminal.v1", runTerminalState: "failed", turnTerminalState: "failed", reportedWorkDisposition: "yielded" },
|
||||
terminalTurns: [{ turnId: "turn-recovery", fingerprint: JSON.stringify({ terminalState: "failed", error: failure, result: null }) }],
|
||||
semanticResult: null, pendingRuntimeRequests: [], lineage: [],
|
||||
dispositionOnlyRecoveryConsumed: true, dispositionOnlyRecoveryTurnId: "turn-continuation",
|
||||
};
|
||||
const startTurn = vi.fn<NativeSession["startTurn"]>(async () => ({ turnId: "turn-unexpected" }));
|
||||
const session: NativeSession = {
|
||||
identity: () => identity,
|
||||
async capabilities() { return { resume: true, typedEvents: true, steering: false, interruption: true, structuredResult: true }; },
|
||||
async *events() {
|
||||
yield { ...runnerEvent(1, "turn.completed", { status: "completed" }), turnId: "turn-continuation" };
|
||||
},
|
||||
startTurn,
|
||||
async result() { return { result, terminal, turnId: "turn-continuation" }; },
|
||||
async snapshot() { return structuredClone(checkpoint); },
|
||||
async close() {},
|
||||
};
|
||||
const backend: NativeSessionBackend = {
|
||||
async descriptor() { return { kind: "runner", name: "restart", version: "1", capabilities: await session.capabilities() }; },
|
||||
async openSession() { throw new Error("must preserve the provider session"); },
|
||||
async recoverSession() { return { recovered: true, session }; },
|
||||
};
|
||||
const port: ControlPlanePort = {
|
||||
async openRun() {},
|
||||
async checkpointSession() {},
|
||||
async appendEvent(event: PrpEvent) { return { cursor: event.sourceSeq, highestContiguousSourceSeq: event.sourceSeq, disposition: "committed" }; },
|
||||
async replayEvents() { return { events: [], highestContiguousSourceSeq: 0 }; },
|
||||
async completeRun() {},
|
||||
};
|
||||
await expect(executeNativeSession({
|
||||
input, backend, controlPlane: port, persistedSession: checkpoint,
|
||||
runnerInstanceId: "runner-recovery", controlPlaneInstanceId: "control-recovery",
|
||||
resumeInterruptedTurn: true,
|
||||
})).resolves.toMatchObject({ result, turnId: "turn-continuation", terminal: { runTerminalState: "succeeded" } });
|
||||
expect(startTurn).not.toHaveBeenCalled();
|
||||
expect(nativeRestartContinuationInFlight(checkpoint)).toBe(true);
|
||||
expect(nativeRestartContinuationInFlight({ ...checkpoint, activeTurnId: "turn-recovery" })).toBe(false);
|
||||
expect(nativeRestartContinuationInFlight({ ...checkpoint, terminalTurns: [{ turnId: "turn-recovery",
|
||||
fingerprint: JSON.stringify({ terminalState: "failed", error: { code: "model_not_found", recoverable: false }, result: null }) }] }))
|
||||
.toBe(false);
|
||||
});
|
||||
|
||||
it.each(["acpx_runtime", "opencode_server"] as const)("continues a restart-lost %s turn once in the same conversation", async (driverKind) => {
|
||||
const qualified = resolveQualifiedAcpxProfile("claude", "claude-sonnet-5");
|
||||
const acpxInput: NativeExecutionInputV1 = (driverKind === "acpx_runtime" ? {
|
||||
|
||||
@@ -33,6 +33,7 @@ import {
|
||||
NativeSessionCleanupQuarantinedError,
|
||||
NativeSessionProtocolIntegrityError,
|
||||
isNativeRestartInterruption,
|
||||
nativeRestartContinuationInFlight,
|
||||
nativeRestartInterruptedTurnId,
|
||||
} from "./contracts/native-session-backend.js";
|
||||
import {
|
||||
@@ -2327,7 +2328,7 @@ export async function executeNativeSession(
|
||||
} | null = null;
|
||||
let reconciledRecoveryCheckpoint: PersistedNativeSession | null = null;
|
||||
const checkpointedInterruption = persistedSession && nativeRestartInterruptedTurnId(persistedSession);
|
||||
if (checkpointedInterruption && persistedSession) {
|
||||
if (persistedSession && (checkpointedInterruption || nativeRestartContinuationInFlight(persistedSession))) {
|
||||
// The turn failed because its process vanished; the conversation is still
|
||||
// usable. Keep its terminal fingerprint so replay cannot execute it twice.
|
||||
persistedSession = { ...persistedSession, terminal: null };
|
||||
|
||||
@@ -47,7 +47,7 @@ import { logger } from "../../middleware/logger.js";
|
||||
import { issueRecoveryActionService } from "../issue-recovery-actions.js";
|
||||
import { isNewTerminalRunFailure, reportRunFailure } from "../run-failure-report.js";
|
||||
import { isNativeRunnerOwnershipHeld } from "./native-runner-ownership.js";
|
||||
import { nativeRestartInterruptedTurnId, type PersistedNativeSession } from "../../vendor/paperclip-runner/index.js";
|
||||
import { nativeRestartContinuationInFlight, nativeRestartInterruptedTurnId, type PersistedNativeSession } from "../../vendor/paperclip-runner/index.js";
|
||||
import { retireSupersededNativeRetry } from "./native-superseded-retry.js";
|
||||
import { legacyWorkspaceRecoveryNotMigrated } from "../legacy-workspace-recovery-migration.js";
|
||||
import { requestNativeStartupCleanup, readNativeStartupContext, promoteNativeStartupCleanup } from "./native-startup-cleanup.js";
|
||||
@@ -1436,7 +1436,8 @@ export async function claimNativeRestartRecoveries(input: {
|
||||
checkpointFailed:
|
||||
(checkpointRecord.terminal as Record<string, unknown> | undefined)
|
||||
?.runTerminalState === "failed" &&
|
||||
nativeRestartInterruptedTurnId(checkpointRecord as unknown as PersistedNativeSession) === null,
|
||||
nativeRestartInterruptedTurnId(checkpointRecord as unknown as PersistedNativeSession) === null &&
|
||||
!nativeRestartContinuationInFlight(checkpointRecord as unknown as PersistedNativeSession),
|
||||
providerAttempt: row.coordinator.attempt,
|
||||
resumeChargesAttempt: !resumeKeepsAttempt,
|
||||
});
|
||||
|
||||
+26
-3
@@ -2892,8 +2892,10 @@ process.on("SIGTERM", () => child.kill("SIGTERM"));
|
||||
{ attempt: 2, driverKind: "opencode_server" },
|
||||
// A second restart that loses the first restart's continuation turn.
|
||||
{ attempt: 2, driverKind: "codex_app_server", lostContinuation: true },
|
||||
])("recovers a checkpointed $driverKind restart interruption within provider budget $attempt", async ({ attempt, driverKind, lostContinuation }) => {
|
||||
const fixture = await seedRun(`INTERRUPTED-${driverKind}-${attempt}${lostContinuation ? "-CONTINUATION" : ""}`);
|
||||
{ attempt: 2, driverKind: "codex_app_server", lostContinuation: true, staleTerminal: true },
|
||||
{ attempt: 2, driverKind: "acpx_runtime", lostContinuation: true, staleTerminal: true, heldResult: true },
|
||||
])("recovers a checkpointed $driverKind restart interruption within provider budget $attempt", async ({ attempt, driverKind, lostContinuation, staleTerminal, heldResult }) => {
|
||||
const fixture = await seedRun(`INTERRUPTED-${driverKind}-${attempt}${lostContinuation ? "-CONTINUATION" : ""}${staleTerminal ? "-STALE" : ""}${heldResult ? "-RESULT" : ""}`);
|
||||
await fixture.db.update(heartbeatRuns).set({ runnerProfileJson: { sessionCheckpoint: {
|
||||
driverKind,
|
||||
sessionId: fixture.native.normalizedSessionId,
|
||||
@@ -2908,7 +2910,8 @@ process.on("SIGTERM", () => child.kill("SIGTERM"));
|
||||
}) }],
|
||||
// A restart during the first continuation: its turn is still the
|
||||
// checkpointed active turn and the consumed marker names it.
|
||||
...(lostContinuation ? { activeTurnId: "continuation-turn", terminal: null,
|
||||
...(heldResult ? { semanticResult: { schema: "paperclip.run_result.v1", summary: "Saved before the restart." } } : {}),
|
||||
...(lostContinuation ? { activeTurnId: "continuation-turn", ...(staleTerminal ? {} : { terminal: null }),
|
||||
dispositionOnlyRecoveryConsumed: true, dispositionOnlyRecoveryTurnId: "continuation-turn" } : {}),
|
||||
} } }).where(eq(heartbeatRuns.id, fixture.runId));
|
||||
await fixture.db.update(nativeRunFinalizations).set({ attempt }).where(eq(nativeRunFinalizations.runId, fixture.runId));
|
||||
@@ -2923,6 +2926,26 @@ process.on("SIGTERM", () => child.kill("SIGTERM"));
|
||||
: { kind: "blocked", runId: fixture.runId, reason: "execution_recovery_budget_exhausted" })]);
|
||||
});
|
||||
|
||||
it("keeps a run whose active turn follows a real provider failure blocked as permanently failed", async () => {
|
||||
const fixture = await seedRun("ACTIVE-AFTER-PROVIDER-FAILURE");
|
||||
await fixture.db.update(heartbeatRuns).set({ runnerProfileJson: { sessionCheckpoint: {
|
||||
driverKind: "codex_app_server",
|
||||
sessionId: fixture.native.normalizedSessionId,
|
||||
identity: { companyId, issueId: fixture.issueId, runId: fixture.runId, agentId, sessionId: fixture.native.normalizedSessionId },
|
||||
providerSessionId: "failed-provider",
|
||||
activeTurnId: "later-turn",
|
||||
semanticResult: null,
|
||||
terminal: { runTerminalState: "failed", turnTerminalState: "failed" },
|
||||
terminalTurns: [{ turnId: "failed-turn", fingerprint: JSON.stringify({
|
||||
terminalState: "failed", result: null, error: { code: "model_not_found", recoverable: false },
|
||||
}) }],
|
||||
} } }).where(eq(heartbeatRuns.id, fixture.runId));
|
||||
await fixture.db.update(nativeRunFinalizations).set({ attempt: 1 }).where(eq(nativeRunFinalizations.runId, fixture.runId));
|
||||
await appendRunEvents(fixture, [{ eventType: "native.process_start_requested" }, { eventType: "turn.accepted", turnId: "later-turn" }]);
|
||||
expect(await claimNativeRestartRecoveries({ db: fixture.db, controller: successor, restartKind: "hot", runIds: [fixture.runId] }))
|
||||
.toEqual([expect.objectContaining({ kind: "blocked", runId: fixture.runId, reason: "provider_checkpoint_permanently_failed" })]);
|
||||
});
|
||||
|
||||
// RC-22b: controller crashes (no drain record) that each interrupted a resume
|
||||
// which had already done work exhausted the three-attempt budget.
|
||||
async function seedInterruptedCheckpoint(label: string) {
|
||||
|
||||
+1
@@ -193,6 +193,7 @@ export const NativeProviderStateIncompatibleError = runner.NativeProviderStateIn
|
||||
export const NativeProviderSideEffectPendingError = runner.NativeProviderSideEffectPendingError;
|
||||
export const NativeRunnerSourceReconciliationRequiredError = runner.NativeRunnerSourceReconciliationRequiredError;
|
||||
export const nativeRestartInterruptedTurnId = runner.nativeRestartInterruptedTurnId;
|
||||
export const nativeRestartContinuationInFlight = runner.nativeRestartContinuationInFlight;
|
||||
|
||||
export const completeTerminatedRemoteNativeSessionCleanup = runner.completeTerminatedRemoteNativeSessionCleanup;
|
||||
export const completeTerminatedLocalNativeSessionCleanup = runner.completeTerminatedLocalNativeSessionCleanup;
|
||||
|
||||
Reference in new issue
Block a user