From ef37203a4805b8712bef4582492df0fff8eda76f Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Tue, 23 Jun 2026 17:52:12 -0700 Subject: [PATCH] perf(ci): build standalone public packages concurrently (#8567) 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 > - CI runs a Canary Dry Run job that exercises `release.sh`, which builds the standalone sandbox-provider packages for publish > - That step (`scripts/build-standalone-public-packages.mjs`) built the 7 provider plugins serially — each doing `rm -rf dist && tsc` — making it the dominant cost (~49s) inside the slowest PR check (~4.9m wall) after the general-server lane was already sharded > - The packages are independent (their own `node_modules` via `--ignore-workspace`, their own `dist`), so the serial build is pure latency with no correctness benefit > - This pull request builds them with a bounded-concurrency pool sized to the runner CPU count (overridable via `STANDALONE_BUILD_CONCURRENCY`), buffering each package's output and flushing it as one block so parallel logs stay readable, and aggregating failures by original index > - The benefit is a faster Canary Dry Run / PR feedback loop without changing what gets built or published ## Linked Issues or Issue Description No public GitHub issue exists. Inline feature/perf description: ### Problem or motivation `build-standalone-public-packages.mjs` builds standalone provider packages serially, making it the largest single cost inside the slowest PR check. ### Proposed solution Run independent per-package builds through a bounded-concurrency worker pool sized to runner CPU count, with an env override and readable buffered logs. ### Alternatives considered Keep the serial build for simpler logs, but that preserves the avoidable CI latency. ### Roadmap alignment This is CI maintenance and does not overlap planned core roadmap work. ## What Changed - `scripts/build-standalone-public-packages.mjs`: replaced the serial per-package build loop with a bounded-concurrency pool (default = runner CPU count, override via `STANDALONE_BUILD_CONCURRENCY`); per-package stdout/stderr is buffered and flushed as a single block; failures are aggregated by original package index so one failure neither aborts the others mid-flight nor obscures which package broke. - `scripts/__tests__/build-standalone-concurrency.test.mjs`: new `node:test` unit suite covering the pool (limit respected, all items run, ordered failure aggregation, env-override resolution). - `.github/workflows/pr.yml`: wired the new unit test into the policy job. ## Verification - `node --test ./scripts/__tests__/build-standalone-concurrency.test.mjs` → 6/6 pass - `node ./scripts/release-package-map.mjs check` → OK (29 enabled for CI publish) - `git diff --check origin/master..HEAD` → clean ## Risks - Low risk. Build inputs/outputs are unchanged; only scheduling differs. The concurrency is bounded by CPU count and overridable; output is buffered per package so logs remain attributable. If a package fails, all failures are still reported with their package index. ## Model Used - Claude (Anthropic), `claude-opus-4-8`, extended thinking with tool use. ## 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 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 - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .github/workflows/pr.yml | 3 + .../build-standalone-concurrency.test.mjs | 131 ++++++++++++++ scripts/build-standalone-public-packages.mjs | 171 +++++++++++++++--- 3 files changed, 276 insertions(+), 29 deletions(-) create mode 100644 scripts/__tests__/build-standalone-concurrency.test.mjs diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 42c7c4ec13..af04adf61d 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -58,6 +58,9 @@ jobs: - name: Test general-server shard partition run: node --test ./scripts/__tests__/run-vitest-stable-shard.test.mjs + - name: Test standalone package build concurrency + run: node --test ./scripts/__tests__/build-standalone-concurrency.test.mjs + - name: Validate release package manifest run: node ./scripts/release-package-map.mjs check diff --git a/scripts/__tests__/build-standalone-concurrency.test.mjs b/scripts/__tests__/build-standalone-concurrency.test.mjs new file mode 100644 index 0000000000..ef22755f57 --- /dev/null +++ b/scripts/__tests__/build-standalone-concurrency.test.mjs @@ -0,0 +1,131 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + resolveConcurrency, + runWithConcurrency, +} from "../build-standalone-public-packages.mjs"; + +function deferred() { + let resolve; + let reject; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +test("runs every item and preserves result order", async () => { + const items = [1, 2, 3, 4, 5]; + const results = await runWithConcurrency(items, 2, async (value) => value * 10); + assert.deepEqual(results, [10, 20, 30, 40, 50]); +}); + +test("never exceeds the concurrency limit", async () => { + const items = Array.from({ length: 8 }, (_, index) => index); + const limit = 3; + let active = 0; + let peak = 0; + const gates = items.map(() => deferred()); + + const run = runWithConcurrency(items, limit, async (value) => { + active += 1; + peak = Math.max(peak, active); + assert.ok(active <= limit, `active ${active} exceeded limit ${limit}`); + await gates[value].promise; + active -= 1; + return value; + }); + + // Release gates progressively; the pool must keep at most `limit` in flight. + for (const gate of gates) { + await Promise.resolve(); + gate.resolve(); + } + + await run; + assert.equal(peak, limit); +}); + +test("aggregates failures and still runs remaining items", async () => { + const items = [0, 1, 2, 3]; + const processed = []; + + await assert.rejects( + () => + runWithConcurrency(items, 2, async (value) => { + processed.push(value); + if (value === 1) { + throw new Error(`boom-${value}`); + } + return value; + }), + (error) => { + assert.match(error.message, /1 standalone package build\(s\) failed/); + assert.equal(error.failures.length, 1); + assert.equal(error.failures[0].index, 1); + assert.match(error.failures[0].error.message, /boom-1/); + return true; + }, + ); + + // A single failure must not abort the rest of the queue. + assert.deepEqual(processed.sort((a, b) => a - b), [0, 1, 2, 3]); +}); + +test("reports failures sorted by original index", async () => { + const items = [0, 1, 2, 3, 4]; + + await assert.rejects( + () => + runWithConcurrency(items, 5, async (value) => { + if (value === 3 || value === 1) { + throw new Error(`fail-${value}`); + } + return value; + }), + (error) => { + assert.deepEqual( + error.failures.map(({ index }) => index), + [1, 3], + ); + return true; + }, + ); +}); + +test("resolveConcurrency honors a valid env override, capped by package count", () => { + const previous = process.env.STANDALONE_BUILD_CONCURRENCY; + try { + process.env.STANDALONE_BUILD_CONCURRENCY = "2"; + assert.equal(resolveConcurrency(7), 2); + assert.equal(resolveConcurrency(1), 1); + assert.equal(resolveConcurrency(0), 1); + + process.env.STANDALONE_BUILD_CONCURRENCY = "0"; + assert.ok(resolveConcurrency(7) >= 1); + + process.env.STANDALONE_BUILD_CONCURRENCY = "not-a-number"; + assert.ok(resolveConcurrency(7) >= 1); + } finally { + if (previous === undefined) { + delete process.env.STANDALONE_BUILD_CONCURRENCY; + } else { + process.env.STANDALONE_BUILD_CONCURRENCY = previous; + } + } +}); + +test("resolveConcurrency never returns more than the package count", () => { + const previous = process.env.STANDALONE_BUILD_CONCURRENCY; + try { + delete process.env.STANDALONE_BUILD_CONCURRENCY; + assert.equal(resolveConcurrency(0), 1); + assert.ok(resolveConcurrency(3) <= 3); + } finally { + if (previous !== undefined) { + process.env.STANDALONE_BUILD_CONCURRENCY = previous; + } + } +}); diff --git a/scripts/build-standalone-public-packages.mjs b/scripts/build-standalone-public-packages.mjs index 7d004bb7ab..7b5e6b4902 100644 --- a/scripts/build-standalone-public-packages.mjs +++ b/scripts/build-standalone-public-packages.mjs @@ -1,13 +1,17 @@ #!/usr/bin/env node -import { execFileSync } from "node:child_process"; +import { execFile, execFileSync } from "node:child_process"; import { existsSync, readFileSync, rmSync } from "node:fs"; +import { availableParallelism } from "node:os"; import path, { dirname } from "node:path"; import process from "node:process"; import { fileURLToPath } from "node:url"; +import { promisify } from "node:util"; import { linkSdkInto } from "./link-plugin-dev-sdk.mjs"; +const execFileAsync = promisify(execFile); + const scriptDir = dirname(fileURLToPath(import.meta.url)); const repoRoot = path.resolve(scriptDir, ".."); const workspacePath = path.join(repoRoot, "pnpm-workspace.yaml"); @@ -92,34 +96,41 @@ function readPackageJson(pkgDir) { ); } -function run(command, args, cwd) { - execFileSync(command, args, { - cwd, - env: { - ...process.env, - CI: "true", - }, - stdio: "inherit", - }); +async function runCaptured(command, args, cwd, log) { + try { + const { stdout, stderr } = await execFileAsync(command, args, { + cwd, + env: { + ...process.env, + CI: "true", + }, + maxBuffer: 64 * 1024 * 1024, + }); + if (stdout?.trim()) log(stdout.trimEnd()); + if (stderr?.trim()) log(stderr.trimEnd()); + } catch (error) { + if (error.stdout?.toString().trim()) log(error.stdout.toString().trimEnd()); + if (error.stderr?.toString().trim()) log(error.stderr.toString().trimEnd()); + throw error; + } } -function main() { - const workspaceEntries = parseWorkspaceEntries(readFileSync(workspacePath, "utf8")); - const standalonePackages = listPublicPackages() - .filter(({ dir }) => !isWorkspacePackage(dir, workspaceEntries)); +// Each standalone package installs into its own directory (`--ignore-workspace`) +// and builds into its own `dist`, so there is no shared mutable state between +// packages beyond pnpm's content-addressable global store, which is safe for +// concurrent access. Buffer each package's output and flush it as one block so +// interleaved parallel logs stay readable. +async function prepareAndBuildPackage(pkg) { + const logs = []; + const log = (line) => logs.push(line); - if (standalonePackages.length === 0) { - console.log(" i No standalone public packages detected outside the pnpm workspace"); - return; - } + const pkgDir = path.join(repoRoot, pkg.dir); + const pkgJson = readPackageJson(pkg.dir); + const nodeModulesDir = path.join(pkgDir, "node_modules"); + const packageLockfilePath = path.join(pkgDir, "pnpm-lock.yaml"); - for (const pkg of standalonePackages) { - const pkgDir = path.join(repoRoot, pkg.dir); - const pkgJson = readPackageJson(pkg.dir); - const nodeModulesDir = path.join(pkgDir, "node_modules"); - const packageLockfilePath = path.join(pkgDir, "pnpm-lock.yaml"); - - console.log(` Preparing standalone package ${pkg.name} (${pkg.dir})`); + log(` Preparing standalone package ${pkg.name} (${pkg.dir})`); + try { if (existsSync(nodeModulesDir)) { rmSync(nodeModulesDir, { force: true, recursive: true }); } @@ -133,7 +144,7 @@ function main() { // Standalone packages intentionally avoid committed lockfile churn in the repo. ]; - run("pnpm", installArgs, pkgDir); + await runCaptured("pnpm", installArgs, pkgDir, log); // The fresh install above wipes node_modules and no longer fires a // per-plugin postinstall (removed for supply-chain safety), so link the @@ -141,11 +152,113 @@ function main() { linkSdkInto(pkgDir); if (pkgJson.scripts?.build) { - run("pnpm", ["run", "build"], pkgDir); + await runCaptured("pnpm", ["run", "build"], pkgDir, log); } else { - console.log(" i No build script; skipped build"); + log(" i No build script; skipped build"); + } + } finally { + if (logs.length > 0) { + console.log(logs.join("\n")); } } } -main(); +export function resolveConcurrency(packageCount) { + const raw = process.env.STANDALONE_BUILD_CONCURRENCY; + if (raw !== undefined && raw !== "") { + const parsed = Number.parseInt(raw, 10); + if (Number.isFinite(parsed) && parsed > 0) { + return Math.max(1, Math.min(parsed, packageCount)); + } + } + + let cpus = 4; + try { + cpus = availableParallelism(); + } catch { + // Fall back to the default when parallelism cannot be determined. + } + return Math.max(1, Math.min(cpus, packageCount)); +} + +// Bounded-concurrency task pool. Workers pull from a shared cursor so no more +// than `limit` tasks run at once. Every task is awaited even if an earlier one +// fails, and failures are aggregated (sorted by original index) so a single bad +// package neither aborts the others mid-flight nor hides which one broke. +export async function runWithConcurrency(items, limit, worker) { + const results = new Array(items.length); + const failures = []; + let nextIndex = 0; + + async function runWorker() { + while (true) { + const current = nextIndex; + nextIndex += 1; + if (current >= items.length) { + return; + } + try { + results[current] = await worker(items[current], current); + } catch (error) { + failures.push({ index: current, error }); + } + } + } + + const poolSize = Math.max(1, Math.min(limit, items.length)); + await Promise.all(Array.from({ length: poolSize }, () => runWorker())); + + if (failures.length > 0) { + failures.sort((a, b) => a.index - b.index); + const aggregate = new Error( + `${failures.length} standalone package build(s) failed`, + ); + aggregate.failures = failures; + throw aggregate; + } + + return results; +} + +async function main() { + const workspaceEntries = parseWorkspaceEntries(readFileSync(workspacePath, "utf8")); + const standalonePackages = listPublicPackages() + .filter(({ dir }) => !isWorkspacePackage(dir, workspaceEntries)); + + if (standalonePackages.length === 0) { + console.log(" i No standalone public packages detected outside the pnpm workspace"); + return; + } + + const concurrency = resolveConcurrency(standalonePackages.length); + console.log( + ` Building ${standalonePackages.length} standalone package(s) with concurrency ${concurrency}`, + ); + + try { + await runWithConcurrency( + standalonePackages, + concurrency, + prepareAndBuildPackage, + ); + } catch (error) { + if (Array.isArray(error.failures)) { + const names = error.failures + .map(({ index }) => standalonePackages[index]?.name) + .filter(Boolean) + .join(", "); + console.error(` ✗ Failed standalone package build(s): ${names}`); + } + throw error; + } +} + +const invokedDirectly = + process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url); + +if (invokedDirectly) { + main().catch((error) => { + console.error(error); + process.exit(1); + }); +}