From d866ff374e816730b60960d021fa98b20e80e5d3 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Tue, 25 Aug 2026 14:24:13 -0700 Subject: [PATCH] fix(adapter-utils): report real transferred bytes for project sync, git-history export, and workspace restore (#12180) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent adapters move files between the host and sandbox during a run > - The sync transport reports transferred bytes, but some progress lines discard this value > - Discarded byte totals make large transfers display as `0.0 MB` > - This pull request passes the transport total to the affected progress lines > - The benefit is accurate transfer progress without changing file movement or confinement checks ## Linked Issues or Issue Description **What happened?** Three file-sync progress lines displayed `0.0 MB` when the transport moved data. The affected paths cover referenced-project staging, native git-history export, and native workspace restore. **Expected behavior** Each progress line should display the bytes that the sync transport transfers. A provider that reports zero bytes should preserve the known host-side value for inbound workspace sync. **Steps to reproduce** 1. Run a sandbox task that stages a referenced project. 2. Run a task that uses native git-history export or native workspace restore. 3. Inspect the file-sync progress lines during each transfer. **Paperclip version or commit** Commit `8062612baa20036a1defce8bbd683c038ba187d5`. **Deployment mode** Built from source with the adapter-utils Vitest suite. ## What Changed - Add a helper that sums valid `bytesTransferred` values from a `SandboxSyncResult`. - Use the transport total for referenced-project staging, native git-history export, and native workspace restore. - Preserve the caller count when referenced-project staging reports zero bytes. - Add tests for non-zero progress and the zero-byte fallback. ## Verification - Run `npx vitest run packages/adapter-utils/src/sandbox-managed-runtime.test.ts` from the repository root. - Run the TypeScript check for `packages/adapter-utils`. - Confirm that the new tests cover referenced-project staging, native workspace restore, native git-history export, and the zero-byte fallback. ## Risks This change affects progress reporting only. It does not change transferred files, transfer order, provider behavior, or confinement checks. ## Model Used OpenAI Codex, GPT-5, with tool use and code execution. The model reviewed and routed the author-provided change. The implementing engineer authored the code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I have addressed all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- .../src/sandbox-managed-runtime.test.ts | 194 +++++++++++++++++- .../src/sandbox-managed-runtime.ts | 41 +++- 2 files changed, 216 insertions(+), 19 deletions(-) diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.test.ts b/packages/adapter-utils/src/sandbox-managed-runtime.test.ts index 47d744e42f..9eb425002d 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.test.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.test.ts @@ -1,5 +1,6 @@ +import { randomBytes } from "node:crypto"; import { promises as fsPromises } from "node:fs"; -import { lstat, mkdir, mkdtemp, readFile, readdir, rm, symlink, writeFile } from "node:fs/promises"; +import { lstat, mkdir, mkdtemp, readFile, readdir, rm, stat, symlink, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { execFile as execFileCallback, spawn } from "node:child_process"; @@ -40,6 +41,23 @@ function toArrayBuffer(bytes: Buffer): ArrayBuffer { return bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength) as ArrayBuffer; } +// Sum the file sizes under a directory, recursively. Test-only: it stands in +// for a real provider's own byte count on a `kind: "directory"` mapping, so a +// fake `syncIn`/`syncOut` can report a real, non-zero `bytesTransferred`. +async function directoryByteSize(dir: string): Promise { + let total = 0; + const entries = await readdir(dir, { withFileTypes: true }); + for (const entry of entries) { + const entryPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + total += await directoryByteSize(entryPath); + } else if (entry.isFile()) { + total += (await stat(entryPath)).size; + } + } + return total; +} + // Give a bare fake client a `syncIn` that reproduces the non-native base64-tar // FALLBACK (place each file mapping via `writeFile`, then run the operation's // ordered `postUploadCommands` fail-fast via `run`) — byte-for-byte the prior @@ -111,7 +129,9 @@ function attachNativeRecordingSyncIn( // A capturing `syncIn` that records every operation for assertion and // materializes file AND directory mappings. A directory mapping (a referenced // project) uses `mirrorDirectory`, so a test can assert the advisory `access` -// intent on directory mappings as well as file mappings. +// intent on directory mappings as well as file mappings. It reports a real +// `bytesTransferred` for a directory mapping too, from a post-mirror byte +// count, so a test can assert on the emitted progress line. function attachCapturingSyncIn( client: SandboxManagedRuntimeClient, captured: SandboxSyncOperation[], @@ -126,6 +146,7 @@ function attachCapturingSyncIn( await mkdir(path.posix.dirname(mapping.targetPath), { recursive: true }); if (mapping.kind === "directory") { await mirrorDirectory(mapping.sourcePath, mapping.targetPath); + bytesTransferred += await directoryByteSize(mapping.targetPath); } else { const bytes = await readFile(mapping.sourcePath); await writeFile(mapping.targetPath, bytes); @@ -502,6 +523,54 @@ describe("sandbox managed runtime", () => { expect(runtimeStatuses.at(-1)).toBe("finalize:Finalizing workspace"); }); + it("falls back to the host-known byte count when the provider reports 0 for inbound workspace sync", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-sandbox-zero-report-")); + cleanupDirs.push(rootDir); + const localWorkspaceDir = path.join(rootDir, "local-workspace"); + const remoteWorkspaceDir = path.join(rootDir, "remote-workspace"); + await mkdir(localWorkspaceDir, { recursive: true }); + // Sizeable content, well above the 0.1 MB rounding step, so a fallback bug + // that stays at 0 is distinguishable from a small transfer that would + // still round down to "0.0 MB". + await writeFile(path.join(localWorkspaceDir, "large.bin"), Buffer.alloc(300 * 1024, "a")); + + const client = makeFilesystemClient(); + const realSyncIn = client.syncIn!; + // Simulate a provider that under-reports: it moves the real bytes but + // returns 0 in `bytesTransferred`, exactly like a buggy or minimal plugin. + client.syncIn = async (operations) => { + const result = await realSyncIn(operations); + return { + operations: result.operations.map((operation) => ({ ...operation, bytesTransferred: 0 })), + }; + }; + + const lines: string[] = []; + await prepareSandboxManagedRuntime({ + spec: { + transport: "sandbox", + provider: "test", + sandboxId: "sandbox-1", + remoteCwd: remoteWorkspaceDir, + timeoutMs: 30_000, + apiKey: null, + }, + adapterKey: "test-adapter", + client, + workspaceLocalDir: localWorkspaceDir, + onProgress: (line) => { lines.push(line); }, + }); + + await expect(readFile(path.join(remoteWorkspaceDir, "large.bin"))).resolves.toHaveLength(300 * 1024); + + const workspaceLines = lines.filter((line) => line.includes("Syncing workspace to environment")); + expect(workspaceLines.length).toBeGreaterThan(0); + // Even though the provider reported 0, the line still shows the real, + // host-known workspace size, from the caller-supplied fallback. + expect(workspaceLines.some((line) => /\(\d+\.\d\/\d+\.\d MB\)/.test(line))).toBe(true); + expect(workspaceLines.some((line) => line.includes("(0.0/0.0 MB)"))).toBe(false); + }); + it.each(["workspace", "git-workspace"])( "rejects an asset key that collides with the reserved %s archive name", async (reservedKey) => { @@ -2166,6 +2235,70 @@ describe("sandbox managed runtime", () => { } }); + it("reports the real transferred bytes for a referenced project's inbound staging", async () => { + const flagKey = "PAPERCLIP_MULTI_PROJECT_WORKSPACE_SYNC"; + const priorFlag = process.env[flagKey]; + process.env[flagKey] = "1"; + try { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-project-bytes-")); + cleanupDirs.push(rootDir); + const localWorkspaceDir = path.join(rootDir, "local-workspace"); + const remoteWorkspaceDir = path.join(rootDir, "remote-workspace"); + const referencedDir = path.join(rootDir, "referenced-project"); + await mkdir(localWorkspaceDir, { recursive: true }); + await mkdir(referencedDir, { recursive: true }); + await writeFile(path.join(localWorkspaceDir, "README.md"), "anchor\n", "utf8"); + // A referenced project has no host tarball to `fs.stat`, so its progress + // line depends entirely on the transport's own `bytesTransferred`. Give it + // real, sizeable content (well above the 0.1 MB rounding step), so a bug + // that keeps the line at 0 stays distinguishable from a correctly-reported + // small transfer that would still round down to "0.0 MB". + await writeFile(path.join(referencedDir, "notes.md"), Buffer.alloc(300 * 1024, "a")); + + const client: SandboxManagedRuntimeClient = { + makeDir: async (remotePath) => { await mkdir(remotePath, { recursive: true }); }, + writeFile: async (remotePath, bytes) => { + await mkdir(path.dirname(remotePath), { recursive: true }); + await writeFile(remotePath, Buffer.from(bytes)); + }, + readFile: async (remotePath) => await readFile(remotePath), + listFiles: async () => [], + remove: async (remotePath) => { await rm(remotePath, { recursive: true, force: true }); }, + run: async (command) => { await execFile("sh", ["-c", command], { maxBuffer: 32 * 1024 * 1024 }); }, + }; + attachCapturingSyncIn(client, []); + + const lines: string[] = []; + await prepareSandboxManagedRuntime({ + spec: { + transport: "sandbox", + provider: "test", + sandboxId: "sandbox-1", + remoteCwd: remoteWorkspaceDir, + timeoutMs: 30_000, + apiKey: null, + }, + adapterKey: "test-adapter", + client, + workspaceLocalDir: localWorkspaceDir, + additionalSources: [{ localPath: referencedDir, projectId: "proj-first" }], + onProgress: (line) => { lines.push(line); }, + }); + + const projectLines = lines.filter((line) => line.includes("Syncing project-proj-first to environment")); + expect(projectLines.length).toBeGreaterThan(0); + // The transfer landed real bytes, so the terminal line carries the actual + // byte total and a 100% completion, not the "0.0 MB" that a discarded + // sync result would show. + expect(projectLines.some((line) => line.includes("100%"))).toBe(true); + expect(projectLines.every((line) => /\(\d+\.\d\/\d+\.\d MB\)/.test(line))).toBe(true); + expect(projectLines.some((line) => line.includes("(0.0/0.0 MB)"))).toBe(false); + } finally { + if (priorFlag === undefined) delete process.env[flagKey]; + else process.env[flagKey] = priorFlag; + } + }); + it("keeps the sandbox runtime core free of Codex-specific string literals", async () => { const coreSource = await readFile(new URL("./sandbox-managed-runtime.ts", import.meta.url), "utf8"); // The seam must be generic: no adapter (Codex) knowledge may live in the core. @@ -3482,24 +3615,31 @@ describe("sandbox git-bundle export transport", () => { attachFallbackSyncIn(client); if (native) { client.syncOut = async (operations) => { + const resultOperations: SandboxSyncResult["operations"] = []; for (const operation of operations) { capture.syncOutOperations.push(operation); + // Report a real `bytesTransferred`, from a post-copy byte count, so a + // test can assert on the emitted progress line — the same way a real + // provider reports the bytes it actually moved. + let bytesTransferred = 0; for (const mapping of operation.files) { if (mapping.kind === "directory") { await copyDirectoryWithExclude(mapping.sourcePath, mapping.targetPath, mapping.exclude); + bytesTransferred += await directoryByteSize(mapping.targetPath); } else { await mkdir(path.dirname(mapping.targetPath), { recursive: true }); - await writeFile(mapping.targetPath, await readFile(mapping.sourcePath)); + const bytes = await readFile(mapping.sourcePath); + await writeFile(mapping.targetPath, bytes); + bytesTransferred += bytes.byteLength; } } - } - return { - operations: operations.map((operation) => ({ + resultOperations.push({ operationId: operation.operationId, filesTransferred: operation.files.length, - bytesTransferred: 0, - })), - }; + bytesTransferred, + }); + } + return { operations: resultOperations }; }; } return client; @@ -3664,4 +3804,40 @@ describe("sandbox git-bundle export transport", () => { // sandbox head commit. await expect(git(localWorkspaceDir, ["cat-file", "-e", `${sandboxHead}^{commit}`])).resolves.toBe(""); }); + + it("reports the real transferred bytes for the native git-history export and workspace restore", async () => { + const capture: TransportCapture = { syncOutOperations: [], readFilePaths: [] }; + const { localWorkspaceDir, remoteWorkspaceDir } = await setupGitBackedWorkspace("paperclip-restore-bytes-"); + const client = makeTransportClient(true, capture); + const prepared = await prepareSandboxManagedRuntime({ + spec: gitSpec(remoteWorkspaceDir), + adapterKey: "test-adapter", + client, + workspaceLocalDir: localWorkspaceDir, + }); + + // Add a sizeable, incompressible tracked file in the sandbox, so both the + // packed git-history bundle and the restored working tree carry real, + // non-trivial bytes well above the 0.1 MB rounding step. Random bytes, + // not a repeated byte, so git's own pack compression cannot shrink the + // bundle back down to a trivial size. + await writeFile(path.join(remoteWorkspaceDir, "large.bin"), randomBytes(300 * 1024)); + await commitInSandbox(remoteWorkspaceDir); + + const lines: string[] = []; + await prepared.restoreWorkspace((line) => { lines.push(line); }); + + const exportLines = lines.filter((line) => line.includes("Exporting git history from environment")); + const restoreLines = lines.filter((line) => line.includes("Restoring workspace from environment")); + + // Both terminal lines carry the real transferred byte total, not the + // "0.0 MB" a discarded native `syncOut` result would leave behind. + expect(exportLines.length).toBeGreaterThan(0); + expect(exportLines.some((line) => /\(\d+\.\d\/\d+\.\d MB\)/.test(line))).toBe(true); + expect(exportLines.some((line) => line.includes("(0.0/0.0 MB)"))).toBe(false); + + expect(restoreLines.length).toBeGreaterThan(0); + expect(restoreLines.some((line) => /\(\d+\.\d\/\d+\.\d MB\)/.test(line))).toBe(true); + expect(restoreLines.some((line) => line.includes("(0.0/0.0 MB)"))).toBe(false); + }); }); diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.ts b/packages/adapter-utils/src/sandbox-managed-runtime.ts index bfa21b7065..c409533af0 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.ts @@ -774,6 +774,16 @@ function toBuffer(bytes: Buffer | Uint8Array | ArrayBuffer): Buffer { return Buffer.from(bytes.buffer, bytes.byteOffset, bytes.byteLength); } +// Sum `bytesTransferred` over a `SandboxSyncResult`. The result crosses the +// plugin boundary, so a provider can return anything: guard each value and +// treat a missing, non-finite, or negative number as 0. +function sumSyncResultBytes(result: SandboxSyncResult): number { + return result.operations.reduce((total, operation) => { + const bytes = operation.bytesTransferred; + return Number.isFinite(bytes) && bytes > 0 ? total + bytes : total; + }, 0); +} + function tarExcludeFlags(exclude: string[] | undefined): string { return ["._*", ...(exclude ?? [])].map((entry) => `--exclude ${shellQuote(entry)}`).join(" "); } @@ -1039,8 +1049,15 @@ export async function prepareSandboxManagedRuntime(input: { params.progressLabel, { sink: input.onRuntimeProgress, phase: params.statusPhase }, ); - await syncIn(operations); - await upload.finish(params.progressBytes, params.progressBytes); + const syncResult = await syncIn(operations); + // Prefer the transport's own byte total. It is the real count for a + // provider that has no host tarball to stat (a referenced project rides + // a `directory` mapping). Fall back to the caller-supplied count when + // the transport reports 0, so a provider that under-reports still + // shows the host-known workspace total. + const transferredBytes = sumSyncResultBytes(syncResult); + const reportedBytes = transferredBytes > 0 ? transferredBytes : params.progressBytes; + await upload.finish(reportedBytes, reportedBytes); }; // Build the ordered inbound operation task list. Each task stages one inbound @@ -1413,9 +1430,10 @@ export async function prepareSandboxManagedRuntime(input: { // one `kind: "file"` mapping. The host does not buffer the full // bundle in RAM and does not move the bytes through the base64 // read loop. The git import step below reads the bundle from - // `localBundlePath`. The provider transfer reports no byte counts, - // so the "Exporting git history" progress degrades to - // start-and-finish only (mirrors the workspace restore below). + // `localBundlePath`. The provider transfer reports its own byte + // total, so the "Exporting git history" progress still degrades to + // start-and-finish (mirrors the workspace restore below), but the + // finish line carries the real transferred byte count. const operations: SandboxSyncOperation[] = [{ operationId: nextSyncOperationId(), files: [{ @@ -1428,8 +1446,9 @@ export async function prepareSandboxManagedRuntime(input: { sourceRoots: [runtimeRootDir], targetRoots: [tempDir], }); - await input.client.syncOut!(operations); - await gitExport.finish(0, 0); + const syncResult = await input.client.syncOut!(operations); + const transferredBytes = sumSyncResultBytes(syncResult); + await gitExport.finish(transferredBytes, transferredBytes); } else { const bundleBytes = await input.client.readFile(remoteGitBundle, gitExport.options); const bundleBuffer = toBuffer(bundleBytes); @@ -1468,7 +1487,8 @@ export async function prepareSandboxManagedRuntime(input: { // a fresh host directory. It is a clean destroy-then-replace into a // temp dir the orchestrator just created, so it maps exactly to a // generic directory file mapping; the host-side baseline merge below is - // unchanged. + // unchanged. The provider transfer reports its own byte total, so the + // finish line carries the real transferred byte count. const operations: SandboxSyncOperation[] = [{ operationId: nextSyncOperationId(), files: [{ @@ -1490,8 +1510,9 @@ export async function prepareSandboxManagedRuntime(input: { "workspace", { sink: input.onRuntimeProgress, phase: "restore" }, ); - await input.client.syncOut!(operations); - await workspaceRestore.finish(0, 0); + const syncResult = await input.client.syncOut!(operations); + const transferredBytes = sumSyncResultBytes(syncResult); + await workspaceRestore.finish(transferredBytes, transferredBytes); } else { const remoteWorkspaceTar = path.posix.join(runtimeRootDir, "workspace-download.tar"); await input.client.run(