diff --git a/doc/run-log-events.md b/doc/run-log-events.md index 9ac6842d450..26d6e9782b3 100644 --- a/doc/run-log-events.md +++ b/doc/run-log-events.md @@ -355,6 +355,12 @@ unfinished `session.open`, with no committed or acknowledged event. The executor restores that checkpoint and checks the same proof before it bootstraps again. After three provider attempts it records `execution_recovery_budget_exhausted` instead. +A claimed remote run whose previous controller is gone, with no +`native.process_start_requested`, process identity, adapter invocation, +runner-sourced event or controller checkpoint, records +`remote_runner_never_launched` with the same disposition and budget. The +executor records `native.process_start_requested` before a backend may spawn a +runner, so its absence (with no checkpoint) proves nothing reached a runner. For a local Codex, OpenCode, or ACPX turn lost while runnerd was stopped, the canonical `turn.failed` event retains `error.code: "provider_turn_lost_on_restore"` and diff --git a/server/src/services/execution-recovery-resolution.test.ts b/server/src/services/execution-recovery-resolution.test.ts new file mode 100644 index 00000000000..a7c0fc57143 --- /dev/null +++ b/server/src/services/execution-recovery-resolution.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from "vitest"; +import { executionRecoverySettlementNote } from "./execution-recovery-resolution.js"; + +describe("executionRecoverySettlementNote", () => { + it("names a restart-recovery hold instead of asking for a workspace repair", () => { + const note = executionRecoverySettlementNote({ current: true, cause: "native_restart_recovery_blocked", + workspaceRestoreFailed: true }); + expect(note).not.toContain("Workspace repair required"); + expect(note).toContain("after a server restart"); + }); + + it("keeps the workspace repair note for other holds with a restore failure", () => { + expect(executionRecoverySettlementNote({ current: true, cause: "execution_finalization_deadline_exceeded", + workspaceRestoreFailed: true })).toContain("Workspace repair required"); + expect(executionRecoverySettlementNote({ current: true, cause: "execution_finalization_deadline_exceeded", + workspaceRestoreFailed: false })).toContain("Automatic recovery stopped. Recorded work is preserved"); + expect(executionRecoverySettlementNote({ current: false, cause: "native_restart_recovery_blocked", + workspaceRestoreFailed: true })).toContain("Recovery closed"); + }); +}); diff --git a/server/src/services/execution-recovery-resolution.ts b/server/src/services/execution-recovery-resolution.ts index f579d908d14..7c1ed06d034 100644 --- a/server/src/services/execution-recovery-resolution.ts +++ b/server/src/services/execution-recovery-resolution.ts @@ -36,6 +36,23 @@ import { readSystemRecovery } from "./system-recovery-state.js"; import { alias } from "drizzle-orm/pg-core"; /** An operator records observed outcomes; this is not permission to blindly retry. */ +/** + * The user-facing note for a settled recovery hold. A restart-recovery hold + * names its own cause: a workspace export retried after that hold does not + * mean the user must repair the workspace. + */ +export function executionRecoverySettlementNote(input: { + current: boolean; cause: string; workspaceRestoreFailed: boolean; +}): string { + if (!input.current) return "Recovery closed because the task's owner, execution, or status changed. No work was replayed."; + if (input.cause === "native_restart_recovery_blocked") { + return "Automatic recovery stopped after a server restart because the previous execution could not be verified. Saved work is preserved; actions with unverified outcomes will not be repeated."; + } + return input.workspaceRestoreFailed + ? "Workspace repair required. Verify safe staging or repair before continuing. Saved work and approval decisions remain in force." + : "Automatic recovery stopped. Recorded work is preserved; actions with unverified outcomes will not be repeated."; +} + export async function validateExecutionReconciliation(input: { db: Db; companyId: string; @@ -626,11 +643,8 @@ export async function settleUnrecoverableExecutions( !["done", "cancelled"].includes(task.status) && (!task.executionRunId || task.executionRunId === run.id) && (!task.checkoutRunId || task.checkoutRunId === run.id); - const note = current - ? hasWorkspaceRestoreFailure(run.resultJson) - ? "Workspace repair required. Verify safe staging or repair before continuing. Saved work and approval decisions remain in force." - : "Automatic recovery stopped. Recorded work is preserved; actions with unverified outcomes will not be repeated." - : "Recovery closed because the task's owner, execution, or status changed. No work was replayed."; + const note = executionRecoverySettlementNote({ current, cause: action.cause, + workspaceRestoreFailed: hasWorkspaceRestoreFailure(run.resultJson) }); await notifyUnsafeWorkspaceExportRecovery(tx, { companyId: task.companyId, issueId: task.id }); let nativeFailureBlock = action.evidence.nativeFailureBlock; let nativeBootstrapFailureBlock = action.evidence.nativeBootstrapFailureBlock; diff --git a/server/src/services/native-runtime/native-restart-recovery.ts b/server/src/services/native-runtime/native-restart-recovery.ts index 089d421450e..d46a0ffe566 100644 --- a/server/src/services/native-runtime/native-restart-recovery.ts +++ b/server/src/services/native-runtime/native-restart-recovery.ts @@ -453,8 +453,9 @@ export function classifyNativeRunnerRecoveryEvidence(input: { runnerGroupAlive: boolean; remoteSandbox?: boolean; unclaimedBootstrap?: boolean; - /** The durable controller checkpoint of a claimed remote bootstrap proves no provider work. */ + /** Durable evidence proves a claimed remote bootstrap reached no provider work. */ remoteBootstrapProvenIncomplete?: boolean; + remoteBootstrapIncompleteReason?: "remote_runner_never_launched"; processStartMatches: boolean; knownProviderProcessAlive?: boolean; knownProviderProcessIdentityAmbiguous?: boolean; @@ -487,7 +488,8 @@ export function classifyNativeRunnerRecoveryEvidence(input: { // Each same-run bootstrap is another provider attempt; keep restarts bounded. if ((input.providerAttempt ?? 0) >= 3) return { claimKind: null, reason: "execution_recovery_budget_exhausted" }; - return { claimKind: "bootstrap_incomplete", reason: "remote_bootstrap_proven_incomplete" }; + return { claimKind: "bootstrap_incomplete", + reason: input.remoteBootstrapIncompleteReason ?? "remote_bootstrap_proven_incomplete" }; } return input.hasCheckpoint && input.checkpointIdentityMatches === true && @@ -547,6 +549,31 @@ export function classifyNativeRunnerRecoveryEvidence(input: { return { claimKind: null, reason: "ambiguous_provider_recovery_evidence" }; } +/** Any record that execution reached a runner: a requested or identified + * process, an adapter invocation, a tool start, or a runner-sourced event. */ +async function hasNativeExecutionStartEvidence( + tx: Pick, run: { companyId: string; id: string }, +): Promise { + const [executionEvent] = await tx + .select({ id: heartbeatRunEvents.id }) + .from(heartbeatRunEvents) + .where(and( + eq(heartbeatRunEvents.companyId, run.companyId), + eq(heartbeatRunEvents.runId, run.id), + or( + isNotNull(heartbeatRunEvents.sourceEventId), + inArray(heartbeatRunEvents.eventType, [ + "adapter.invoke", + PROCESS_START_REQUESTED, + PROCESS_IDENTITY_RECORDED, + "tool.execution.started", + ]), + ), + )) + .limit(1); + return executionEvent !== undefined; +} + function historyEntry(input: { now: Date; restartKind: NativeRestartKind; @@ -1051,26 +1078,27 @@ export async function claimNativeRestartRecoveries(input: { controller.bootId === (await currentNativeControllerIdentity()).bootId) { return { kind: "awaiting_evidence", runId: row.run.id, reason: "current_controller_preparing" } as const; } - const [executionEvent] = await tx - .select({ id: heartbeatRunEvents.id }) - .from(heartbeatRunEvents) - .where(and( - eq(heartbeatRunEvents.companyId, row.run.companyId), - eq(heartbeatRunEvents.runId, row.run.id), - or( - isNotNull(heartbeatRunEvents.sourceEventId), - inArray(heartbeatRunEvents.eventType, [ - "adapter.invoke", - PROCESS_START_REQUESTED, - PROCESS_IDENTITY_RECORDED, - "tool.execution.started", - ]), - ), - )) - .limit(1); - unclaimedBootstrap = !executionEvent; + unclaimedBootstrap = !(await hasNativeExecutionStartEvidence(tx, row.run)); } + // A claimed remote run whose executor died while still preparing. The + // executor durably records native.process_start_requested before any + // 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 remoteRunnerNeverLaunched = !unclaimedBootstrap && remoteWorkspace?.state === "prepared" && !pluginTask + && !durableAuthority && stagedAttachment === null + && row.run.runnerProfileJson?.nativePreparationFencingVersion === NATIVE_PREPARATION_FENCING_VERSION + && row.coordinator.phase === "observed" && row.coordinator.attempt >= 1 && row.coordinator.controllerGeneration >= 1 + && Boolean(row.coordinator.controllerBootId && row.coordinator.controllerPid + && row.coordinator.controllerProcessStartedAt && row.coordinator.leaseOwner) + && !row.coordinator.failureDetail?.successorRunId + && !row.run.processPid && !row.run.processGroupId && !row.run.processStartedAt + && !row.run.sessionIdBefore && !row.run.sessionIdAfter + && Boolean(row.run.nativeSessionId && row.run.runnerInstanceId) + && !hasCheckpoint && !hasProviderEvidence + && !(await hasNativeExecutionStartEvidence(tx, row.run)); + // A claimed remote bootstrap (runner launched, perhaps connected) has no // provider evidence when its controller dies before session.open // completes. The owner's lease has expired above, and the controller @@ -1087,7 +1115,8 @@ export async function claimNativeRestartRecoveries(input: { const classification = classifyNativeRunnerRecoveryEvidence({ remoteSandbox: remoteSandbox || Boolean(pluginTask && durableAuthority), unclaimedBootstrap, - remoteBootstrapProvenIncomplete, + remoteBootstrapProvenIncomplete: remoteBootstrapProvenIncomplete || remoteRunnerNeverLaunched, + remoteBootstrapIncompleteReason: remoteRunnerNeverLaunched ? "remote_runner_never_launched" : undefined, runnerPidAlive, runnerGroupAlive, processStartMatches: exactRunnerIdentity, diff --git a/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts b/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts index a9cc8f325cc..29881952bc1 100644 --- a/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts +++ b/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts @@ -2310,7 +2310,7 @@ describeEmbeddedPostgres("native runner restart recovery with real processes", ( contract: { revision: "1", objective: "Restart bootstrap", criteria: [{ id: "result", requirement: "Complete work" }] } }, }); const root = resolve(runtimeRoot, `marker-${fixture.runId}`); - await mkdir(resolve(root, "control-plane"), { recursive: true, mode: 0o700 }); + await mkdir(root, { recursive: true, mode: 0o700 }); const writeMarker = (runId: string) => writeFile(resolve(root, "control-plane", "control-plane-state.json"), JSON.stringify({ schema: "paperclip.runner.external-control-plane-state.v1", identity: { runnerInstanceId: fixture.native.runnerInstanceId, environmentLeaseId: fixture.runId, runId, @@ -2320,6 +2320,15 @@ describeEmbeddedPostgres("native runner restart recovery with real processes", ( controllerGeneration: 2, providerAttempt: 1, restartKind: "hard" as const, recoveryRequestId: null }; const target = { kind: "remote", transport: "sandbox", remoteCwd: "/workspace" } as AdapterExecutionTarget; try { + // RC-29b: preparation may leave an empty session root before any + // controller state exists. It carries no evidence and must not block. + await restoreRemoteBootstrapCheckpointForRecovery({ db: fixture.db, execution, root, + restartRecovery: undefined, runnerExecutionTarget: target }); + expect(existsSync(root)).toBe(true); + await restoreRemoteBootstrapCheckpointForRecovery({ db: fixture.db, execution, root, + restartRecovery: claim, runnerExecutionTarget: target }); + expect(existsSync(root)).toBe(false); + await mkdir(resolve(root, "control-plane"), { recursive: true, mode: 0o700 }); await writeMarker(fixture.runId); expect(runnerdStateProvesIncompleteBootstrap(root)).toBe(false); // Only a bootstrap recovery on a classic sandbox restores the checkpoint. @@ -2357,6 +2366,73 @@ describeEmbeddedPostgres("native runner restart recovery with real processes", ( restartKind: "hard", runIds: [fixture.runId] })).toEqual([{ kind: "blocked", runId: fixture.runId, reason }]); }); + // RC-29b: a restart while the executor was still preparing a claimed Daytona + // run, before it asked for a runner process. The executor records + // native.process_start_requested (and only then may a backend spawn or a + // controller checkpoint exist), so its absence proves nothing reached a runner. + async function seedUnlaunchedRemoteClaim(label: string, options: { + attempt?: number; leaseExpired?: boolean; liveController?: boolean; + event?: { eventType: string; sourceEventId?: string }; + run?: Partial; + } = {}) { + const fixture = await seedRun(`REMOTE-UNLAUNCHED-${label}`); + await fixture.db.update(heartbeatRuns).set({ + runnerProfileJson: { ...fixture.preparedProfile, nativeWorkspaceSync: remotePreparedWorkspace() }, + ...options.run, + }).where(eq(heartbeatRuns.id, fixture.runId)); + if (options.event) await fixture.db.insert(heartbeatRunEvents).values({ companyId, runId: fixture.runId, agentId, + seq: 900, stream: "system", level: "info", ...options.event }); + const current = await currentNativeControllerIdentity(); + await fixture.db.update(nativeRunFinalizations).set({ + attempt: options.attempt ?? 1, controllerGeneration: 1, + controllerBootId: options.liveController ? current.bootId : randomUUID(), + controllerPid: options.liveController ? current.pid : 2_000_000_001, + controllerProcessStartedAt: options.liveController ? current.processStartedAt : new Date(Date.now() - 60_000), + leaseOwner: `previous:${randomUUID()}`, + leaseExpiresAt: new Date(Date.now() + (options.leaseExpired ? -1_000 : 20 * 60_000)), + }).where(eq(nativeRunFinalizations.runId, fixture.runId)); + return fixture; + } + + 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, + restartKind: "hard", runIds: [fixture.runId] })).toEqual([expect.objectContaining({ + kind: "bootstrap_incomplete", runId: fixture.runId, providerAttempt: 1, controllerGeneration: 2 })]); + const [coordinator] = await fixture.db.select().from(nativeRunFinalizations) + .where(eq(nativeRunFinalizations.runId, fixture.runId)); + expect(coordinator).toMatchObject({ phase: "observed", recoveryState: "bootstrap_incomplete", failureCode: null }); + expect(coordinator.recoveryHistory.at(-1)).toMatchObject({ disposition: "bootstrap_incomplete", + reason: "remote_runner_never_launched" }); + const [run] = await fixture.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, fixture.runId)); + expect(run).toMatchObject({ status: "running", errorCode: null }); + expect(await fixture.db.select().from(issueRecoveryActions) + .where(eq(issueRecoveryActions.sourceIssueId, fixture.issueId))).toHaveLength(0); + }); + + it.each<[string, Parameters[1], { kind: string; reason: string }]>([ + ["a runner process start was requested", { event: { eventType: "native.process_start_requested" } }, + { kind: "blocked", reason: "ambiguous_remote_recovery_evidence" }], + ["a runner process identity was recorded", { event: { eventType: "native.process_identity_recorded" } }, + { kind: "blocked", reason: "ambiguous_remote_recovery_evidence" }], + ["the adapter was invoked", { event: { eventType: "adapter.invoke" } }, + { kind: "blocked", reason: "ambiguous_remote_recovery_evidence" }], + ["a runner event was committed", { event: { eventType: "item.updated", sourceEventId: "runner-event-1" } }, + { kind: "blocked", reason: "ambiguous_remote_recovery_evidence" }], + ["a process pid is recorded", { run: { processPid: 443 } }, + { kind: "blocked", reason: "ambiguous_remote_recovery_evidence" }], + ["a provider session is recorded", { run: { sessionIdAfter: "provider-session" } }, + { kind: "blocked", reason: "ambiguous_remote_recovery_evidence" }], + ["the bootstrap budget is spent", { attempt: 3 }, + { kind: "blocked", reason: "execution_recovery_budget_exhausted" }], + ["the claiming controller is still alive", { liveController: true }, + { kind: "awaiting_evidence", reason: "controller_still_alive" }], + ])("keeps a claimed unlaunched remote run held when %s", async (label, options, expected) => { + const fixture = await seedUnlaunchedRemoteClaim(label.replace(/\W+/g, "-"), options); + expect(await claimNativeRestartRecoveries({ db: fixture.db, controller: successor, + restartKind: "hard", runIds: [fixture.runId] })).toEqual([expect.objectContaining({ runId: fixture.runId, ...expected })]); + }); + it.each([ { attempt: 2, driverKind: "codex_app_server" }, { attempt: 3, driverKind: "codex_app_server" }, diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index acc9ace7e60..d0243073dd5 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -126,6 +126,7 @@ import { opendirSync, readFileSync, readdirSync, + rmdirSync, readlinkSync, readSync, renameSync, @@ -6044,8 +6045,22 @@ export async function restoreRemoteBootstrapCheckpointForRecovery(input: { }): Promise { const target = input.runnerExecutionTarget; if (input.restartRecovery?.kind !== "bootstrap_incomplete" || target?.kind !== "remote" - || target.transport !== "sandbox" || target.runnerTask || !existsSync(input.root) - || readRunnerdDurableIdentity(input.root) !== null) return; + || target.transport !== "sandbox" || target.runnerTask || !isSafeNativeStateDirectory(input.root)) return; + // Preparation can create the session root before the executor opens any + // controller state. An empty root holds no evidence; remove it so the fresh + // bootstrap does not read it as an unreadable identity. + if (readdirSync(input.root).length === 0) { + try { + rmdirSync(input.root); + return; + } catch (error) { + // An entry that appeared after the read goes through the checks below. + const code = (error as NodeJS.ErrnoException).code; + if (code === "ENOENT") return; + if (code !== "ENOTEMPTY" && code !== "EEXIST") throw error; + } + } + if (readRunnerdDurableIdentity(input.root) !== null) return; const marker = readRunnerdDurableIdentity(input.root, true); if (!marker || marker.runId !== input.execution.binding.runId) return; await restoreNativeRunnerCheckpoint(input.db, input.execution.binding, input.root, nativeSessionKey(input.execution));