From aec14bbab829e75b93d412a79134bc190a41fbaa Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 7 Sep 2026 15:03:16 -0500 Subject: [PATCH] fix: serialize warm binding validation with lifecycle transitions Persist the validated workspace mode and run identical executable acceptance scripts across live engines. Co-Authored-By: Paperclip --- .../__tests__/sandbox-work-folders.test.ts | 26 +++++++++- .../src/services/sandbox-workspace-binding.ts | 9 +++- .../runner-e2e/deployed-work-folders.spec.ts | 31 +++--------- .../work-folder-acceptance-prompts.test.ts | 38 +++++++++++++++ .../work-folder-acceptance-prompts.ts | 47 +++++++++++++++++++ 5 files changed, 123 insertions(+), 28 deletions(-) create mode 100644 tests/runner-e2e/work-folder-acceptance-prompts.test.ts create mode 100644 tests/runner-e2e/work-folder-acceptance-prompts.ts diff --git a/server/src/__tests__/sandbox-work-folders.test.ts b/server/src/__tests__/sandbox-work-folders.test.ts index 381fe2f5eb..593c3531a5 100644 --- a/server/src/__tests__/sandbox-work-folders.test.ts +++ b/server/src/__tests__/sandbox-work-folders.test.ts @@ -2,7 +2,7 @@ import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { execFile } from "node:child_process"; import { promisify } from "node:util"; import { createHash, randomUUID } from "node:crypto"; -import { eq } from "drizzle-orm"; +import { eq, sql } from "drizzle-orm"; import fs from "node:fs/promises"; import path from "node:path"; import os from "node:os"; @@ -59,13 +59,35 @@ describe("shared sandbox work-folder lifecycle", () => { const input = { companyId, issueId: task, runId, agentId, workspaceId }; await bindWarmSandboxWorkspace(db, input); const [bound] = await db.select().from(issues).where(eq(issues.id, task)); - expect(bound).toMatchObject({ executionWorkspaceId: workspaceId, executionWorkspacePreference: "reuse_existing", executionWorkspaceSettings: null }); + expect(bound).toMatchObject({ executionWorkspaceId: workspaceId, executionWorkspacePreference: "reuse_existing", executionWorkspaceSettings: { mode: "shared_workspace" } }); for (const bad of [{ companyId: randomUUID() }, { agentId: randomUUID() }, { issueId: taskId }, { runId: randomUUID() }]) { await expect(bindWarmSandboxWorkspace(db, { ...input, ...bad })).rejects.toThrow("active task run"); } await db.update(executionWorkspaces).set({ sourceIssueId: taskId }).where(eq(executionWorkspaces.id, workspaceId)); await expect(bindWarmSandboxWorkspace(db, input)).rejects.toThrow("active task run"); await db.update(executionWorkspaces).set({ sourceIssueId: task }).where(eq(executionWorkspaces.id, workspaceId)); + const originalLogActivity = activityLog.logActivity; + let release!: () => void; + let entered!: () => void; + const gate = new Promise((resolve) => { release = resolve; }); + const reached = new Promise((resolve) => { entered = resolve; }); + const audit = vi.spyOn(activityLog, "logActivity").mockImplementationOnce(async (...args) => { + entered(); await gate; return originalLogActivity(...args); + }); + const pendingBinding = bindWarmSandboxWorkspace(db, input); + try { + await reached; + // These state changes must wait until the validated binding commits. + for (const target of ["run", "workspace"]) { + await expect(db.transaction(async (tx) => { + await tx.execute(sql`set local lock_timeout = '100ms'`); + if (target === "run") await tx.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, runId)); + else await tx.update(executionWorkspaces).set({ status: "closed" }).where(eq(executionWorkspaces.id, workspaceId)); + })).rejects.toMatchObject({ cause: { code: "55P03" } }); + } + } finally { + release(); await pendingBinding; audit.mockRestore(); + } await db.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, runId)); await expect(bindWarmSandboxWorkspace(db, input)).rejects.toThrow("active task run"); }); diff --git a/server/src/services/sandbox-workspace-binding.ts b/server/src/services/sandbox-workspace-binding.ts index 0c118622f5..9034fe3416 100644 --- a/server/src/services/sandbox-workspace-binding.ts +++ b/server/src/services/sandbox-workspace-binding.ts @@ -1,6 +1,7 @@ import { and, eq } from "drizzle-orm"; import { executionWorkspaces, heartbeatRuns, issues, type Db } from "@paperclipai/db"; import { logActivity, publishActivity, type ActivityPublication } from "./activity-log.js"; +import { issueExecutionWorkspaceModeForPersistedWorkspace } from "./execution-workspace-policy.js"; /** Host runtime state must survive even when user-configurable worktrees are disabled. */ export async function bindWarmSandboxWorkspace(db: Db, input: { @@ -14,17 +15,21 @@ export async function bindWarmSandboxWorkspace(db: Db, input: { )).for("update"); const [workspace] = await tx.select().from(executionWorkspaces).where(and( eq(executionWorkspaces.id, input.workspaceId), eq(executionWorkspaces.companyId, input.companyId), - )); + )).for("update"); const [run] = await tx.select().from(heartbeatRuns).where(and( eq(heartbeatRuns.id, input.runId), eq(heartbeatRuns.companyId, input.companyId), eq(heartbeatRuns.agentId, input.agentId), eq(heartbeatRuns.status, "running"), - )); + )).for("update"); if (!issue || !run || !workspace || workspace.projectId !== issue.projectId || workspace.status !== "active" || (workspace.sourceIssueId !== null && workspace.sourceIssueId !== issue.id)) { throw new Error("Warm sandbox workspace no longer belongs to this active task run"); } await tx.update(issues).set({ executionWorkspaceId: workspace.id, executionWorkspacePreference: "reuse_existing", + executionWorkspaceSettings: { + ...(issue.executionWorkspaceSettings ?? {}), + mode: issueExecutionWorkspaceModeForPersistedWorkspace(workspace.mode), + }, ...(workspace.projectWorkspaceId ? { projectWorkspaceId: workspace.projectWorkspaceId } : {}), updatedAt: new Date(), }).where(eq(issues.id, issue.id)); diff --git a/tests/runner-e2e/deployed-work-folders.spec.ts b/tests/runner-e2e/deployed-work-folders.spec.ts index b20e9a25e3..d6f461a2ae 100644 --- a/tests/runner-e2e/deployed-work-folders.spec.ts +++ b/tests/runner-e2e/deployed-work-folders.spec.ts @@ -2,10 +2,12 @@ import { randomUUID } from "node:crypto"; import { test, expect } from "@playwright/test"; import type { EnvironmentCapabilities } from "../../packages/shared/src/environment-support.js"; import type { SandboxWorkFolderManifest, WorkFolderListing, WorkFolderSyncStatus } from "../../packages/shared/src/work-folders.js"; -import { QUALIFIED_ACPX_PROFILES } from "../../packages/paperclip-runner/src/drivers/acpx/qualified-profiles.js"; +import { QUALIFIED_ACPX_RUNNER_MODELS } from "../../server/src/services/native-runtime/provider-profile.js"; import { pollUntil } from "./api.js"; import { DeployedStackApi, deployedAgentEngine, loadDeployedStack } from "./deployed-stack.js"; +import { repoAcceptancePrompt } from "./work-folder-acceptance-prompts.js"; + const stack = loadDeployedStack(); const api = new DeployedStackApi(stack); const folder = (scope: string, ownerId: string) => `/api/companies/${stack.companyId}/work-folders/${scope}/${encodeURIComponent(ownerId)}`; @@ -26,7 +28,7 @@ test("deployed candidate and complete supported adapter inventory", async ({}, i if (capabilities.adapters.find((entry) => entry.adapterType === adapter.type)?.drivers.sandbox !== "supported") continue; if (adapter.type === "paperclip_runner") { required.push("paperclip_runner:codex", "paperclip_runner:opencode", - ...Object.keys(QUALIFIED_ACPX_PROFILES).map((name) => `paperclip_runner:acpx:${name}`)); + ...Object.keys(QUALIFIED_ACPX_RUNNER_MODELS).map((name) => `paperclip_runner:acpx:${name}`)); } else { required.push(`${adapter.type}:cli`); if (adapter.capabilities.supportsAcp) required.push(`${adapter.type}:acp`); @@ -69,6 +71,7 @@ for (const [scope, owner] of [["task", stack.taskId], ["agent", stack.agentId], }); } + for (const profile of stack.profiles) { test(`${profile.id} preserves task-specific repo and file state across cold and warm runs`, async ({}, info) => { test.setTimeout(1_800_000); @@ -76,19 +79,7 @@ for (const profile of stack.profiles) { const issue = await api.json<{ id: string; identifier: string }>(`/api/companies/${stack.companyId}/issues`, "POST", { title: `Work folder acceptance ${profile.id} ${nonce}`, projectId: stack.projectId, assigneeAgentId: profile.agentId, status: "todo", - description: [ - "Perform this sandbox acceptance task using real filesystem tools.", - "Verify cwd equals the operating-system HOME and task, agent, user, project, repos, .codex, .cache are directories beneath it.", - "Verify repos contains at least two independent Git checkouts. Fail the task with the actual error if either assertion fails.", - "In each repo, assert .acceptance-owner does not exist (another task must not share this checkout).", - `In each repo write '${nonce}' without a newline to .acceptance-owner, git add ONLY that file, and create a local commit using git -c user.name=Acceptance -c user.email=acceptance@example.invalid commit -m acceptance. Do not push.`, - "Save each repo's HEAD to $HOME/task/head-.txt.", - "In each repo write 'staged' without newline to .acceptance-state, git add ONLY that file, then replace its working-tree content with 'unstaged' without newline. Write 'untracked' without newline to .acceptance-untracked and leave it untracked.", - "If .acceptance-setup-count exists, assert it has exactly one line. Never run setup yourself.", - `Write exactly '${nonce}' without a newline into $HOME/task/acceptance.txt and $HOME/agent/acceptance-${nonce}.txt.`, - `Also write exactly '${nonce}' to $HOME/.cache/warm-${nonce}; this disposable cache marker must survive an actual warm reuse.`, - "Then complete this task successfully. Do not print credentials or modify unrelated files.", - ].join("\n"), + description: repoAcceptancePrompt(nonce, false), }); await info.attach("task", { contentType: "application/json", body: Buffer.from(JSON.stringify({ profile: profile.id, ...issue })) }); const base = folder("task", issue.id); @@ -107,15 +98,7 @@ for (const profile of stack.profiles) { const coldRun = await api.json<{ contextSnapshot: { paperclipWorkFolders: SandboxWorkFolderManifest } }>(`/api/heartbeat-runs/${coldSave.runId}`); const coldManifest = coldRun.contextSnapshot.paperclipWorkFolders; expect(coldManifest.sandboxKey).toBeTruthy(); - await api.json(`/api/issues/${issue.id}`, "PATCH", { status: "todo", description: [ - "Continue this sandbox acceptance task. This is a warm run; inspect the existing work without repairing it.", - "Verify cwd equals HOME and all seven directories still exist.", - `Assert $HOME/task/acceptance.txt and every repo's committed HEAD:.acceptance-owner equal '${nonce}'.`, - `Assert $HOME/.cache/warm-${nonce} still contains exactly '${nonce}'. A replacement is not a warm pass; do not recreate the marker.`, - "For each repo assert HEAD equals the saved task/head-.txt, index :.acceptance-state equals 'staged', working .acceptance-state equals 'unstaged', and .acceptance-untracked equals 'untracked' and remains untracked.", - "If .acceptance-setup-count exists, assert exactly one line. Fail with the actual discrepancy; do not recreate missing state or rerun setup.", - `Write exactly '${nonce}' without a newline to $HOME/task/warm.txt, then complete the task.`, - ].join("\n") }); + await api.json(`/api/issues/${issue.id}`, "PATCH", { status: "todo", description: repoAcceptancePrompt(nonce, true) }); const warm = await pollUntil({ label: `${profile.id} warm run preserves saved work`, deadlineAt: Date.now() + 840_000, intervalMs: 5_000, load: async () => ({ issue: await api.json<{ status: string }>(`/api/issues/${issue.id}`), diff --git a/tests/runner-e2e/work-folder-acceptance-prompts.test.ts b/tests/runner-e2e/work-folder-acceptance-prompts.test.ts new file mode 100644 index 0000000000..2d138c134a --- /dev/null +++ b/tests/runner-e2e/work-folder-acceptance-prompts.test.ts @@ -0,0 +1,38 @@ +import { afterAll, beforeAll, expect, it } from "vitest"; +import fs from "node:fs/promises"; +import path from "node:path"; +import os from "node:os"; +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; +import { randomUUID } from "node:crypto"; +import { repoAcceptanceScript } from "./work-folder-acceptance-prompts.js"; +const exec = promisify(execFile); +let root: string; +beforeAll(async () => { + root = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "work-folder-acceptance-script-"))); + for (const folder of ["task", "agent", "user", "project", "repos", ".codex", ".cache"]) await fs.mkdir(path.join(root, folder)); + for (const name of ["first repo", "second-repo"]) { + const repo = path.join(root, "repos", name); + await exec("git", ["init", repo]); + await fs.writeFile(path.join(repo, "README"), "fixture"); + await exec("git", ["-C", repo, "add", "README"]); + await exec("git", ["-C", repo, "-c", "user.name=Acceptance", "-c", "user.email=acceptance@example.invalid", "commit", "-m", "fixture"]); + await fs.writeFile(path.join(repo, ".acceptance-setup-count"), "initialized\n"); + } +}); +afterAll(async () => { if (root) await fs.rm(root, { recursive: true, force: true }); }); +it("proves actual warm state and fails instead of repairing lost files", async () => { + const nonce = randomUUID(); + const run = (warm: boolean) => exec("/bin/sh", ["-c", repoAcceptanceScript(nonce, warm)], { cwd: root, env: { ...process.env, HOME: root } }); + expect((await run(false)).stdout).toContain("ACCEPTANCE_SCRIPT_PASSED"); + expect((await run(true)).stdout).toContain("ACCEPTANCE_SCRIPT_PASSED"); + // Cold tasks cannot silently share another task's checkouts. + await expect(run(false)).rejects.toThrow(); + const untracked = path.join(root, "repos", "first repo", ".acceptance-untracked"); + await fs.writeFile(untracked, "corrupt"); + await expect(run(true)).rejects.toThrow(); + expect(await fs.readFile(untracked, "utf8")).toBe("corrupt"); + await fs.writeFile(untracked, "untracked"); + await fs.rm(path.join(root, ".cache", `warm-${nonce}`)); + await expect(run(true)).rejects.toThrow(); +}); diff --git a/tests/runner-e2e/work-folder-acceptance-prompts.ts b/tests/runner-e2e/work-folder-acceptance-prompts.ts new file mode 100644 index 0000000000..593a7ce50d --- /dev/null +++ b/tests/runner-e2e/work-folder-acceptance-prompts.ts @@ -0,0 +1,47 @@ +export function repoAcceptanceScript(nonce: string, warm: boolean): string { + if (!/^[a-f0-9-]{36}$/.test(nonce)) throw new Error("Acceptance nonce must be a UUID"); + const checkLayout = [ + 'set -eu', 'test "$PWD" = "$HOME"', + 'for folder in task agent user project repos .codex .cache; do test -d "$HOME/$folder"; done', + 'count=0', 'for repo in "$HOME"/repos/*; do', + ' test -d "$repo/.git" || continue', ' count=$((count + 1))', ' name=$(basename "$repo")', + ]; + const repoSteps = warm ? [ + ` test "$(git -C "$repo" show HEAD:.acceptance-owner)" = '${nonce}'`, + ' test "$(git -C "$repo" rev-parse HEAD)" = "$(cat "$HOME/task/head-$name.txt")"', + ' test "$(git -C "$repo" show :.acceptance-state)" = staged', + ' test "$(cat "$repo/.acceptance-state")" = unstaged', + ' test "$(cat "$repo/.acceptance-untracked")" = untracked', + ' if git -C "$repo" ls-files --error-unmatch .acceptance-untracked >/dev/null 2>&1; then exit 1; fi', + ] : [ + ' test ! -e "$repo/.acceptance-owner"', + ` printf '%s' '${nonce}' > "$repo/.acceptance-owner"`, + ' git -C "$repo" add -- .acceptance-owner', + ' git -C "$repo" -c user.name=Acceptance -c user.email=acceptance@example.invalid commit -m acceptance', + ' git -C "$repo" rev-parse HEAD > "$HOME/task/head-$name.txt"', + ' printf staged > "$repo/.acceptance-state"', ' git -C "$repo" add -- .acceptance-state', + ' printf unstaged > "$repo/.acceptance-state"', ' printf untracked > "$repo/.acceptance-untracked"', + ]; + return [...checkLayout, ...repoSteps, + ' if test -f "$repo/.acceptance-setup-count"; then test "$(wc -l < "$repo/.acceptance-setup-count")" -eq 1; fi', + 'done', 'test "$count" -ge 2', + ...(warm ? [ + `test "$(cat "$HOME/task/acceptance.txt")" = '${nonce}'`, + `test "$(cat "$HOME/.cache/warm-${nonce}")" = '${nonce}'`, + `printf '%s' '${nonce}' > "$HOME/task/warm.txt"`, + ] : [ + `printf '%s' '${nonce}' > "$HOME/task/acceptance.txt"`, + `printf '%s' '${nonce}' > "$HOME/agent/acceptance-${nonce}.txt"`, + `printf '%s' '${nonce}' > "$HOME/.cache/warm-${nonce}"`, + ]), 'printf "ACCEPTANCE_SCRIPT_PASSED\\n"', + ].join("\n"); +} + +export function repoAcceptancePrompt(nonce: string, warm: boolean): string { + return [ + "Execute this exact acceptance shell script from your initial working directory in one tool call. Use real filesystem tools; do not simulate its result.", + warm ? "This must reuse the same warm sandbox. Do not repair, recreate, or reset missing state." : "This creates disposable local commits and staged, unstaged, and untracked test files. Do not push.", + "If any assertion fails, mark the task blocked with the actual error. Otherwise mark the Paperclip task done after the script succeeds.", + "```sh", repoAcceptanceScript(nonce, warm), "```", + ].join("\n"); +}