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 <noreply@paperclip.ing>
This commit is contained in:
parent
850354d79c
commit
aec14bbab8
|
|
@ -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<void>((resolve) => { release = resolve; });
|
||||
const reached = new Promise<void>((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");
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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));
|
||||
|
|
|
|||
|
|
@ -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-<repo-directory-name>.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-<repo-directory-name>.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}`),
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
|
|
@ -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");
|
||||
}
|
||||
Loading…
Reference in New Issue