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 <noreply@paperclip.ing>
This commit is contained in:
Nicky Leach
GitHub
parent
6880213de5
commit
d866ff374e
|
|
@ -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<number> {
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
Loading…
Reference in New Issue