From 444b4de693e5e0df3fab1f759a1c9bde4e7b4335 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Fri, 6 Nov 2026 16:09:15 -0800 Subject: [PATCH] fix(cli): use native CRC-32 for import zips and give the e2e server time to start (#19667) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors wait for PR CI before merge, and a flaky test forces a re-run of the whole CI run > - Two CLI tests failed several times on 2026-11-06 on commits that passed on a later attempt > - Both failures came from time limits on CPU-contended 4-vCPU runners, and both appeared in the same jobs > - One cause is a slow checksum loop that the CLI's own zip writer also uses > - This pull request removes that cost and gives the e2e child server a startup budget that covers a cold start > - The benefit is fewer whole-run re-runs, and faster large-package zips for CLI users ## Linked Issues or Issue Description **What happened?** Two tests in `cli/src/__tests__` failed in PR CI and passed on a re-run of the same commit: - `company-import-transfer.test.ts` › "resolveChunkedImportZip › takes the chunked path…" failed with `Test timed out in 5000ms` (2 times). - `company-import-export-e2e.test.ts` failed with `Timed out waiting for the child server` (3 times). **Expected behavior** Both tests pass on a loaded CI runner. **Steps to reproduce** 1. Run the `cli` Vitest project with `--maxWorkers=4` in a 4-CPU container (`docker run --cpus=4 --memory=16g`), with a 4-thread CPU load in that container and a fresh `TMPDIR`. 2. Before this change, both tests fail in 3 of 3 runs. ## What Changed - `cli/src/commands/client/zip.ts`: the stored-zip writer uses `crc32` from `node:zlib` instead of a bitwise JavaScript loop. The CLI uses this writer for packages past the chunked-import threshold. Over about 64 MB, the old loop takes about 14 seconds and `zlib.crc32` takes 35 ms. `zlib.crc32` has been in Node since 22.2, and the engines field requires Node 24.11 or later. - `cli/src/__tests__/company-import-transfer.test.ts`: the fixture's zip builder uses `zlib.crc32` too. - `cli/src/__tests__/company-import-export-e2e.test.ts`: - add `SERVER_STARTUP_TIMEOUT_MS = 90_000` for `waitForServer`, which was 30 seconds; - set the `beforeAll` timeout to that value plus 30 seconds for the database start; - a comment gives the measurements. A child that exits early still fails at once. No assertions change. ## Verification - **Cause of the e2e failure:** `paperclipai run` loads the CLI and the server from source through tsx, and every CI run starts with a cold tsx cache. - The server is slow to start, not hung. With a larger budget at heavy load, startup took 50 to 55 seconds. - Passing CI jobs already used 15 to 25 seconds of the 30-second budget. - In the failed logs, nothing comes from the server after "Starting Paperclip server...". - **Container reproduction** (4 CPUs, 16 GB, a 4-thread load in the same container, cold tsx cache, full `cli` project with 4 workers): | State | Result | Dense-zip test | Oversized-folder test | | --- | --- | --- | --- | | Before | failed 3/3 (child-server timeout; the dense test also timed out in one run) | 4.6–5.5 s | 2.7–3.2 s | | CRC fix only | the e2e test still failed 3/3 | 0.8–1.3 s | — | | Full fix | all 65 files passed 3/3 | 0.7–1.3 s | 0.15–0.2 s | - `pnpm typecheck` in `cli`: passes. - Host run with `--maxWorkers=2` of the changed and related files: 4 files, 23/23 pass. ## Risks - The e2e change is a larger time budget, not a faster start. A real startup regression now takes up to 90 seconds to fail. - Each "after" result comes from 3 runs, which is a small sample. - `zlib.crc32` returns the same unsigned 32-bit value as the old loop. The existing zip round-trip tests cover this. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use and code execution. A Claude sub-agent found the causes and made the fix, and the parent session reviewed it. ## 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 will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- .../__tests__/company-import-export-e2e.test.ts | 15 +++++++++++++-- cli/src/__tests__/company-import-transfer.test.ts | 9 ++------- cli/src/commands/client/zip.ts | 13 ++----------- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/cli/src/__tests__/company-import-export-e2e.test.ts b/cli/src/__tests__/company-import-export-e2e.test.ts index 5d49d4a8b8..5182afd3cd 100644 --- a/cli/src/__tests__/company-import-export-e2e.test.ts +++ b/cli/src/__tests__/company-import-export-e2e.test.ts @@ -249,6 +249,16 @@ async function runCliJson( return JSON.parse(stdout.slice(jsonStart)) as T; } +// `paperclipai run` loads the CLI and the whole server graph from source +// through tsx. The test runner gives every run a fresh TMPDIR, so tsx's +// transform cache is always cold: startup then costs about twice a warm start +// (11.5 s against 5.2 s from "Starting Paperclip server..." to the first server +// log on an idle 16-core machine). On contended 4-vCPU CI runners the hooks of +// passing runs took 15-25 s, and the old 30 s budget failed in the tail. This budget +// matches the 90 s test-body timeout; a child that exits early still fails at +// once. +const SERVER_STARTUP_TIMEOUT_MS = 90_000; + async function waitForServer( instanceId: string, runtimeInfoPath: string, @@ -256,7 +266,7 @@ async function waitForServer( output: { stdout: string[]; stderr: string[] }, ) { const startedAt = Date.now(); - while (Date.now() - startedAt < 30_000) { + while (Date.now() - startedAt < SERVER_STARTUP_TIMEOUT_MS) { if (child.exitCode !== null) { throw new Error( `paperclipai run exited before healthcheck succeeded.\nstdout:\n${output.stdout.join("")}\nstderr:\n${output.stderr.join("")}`, @@ -342,7 +352,8 @@ describeEmbeddedPostgres("paperclipai company import/export e2e", () => { output, ); expect(new URL(apiBase).port).not.toBe(String(port)); - }, 60_000); + // Room for the embedded database start on top of the server budget. + }, SERVER_STARTUP_TIMEOUT_MS + 30_000); afterAll(async () => { await stopServerProcess(serverProcess); diff --git a/cli/src/__tests__/company-import-transfer.test.ts b/cli/src/__tests__/company-import-transfer.test.ts index 37c0b150bc..0523169bc9 100644 --- a/cli/src/__tests__/company-import-transfer.test.ts +++ b/cli/src/__tests__/company-import-transfer.test.ts @@ -1,5 +1,5 @@ import { createHash } from "node:crypto"; -import { deflateRawSync } from "node:zlib"; +import { crc32, deflateRawSync } from "node:zlib"; import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -73,12 +73,7 @@ function buildDeflateZip(entryPath: string, text: string): Uint8Array { const raw = Buffer.from(text, "utf8"); const body = deflateRawSync(raw); const name = Buffer.from(entryPath, "utf8"); - let crc = 0xffffffff; - for (const byte of raw) { - crc ^= byte; - for (let bit = 0; bit < 8; bit += 1) crc = (crc & 1) === 1 ? (crc >>> 1) ^ 0xedb88320 : crc >>> 1; - } - crc = (crc ^ 0xffffffff) >>> 0; + const crc = crc32(raw); const local = Buffer.alloc(30 + name.length); local.writeUInt32LE(0x04034b50, 0); local.writeUInt16LE(20, 4); diff --git a/cli/src/commands/client/zip.ts b/cli/src/commands/client/zip.ts index a927d239c1..c2c7a4cd06 100644 --- a/cli/src/commands/client/zip.ts +++ b/cli/src/commands/client/zip.ts @@ -1,3 +1,5 @@ +import { crc32 } from "node:zlib"; + // The node-side portability zip reader lives in @paperclipai/shared so the // server can consume the same codec (a raw uploaded zip is unzipped into the // exact `{ rootPath, files }` bundle the inline import source carries). This @@ -31,17 +33,6 @@ function writeUint32(target: Uint8Array, offset: number, value: number) { target[offset + 3] = (value >>> 24) & 0xff; } -function crc32(bytes: Uint8Array) { - let crc = 0xffffffff; - for (const byte of bytes) { - crc ^= byte; - for (let bit = 0; bit < 8; bit += 1) { - crc = (crc & 1) === 1 ? (crc >>> 1) ^ 0xedb88320 : crc >>> 1; - } - } - return (crc ^ 0xffffffff) >>> 0; -} - /** * Build a stored (uncompressed) zip of `files` under a single `rootPath/` * top-level directory — the layout `readZipArchive` folds back into the