mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 21:05:21 +02:00
## 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 <noreply@paperclip.ing>
132 lines
3.6 KiB
JavaScript
132 lines
3.6 KiB
JavaScript
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;
|
|
}
|
|
}
|
|
});
|