From 0eec4cd2951d3c7ca7105134a6a823432452df89 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 7 Sep 2026 12:39:24 -0500 Subject: [PATCH] Collect superseded repository checkpoints and complete file API coverage Co-Authored-By: Paperclip --- doc/sandbox-work-folders.md | 5 +- packages/shared/src/work-folders.ts | 1 + server/src/__tests__/openapi-routes.test.ts | 6 +++ .../__tests__/sandbox-work-folders.test.ts | 50 +++++++++++++++++-- server/src/routes/openapi.ts | 38 ++++++++++++++ server/src/routes/work-folders.ts | 4 +- .../src/services/work-folder-repositories.ts | 31 ++++++++++-- ui/src/pages/IssueDetail.test.tsx | 1 + 8 files changed, 125 insertions(+), 11 deletions(-) diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index b79240193f..79d71abf8c 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -68,7 +68,10 @@ including a lease originally configured as ephemeral. Deletion moves files to recoverable trash. Restore rejects path collisions. Explicit purge and permanent owner deletion schedule object cleanup through a durable deletion journal. Overwritten scoped-file content is not versioned. -Repository checkpoint objects remain retained while their task binding exists. +The current complete repository checkpoint remains retained while its task +binding exists. Superseded manifests and unused blobs enter a 24-hour deletion +queue; shared blobs in the current checkpoint stay protected. A save that takes +more than one hour fails visibly and must retry before its old references expire. The scheduler retries object cleanup every three minutes; disabled heartbeat scheduling also disables this cleanup sweep. diff --git a/packages/shared/src/work-folders.ts b/packages/shared/src/work-folders.ts index 9c51b57cb8..023a09d217 100644 --- a/packages/shared/src/work-folders.ts +++ b/packages/shared/src/work-folders.ts @@ -2,6 +2,7 @@ export const WORK_FOLDER_SCOPES = ["task", "agent", "user", "project"] as const; export type WorkFolderScope = (typeof WORK_FOLDER_SCOPES)[number]; export const WORK_FOLDER_SYNC_INTERVAL_MS = 180_000; +export const WORK_FOLDER_ROUTE_PATH = "/companies/:companyId/work-folders/:scope/:ownerId"; export interface WorkFolderOwner { companyId: string; diff --git a/server/src/__tests__/openapi-routes.test.ts b/server/src/__tests__/openapi-routes.test.ts index 238816b7ed..c623323b21 100644 --- a/server/src/__tests__/openapi-routes.test.ts +++ b/server/src/__tests__/openapi-routes.test.ts @@ -5,6 +5,7 @@ import { fileURLToPath } from "node:url"; import request from "supertest"; import { describe, expect, it } from "vitest"; import { COMPANY_IMPORT_TRANSFERS_ROUTE_PATH } from "@paperclipai/shared/company-import-transfer"; +import { WORK_FOLDER_ROUTE_PATH } from "@paperclipai/shared"; import { errorHandler } from "../middleware/index.js"; import { buildOpenApiSpec, openApiRoutes } from "../routes/openapi.js"; @@ -63,6 +64,7 @@ const apiPrefixes: Record = { "tool-access.ts": "/api", "tool-gateway.ts": "/api", "user-profiles.ts": "/api", + "work-folders.ts": "/api", }; const ROUTE_LITERAL_PATTERN = /router\.(get|post|put|patch|delete)\(\s*["'`]([^"'`]+)["'`]/g; @@ -94,6 +96,7 @@ function createApp() { // literals; substitute the constants' values before normalizing. const routePathConstantSubstitutions: Record = { "${COMPANY_IMPORT_TRANSFERS_ROUTE_PATH}": COMPANY_IMPORT_TRANSFERS_ROUTE_PATH, + "${base}": WORK_FOLDER_ROUTE_PATH, }; function normalizeExpressPath(routePath: string) { @@ -153,6 +156,9 @@ function loadActualRoutes() { if (file === "companies.ts" && source.includes("router.post(COMPANY_IMPORT_ROUTE_PATH")) { routes.add("POST /api/companies/import"); } + if (file === "work-folders.ts" && source.includes("router.get(base,")) { + routes.add(`GET ${normalizeExpressPath(`/api${WORK_FOLDER_ROUTE_PATH}`)}`); + } if (file === "companies.ts" && source.includes("router.post(COMPANY_IMPORT_TRANSFERS_ROUTE_PATH")) { routes.add(`POST /api/companies${COMPANY_IMPORT_TRANSFERS_ROUTE_PATH}`); } diff --git a/server/src/__tests__/sandbox-work-folders.test.ts b/server/src/__tests__/sandbox-work-folders.test.ts index ba0b68fe78..5301e80281 100644 --- a/server/src/__tests__/sandbox-work-folders.test.ts +++ b/server/src/__tests__/sandbox-work-folders.test.ts @@ -6,11 +6,12 @@ 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, startEmbeddedPostgresTestDatabase, type Db } from "@paperclipai/db"; +import { agents, assets, companyMemberships, issueAttachments, companies, createDb, heartbeatRuns, issues, environments, environmentLeases, projects, projectWorkspaces, taskRepositoryBindings, workFolderObjects, startEmbeddedPostgresTestDatabase, type Db } from "@paperclipai/db"; import { createLocalDiskStorageProvider } from "../storage/local-disk-provider.js"; import { prepareSandboxWorkFolders } from "../services/sandbox-work-folders.js"; import { retainUnsavedWorkFolderLease, workFolderSandboxKey } from "../services/work-folder-retention.js"; import { workFolderService } from "../services/work-folders.js"; +import { collectWorkFolderGarbage } from "../services/work-folder-garbage.js"; import { localTestWorkFolderRunner } from "./helpers/work-folder-runner.js"; const exec = promisify(execFile); @@ -45,13 +46,16 @@ 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 }); }); - async function prepare(home: string, leaseId: string, physicalId = leaseId, responsibleUserId: string | null = null) { + async function prepare(home: string, leaseId: string, physicalId = leaseId, responsibleUserId: string | null = null, + options: { taskId?: string; branchName?: string } = {}) { await fs.mkdir(home, { recursive: true }); const runId = randomUUID(); await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, responsibleUserId, status: "running" }); const lease = { id: leaseId, companyId, environmentId, provider: "test", providerLeaseId: physicalId }; await db.insert(environmentLeases).values({ ...lease, heartbeatRunId: runId }).onConflictDoUpdate({ target: environmentLeases.id, set: { heartbeatRunId: runId } }); - const run = await prepareSandboxWorkFolders({ db, companyId, agentId, projectId, taskId, runId, + const [primary] = await db.select().from(projectWorkspaces).where(eq(projectWorkspaces.projectId, projectId)); + const run = await prepareSandboxWorkFolders({ db, companyId, agentId, projectId, taskId: options.taskId ?? taskId, runId, + primaryWorkspaceId: primary!.id, primaryBranchName: options.branchName, responsibleUserId, storage, sandboxKey: workFolderSandboxKey(lease), target: { kind: "remote", transport: "sandbox", leaseId, remoteCwd: home, runner: { execute: (input) => localTestWorkFolderRunner.execute({ ...input, env: { ...input.env, HOME: home } }) } } }); active.push(run); return run; @@ -198,4 +202,44 @@ describe("shared sandbox work-folder lifecycle", () => { expect(Buffer.concat(chunks)).toEqual(content); } }, 120_000); + + it("gives another task independent checkouts and preserves its task branch on warm starts", async () => { + const secondTaskId = randomUUID(); + await db.insert(issues).values({ id: secondTaskId, companyId, projectId, title: "Independent task", assigneeAgentId: agentId }); + const lease = randomUUID(); + const home = path.join(root, "independent-task"); + const run = await prepare(home, lease, lease, null, { taskId: secondTaskId, branchName: "acceptance/second-task" }); + expect(run.manifest.repositories).toHaveLength(2); + expect((await exec("git", ["-C", run.primaryRepo, "branch", "--show-current"])).stdout.trim()).toBe("acceptance/second-task"); + expect(await fs.readFile(path.join(run.primaryRepo, "tracked"), "utf8")).toBe("initial\n"); + await fs.writeFile(path.join(run.primaryRepo, "tracked"), "second task edit"); + await run.stop(); active.splice(active.indexOf(run), 1); + const warm = await prepare(home, randomUUID(), lease, null, { taskId: secondTaskId, branchName: "must-not-reset" }); + expect((await exec("git", ["-C", warm.primaryRepo, "branch", "--show-current"])).stdout.trim()).toBe("acceptance/second-task"); + expect(await fs.readFile(path.join(warm.primaryRepo, "tracked"), "utf8")).toBe("second task edit"); + await warm.stop(); active.splice(active.indexOf(warm), 1); + }, 120_000); + + it("collects superseded repository objects while retaining the complete current checkpoint", async () => { + const run = await prepare(path.join(root, "checkpoint-garbage"), randomUUID()); + const filename = path.join(run.primaryRepo, "garbage-fixture"); + await fs.writeFile(filename, "old unique checkpoint content"); + await run.flush(); + const bindingId = run.manifest.repositories.find((binding) => binding.primary)!.bindingId; + const [before] = await db.select().from(taskRepositoryBindings).where(eq(taskRepositoryBindings.id, bindingId)); + const oldBlob = `${companyId}/task-repositories/${bindingId}/blobs/${createHash("sha256").update("old unique checkpoint content").digest("hex")}`; + await fs.writeFile(filename, "current checkpoint content"); + await run.stop(); active.splice(active.indexOf(run), 1); + const [after] = await db.select().from(taskRepositoryBindings).where(eq(taskRepositoryBindings.id, bindingId)); + const [retired] = await db.select().from(workFolderObjects).where(eq(workFolderObjects.objectKey, before!.checkpointKey!)); + expect(retired!.deleteAfter).not.toBeNull(); + await collectWorkFolderGarbage(db, storage, new Date(Date.now() + 25 * 60 * 60 * 1000), 1000); + expect((await storage.headObject({ objectKey: before!.checkpointKey! })).exists).toBe(false); + expect((await storage.headObject({ objectKey: oldBlob })).exists).toBe(false); + expect((await storage.headObject({ objectKey: after!.checkpointKey! })).exists).toBe(true); + await fs.rm(run.home, { recursive: true }); + const restored = await prepare(path.join(root, "checkpoint-garbage-restored"), randomUUID()); + expect(await fs.readFile(path.join(restored.primaryRepo, "garbage-fixture"), "utf8")).toBe("current checkpoint content"); + await restored.stop(); active.splice(active.indexOf(restored), 1); + }, 120_000); }); diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 56da28aca7..37e352ca97 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -5260,6 +5260,44 @@ registry.registerPath({ // ─── Assets ────────────────────────────────────────────────────────────────── +const workFolderPath = "/api/companies/{companyId}/work-folders/{scope}/{ownerId}"; +const workFolderParams = z.object({ companyId: z.uuid(), scope: z.enum(["task", "agent", "user", "project"]), ownerId: z.string() }); +const workFileResponse = z.object({ id: z.uuid(), path: z.string(), kind: z.enum(["file", "directory"]), + byteSize: z.number(), sha256: z.string().nullable(), executable: z.boolean(), contentType: z.string(), + deletedAt: z.string().nullable(), updatedAt: z.string() }); +const workFolderErrors = { 400: r.badRequest, 401: r.unauthorized, 404: r.notFound, 409: r.conflict }; +registry.registerPath({ method: "get", path: workFolderPath, tags: ["work-folders"], summary: "List scoped sandbox files or recoverable trash", + description: "User files require the owning user or an authorized run acting for that user. Company access alone does not grant access.", + request: { params: workFolderParams, query: z.object({ trash: z.enum(["true", "false"]).optional(), cursor: z.uuid().optional(), limit: z.coerce.number().int().min(1).max(1000).optional() }) }, + responses: { ...workFolderErrors, 200: r.ok(z.object({ id: z.uuid(), owner: workFolderParams, files: z.array(workFileResponse), nextCursor: z.string().nullable() })) }, +}); +registry.registerPath({ method: "get", path: `${workFolderPath}/content`, tags: ["work-folders"], summary: "Download a scoped file", + request: { params: workFolderParams, query: z.object({ path: z.string() }) }, + responses: { ...workFolderErrors, 200: { description: "File bytes with private cache policy and attachment disposition", content: { "application/octet-stream": { schema: { type: "string", format: "binary" } } } } }, +}); +registry.registerPath({ method: "put", path: `${workFolderPath}/content`, tags: ["work-folders"], summary: "Upload or replace a scoped file", + description: "Send raw bytes as application/octet-stream, including an empty body for an empty file. Idempotency-Key identifies a retry. X-File-Executable: true preserves executable permission. X-File-Content-Type specifies the stored media type.", + request: { params: workFolderParams, query: z.object({ path: z.string() }), body: { required: true, content: { "application/octet-stream": { schema: { type: "string", format: "binary" } } } } }, + responses: { ...workFolderErrors, 200: r.ok() }, +}); +registry.registerPath({ method: "post", path: `${workFolderPath}/operations`, tags: ["work-folders"], summary: "Create a directory, delete, restore, or permanently purge files", + description: "Deletion retains a recoverable copy. Restore rejects occupied paths. Purge removes the deleted copy permanently. Idempotency-Key identifies retries.", + request: { params: workFolderParams, body: jsonBody(z.discriminatedUnion("action", [ + z.object({ action: z.literal("mkdir"), path: z.string() }), z.object({ action: z.literal("delete"), path: z.string() }), + z.object({ action: z.literal("restore"), fileId: z.uuid() }), z.object({ action: z.literal("purge"), fileId: z.uuid() }), + ])) }, responses: { ...workFolderErrors, 200: r.ok(z.object({ applied: z.boolean() })) }, +}); +registry.registerPath({ method: "get", path: `${workFolderPath}/sync`, tags: ["work-folders"], summary: "Read save status for runs bound to this folder", + request: { params: workFolderParams }, responses: { ...workFolderErrors, 200: r.ok(z.array(z.object({ runId: z.uuid(), + state: z.enum(["starting", "saved", "saving", "failed"]), lastSavedAt: z.string().nullable(), error: z.string().nullable(), + refreshRequested: z.boolean(), active: z.boolean() }))) }, +}); +registry.registerPath({ method: "post", path: `${workFolderPath}/refresh`, tags: ["work-folders"], summary: "Queue incoming refresh at the next safe run boundary", + description: "The active run first flushes outgoing edits. Incoming files refresh after the agent stops editing. Ended runs refresh at their next startup.", + request: { params: workFolderParams, body: jsonBody(z.object({ runId: z.uuid() })) }, + responses: { ...workFolderErrors, 202: { description: "Refresh queued", content: { "application/json": { schema: z.object({ queued: z.literal(true) }) } } } }, +}); + registry.registerPath({ method: "post", path: "/api/companies/{companyId}/assets/images", diff --git a/server/src/routes/work-folders.ts b/server/src/routes/work-folders.ts index d67ba52057..ac119ef99b 100644 --- a/server/src/routes/work-folders.ts +++ b/server/src/routes/work-folders.ts @@ -5,7 +5,7 @@ import { randomUUID } from "node:crypto"; import { z } from "zod"; import { type Db, workFolderRuns, heartbeatRuns } from "@paperclipai/db"; import { and, desc, eq, sql } from "drizzle-orm"; -import { WORK_FOLDER_SCOPES } from "@paperclipai/shared"; +import { WORK_FOLDER_SCOPES, WORK_FOLDER_ROUTE_PATH } from "@paperclipai/shared"; import { loadConfig } from "../config.js"; import { createStorageProviderFromConfig } from "../storage/provider-registry.js"; import type { StorageProvider } from "../storage/types.js"; @@ -24,7 +24,7 @@ export function workFolderRoutes(db: Db, provider?: StorageProvider) { const router = Router(); // Resolve lazily: route registration and tests need not initialize cloud credentials. const service = () => workFolderService(db, provider ?? createStorageProviderFromConfig(loadConfig())); - const base = "/companies/:companyId/work-folders/:scope/:ownerId"; + const base = WORK_FOLDER_ROUTE_PATH; router.use(base, async (req, _res, next) => { const owner = ownerSchema.parse(req.params); await assertWorkFolderAccess(db, req.actor, owner, !["GET", "HEAD"].includes(req.method)); diff --git a/server/src/services/work-folder-repositories.ts b/server/src/services/work-folder-repositories.ts index 70910a0bb2..6eab10ea95 100644 --- a/server/src/services/work-folder-repositories.ts +++ b/server/src/services/work-folder-repositories.ts @@ -1,6 +1,6 @@ import { createHash, randomUUID } from "node:crypto"; import { Readable, Transform } from "node:stream"; -import { and, eq, inArray } from "drizzle-orm"; +import { and, eq, inArray, isNull } from "drizzle-orm"; import { taskRepositoryBindings, workFolderObjects, type Db } from "@paperclipai/db"; import { validateWorkFilePath } from "@paperclipai/shared"; import { z } from "zod"; @@ -21,6 +21,14 @@ function signature(entries: WorkTreeEntry[]) { export function workFolderRepositoryService(db: Db, storage: StorageProvider, transport: WorkFolderTransport) { const knownByBinding = new Map>(); async function checkpoint(binding: Binding, root: string) { + // Another sandbox can have published since this coordinator loaded the + // binding. Cache only the current complete checkpoint's protected objects. + const [current] = await db.select().from(taskRepositoryBindings).where(and( + eq(taskRepositoryBindings.id, binding.id), eq(taskRepositoryBindings.companyId, binding.companyId))); + if (!current) throw new Error("Repository owner was deleted during checkpoint"); + if (current.checkpointKey !== binding.checkpointKey) knownByBinding.delete(binding.id); + Object.assign(binding, current); + const startedAt = Date.now(); const entries = await transport.scan(root, true); const digest = signature(entries); if (binding.checkpointSha256 === digest) return; @@ -49,12 +57,25 @@ export function workFolderRepositoryService(db: Db, storage: StorageProvider, tr const body = Buffer.from(JSON.stringify({ version: 1, bindingId: binding.id, files })); await registerWorkFolderObject(db, storage, { objectKey: checkpointKey, companyId: binding.companyId, repositoryBindingId: binding.id }); await storage.putObject({ objectKey: checkpointKey, body, contentType: "application/json", contentLength: body.length }); + // Retired objects have a 24-hour grace period. A checkpoint must finish + // within that window even when a competing run advances the pointer. + if (Date.now() - startedAt > 60 * 60 * 1000) throw new Error("Repository checkpoint exceeded the one-hour save limit; retry required"); await db.transaction(async (tx) => { - // Retain every object in this complete binding before making the pointer - // visible. Unpublished uploads keep their expiry for later collection. - const published = [checkpointKey, ...new Set(files.flatMap((file) => file.objectKey && !known.has(file.objectKey) ? [file.objectKey] : []))]; + const [owner] = await tx.select({ id: taskRepositoryBindings.id }).from(taskRepositoryBindings) + .where(and(eq(taskRepositoryBindings.id, binding.id), eq(taskRepositoryBindings.companyId, binding.companyId))).for("update"); + if (!owner) throw new Error("Repository owner was deleted during checkpoint"); + // Retire superseded manifests and blobs in the same transaction that + // protects ALL current objects and advances the complete-checkpoint pointer. + // The grace period also lets an already-started restore finish safely. + await tx.update(workFolderObjects).set({ deleteAfter: new Date(Date.now() + 24 * 60 * 60 * 1000) }) + .where(and(eq(workFolderObjects.repositoryBindingId, binding.id), eq(workFolderObjects.companyId, binding.companyId), isNull(workFolderObjects.deleteAfter))); + const published = [checkpointKey, ...new Set(files.flatMap((file) => file.objectKey ? [file.objectKey] : []))]; for (let offset = 0; offset < published.length; offset += 1000) { - await tx.update(workFolderObjects).set({ deleteAfter: null }).where(inArray(workFolderObjects.objectKey, published.slice(offset, offset + 1000))); + const batch = published.slice(offset, offset + 1000); + const protectedObjects = await tx.update(workFolderObjects).set({ deleteAfter: null }) + .where(and(eq(workFolderObjects.repositoryBindingId, binding.id), inArray(workFolderObjects.objectKey, batch))) + .returning({ key: workFolderObjects.objectKey }); + if (protectedObjects.length !== batch.length) throw new Error("Repository objects expired during checkpoint; retry required"); } const updated = await tx.update(taskRepositoryBindings).set({ checkpointKey, checkpointSha256: digest, checkpointAt: new Date() }) .where(and(eq(taskRepositoryBindings.id, binding.id), eq(taskRepositoryBindings.companyId, binding.companyId))).returning({ id: taskRepositoryBindings.id }); diff --git a/ui/src/pages/IssueDetail.test.tsx b/ui/src/pages/IssueDetail.test.tsx index f570fa1276..7f30fc9afe 100644 --- a/ui/src/pages/IssueDetail.test.tsx +++ b/ui/src/pages/IssueDetail.test.tsx @@ -590,6 +590,7 @@ vi.mock("@/components/ui/popover", () => ({ })); vi.mock("@/components/ui/dialog", () => ({ + DialogTrigger: ({ children }: { children?: ReactNode }) => <>{children}, Dialog: ({ children, open }: { children?: ReactNode; open?: boolean }) => open ?
{children}
: null, DialogContent: ({