diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index 2a4f5b9dee..a13875808a 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -224,6 +224,12 @@ success. Optional SDK streaming checksums are disabled to avoid an unhandled digest rejection when a source file changes during transfer. Work-folder SHA-256 verification and complete-checkpoint publication remain required. A changing source must fail its save without stopping the application or another run. +Scoped-file and repository-blob uploads retry transient network errors and +retryable HTTP responses up to three attempts, using the same object key. Each +attempt opens a fresh source and verifies its complete size and SHA-256; the +previous request and reader must settle before another attempt starts. Changed +content, ownership errors, and authentication failures are not retried. Exhausted +retries retain the previous complete checkpoint and the recoverable working copy. Deletion moves files to recoverable trash. Restore rejects path collisions. Explicit purge and permanent owner deletion schedule object cleanup through a diff --git a/server/src/__tests__/sandbox-work-folders.test.ts b/server/src/__tests__/sandbox-work-folders.test.ts index efbd9fec8e..62ec64a620 100644 --- a/server/src/__tests__/sandbox-work-folders.test.ts +++ b/server/src/__tests__/sandbox-work-folders.test.ts @@ -1,6 +1,7 @@ import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { execFile } from "node:child_process"; import { promisify } from "node:util"; +import { Readable } from "node:stream"; import { createHash, randomUUID } from "node:crypto"; import { eq, inArray, sql } from "drizzle-orm"; import fs from "node:fs/promises"; @@ -347,6 +348,54 @@ describe("shared sandbox work-folder lifecycle", () => { expect((await svc.list(folder)).files.map((file) => file.path)).toContain("private"); }, 120_000); + it("reopens repository blobs after transient PUT failures and retains the previous checkpoint when retries exhaust", async () => { + const leaseId = randomUUID(); + const run = await prepare(path.join(root, "replayed-repository-upload"), leaseId); + await run.flush(); + const filename = path.join(run.primaryRepo, "retry-upload"); + const content = "replay the entire repository file"; + await fs.writeFile(filename, content); + const targetHash = createHash("sha256").update(content).digest("hex"); + const put = storage.putObject.bind(storage); + const streams: Readable[] = []; + let attempts = 0; + const recover = vi.spyOn(storage, "putObject").mockImplementation(async (input) => { + if (input.objectKey.endsWith("/" + targetHash)) { + streams.push(input.body as Readable); + if (++attempts === 1) { + await (input.body as Readable).iterator({ destroyOnReturn: false }).next(); + throw Object.assign(new Error("socket hang up"), { code: "ECONNRESET" }); + } + } + return put(input); + }); + try { await run.flush(); } finally { recover.mockRestore(); } + expect(attempts).toBe(2); + expect(new Set(streams).size).toBe(2); + expect(streams.every((stream) => stream.destroyed)).toBe(true); + const bindingId = run.manifest.repositories[0]!.bindingId; + const [before] = await db.select().from(taskRepositoryBindings).where(eq(taskRepositoryBindings.id, bindingId)); + await fs.writeFile(filename, "retain this unsaved edit"); + const nextHash = createHash("sha256").update("retain this unsaved edit").digest("hex"); + let failedAttempts = 0; + const fail = vi.spyOn(storage, "putObject").mockImplementation(async (input) => { + if (input.objectKey.endsWith("/" + nextHash)) { + failedAttempts++; + for await (const _chunk of input.body as Readable) { /* Lost response after consuming the body. */ } + throw Object.assign(new Error("socket hang up"), { code: "ECONNRESET" }); + } + return put(input); + }); + try { + await expect(run.stop()).rejects.toThrow("socket hang up"); + expect(failedAttempts).toBe(3); + const [after] = await db.select().from(taskRepositoryBindings).where(eq(taskRepositoryBindings.id, bindingId)); + expect(after!.checkpointKey).toBe(before!.checkpointKey); + expect(await fs.readFile(filename, "utf8")).toBe("retain this unsaved edit"); + expect(await retainUnsavedWorkFolderLease(db, { id: leaseId, companyId })).toBe(true); + } finally { fail.mockRestore(); active.splice(active.indexOf(run), 1); } + }, 120_000); + it("does not publish a partial repository checkpoint and recovers a failed final save in a new run", async () => { const leaseId = randomUUID(); const run = await prepare(path.join(root, "interrupted-checkpoint"), leaseId); diff --git a/server/src/__tests__/work-folder-upload.test.ts b/server/src/__tests__/work-folder-upload.test.ts new file mode 100644 index 0000000000..de1e60db9e --- /dev/null +++ b/server/src/__tests__/work-folder-upload.test.ts @@ -0,0 +1,109 @@ +import { createHash } from "node:crypto"; +import { Readable } from "node:stream"; +import { describe, expect, it, vi } from "vitest"; +import { uploadWorkFolderObject } from "../services/work-folder-upload.js"; + +const data = Buffer.from("verified file contents\n"); +const reset = () => Object.assign(new Error("socket hang up"), { name: "TimeoutError", code: "ECONNRESET" }); +const digest = (bytes: Buffer) => createHash("sha256").update(bytes).digest("hex"); +const input = { objectKey: "company/repository/blob", contentType: "application/octet-stream", contentLength: data.length, sha256: digest(data) }; +async function bytes(body: Buffer | Readable) { + const chunks: Buffer[] = []; + for await (const chunk of body) chunks.push(Buffer.from(chunk)); + return Buffer.concat(chunks); +} + +describe("replayable verified work-folder uploads", () => { + it.each(["partial", "complete"])("reopens and verifies the whole source after a reset following %s consumption", async (consumption) => { + const sources: Readable[] = [], bodies: Readable[] = []; + let attempt = 0; + const putObject = vi.fn(async ({ body, objectKey }) => { + expect(objectKey).toBe(input.objectKey); + bodies.push(body); + if (++attempt === 1) { + if (consumption === "complete") expect(await bytes(body)).toEqual(data); + else await body.iterator({ destroyOnReturn: false }).next(); + throw reset(); + } + expect(await bytes(body)).toEqual(data); + }); + await uploadWorkFolderObject({ putObject }, { ...input, createSource: () => { + const stream = Readable.from([data.subarray(0, 3), data.subarray(3)]); sources.push(stream); return stream; + } }); + expect(putObject).toHaveBeenCalledTimes(2); + expect(new Set(bodies).size).toBe(2); + expect(sources.every((source) => source.destroyed)).toBe(true); + expect(bodies.every((body) => body.destroyed)).toBe(true); + }); + + it("waits for the previous PUT to settle before reopening a failed source", async () => { + let release!: () => void; + let failed!: () => void; + const held = new Promise((resolve) => { release = resolve; }); + const sourceFailed = new Promise((resolve) => { failed = resolve; }); + let attempts = 0; + const createSource = vi.fn(() => ++attempts === 1 + ? Readable.from((async function* () { yield data.subarray(0, 3); failed(); throw reset(); })()) + : Readable.from([data])); + const putObject = vi.fn(async ({ body }) => { + if (attempts === 1) { await held; throw reset(); } + expect(await bytes(body)).toEqual(data); + }); + const upload = uploadWorkFolderObject({ putObject }, { ...input, createSource }); + await sourceFailed; + // Longer than the retry delay: an unsettled old request must still own the key. + await new Promise((resolve) => setTimeout(resolve, 300)); + expect(createSource).toHaveBeenCalledTimes(1); + release(); + await upload; + expect(createSource).toHaveBeenCalledTimes(2); + }); + + it("stops after three failed attempts and destroys every source", async () => { + const sources: Readable[] = []; + const putObject = vi.fn(async ({ body }) => { await bytes(body); throw reset(); }); + await expect(uploadWorkFolderObject({ putObject }, { ...input, createSource: () => { + const source = Readable.from([data]); sources.push(source); return source; + } })).rejects.toMatchObject({ code: "ECONNRESET" }); + expect(putObject).toHaveBeenCalledTimes(3); + expect(sources.every((source) => source.destroyed)).toBe(true); + }); + + it("rejects a changed source after a reset without publishing or another retry", async () => { + let attempt = 0; + const putObject = vi.fn(async ({ body }) => { await bytes(body); throw reset(); }); + await expect(uploadWorkFolderObject({ putObject }, { ...input, createSource: () => Readable.from([ + ++attempt === 1 ? data : Buffer.alloc(data.length, 120), + ]) })).rejects.toThrow("Work file changed during upload"); + expect(putObject).toHaveBeenCalledTimes(2); + }); + + it.each([Buffer.from("short"), Buffer.alloc(data.length + 1), Buffer.alloc(data.length, 120)])("rejects size or hash changes without retrying", async (body) => { + const putObject = vi.fn(async ({ body: stream }) => { await bytes(stream); }); + await expect(uploadWorkFolderObject({ putObject }, { ...input, createSource: () => Readable.from([body]) })) + .rejects.toThrow("Work file changed during upload"); + expect(putObject).toHaveBeenCalledTimes(1); + }); + + it.each([ + Object.assign(new Error("denied"), { $metadata: { httpStatusCode: 403 } }), + Object.assign(new Error("missing"), { code: "ENOENT" }), + new Error("owner was deleted"), + ])("does not replay a permanent or unclassified error", async (error) => { + const source = Readable.from([data]); + const putObject = vi.fn(async () => { throw error; }); + await expect(uploadWorkFolderObject({ putObject }, { ...input, createSource: () => source })).rejects.toBe(error); + expect(putObject).toHaveBeenCalledTimes(1); + expect(source.destroyed).toBe(true); + }); + + it("retries a classified temporary HTTP failure and supports empty files", async () => { + let attempt = 0; + const putObject = vi.fn(async ({ body }) => { + expect(await bytes(body)).toEqual(Buffer.alloc(0)); + if (++attempt === 1) throw Object.assign(new Error("unavailable"), { $metadata: { httpStatusCode: 503 } }); + }); + await uploadWorkFolderObject({ putObject }, { ...input, contentLength: 0, sha256: digest(Buffer.alloc(0)), createSource: () => Readable.from([]) }); + expect(putObject).toHaveBeenCalledTimes(2); + }); +}); diff --git a/server/src/__tests__/work-folders.test.ts b/server/src/__tests__/work-folders.test.ts index d20ef577b7..8f94ce2c69 100644 --- a/server/src/__tests__/work-folders.test.ts +++ b/server/src/__tests__/work-folders.test.ts @@ -1,4 +1,4 @@ -import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { randomUUID } from "node:crypto"; import { mkdtemp, rm } from "node:fs/promises"; import path from "node:path"; @@ -97,6 +97,36 @@ describe("durable work folders", () => { await expect(svc.write(f, { path: "large", body: Buffer.from("large"), maxBytes: 2, operationId: "large" })).rejects.toMatchObject({ status: 413 }); expect((await svc.list(f)).files).toHaveLength(0); }); + it("replays a failed spool upload without publishing a receipt or replacing the previous file", async () => { + const f = await folder(); + await svc.write(f, { path: "note", body: Buffer.from("old"), operationId: "old" }); + const old = await svc.get(f, "note"); + const put = storage.putObject.bind(storage); + const bodies: Readable[] = []; + const failed = vi.spyOn(storage, "putObject").mockImplementation(async (input) => { + bodies.push(input.body as Readable); + for await (const _chunk of input.body as Readable) { /* Consume the uncertain request. */ } + throw Object.assign(new Error("socket hang up"), { code: "ECONNRESET" }); + }); + try { + await expect(svc.write(f, { path: "note", body: Buffer.from("new"), operationId: "new" })).rejects.toThrow("socket hang up"); + expect(failed).toHaveBeenCalledTimes(3); + expect(new Set(bodies).size).toBe(3); + expect(bodies.every((body) => body.destroyed)).toBe(true); + expect((await svc.get(f, "note")).objectKey).toBe(old.objectKey); + expect(await textContent(f, "note")).toBe("old"); + } finally { failed.mockRestore(); } + let attempts = 0; + const recovered = vi.spyOn(storage, "putObject").mockImplementation(async (input) => { + await put(input); + if (++attempts === 1) throw Object.assign(new Error("lost response"), { code: "ECONNRESET" }); + }); + try { + expect(await svc.write(f, { path: "note", body: Buffer.from("new"), operationId: "new" })).toEqual({ applied: true }); + expect(recovered).toHaveBeenCalledTimes(2); + expect(await textContent(f, "note")).toBe("new"); + } finally { recovered.mockRestore(); } + }); it("serializes conflicting parent/file creation", async () => { const f = await folder(); const results = await Promise.allSettled([ diff --git a/server/src/services/sandbox-work-folders.ts b/server/src/services/sandbox-work-folders.ts index c7b2da986f..dbab5cea0f 100644 --- a/server/src/services/sandbox-work-folders.ts +++ b/server/src/services/sandbox-work-folders.ts @@ -7,6 +7,7 @@ import { WORK_FOLDER_SCOPES, type SandboxWorkFolderManifest, type WorkFolderScop import type { AdapterSandboxExecutionTarget } from "@paperclipai/adapter-utils/execution-target"; import type { StorageProvider } from "../storage/types.js"; import { loadConfig } from "../config.js"; +import { logger } from "../middleware/logger.js"; import { createStorageProviderFromConfig } from "../storage/provider-registry.js"; import { resolveDefaultAgentWorkspaceDir } from "../home-paths.js"; import { createGitRemoteAuthProvider } from "./git-credentials.js"; @@ -319,7 +320,15 @@ export async function prepareSandboxWorkFolders(input: { for (const { binding, root } of bindings) await repositories.checkpoint(binding, root); await saveState("saved"); }, - async onError() { await saveState("failed", "Files could not be saved; the sandbox must be retained for recovery"); }, + async onError(error) { + const failure = error as { name?: unknown; code?: unknown; $metadata?: { httpStatusCode?: unknown } } | null; + // Do not log SDK request objects, headers, file contents or credentials. + const label = (value: unknown) => typeof value === "string" && /^[A-Za-z0-9_]{1,80}$/.test(value) ? value : null; + logger.warn({ runId: input.runId, errorName: label(failure?.name), errorCode: label(failure?.code), + httpStatus: typeof failure?.$metadata?.httpStatusCode === "number" ? failure.$metadata.httpStatusCode : null }, + "Work folder checkpoint failed; retaining sandbox for recovery"); + await saveState("failed", "Files could not be saved; the sandbox must be retained for recovery"); + }, }); let completion: Promise | null = null; function stop(beforeCompletion?: () => Promise) { diff --git a/server/src/services/work-folder-repositories.ts b/server/src/services/work-folder-repositories.ts index 6eab10ea95..1822f6e7d8 100644 --- a/server/src/services/work-folder-repositories.ts +++ b/server/src/services/work-folder-repositories.ts @@ -1,10 +1,10 @@ import { createHash, randomUUID } from "node:crypto"; -import { Readable, Transform } from "node:stream"; 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"; import { registerWorkFolderObject } from "./work-folder-garbage.js"; +import { uploadWorkFolderObject } from "./work-folder-upload.js"; import type { StorageProvider } from "../storage/types.js"; import type { WorkFolderTransport, WorkTreeEntry } from "./work-folder-transport.js"; @@ -40,14 +40,9 @@ export function workFolderRepositoryService(db: Db, storage: StorageProvider, tr const objectKey = entry.kind === "file" && !entry.linkTarget ? `${prefix}blobs/${entry.sha256}` : null; if (objectKey && !known.has(objectKey)) await registerWorkFolderObject(db, storage, { objectKey, companyId: binding.companyId, repositoryBindingId: binding.id }); if (objectKey && !known.has(objectKey) && !(await storage.headObject({ objectKey })).exists) { - const hash = createHash("sha256"); - const verify = new Transform({ transform(chunk: Buffer, _encoding, callback) { hash.update(chunk); callback(null, chunk); }, - flush(callback) { callback(hash.digest("hex") === entry.sha256 ? undefined : new Error("Repository changed during checkpoint")); } }); - const source = transport.read(root, entry.path, entry.byteSize); - source.on("error", (error) => verify.destroy(error)); - try { - await storage.putObject({ objectKey, body: source.pipe(verify), contentType: "application/octet-stream", contentLength: entry.byteSize }); - } finally { source.destroy(); verify.destroy(); } + await uploadWorkFolderObject(storage, { objectKey, contentType: "application/octet-stream", + contentLength: entry.byteSize, sha256: entry.sha256!, + createSource: () => transport.read(root, entry.path, entry.byteSize) }); } files.push({ ...entry, objectKey }); } diff --git a/server/src/services/work-folder-upload.ts b/server/src/services/work-folder-upload.ts new file mode 100644 index 0000000000..11e4cffd6a --- /dev/null +++ b/server/src/services/work-folder-upload.ts @@ -0,0 +1,62 @@ +import { createHash } from "node:crypto"; +import { Readable, Transform } from "node:stream"; +import { pipeline } from "node:stream/promises"; +import { setTimeout as delay } from "node:timers/promises"; +import type { StorageProvider } from "../storage/types.js"; + +function transientUploadFailure(error: unknown) { + if (!(error instanceof Error)) return false; + const failure = error as Error & { code?: string; $metadata?: { httpStatusCode?: number } }; + // Authentication, validation and ownership failures must never be retried. + const status = failure.$metadata?.httpStatusCode; + if (status !== undefined) return [408, 429, 500, 502, 503, 504].includes(status); + return ["ECONNRESET", "EPIPE", "ETIMEDOUT", "ECONNABORTED", "EAI_AGAIN"].includes(failure.code ?? ""); +} + +/** Retry an uncertain object PUT with a fresh, fully verified stream and the same key. */ +export async function uploadWorkFolderObject(storage: Pick, input: { + objectKey: string; + contentType: string; + contentLength: number; + sha256: string; + createSource: () => Readable; +}) { + for (let attempt = 0; attempt < 3; attempt++) { + const source = input.createSource(); + const hash = createHash("sha256"); + let bytes = 0; + let validationError: Error | undefined; + const changed = () => validationError ??= new Error("Work file changed during upload; retry the checkpoint"); + const verify = new Transform({ + transform(chunk: Buffer, _encoding, callback) { + bytes += chunk.length; + if (bytes > input.contentLength) return callback(changed()); + hash.update(chunk); + callback(null, chunk); + }, + flush(callback) { + callback(bytes === input.contentLength && hash.digest("hex") === input.sha256 ? undefined : changed()); + }, + }); + const transferred = pipeline(source, verify); + const uploaded = Promise.resolve().then(() => storage.putObject({ + objectKey: input.objectKey, contentType: input.contentType, + contentLength: input.contentLength, body: verify, + })); + let failure: unknown; + try { + // Observe both promises immediately. An early HTTP success cannot publish + // unvalidated content, and a failed request must release its source reader. + await Promise.all([transferred, uploaded]); + return; + } catch (error) { + failure = validationError ?? error; + } finally { + source.destroy(); + verify.destroy(); + await Promise.allSettled([transferred, uploaded]); + } + if (validationError || attempt === 2 || !transientUploadFailure(failure)) throw failure; + await delay(250 * (attempt + 1)); + } +} diff --git a/server/src/services/work-folders.ts b/server/src/services/work-folders.ts index 5c955c4fa3..c8f015cfe4 100644 --- a/server/src/services/work-folders.ts +++ b/server/src/services/work-folders.ts @@ -9,6 +9,7 @@ import { and, asc, eq, gt, isNull, isNotNull, max, sql } from "drizzle-orm"; import { workFolders, workFiles, workFileOperations, workFolderObjects, type Db } from "@paperclipai/db"; import { validateWorkFilePath, type WorkFile, type WorkFolderOwner } from "@paperclipai/shared"; import { registerWorkFolderObject } from "./work-folder-garbage.js"; +import { uploadWorkFolderObject } from "./work-folder-upload.js"; import type { StorageProvider } from "../storage/types.js"; import { badRequest, conflict, notFound, payloadTooLarge } from "../errors.js"; @@ -92,6 +93,7 @@ export function workFolderService(db: Db, storage: StorageProvider) { const spool = path.join(directory, "content"); const hash = createHash("sha256"); let byteSize = 0; + let sha256: string | null = null; let objectKey: string | null = null; let discardUpload = false; try { @@ -103,12 +105,13 @@ export function workFolderService(db: Db, storage: StorageProvider) { hash.update(chunk); callback(null, chunk); } }), createWriteStream(spool, { mode: 0o600 })); + sha256 = hash.digest("hex"); + if (input.expectedSha256 && input.expectedSha256 !== sha256) throw conflict("File changed during transfer; retry the checkpoint"); objectKey = `${folder.companyId}/work-folders/${folder.id}/${randomUUID()}`; await registerWorkFolderObject(db, storage, { objectKey, companyId: folder.companyId, folderId: folder.id }); - await storage.putObject({ objectKey, body: createReadStream(spool), contentLength: byteSize, - contentType: input.contentType ?? "application/octet-stream" }); + await uploadWorkFolderObject(storage, { objectKey, createSource: () => createReadStream(spool), + contentLength: byteSize, sha256, contentType: input.contentType ?? "application/octet-stream" }); } - const sha256 = kind === "file" ? hash.digest("hex") : null; if (input.expectedSha256 && input.expectedSha256 !== sha256) throw conflict("File changed during transfer; retry the checkpoint"); const value = { kind, objectKey, byteSize, sha256, executable: input.executable ?? false, contentType: input.contentType ?? "application/octet-stream", updatedAt: new Date() };