mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(native): restart an unclaimed remote follow-up that carries only its inherited checkpoint (#20214)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A remote per-turn follow-up inherits the previous run's session checkpoint at admission, rebound to its own identity. Its session root still holds the previous owner's remote marker until the follow-up rotates authority > - If a restart lands before the follow-up's controller ever claims execution, recovery should restart its bootstrap: nothing of this run reached any runner > - Recovery's unclaimed-bootstrap check required no checkpoint and no provider evidence. The inherited checkpoint failed both, so recovery claimed `reattach_remote_runner` > - Even under a bootstrap claim, the executor skipped the per-turn projection restore for every restart claim. It therefore never rehydrated the previous owner's external marker, and failed with `runner_state_identity_mismatch: durable_identity_unreadable` > - This pull request does not count the inherited checkpoint as evidence for an unclaimed remote run, and lets a bootstrap claim restore the previous owner's projection, as a fresh follow-up start does > - The benefit is that a follow-up drained in its first second on Daytona restarts on its own ## Linked Issues or Issue Description Refs #20153 **What happened?** Daytona per-turn restart-chaos K13 storm w-20261109T074128-s9503 (revision9046884811), t26 (codex C1): follow-up run `f1ddc43d` started at 07:54:18 and was drained by SIGTERM at 07:54:19, during workspace staging. - It had not requested a runner process, and its coordinator was never claimed (provider attempt 0). - Recovery claimed `reattach_remote_runner` (`hasCheckpoint: true`, `hasProviderEvidence: true`, both from the inherited checkpoint). - The executor failed with `runner_state_identity_mismatch: durable_identity_unreadable`, because the session root held the previous run's external marker. A Retry succeeded. **Expected behavior** An unclaimed remote run whose only checkpoint is the admission-rebound one, with no durable controller record and no provider events, restarts its bootstrap (`remote_execution_never_claimed`). The executor rehydrates the previous owner's projection and starts the follow-up from it. **Steps to reproduce** 1. Start a remote per-turn follow-up whose session root holds the previous owner's external marker. Drain the control plane before the follow-up's controller claims execution. 2. Restart. Before this change, recovery claims `reattach_remote_runner` and the executor fails with `durable_identity_unreadable`. **Paperclip version or commit**9046884811(wave). Branch rebased on current master. ## What Changed - `native-restart-recovery.ts`: - #20153's admission-rebound checkpoint test moves earlier. A new `inheritedCheckpointOnly` holds for a remote sandbox run (not a plugin task) with only that checkpoint, no durable controller record, no staged attachment and no provider events. - The unclaimed-bootstrap path and its classification then treat that checkpoint as neither a checkpoint nor provider evidence. - `native-inherited-runner-retry.ts` / `native-restart-recovery.ts`: a warm continuation carries its settled predecessor's runner process identity until it launches its own runner, and t26's row still held pid 596 at the restart. - The new `carriesInheritedRunnerIdentity` reuses the same-run retry path's proof (the exact pid, group and start time a settled predecessor recorded). - The restart classifier then ignores that inherited identity on the unclaimed path, for a run that carries only its inherited checkpoint. - A process identity that is the run's own still holds the run. - `native-session-executor.ts`: `restorePerTurnRunnerdController` now also runs for a `bootstrap_incomplete` claim, but only when the root's reference identity belongs to a previous run. A claim on the run's own root returns early, as before. - Tests: - `native-runner-restart-recovery.integration.test.ts`: an unclaimed remote follow-up with only its inherited checkpoint restarts, with no process identity and with its predecessor's runner identity. It stays held with its own runner identity, or when a provider event was projected; - `native-recovered-owner-followup.test.ts`: a bootstrap claim over the previous owner's external marker is admitted, rehydrating the marker, keeping the backup and quarantining nothing. ## Verification - Red/green: both new tests fail on master. The classifier test claims `reattach_remote_runner`, and the executor test fails with exactly `durable_identity_unreadable`. Both pass with the change, and the held case holds. - `vitest run` over the restart-recovery, bootstrap, recovered-owner and failed-workspace-recovery test files and `native-session-executor.test.ts`: 9 files, 1715 tests passed (14 skipped). After the inherited-identity change, the restart-recovery, bootstrap, recovered-owner, failed-workspace-recovery and inherited-runner test files: 8 files, 407 tests passed (14 skipped). Without the identity check, the inherited case is not restarted. - `PAPERCLIP_TYPECHECK_LOW_MEMORY=1 pnpm typecheck` (`server`): passed. ## Risks - Low risk: - The inherited checkpoint stops counting only when the database shows no controller record and no provider event for the run, and the existing unclaimed-bootstrap conditions all still hold. - The executor change runs the existing fresh-follow-up projection restore, with all of its own checks, only for a root that still belongs to the previous run. ## 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
98e0f1fc0d
commit
ad57d5cc12
5 files changed
+97
-21
No files matched your search
@@ -89,6 +89,28 @@ export function readInheritedRunnerRetainedSandbox(profile: Record<string, unkno
|
||||
*/
|
||||
export type InheritedRunnerRetirement = "retired" | "not_applicable" | "transient";
|
||||
|
||||
type RunnerIdentityRun = Pick<typeof heartbeatRuns.$inferSelect,
|
||||
"id" | "companyId" | "agentId" | "nativeSessionId" | "createdAt" | "processPid" | "processGroupId" | "processStartedAt">;
|
||||
|
||||
/** The settled predecessor whose exact runner process identity this run carries. */
|
||||
async function settledPredecessorWithRunnerIdentity(tx: Pick<Db, "select">, run: RunnerIdentityRun) {
|
||||
if (!run.nativeSessionId || (run.processPid === null && run.processGroupId === null)) return null;
|
||||
const [predecessor] = await tx.select({ id: heartbeatRuns.id, processStartedAt: heartbeatRuns.processStartedAt })
|
||||
.from(heartbeatRuns).where(and(
|
||||
eq(heartbeatRuns.companyId, run.companyId), eq(heartbeatRuns.agentId, run.agentId),
|
||||
eq(heartbeatRuns.nativeSessionId, run.nativeSessionId), ne(heartbeatRuns.id, run.id),
|
||||
lt(heartbeatRuns.createdAt, run.createdAt), inArray(heartbeatRuns.status, TERMINAL_RUN_STATUSES),
|
||||
run.processPid === null ? isNull(heartbeatRuns.processPid) : eq(heartbeatRuns.processPid, run.processPid),
|
||||
run.processGroupId === null ? isNull(heartbeatRuns.processGroupId) : eq(heartbeatRuns.processGroupId, run.processGroupId),
|
||||
)).orderBy(desc(heartbeatRuns.createdAt)).limit(1);
|
||||
return predecessor && sameTime(predecessor.processStartedAt, run.processStartedAt) ? predecessor : null;
|
||||
}
|
||||
|
||||
/** The run's recorded runner process identity is exactly a settled predecessor's, not its own. */
|
||||
export async function carriesInheritedRunnerIdentity(tx: Pick<Db, "select">, run: RunnerIdentityRun): Promise<boolean> {
|
||||
return (await settledPredecessorWithRunnerIdentity(tx, run)) !== null;
|
||||
}
|
||||
|
||||
export async function retireInheritedRemoteRunnerIdentity(db: Db, runId: string): Promise<InheritedRunnerRetirement> {
|
||||
return db.transaction(async transaction => {
|
||||
const tx = transaction as unknown as Db;
|
||||
@@ -127,16 +149,8 @@ export async function retireInheritedRemoteRunnerIdentity(db: Db, runId: string)
|
||||
if ((await nativeSessionProviderEvidence(tx, [run.id])).has(run.id)) return "not_applicable";
|
||||
if (await hasNativeRunnerCheckpoint(tx, { companyId: run.companyId, issueId: issue.id, runId: run.id })) return "not_applicable";
|
||||
|
||||
// The identity must be exactly the one a settled predecessor recorded.
|
||||
const [predecessor] = await tx.select({ id: heartbeatRuns.id, processStartedAt: heartbeatRuns.processStartedAt })
|
||||
.from(heartbeatRuns).where(and(
|
||||
eq(heartbeatRuns.companyId, run.companyId), eq(heartbeatRuns.agentId, run.agentId),
|
||||
eq(heartbeatRuns.nativeSessionId, run.nativeSessionId), ne(heartbeatRuns.id, run.id),
|
||||
lt(heartbeatRuns.createdAt, run.createdAt), inArray(heartbeatRuns.status, TERMINAL_RUN_STATUSES),
|
||||
run.processPid === null ? isNull(heartbeatRuns.processPid) : eq(heartbeatRuns.processPid, run.processPid),
|
||||
run.processGroupId === null ? isNull(heartbeatRuns.processGroupId) : eq(heartbeatRuns.processGroupId, run.processGroupId),
|
||||
)).orderBy(desc(heartbeatRuns.createdAt)).limit(1);
|
||||
if (!predecessor || !sameTime(predecessor.processStartedAt, run.processStartedAt)) return "not_applicable";
|
||||
const predecessor = await settledPredecessorWithRunnerIdentity(tx, run);
|
||||
if (!predecessor) return "not_applicable";
|
||||
|
||||
// Keep the run's own sandbox when its lease is still live.
|
||||
const [lease] = await tx.select({ id: environmentLeases.id }).from(environmentLeases).where(and(
|
||||
|
||||
@@ -337,6 +337,16 @@ describe.skipIf(!recoveryPostgresSupport.supported)("follow-up after a recovered
|
||||
expect(followUp.logs.join("")).not.toContain("starting a new runner for this conversation");
|
||||
}, 60_000);
|
||||
|
||||
it("starts an unclaimed remote follow-up from the previous owner's external marker under a bootstrap claim", async () => {
|
||||
const fixture = await staleProjection("codex", true);
|
||||
expect(await fixture.followUp.start("created", { kind: "bootstrap_incomplete", runId: fixture.followUp.runId,
|
||||
leaseOwner: "successor", controllerGeneration: 1, providerAttempt: 0, restartKind: "hard", recoveryRequestId: null } as never))
|
||||
.toBe(ADMITTED);
|
||||
expect(JSON.parse(await readFile(fixture.path, "utf8")).identity).toEqual(fixture.latest.identity);
|
||||
await fixture.assertBackup();
|
||||
expect(await quarantined()).toEqual([]);
|
||||
}, 60_000);
|
||||
|
||||
it("rehydrates a same-source external marker before reading the suspended resumed sandbox", async () => {
|
||||
const fixture = await staleProjection("codex", true), { latest, followUp, root, path } = fixture;
|
||||
await writeFile(path, JSON.stringify({ schema: "paperclip.runner.external-control-plane-state.v1", identity: latest.identity }));
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { carriesInheritedRunnerIdentity } from "./native-inherited-runner-retry.js";
|
||||
import { readPendingNativeRunStop } from "./native-cancellation-request.js";
|
||||
import { CONTROLLER_LEASE_RENEW_MS } from "../controller-instance-lease.js";
|
||||
import { currentControllerProcessStamp, UNKNOWN_PROCESS_SPACE } from "../controller-process-identity.js";
|
||||
@@ -1324,6 +1325,13 @@ export async function claimNativeRestartRecoveries(input: {
|
||||
const hasProviderEvidence =
|
||||
hasCheckpointProviderIdentity || providerEvents.length > 0;
|
||||
|
||||
const admissionCheckpointOnly = !hasCheckpoint || (checkpointIdentityMatches && checkpointRecord.cursor == null
|
||||
&& (!Array.isArray(checkpointRecord.terminalTurns) || checkpointRecord.terminalTurns.length === 0)
|
||||
&& checkpointRecord.activeTurnId == null && checkpointRecord.semanticResult == null && checkpointRecord.terminal == null);
|
||||
const inheritedCheckpointOnly = hasCheckpoint && admissionCheckpointOnly && remoteSandbox && !pluginTask
|
||||
&& !durableAuthority && stagedAttachment === null && providerEvents.length === 0;
|
||||
const inheritedRunnerIdentity = inheritedCheckpointOnly && Boolean(row.run.processPid || row.run.processGroupId)
|
||||
&& await carriesInheritedRunnerIdentity(tx, row.run);
|
||||
let unclaimedBootstrap =
|
||||
remoteWorkspace?.state === "prepared" &&
|
||||
row.run.runnerProfileJson?.nativePreparationFencingVersion === NATIVE_PREPARATION_FENCING_VERSION &&
|
||||
@@ -1338,14 +1346,12 @@ export async function claimNativeRestartRecoveries(input: {
|
||||
// suspends. Without an owner, an elapsed expiry grants no authority.
|
||||
(!row.coordinator.leaseExpiresAt || row.coordinator.leaseExpiresAt <= now) &&
|
||||
!row.coordinator.failureDetail?.successorRunId &&
|
||||
!row.run.processPid &&
|
||||
!row.run.processGroupId &&
|
||||
!row.run.processStartedAt &&
|
||||
(inheritedRunnerIdentity || (!row.run.processPid && !row.run.processGroupId && !row.run.processStartedAt)) &&
|
||||
!row.run.sessionIdBefore &&
|
||||
!row.run.sessionIdAfter &&
|
||||
Boolean(row.run.nativeSessionId && row.run.runnerInstanceId) &&
|
||||
!hasCheckpoint &&
|
||||
!hasProviderEvidence;
|
||||
(!hasCheckpoint || inheritedCheckpointOnly) &&
|
||||
(!hasProviderEvidence || inheritedCheckpointOnly);
|
||||
if (unclaimedBootstrap) {
|
||||
if (activeNativeExecutionRunIds.has(row.run.id) &&
|
||||
controller.bootId === (await currentNativeControllerIdentity()).bootId) {
|
||||
@@ -1430,9 +1436,6 @@ export async function claimNativeRestartRecoveries(input: {
|
||||
const remoteBootstrapProvenIncomplete = remoteBootstrapProof !== null
|
||||
&& (!hasProviderEvidence || remoteBootstrapProof === "session_opened_without_turn");
|
||||
|
||||
const admissionCheckpointOnly = !hasCheckpoint || (checkpointIdentityMatches && checkpointRecord.cursor == null
|
||||
&& (!Array.isArray(checkpointRecord.terminalTurns) || checkpointRecord.terminalTurns.length === 0)
|
||||
&& checkpointRecord.activeTurnId == null && checkpointRecord.semanticResult == null && checkpointRecord.terminal == null);
|
||||
const remoteRunnerNeverReceivedCommands = !unclaimedBootstrap && !remoteRunnerNeverLaunched
|
||||
&& !remoteRunnerExitedBeforeProviderWork && !remoteBootstrapProvenIncomplete
|
||||
&& remoteSandbox && remoteWorkspace !== null && !pluginTask && !durableAuthority && stagedAttachment === null
|
||||
@@ -1468,12 +1471,12 @@ export async function claimNativeRestartRecoveries(input: {
|
||||
knownProviderProcessAlive: providerProcesses.livePids.length > 0,
|
||||
knownProviderProcessIdentityAmbiguous:
|
||||
providerProcesses.ambiguousLivePids.length > 0,
|
||||
hasCheckpoint: hasCheckpoint && !remoteRunnerNeverReceivedCommands,
|
||||
hasCheckpoint: hasCheckpoint && !remoteRunnerNeverReceivedCommands && !(unclaimedBootstrap && inheritedCheckpointOnly),
|
||||
stagedAttachmentVerified: stagedAttachment !== null,
|
||||
checkpointIdentityMatches:
|
||||
checkpointIdentityMatches &&
|
||||
(!remoteSandbox || remoteWorkspace !== null),
|
||||
hasProviderEvidence,
|
||||
hasProviderEvidence: hasProviderEvidence && !(unclaimedBootstrap && inheritedCheckpointOnly),
|
||||
checkpointFailed:
|
||||
(checkpointRecord.terminal as Record<string, unknown> | undefined)
|
||||
?.runTerminalState === "failed" &&
|
||||
|
||||
@@ -2757,6 +2757,50 @@ process.on("SIGTERM", () => child.kill("SIGTERM"));
|
||||
expect(coordinator.recoveryHistory.at(-1)).toMatchObject({ reason: "remote_runner_never_received_commands" });
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ label: "restarts", providerEvent: false, process: "none" },
|
||||
{ label: "restarts with its predecessor's runner identity", providerEvent: false, process: "inherited" },
|
||||
{ label: "keeps its own runner identity held instead of restarting", providerEvent: false, process: "own" },
|
||||
{ label: "keeps a projected provider event held instead of restarting", providerEvent: true, process: "none" },
|
||||
])("$label an unclaimed remote follow-up that carries only its inherited checkpoint", async ({ providerEvent, process }) => {
|
||||
const fixture = await seedUnlaunchedRemoteClaim(`UNCLAIMED-INHERITED-${providerEvent}-${process}`);
|
||||
if (process !== "none") {
|
||||
const processStartedAt = new Date(Date.now() - 120_000);
|
||||
await fixture.db.update(heartbeatRuns).set({ processPid: 596, processGroupId: null, processStartedAt, processHostHash: "remote" })
|
||||
.where(eq(heartbeatRuns.id, fixture.runId));
|
||||
if (process === "inherited") {
|
||||
const [current] = await fixture.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, fixture.runId));
|
||||
await fixture.db.insert(heartbeatRuns).values({ companyId, agentId, status: "succeeded", runtimeMode: "native",
|
||||
nativeIssueId: fixture.issueId, nativeSessionId: current.nativeSessionId, runnerInstanceId: current.runnerInstanceId,
|
||||
createdAt: new Date(current.createdAt.getTime() - 60_000), finishedAt: new Date(current.createdAt.getTime() - 5_000),
|
||||
processPid: 596, processGroupId: null, processStartedAt, processHostHash: "remote",
|
||||
contextSnapshot: { issueId: fixture.issueId } });
|
||||
}
|
||||
}
|
||||
await fixture.db.update(nativeRunFinalizations).set({ attempt: 0, controllerGeneration: 0, controllerBootId: null,
|
||||
controllerPid: null, controllerProcessStartedAt: null, leaseOwner: null, leaseExpiresAt: null })
|
||||
.where(eq(nativeRunFinalizations.runId, fixture.runId));
|
||||
const [seeded] = await fixture.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, fixture.runId));
|
||||
await fixture.db.update(heartbeatRuns).set({ runnerProfileJson: { ...seeded.runnerProfileJson, sessionCheckpoint: {
|
||||
driverKind: "codex_app_server", backendKind: "runner", sessionId: "inherited-thread", cursor: null,
|
||||
identity: { companyId, issueId: fixture.issueId, runId: fixture.runId, agentId, sessionId: fixture.native.normalizedSessionId },
|
||||
providerSessionId: "inherited-thread", activeTurnId: null, terminal: null, terminalTurns: [], semanticResult: null,
|
||||
pendingRuntimeRequests: [], 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 (providerEvent || process === "own") {
|
||||
expect(claim).not.toMatchObject({ kind: "bootstrap_incomplete" });
|
||||
return;
|
||||
}
|
||||
expect(claim).toMatchObject({ kind: "bootstrap_incomplete", runId: fixture.runId });
|
||||
const [coordinator] = await fixture.db.select().from(nativeRunFinalizations)
|
||||
.where(eq(nativeRunFinalizations.runId, fixture.runId));
|
||||
expect(coordinator.recoveryHistory.at(-1)).toMatchObject({ reason: "remote_execution_never_claimed" });
|
||||
});
|
||||
|
||||
it("restarts a remote bootstrap retry that shutdown cut before any runner was requested", async () => {
|
||||
const fixture = await seedUnlaunchedRemoteClaim("BOOTSTRAP-RETRY", { bootstrapRetry: true });
|
||||
expect(await claimNativeRestartRecoveries({ db: fixture.db, controller: successor,
|
||||
|
||||
@@ -6744,9 +6744,14 @@ async function restorePerTurnRunnerdController(input: {
|
||||
runnerExecutionTarget?: AdapterExecutionTarget | null;
|
||||
restartRecovery?: NativeRestartRecoveryClaim;
|
||||
}): Promise<void> {
|
||||
if (input.restartRecovery || input.runnerExecutionTarget?.kind !== "remote" ||
|
||||
if ((input.restartRecovery && input.restartRecovery.kind !== "bootstrap_incomplete") ||
|
||||
input.runnerExecutionTarget?.kind !== "remote" ||
|
||||
input.execution.session.lifecyclePolicy.mode !== "per_turn") return;
|
||||
const root = scopedRunnerdStateRoot(input.execution);
|
||||
if (input.restartRecovery) {
|
||||
const prior = readRunnerdDurableIdentity(root) ?? readRunnerdDurableIdentity(root, true);
|
||||
if (!prior || prior.runId === input.execution.binding.runId) return;
|
||||
}
|
||||
await restoreDurableHarnessForExecution(input.db, input.execution, root);
|
||||
const identity = readRunnerdDurableIdentity(root);
|
||||
const reference = identity ?? readRunnerdDurableIdentity(root, true);
|
||||
|
||||
Reference in new issue
Block a user