From 850354d79c067d4244c34f05d20cfcd9872bdfe9 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 7 Sep 2026 14:56:30 -0500 Subject: [PATCH] fix: retain warm sandbox bindings independently of worktree settings Validate live engine coverage and physical sandbox reuse in deployed acceptance. Co-Authored-By: Paperclip --- doc/sandbox-work-folders.md | 6 +++ .../__tests__/sandbox-work-folders.test.ts | 22 ++++++++++- server/src/services/heartbeat.ts | 9 ++++- .../src/services/sandbox-workspace-binding.ts | 39 +++++++++++++++++++ tests/runner-e2e/deployed-stack.test.ts | 12 +++++- tests/runner-e2e/deployed-stack.ts | 13 +++++++ .../runner-e2e/deployed-work-folders.spec.ts | 23 ++++++++--- 7 files changed, 116 insertions(+), 8 deletions(-) create mode 100644 server/src/services/sandbox-workspace-binding.ts diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index 9f216853a0..b972507852 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -8,6 +8,9 @@ and `PAPERCLIP_DEPLOYED_STACK_EVIDENCE` to an absolute output directory. The harness never launches a local server. It checks the deployed commit and adapter inventory, exercises scoped file APIs, and starts real sandbox tasks for every configured profile, including cold/warm Git state and real timed saves. +Engine coverage is derived from each live agent's configuration. A warm pass +requires the same host-recorded physical sandbox identity and a surviving cache +marker; restoring durable files into a replacement is tested separately. The maintainer-approved matrix is Codex, Claude, OpenCode, and Pi in both runner generations (11 CLI/ACP profiles). Only Cursor, Gemini, Grok, and Kimi are deferred for this campaign. Missing required profiles fail the inventory gate. @@ -18,6 +21,9 @@ scenarios; passing them alone is not staging acceptance. Sandbox runs use the operating-system user's home directory. Both legacy adapters and the native runner enter the same host-owned lifecycle before dispatch. Local execution keeps its existing workspace and home behavior. +Warm sandbox task bindings persist independently of the experimental isolated +workspace setting. Only the active host run can establish that binding; the +setting still controls user-configurable worktree operations. ```text $HOME/ diff --git a/server/src/__tests__/sandbox-work-folders.test.ts b/server/src/__tests__/sandbox-work-folders.test.ts index a7d4e25c2b..381fe2f5eb 100644 --- a/server/src/__tests__/sandbox-work-folders.test.ts +++ b/server/src/__tests__/sandbox-work-folders.test.ts @@ -6,9 +6,10 @@ import { eq } from "drizzle-orm"; import fs from "node:fs/promises"; import path from "node:path"; import os from "node:os"; -import { agents, assets, companyMemberships, issueAttachments, companies, createDb, heartbeatRuns, issues, environments, environmentLeases, projects, projectWorkspaces, taskRepositoryBindings, workFolderObjects, workFolderRuns, startEmbeddedPostgresTestDatabase, type Db } from "@paperclipai/db"; +import { agents, assets, companyMemberships, issueAttachments, companies, createDb, heartbeatRuns, issues, environments, environmentLeases, executionWorkspaces, projects, projectWorkspaces, taskRepositoryBindings, workFolderObjects, workFolderRuns, startEmbeddedPostgresTestDatabase, type Db } from "@paperclipai/db"; import { createLocalDiskStorageProvider } from "../storage/local-disk-provider.js"; import { prepareSandboxWorkFolders } from "../services/sandbox-work-folders.js"; +import { bindWarmSandboxWorkspace } from "../services/sandbox-workspace-binding.js"; import { retainUnsavedWorkFolderLease, workFolderSandboxKey } from "../services/work-folder-retention.js"; import * as activityLog from "../services/activity-log.js"; import { workFolderService } from "../services/work-folders.js"; @@ -49,6 +50,25 @@ describe("shared sandbox work-folder lifecycle", () => { for (const run of active) await run.stop().catch(() => {}); await database?.cleanup(); if (root) await fs.rm(root, { recursive: true, force: true }); }); + it("keeps the host's warm task binding without enabling user-configurable worktrees", async () => { + const task = randomUUID(), runId = randomUUID(), workspaceId = randomUUID(); + await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, status: "running" }); + await db.insert(issues).values({ id: task, companyId, projectId, title: "Warm binding", assigneeAgentId: agentId, executionRunId: runId }); + await db.insert(executionWorkspaces).values({ id: workspaceId, companyId, projectId, sourceIssueId: task, + mode: "shared_workspace", strategyType: "project_primary", name: "Warm binding" }); + 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 }); + 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)); + await db.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, runId)); + await expect(bindWarmSandboxWorkspace(db, input)).rejects.toThrow("active task run"); + }); async function prepare(home: string, leaseId: string, physicalId = leaseId, responsibleUserId: string | null = null, options: { taskId?: string; branchName?: string; agentId?: string } = {}) { await fs.mkdir(home, { recursive: true }); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 22d041ed01..f9750de3cb 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -1,6 +1,7 @@ import fs from "node:fs/promises"; import { retainUnsavedWorkFolderLease, workFolderSandboxKey } from "./work-folder-retention.js"; import { prepareSandboxWorkFolders } from "./sandbox-work-folders.js"; +import { bindWarmSandboxWorkspace } from "./sandbox-workspace-binding.js"; import path from "node:path"; import { execFile as execFileCallback } from "node:child_process"; import { promisify } from "node:util"; @@ -19362,7 +19363,13 @@ export function heartbeatService( }; } if (Object.keys(nextIssuePatch).length > 0) { - await issuesSvc.update(issueId, nextIssuePatch); + if (warmReusableExecutionWorkspace && !isolatedWorkspacesEnabled) { + await bindWarmSandboxWorkspace(db, { + companyId: agent.companyId, issueId, runId: run.id, agentId: agent.id, workspaceId: workspace.id, + }); + } else { + await issuesSvc.update(issueId, nextIssuePatch); + } issueExecutionWorkspaceIdForRun = workspace.id; issueProjectWorkspaceIdForRun = resolvedProjectWorkspaceId ?? issueProjectWorkspaceIdForRun; diff --git a/server/src/services/sandbox-workspace-binding.ts b/server/src/services/sandbox-workspace-binding.ts new file mode 100644 index 0000000000..0c118622f5 --- /dev/null +++ b/server/src/services/sandbox-workspace-binding.ts @@ -0,0 +1,39 @@ +import { and, eq } from "drizzle-orm"; +import { executionWorkspaces, heartbeatRuns, issues, type Db } from "@paperclipai/db"; +import { logActivity, publishActivity, type ActivityPublication } from "./activity-log.js"; + +/** Host runtime state must survive even when user-configurable worktrees are disabled. */ +export async function bindWarmSandboxWorkspace(db: Db, input: { + companyId: string; issueId: string; runId: string; agentId: string; workspaceId: string; +}) { + const publications: ActivityPublication[] = []; + await db.transaction(async (tx) => { + const [issue] = await tx.select().from(issues).where(and( + eq(issues.id, input.issueId), eq(issues.companyId, input.companyId), + eq(issues.executionRunId, input.runId), + )).for("update"); + const [workspace] = await tx.select().from(executionWorkspaces).where(and( + eq(executionWorkspaces.id, input.workspaceId), eq(executionWorkspaces.companyId, input.companyId), + )); + 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"), + )); + 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", + ...(workspace.projectWorkspaceId ? { projectWorkspaceId: workspace.projectWorkspaceId } : {}), + updatedAt: new Date(), + }).where(eq(issues.id, issue.id)); + await logActivity(tx as unknown as Db, { + companyId: input.companyId, actorType: "agent", actorId: input.agentId, agentId: input.agentId, + runId: input.runId, issueId: issue.id, action: "execution_workspace.sandbox_bound", + entityType: "execution_workspace", entityId: workspace.id, + details: { issueId: issue.id, reason: "warm_sandbox_reuse" }, + }, publications); + }); + for (const publication of publications) publishActivity(publication); +} diff --git a/tests/runner-e2e/deployed-stack.test.ts b/tests/runner-e2e/deployed-stack.test.ts index 1382a16333..5b7cd447e4 100644 --- a/tests/runner-e2e/deployed-stack.test.ts +++ b/tests/runner-e2e/deployed-stack.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { isStagingOrigin, assertDeployedAdapterExclusions } from "./deployed-stack.js"; +import { isStagingOrigin, assertDeployedAdapterExclusions, deployedAgentEngine } from "./deployed-stack.js"; describe("deployed stack target", () => { it("requires an explicit HTTPS staging tenant and rejects credential-bearing URLs", () => { @@ -11,6 +11,16 @@ describe("deployed stack target", () => { }); }); +it("derives engine coverage from live adapter configuration", () => { + expect(deployedAgentEngine({ adapterType: "codex_local", adapterConfig: {} })).toBe("cli"); + expect(deployedAgentEngine({ adapterType: "claude_local", adapterConfig: { engine: "acp" } })).toBe("acp"); + expect(deployedAgentEngine({ adapterType: "paperclip_runner", adapterConfig: { provider: "codex" } })).toBe("codex"); + expect(deployedAgentEngine({ adapterType: "paperclip_runner", adapterConfig: { provider: "acpx", acpxAgent: "pi" } })).toBe("acpx:pi"); + expect(deployedAgentEngine({ adapterType: "paperclip_runner", adapterConfig: { provider: "acpx", acpxAgent: "claude" } })).not.toBe("acpx:pi"); + expect(() => deployedAgentEngine({ adapterType: "paperclip_runner", adapterConfig: { provider: "acpx" } })).toThrow("Missing"); + expect(() => deployedAgentEngine({ adapterType: "codex_local", adapterConfig: { engine: "unknown" } })).toThrow("Unknown"); +}); + it("allows only the four explicitly deferred adapters to be excluded", () => { expect(() => assertDeployedAdapterExclusions([{ adapterType: "cursor", reason: "Explicitly deferred by the user" }])).not.toThrow(); for (const adapterType of ["codex_local", "claude_local", "opencode_local", "pi_local", "paperclip_runner"]) { diff --git a/tests/runner-e2e/deployed-stack.ts b/tests/runner-e2e/deployed-stack.ts index a23f2d9dbf..5a8d9b3d9e 100644 --- a/tests/runner-e2e/deployed-stack.ts +++ b/tests/runner-e2e/deployed-stack.ts @@ -16,6 +16,19 @@ export interface DeployedStack { profiles: Array<{ id: string; adapterType: string; engine: string; model: string; qualification: string; agentId: string }>; } +/** Derive coverage from the live configuration, never a manifest's label. */ +export function deployedAgentEngine(agent: { adapterType: string; adapterConfig: Record }): string { + const config = agent.adapterConfig; + if (agent.adapterType === "paperclip_runner") { + assert(config.provider === "codex" || config.provider === "opencode" || config.provider === "acpx", "Unknown native provider"); + if (config.provider !== "acpx") return config.provider; + assert(typeof config.acpxAgent === "string" && config.acpxAgent.length > 0, "Missing native ACPX engine"); + return `acpx:${config.acpxAgent}`; + } + assert(config.engine === undefined || config.engine === "cli" || config.engine === "acp", "Unknown legacy engine"); + return config.engine === "acp" ? "acp" : "cli"; +} + export function loadDeployedStack(): DeployedStack { const filename = process.env.PAPERCLIP_DEPLOYED_STACK_MANIFEST; if (!filename) throw new Error("PAPERCLIP_DEPLOYED_STACK_MANIFEST is required"); diff --git a/tests/runner-e2e/deployed-work-folders.spec.ts b/tests/runner-e2e/deployed-work-folders.spec.ts index 3a381595bf..b20e9a25e3 100644 --- a/tests/runner-e2e/deployed-work-folders.spec.ts +++ b/tests/runner-e2e/deployed-work-folders.spec.ts @@ -1,10 +1,10 @@ import { randomUUID } from "node:crypto"; import { test, expect } from "@playwright/test"; import type { EnvironmentCapabilities } from "../../packages/shared/src/environment-support.js"; -import type { WorkFolderListing, WorkFolderSyncStatus } from "../../packages/shared/src/work-folders.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 { pollUntil } from "./api.js"; -import { DeployedStackApi, loadDeployedStack } from "./deployed-stack.js"; +import { DeployedStackApi, deployedAgentEngine, loadDeployedStack } from "./deployed-stack.js"; const stack = loadDeployedStack(); const api = new DeployedStackApi(stack); @@ -32,13 +32,16 @@ test("deployed candidate and complete supported adapter inventory", async ({}, i if (adapter.capabilities.supportsAcp) required.push(`${adapter.type}:acp`); } } - const configured = new Set(stack.profiles.map((profile) => `${profile.adapterType}:${profile.engine}`)); - expect(required.filter((key) => !configured.has(key)), "Every exposed sandbox adapter/engine requires a qualified fixture").toEqual([]); + const configured = new Set(); for (const profile of stack.profiles) { const agent = await api.json<{ adapterType: string; adapterConfig: Record }>(`/api/agents/${profile.agentId}`); expect(agent.adapterType).toBe(profile.adapterType); expect(agent.adapterConfig.model).toBe(profile.model); + const engine = deployedAgentEngine(agent); + expect(engine, `${profile.id} must exercise its declared live engine`).toBe(profile.engine); + configured.add(`${agent.adapterType}:${engine}`); } + expect(required.filter((key) => !configured.has(key)), "Every exposed sandbox adapter/engine requires a qualified fixture").toEqual([]); await info.attach("deployed-candidate-and-inventory", { contentType: "application/json", body: Buffer.from(JSON.stringify({ stack, required }, null, 2)) }); }); @@ -83,6 +86,7 @@ for (const profile of stack.profiles) { "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"), }); @@ -99,10 +103,15 @@ for (const profile of stack.profiles) { const content = await api.request(`${base}/content?path=acceptance.txt`); expect(content.status).toBe(200); expect(await content.text()).toBe(nonce); const coldRunIds = new Set(cold.saves.map((save) => save.runId)); + const coldSave = cold.saves.find((save) => !save.active && save.state === "saved")!; + 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.`, @@ -117,7 +126,11 @@ for (const profile of stack.profiles) { }); const warmContent = await api.request(`${base}/content?path=warm.txt`); expect(warmContent.status).toBe(200); expect(await warmContent.text()).toBe(nonce); - await info.attach("cold-and-warm-checkpoints", { contentType: "application/json", body: Buffer.from(JSON.stringify({ cold: cold.saves, warm: warm.saves })) }); + const warmSave = warm.saves.find((save) => !save.active && save.state === "saved")!; + const warmRun = await api.json<{ contextSnapshot: { paperclipWorkFolders: SandboxWorkFolderManifest } }>(`/api/heartbeat-runs/${warmSave.runId}`); + const warmManifest = warmRun.contextSnapshot.paperclipWorkFolders; + expect(warmManifest.sandboxKey, "Warm acceptance requires the same physical sandbox").toBe(coldManifest.sandboxKey); + await info.attach("cold-and-warm-checkpoints", { contentType: "application/json", body: Buffer.from(JSON.stringify({ cold: cold.saves, warm: warm.saves, coldManifest, warmManifest })) }); }); }