mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(recovery): bootstrap a remote follow-up again when a restart cut it off after retiring its finished owner (#20241)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A restart must resume in-progress runs on its own. Restart recovery
classifies each claimed native run and the executor acts on that claim.
> - A C1 follow-up on a remote sandbox first retires the finished owner
of the previous turn (it archives the previous controller state), then
launches a new runner.
> - A restart in that window left the run with an inherited session
checkpoint but no runner for this run. Recovery read the inherited
checkpoint as proof of a live remote runner and claimed reattach. The
executor then found no runner identity and failed with
`durable_identity_unreadable`, so the user had to press Retry.
> - This pull request makes recovery recognize that nothing reached a
runner for this run, and makes the executor bootstrap again over the
empty root.
> - The benefit is that this restart window resumes the follow-up by
itself.
## Linked Issues or Issue Description
**What happened?**
In restart-chaos Daytona K13 storm wave seed 9503, task t03 (codex, C1)
graded R3 retry_needed. Follow-up run 0d8b7258 retired its previous
owner at 10:07:32.988 ("Run 4ee1c19a finished without a saved runner
checkpoint; starting a new runner"). A K1 kill at 10:07:34 cut it off
before any runner start was requested. On restart, recovery claimed
`reattach_remote_runner` and the executor failed with
`runner_state_identity_mismatch: durable_identity_unreadable`. The run
ended with `native_continuation_requires_reconciliation` and a Retry
offer.
**Expected behavior**
Recovery sees that no runner was ever requested for this run, claims
`bootstrap_incomplete`, and the executor starts the new runner the first
attempt was about to start.
**Steps to reproduce**
1. Run a remote (Daytona) C1 follow-up whose previous turn finished.
2. Kill the server after the follow-up retires the finished owner and
before it records `native.process_start_requested`.
3. Restart the server. See the `reattach_remote_runner` claim and the
`durable_identity_unreadable` failure.
**Paperclip version or commit**
master at 3963d38560
## What Changed
- `native-restart-recovery.ts`: the `remote_runner_never_launched` proof
also holds when the run carries only its continued session. That means a
session checkpoint with a matching identity and no projected provider
event, active turn, semantic result, terminal or pending runtime
request, plus the predecessor's `sessionIdBefore`. The proof still
requires no `native.process_start_requested`, no controller checkpoint
for this run, no runner process identity and no `sessionIdAfter`. The
classifier then sees no checkpoint for this run.
- `native-session-executor.ts`: a `bootstrap_incomplete` claim over a
root with no runner identity and no controller state quarantines that
root and bootstraps again for a remote target too. Previously only a
local target with no live runner process could do this. Such a root was
never authenticated by a controller on this host.
- Tests: four classifier cases (restart; a requested runner start, a
pending runtime request and a projected provider event stay held). Two
executor cases (created and replacement sandbox) restart the exact state
the kill left after the retire.
## Verification
- Red before the fix: the classifier test got a non-bootstrap claim. The
executor tests failed with `runner_state_identity_mismatch:
durable_identity_unreadable`, the error from the wave. Both pass after
it.
- `native-runner-restart-recovery.integration.test.ts` and
`native-recovered-owner-followup.test.ts`: 239 passed.
- `native-session-executor.test.ts`: 1,310 passed.
- `PAPERCLIP_TYPECHECK_LOW_MEMORY=1 pnpm typecheck` in `server`: passes.
## Risks
- Low to medium. The proof widens only for a claimed remote run with
none of its own start, checkpoint, runner or provider evidence. A
pending runtime request in the inherited checkpoint still holds the run,
as in #20226.
- The quarantined root keeps the archived previous owner for inspection.
The usual quarantine retention applies.
- Previous-release story: no saved state or schema changes. A run that
an older build left in this state is classified with the new rule at its
next claim.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with
tool use and code 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
- [x] All Paperclip CI gates are green
- [x] 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
465da4227a
commit
bd83715a6e
4 files changed
+62
-5
No files matched your search
@@ -521,6 +521,22 @@ describe.skipIf(!recoveryPostgresSupport.supported)("follow-up after a recovered
|
||||
expect(followUp.logs.join("")).toContain(`Run ${prior.runId} finished without a saved runner checkpoint`);
|
||||
}, 60_000);
|
||||
|
||||
it.each(["created", "replacement"] as const)(
|
||||
"bootstraps again when a restart cut the follow-up off after it retired the finished owner: %s sandbox", async outcome => {
|
||||
const prior = await seedRecoveredPriorRun();
|
||||
const followUp = await admitFollowUp(prior);
|
||||
expect(await followUp.start(outcome)).toBe(ADMITTED);
|
||||
const root = await sessionRoot();
|
||||
expect(existsSync(join(root, "control-plane"))).toBe(false);
|
||||
expect(await archivedOwners(root)).toEqual([prior.runId]);
|
||||
followUp.logs.length = 0;
|
||||
expect(await followUp.start(outcome, { kind: "bootstrap_incomplete", runId: followUp.runId, leaseOwner: "successor",
|
||||
controllerGeneration: 2, providerAttempt: 1, restartKind: "hard", recoveryRequestId: null } as never)).toBe(ADMITTED);
|
||||
const [copy] = await quarantined();
|
||||
expect(copy).toMatch(/\.identity_indeterminate\./);
|
||||
expect(existsSync(join(stateDirectory, "quarantine", copy!, "continuity-breaks"))).toBe(true);
|
||||
}, 60_000);
|
||||
|
||||
async function keepLocalRootWithoutPointer(prior: Awaited<ReturnType<typeof seedRecoveredPriorRun>>,
|
||||
followUp: Awaited<ReturnType<typeof admitFollowUp>>) {
|
||||
await db.update(nativeRunFinalizations).set({ phase: "observed" }).where(eq(nativeRunFinalizations.runId, prior.runId));
|
||||
|
||||
@@ -1367,6 +1367,11 @@ export async function claimNativeRestartRecoveries(input: {
|
||||
// backend may spawn a runner, and a remote runner exchanges commands only
|
||||
// through a Postgres controller checkpoint. With neither, nothing reached
|
||||
// a runner. The previous controller is gone (takeover was allowed above).
|
||||
const pendingInheritedRequests = Array.isArray(checkpointRecord.pendingRuntimeRequests)
|
||||
&& checkpointRecord.pendingRuntimeRequests.length > 0;
|
||||
const continuedSessionOnly = hasCheckpoint && checkpointIdentityMatches && providerEvents.length === 0
|
||||
&& !pendingInheritedRequests && checkpointRecord.activeTurnId == null && checkpointRecord.semanticResult == null
|
||||
&& checkpointRecord.terminal == null;
|
||||
const remoteRunnerNeverLaunched = !unclaimedBootstrap && remoteWorkspace?.state === "prepared" && !pluginTask
|
||||
&& !durableAuthority && stagedAttachment === null
|
||||
&& row.run.runnerProfileJson?.nativePreparationFencingVersion === NATIVE_PREPARATION_FENCING_VERSION
|
||||
@@ -1381,9 +1386,9 @@ export async function claimNativeRestartRecoveries(input: {
|
||||
&& row.coordinator.failureDetail?.recoveryMode === "bootstrap_retry" && !row.coordinator.leaseOwner)
|
||||
&& !row.coordinator.failureDetail?.successorRunId
|
||||
&& !row.run.processPid && !row.run.processGroupId && !row.run.processStartedAt
|
||||
&& !row.run.sessionIdBefore && !row.run.sessionIdAfter
|
||||
&& (!row.run.sessionIdBefore || continuedSessionOnly) && !row.run.sessionIdAfter
|
||||
&& Boolean(row.run.nativeSessionId && row.run.runnerInstanceId)
|
||||
&& !hasCheckpoint && !hasProviderEvidence
|
||||
&& (continuedSessionOnly || (!hasCheckpoint && !hasProviderEvidence))
|
||||
&& !(await hasNativeExecutionStartEvidence(tx, row.run));
|
||||
// A settled retry starts new work, so it needs the same current
|
||||
// admission as the resumption reconciler: task pause, agent status,
|
||||
@@ -1473,7 +1478,8 @@ export async function claimNativeRestartRecoveries(input: {
|
||||
knownProviderProcessAlive: providerProcesses.livePids.length > 0,
|
||||
knownProviderProcessIdentityAmbiguous:
|
||||
providerProcesses.ambiguousLivePids.length > 0,
|
||||
hasCheckpoint: hasCheckpoint && !remoteRunnerNeverReceivedCommands && !(unclaimedBootstrap && inheritedCheckpointOnly),
|
||||
hasCheckpoint: hasCheckpoint && !remoteRunnerNeverReceivedCommands && !(unclaimedBootstrap && inheritedCheckpointOnly)
|
||||
&& !remoteRunnerNeverLaunched,
|
||||
stagedAttachmentVerified: stagedAttachment !== null,
|
||||
checkpointIdentityMatches:
|
||||
checkpointIdentityMatches &&
|
||||
|
||||
@@ -2960,6 +2960,42 @@ process.on("SIGTERM", () => child.kill("SIGTERM"));
|
||||
expect(disposition.kind).not.toBe("bootstrap_incomplete");
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ label: "restarts", startRequested: false, pendingRequest: false, providerEvent: false },
|
||||
{ label: "keeps a requested runner start held instead of restarting", startRequested: true, pendingRequest: false,
|
||||
providerEvent: false },
|
||||
{ label: "keeps a pending runtime request held instead of restarting", startRequested: false, pendingRequest: true,
|
||||
providerEvent: false },
|
||||
{ label: "keeps a projected provider event held instead of restarting", startRequested: false, pendingRequest: false,
|
||||
providerEvent: true },
|
||||
])("$label a claimed remote follow-up that never requested a runner and carries its predecessor's session",
|
||||
async ({ startRequested, pendingRequest, providerEvent }) => {
|
||||
const fixture = await seedUnlaunchedRemoteClaim(`PROVEN-CONTINUED-${startRequested}-${pendingRequest}-${providerEvent}`,
|
||||
startRequested ? { event: { eventType: "native.process_start_requested" } } : {});
|
||||
const [seeded] = await fixture.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, fixture.runId));
|
||||
await fixture.db.update(heartbeatRuns).set({ sessionIdBefore: "previous-thread",
|
||||
runnerProfileJson: { ...seeded.runnerProfileJson, sessionCheckpoint: {
|
||||
driverKind: "codex_app_server", backendKind: "runner", sessionId: "previous-thread", cursor: "37",
|
||||
identity: { companyId, issueId: fixture.issueId, runId: fixture.runId, agentId, sessionId: fixture.native.normalizedSessionId },
|
||||
providerSessionId: "previous-thread", activeTurnId: null, terminal: null,
|
||||
terminalTurns: [{ turnId: "previous-turn", fingerprint: "{}" }], semanticResult: null,
|
||||
pendingRuntimeRequests: pendingRequest ? [{ requestId: "input-1", kind: "user_input" }] : [], lineage: [],
|
||||
} } }).where(eq(heartbeatRuns.id, fixture.runId));
|
||||
if (providerEvent) await fixture.db.insert(heartbeatRunEvents).values({ companyId, runId: fixture.runId, agentId, seq: 901,
|
||||
stream: "system", level: "info", eventType: "session.started" });
|
||||
const [claim] = await claimNativeRestartRecoveries({ db: fixture.db, controller: successor,
|
||||
restartKind: "hard", runIds: [fixture.runId] });
|
||||
if (startRequested || pendingRequest || providerEvent) {
|
||||
expect(claim).not.toMatchObject({ kind: "bootstrap_incomplete" });
|
||||
return;
|
||||
}
|
||||
expect(claim).toMatchObject({ kind: "bootstrap_incomplete", runId: fixture.runId, controllerGeneration: 2 });
|
||||
const [coordinator] = await fixture.db.select().from(nativeRunFinalizations)
|
||||
.where(eq(nativeRunFinalizations.runId, fixture.runId));
|
||||
expect(coordinator.recoveryHistory.at(-1)).toMatchObject({ disposition: "bootstrap_incomplete",
|
||||
reason: "remote_runner_never_launched" });
|
||||
});
|
||||
|
||||
it.each([false, true])("restarts a claimed remote run that never requested a runner (lease expired=%s)", async (leaseExpired) => {
|
||||
const fixture = await seedUnlaunchedRemoteClaim(`PROVEN-${leaseExpired}`, { leaseExpired });
|
||||
expect(await claimNativeRestartRecoveries({ db: fixture.db, controller: successor,
|
||||
|
||||
@@ -5809,9 +5809,8 @@ async function migrateRunnerdStateRootForExecution(input: {
|
||||
}
|
||||
const identity = readRunnerdDurableIdentity(scoped);
|
||||
if (!identity && input.restartRecovery?.kind === "bootstrap_incomplete" &&
|
||||
(!input.runnerExecutionTarget || input.runnerExecutionTarget.kind === "local") &&
|
||||
!existsSync(resolve(scoped, "control-plane", "control-plane-state.json")) &&
|
||||
await noLocalRunnerProcessForRun(input.execution.binding.runId)) {
|
||||
(input.runnerExecutionTarget?.kind === "remote" || await noLocalRunnerProcessForRun(input.execution.binding.runId))) {
|
||||
// The server was killed between creating the root and the local runner's
|
||||
// first state write. No runner for the run is alive on this host and none
|
||||
// connected, so no provider started and no command was acknowledged.
|
||||
|
||||
Reference in new issue
Block a user