diff --git a/server/src/services/remote-workspaces.integration.test.ts b/server/src/services/remote-workspaces.integration.test.ts index 1a2990f9458..0ce20d415c9 100644 --- a/server/src/services/remote-workspaces.integration.test.ts +++ b/server/src/services/remote-workspaces.integration.test.ts @@ -1655,6 +1655,50 @@ suite("remote workspace lifecycle with real Postgres and independent filesystem expect(journal).toMatchObject({ status: "running", metadata: { uncertain: true } }); }, 60_000); + // An uncertain provisioning row used to block every later reservation of + // the workspace forever. It settles only once the earlier run's sandbox is + // provably stopped; until then the reservation names the operator's action. + it("settles an uncertain provisioning command once its run's sandbox is provably stopped", async () => { + const reserved = await reserve({ kind: "empty" }), first = await target("provision-stop-proof"); + const execute = first.runner!.execute.bind(first.runner!); + first.runner!.execute = async input => { + const result = await execute(input); + if (input.command === "bash") throw new Error("lost provisioning reply"); + return result; + }; + await expect(prepareRemoteWorkspace({ db, storage, workspace: reserved, runId: ids.run, lease: await lease(), target: first, + provisionCommand: "printf 'one effect\\n' >> setup-result.txt" })).rejects.toThrow("lost provisioning reply"); + await db.update(heartbeatRuns).set({ status: "failed", finishedAt: new Date() }).where(eq(heartbeatRuns.id, ids.run)); + const nextRun = randomUUID(); + await db.insert(heartbeatRuns).values({ id: nextRun, companyId: ids.company, agentId: ids.agent, nativeIssueId: ids.issue, runtimeMode: "native", status: "running" }); + const reserveNext = () => reserveRemoteWorkspace({ db, storage, companyId: ids.company, issueId: ids.issue, runId: nextRun, + agentId: ids.agent, environmentId: ids.environment, projectId: null, projectWorkspaceId: null, existingWorkspaceId: reserved.id, source: { kind: "empty" } }); + const provision = async () => (await db.select().from(workspaceOperations).where(and( + eq(workspaceOperations.companyId, ids.company), eq(workspaceOperations.phase, "remote_workspace_provision"))))[0]!; + // A failed run alone is not proof: its sandbox may still run the command. + await expect(reserveNext()).rejects.toMatchObject({ status: 409, + details: { code: "remote_workspace_provision_unverified", operationId: (await provision()).id, runId: ids.run } }); + expect(await provision()).toMatchObject({ status: "running", metadata: { uncertain: true } }); + const stopped = await lease(); + await db.update(environmentLeases).set({ status: "released", releasedAt: new Date(), cleanupStatus: "success", + metadata: { ...stopped.metadata, remoteExecutionTermination: remoteTerminationReceipt(stopped, + { providerLeaseId: stopped.providerLeaseId, state: "stopped" }) } }).where(eq(environmentLeases.id, ids.lease)); + const outcome = await reserveNext().then(() => null, (error: unknown) => error as { details?: { code?: string } }); + expect(outcome?.details?.code).not.toBe("remote_workspace_provision_unverified"); + expect(await provision()).toMatchObject({ status: "failed", metadata: { uncertain: false, settledBy: "owner_sandbox_stopped" } }); + expect(await db.select({ action: activityLog.action }).from(activityLog).where(and(eq(activityLog.companyId, ids.company), + eq(activityLog.action, "workspace.remote_provision_settled")))).toHaveLength(1); + }, 60_000); + + it("records a provisioning command that reported a failing exit as finished, not uncertain", async () => { + const reserved = await reserve({ kind: "empty" }), first = await target("provision-exit-failure"); + await expect(prepareRemoteWorkspace({ db, storage, workspace: reserved, runId: ids.run, lease: await lease(), target: first, + provisionCommand: "exit 3" })).rejects.toThrow("remote_workspace_provision_unverified"); + const [journal] = await db.select().from(workspaceOperations).where(and(eq(workspaceOperations.companyId, ids.company), + eq(workspaceOperations.phase, "remote_workspace_provision"))); + expect(journal).toMatchObject({ status: "failed", metadata: { uncertain: false, exitCode: 3 } }); + }, 60_000); + it("preserves enrolled workspaces after disabling enrollment and rejects a legacy writer", async () => { const reserved = await reserve({ kind: "empty" }); expect(shouldUseRemoteWorkspaces({ adapterType: "paperclip_runner", environmentDriver: "sandbox", experimentalEnabled: false, existingWorkspace: reserved })).toBe(true); diff --git a/server/src/services/remote-workspaces.ts b/server/src/services/remote-workspaces.ts index 19b9ac31c44..bdc1a54498e 100644 --- a/server/src/services/remote-workspaces.ts +++ b/server/src/services/remote-workspaces.ts @@ -25,6 +25,7 @@ import { notifyCancelledWorkspacePreservation } from "./cancelled-workspace-pres import { assertExecutionWorkspaceRecordAccess, assertTaskWorkspaceExecutionAccess } from "./task-workspace-access.js"; import { readRemoteWorkspaceWriterHold, type RemoteWorkspaceWriter } from "./remote-workspace-writer-hold.js"; import { persistActivity } from "./activity-log.js"; +import { hasRecoveredCleanupStopReceipt, hasRemoteTerminationReceipt } from "./remote-execution-termination.js"; import { captureRemoteWorkspaceArchive, cleanupRemoteWorkspaceArchive, prepareRemoteWorkspaceDirectory, prepareRemoteRepository, remoteWorkspaceRepositoryPath, RemoteWorkspaceOperationTimeoutError, RemoteWorkspacePreparationTimeoutError, remoteWorkspaceFailureCause, hasRemoteRecoveryAttachment, @@ -181,6 +182,9 @@ export async function reserveRemoteWorkspace(input: { }) { assertRemoteWorkspacePersistence(input.storage ?? getStorageService()); const source = remoteWorkspaceSourceSchema.parse(input.source); + // Provisioning rows proven settled commit even when a later reservation + // check (such as a live writer hold) rejects this attempt. + const provenProvisions: string[] = []; return input.db.transaction(async tx => { await tx.select({ id: companies.id }).from(companies).where(eq(companies.id, input.companyId)).for("key share"); const [issue] = await tx.select().from(issues).where(and(eq(issues.companyId, input.companyId), eq(issues.id, input.issueId))).for("update"); @@ -217,12 +221,21 @@ export async function reserveRemoteWorkspace(input: { : { issueId: input.issueId, workspaceId: existing.id }); await assertExecutionWorkspaceRecordAccess(tx, actor, input.companyId, existing.id); } - const [uncertain] = await tx.select({ id: workspaceOperations.id }).from(workspaceOperations).where(and( + const uncertain = await tx.select().from(workspaceOperations).where(and( eq(workspaceOperations.companyId, input.companyId), eq(workspaceOperations.executionWorkspaceId, existing.id), eq(workspaceOperations.phase, "remote_workspace_provision"), sql`${workspaceOperations.metadata}->>'uncertain' = 'true'`, sql`${workspaceOperations.heartbeatRunId} is distinct from ${input.runId}::uuid`, - )).limit(1); - if (uncertain) throw conflict("The prior provisioning operation needs verification before this workspace can run again.", { code: "remote_workspace_provision_unverified" }); + )).for("update"); + for (const operation of uncertain) { + if (!await provisionOwnerProvenStopped(tx as unknown as Db, operation)) { + throw conflict("A provisioning command from an earlier run may still be running in its sandbox, so this workspace cannot " + + "run again until that sandbox is stopped. Stop the earlier run's sandbox, or set the task's Workspace to " + + "\"New isolated workspace\" to continue in a fresh one.", + { code: "remote_workspace_provision_unverified", operationId: operation.id, runId: operation.heartbeatRunId }); + } + await settleUnverifiedProvision(tx as unknown as Db, operation, input.runId); + provenProvisions.push(operation.id); + } const hold = await readRemoteWorkspaceWriterHold(tx as unknown as Db, { companyId: input.companyId, authority, runId: input.runId }); if (hold.kind === "held") throw new RemoteWorkspaceWriterActiveError(existing.id, hold.writer); if (hold.kind === "unattributed") throw conflict("The workspace still has an unfinished writer or save operation.", { code: "remote_workspace_writer_pending" }); @@ -255,6 +268,9 @@ export async function reserveRemoteWorkspace(input: { action: "workspace.remote_created", entityType: "execution_workspace", entityId: id, runId: input.runId, issueId: input.issueId, details: { authority: "sandbox", epoch: initial.epoch } }); return workspace!; + }).catch(async (error: unknown) => { + if (provenProvisions.length > 0) await settleProvenProvisions(input.db, input.companyId, provenProvisions, input.runId); + throw error; }); } @@ -946,6 +962,58 @@ export async function createRemoteWorkspaceGitAuth(db: Db, companyId: string, ru responsibleUserId: run.responsibleUserId === "local-board" ? null : run.responsibleUserId }); } +const TERMINAL_RUN_STATUSES = ["succeeded", "failed", "cancelled", "timed_out", "interrupted"]; + +/** + * True only with proof that an uncertain provisioning command can no longer + * run: its run is terminal, and every sandbox allocation of that run carries a + * provider stop or destroy receipt bound to that lease and run. Run status, + * age, or a dead server process alone never proves it, because the command + * runs in the sandbox, not in the server. + */ +async function provisionOwnerProvenStopped(db: Db, operation: typeof workspaceOperations.$inferSelect): Promise { + if (!operation.heartbeatRunId) return false; + const [run] = await db.select({ status: heartbeatRuns.status }).from(heartbeatRuns).where(and( + eq(heartbeatRuns.companyId, operation.companyId), eq(heartbeatRuns.id, operation.heartbeatRunId))); + if (!run || !TERMINAL_RUN_STATUSES.includes(run.status)) return false; + const leases = await db.select().from(environmentLeases).where(and( + eq(environmentLeases.companyId, operation.companyId), eq(environmentLeases.heartbeatRunId, operation.heartbeatRunId))); + return leases.length > 0 && leases.every(lease => hasRemoteTerminationReceipt(lease) || hasRecoveredCleanupStopReceipt(lease)); +} + +/** The command's effect is still unknown, so the row is settled as failed, + * never as succeeded. The next writer provisions in its own epoch. */ +async function settleUnverifiedProvision(db: Db, operation: typeof workspaceOperations.$inferSelect, reservingRunId: string) { + const now = new Date(); + await db.update(workspaceOperations).set({ status: "failed", finishedAt: now, updatedAt: now, + metadata: { ...operation.metadata, uncertain: false, settledBy: "owner_sandbox_stopped" } }) + .where(eq(workspaceOperations.id, operation.id)); + await persistActivity(db, { companyId: operation.companyId, actorType: "system", actorId: "remote-workspaces", + action: "workspace.remote_provision_settled", entityType: "execution_workspace", + entityId: operation.executionWorkspaceId ?? operation.id, runId: reservingRunId, issueId: operation.issueId ?? undefined, + details: { operationId: operation.id, ownerRunId: operation.heartbeatRunId, + reason: "The earlier run ended and its sandbox was stopped, so its provisioning command cannot still be running. Its effect is unknown." } }); +} + +/** Re-proves and settles provisioning rows after the reservation that found + * them rolled back, so a proven row never blocks again. */ +async function settleProvenProvisions(db: Db, companyId: string, operationIds: string[], reservingRunId: string) { + try { + await db.transaction(async tx => { + const rows = await tx.select().from(workspaceOperations).where(and(eq(workspaceOperations.companyId, companyId), + inArray(workspaceOperations.id, operationIds), sql`${workspaceOperations.metadata}->>'uncertain' = 'true'`)).for("update"); + for (const operation of rows) { + if (await provisionOwnerProvenStopped(tx as unknown as Db, operation)) { + await settleUnverifiedProvision(tx as unknown as Db, operation, reservingRunId); + } + } + }); + } catch (error) { + // The next reservation proves and settles them again. + logger.warn({ err: error, companyId, operationIds }, "remote workspace provisioning settlement will retry"); + } +} + /** Arbitrary provisioning commands cannot be replayed after an ambiguous reply. */ export async function provisionRemoteWorkspace(input: { db: Db; companyId: string; workspaceId: string; runId: string; epoch: number; @@ -979,7 +1047,14 @@ export async function provisionRemoteWorkspace(input: { await input.assertOwned?.(); const result = await input.target.runner!.execute({ command: "bash", args: ["-lc", command], cwd: input.remoteCwd, timeoutMs: 300_000, bypassSession: true }); - if (result.exitCode !== 0 || result.timedOut) throw new Error("remote_workspace_provision_unverified"); + if (result.exitCode !== 0 || result.timedOut) { + // A reported non-zero exit is a finished command, not an ambiguous one. + // A timeout stays uncertain: the runner may not have stopped it. + if (!result.timedOut) await input.db.update(workspaceOperations).set({ status: "failed", finishedAt: new Date(), + updatedAt: new Date(), metadata: { ...operation.metadata, uncertain: false, exitCode: result.exitCode } }) + .where(and(eq(workspaceOperations.id, operation.id), eq(workspaceOperations.status, "running"))); + throw new Error("remote_workspace_provision_unverified"); + } await input.assertOwned?.(); await input.db.transaction(async tx => { const [workspace] = await tx.select({ metadata: executionWorkspaces.metadata }).from(executionWorkspaces)