Skip to content

Commit 352153b

Browse files
devinfoleyPaperclip-Paperclipclaude
authored
fix: run the cargo-building native-runner CI suite in the Rust-cached vitest lane (#13586)
## Thinking Path Post-merge of #13557, the slowest check on the freshest fully-green PR run ([35246999382](https://github.com/paperclipai/paperclip/actions/runs/35246999382)) was `ci / General tests (server (1/12))` at **339s**. The cause is one suite: `server/src/services/native-runtime/native-codex-runner.integration.test.ts` runs 1 test in **277s of a 291s vitest step (95%)** because its `beforeAll` cargo-builds the Runner release binaries, and the general-server shards carry no Rust cache — every PR run cold-compiles the full third-party crate graph. The other 19 suites in that shard finish in under 70ms each. The obvious fix (a dedicated Rust-cached matrix lane) requires editing workflow files, which the available GitHub App credentials cannot push (`workflows` permission). But the `Verify Paperclip Runner` lanes **already restore the shared `release-runner-v1` Rust cache read-only**, and their commands are `pnpm --filter @paperclipai/paperclip-runner <package script>` — so the suite can move into a Rust-cached lane purely through script changes. ## What Changed - `scripts/run-vitest-stable.mjs`: new `general-server-native-runner` group carrying exactly that suite. Under the PR workflow (`GITHUB_WORKFLOW == "PR"`, inherited from `pr.yml` by the reusable `pr-trusted.yml`) the without-chat server shards exclude it and rebalance to ~211s of tests each. Every other caller — local runs, `release-verify.yml` under the Release / Cloud readiness workflows — keeps the suite in the shards, so a renamed or unknown workflow degrades to today's slower-but-covered behavior instead of dropping coverage. - `packages/paperclip-runner`: `test:typescript:vitest` now routes through `scripts/run-pr-vitest-lane.mjs` — the identical `ensure:eval-build-deps && build:rust && vitest run` chain (shard flags passed through), plus the native-runner group on the **final PR shard only** (`--shard=N/M` with `N == M`, i.e. today's `vitest 2/2`, the 122s lane). With the restored cache the suite's cargo build becomes an incremental rebuild. - `scripts/__tests__/run-vitest-stable-shard.test.mjs`: guards pin the whole contract — PR 12-shard coverage (shards + chat + native-runner = full server group exactly), Release/local 10-shard runs keep the suite, `pr.yml` is named `PR`, the vitest lanes partition with exactly one final shard, the package-script wiring, and the wrapper's shard/workflow gating via its `--dry-run` plan output. No workflow files change. `.github/workflows/*` are untouched. ## Verification - `node --test scripts/__tests__/run-vitest-stable-shard.test.mjs scripts/__tests__/release-verify-workflow.test.mjs`: **36/36 pass** locally on this branch (includes the new coverage, wiring, and wrapper-gating guards). Both files run in CI's `Test general-server shard partition` / `Test release verify workflow wiring` steps. - Wrapper `--dry-run` plan matrix verified for all six shard/workflow combinations plus malformed-shard rejection (pinned as a guard test). - The executing proof is this PR's own CI: `ci / Verify Paperclip Runner (vitest 2/2)` must go green while running the native-runner suite (its log will show the `general-server-native-runner` group after the package vitest shard), and the 12 `ci / General tests (server (x/12))` shards must go green without it. ## Risks - The exclusion keys on `GITHUB_WORKFLOW == "PR"`. Failure mode of a rename is safe (suite falls back into the server shards, slower but covered) and the guard test on `pr.yml`'s name makes it loud. - `vitest 2/2` grows from ~122s to an expected ~210–260s — still well under the ~306s `vitest 1/2` and ~326s e2e shards, and inside the 20-minute lane timeout. If the cache misses (key drift), the lane pays a cold compile like the server shard does today; a miss is slow, never wrong. - Double-run/coverage-loss combinations are enumerated in the wrapper header and pinned by tests: each caller runs the suite exactly once. ## Model Used Claude (Bender agent, Paperclip) — Fable 5. --- Expected savings once merged: the 339s `server (1/12)` check drops to ~265s-equivalent shard levels (~211s of tests), the slowest `ci /` check becomes the ~326s e2e shard (~13–33s off PR wall time), and every PR run stops paying ~4.5 min of billed cold Rust compile. For the merger (squash): please keep the trailer below in the squash body to preserve authorship. `Co-Authored-By: Bender (Fable) <Paperclip-Paperclip@users.noreply.github.com>` ## Related PRs Searched the GitHub PR list for prior work on this surface — related groundwork, none duplicate this change: - #13457 — restored master's Rust dependency cache on the PR runner lane (the read-only cache this PR relies on) - #13500 — made that cache key image-toolchain-independent so GitHub-hosted PR runners actually hit it - #13521 — rebalanced PR shards and split the Verify Paperclip Runner lanes this PR extends - #13557 — previous health-check iteration (split the runnerd transport suite); this PR targets the next slowest check ## Checklist - [x] I have searched GitHub for duplicate or related PRs and linked them above Co-authored-by: Bender (Fable) <Paperclip-Paperclip@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent de9414d commit 352153b

4 files changed

Lines changed: 278 additions & 33 deletions

File tree

packages/paperclip-runner/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
"test": "pnpm run test:typescript && pnpm run test:rust",
7676
"test:typescript": "pnpm run test:typescript:prep && vitest run",
7777
"test:typescript:prep": "pnpm run ensure:eval-build-deps && pnpm run build:rust && node --test test/protocol-contract.test.mjs test/acpx-sidecar-contract.test.mjs test/acpx-codex-package-contract.test.mjs scripts/aws-agentcore-provisioning.test.mjs scripts/build-verified-provider-entrypoints.test.mjs scripts/local-provider-smoke-environment.test.mjs scripts/materialize-opencode-binary.test.mjs",
78-
"test:typescript:vitest": "pnpm run ensure:eval-build-deps && pnpm run build:rust && vitest run",
78+
"test:typescript:vitest": "node ./scripts/run-pr-vitest-lane.mjs",
7979
"test:rust": "cargo test --release --manifest-path runner/Cargo.toml --locked --workspace",
8080
"test:codex": "cargo test --manifest-path runner/Cargo.toml --locked -p paperclip-runner-core --test codex_provider",
8181
"test:durable": "cargo test --manifest-path runner/Cargo.toml --locked -p paperclip-runner-core durable::",
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Runs this package's vitest shard for a Verify Paperclip Runner lane, then —
2+
// on the final shard of the PR workflow only — the server package's
3+
// general-server-native-runner group.
4+
//
5+
// That server suite rebuilds the Runner release binaries with cargo in
6+
// beforeAll. The PR workflow's plain General tests server shards carry no
7+
// Rust cache, so hosting it there cold-compiled every third-party crate on
8+
// each run (277s of a 291s shard vitest step, actions run 35246999382,
9+
// 2026-09-17) and made that shard the slowest check of the whole run. The
10+
// Verify Paperclip Runner lanes already restore the shared release-runner-v1
11+
// Rust cache read-only, which turns that build into an incremental rebuild,
12+
// and the workflow files themselves list this lane's command as a package
13+
// script — so the suite moves here without a workflow-file change.
14+
//
15+
// Contract, mirrored in scripts/run-vitest-stable.mjs (prWorkflowName) and
16+
// pinned by scripts/__tests__/run-vitest-stable-shard.test.mjs:
17+
// - Only the PR workflow (pr.yml, whose GITHUB_WORKFLOW the reusable
18+
// pr-trusted.yml jobs inherit) excludes the suite from the server shards,
19+
// and only there does this wrapper run it. Any other caller — local runs,
20+
// release-verify.yml — keeps the suite in the server group, so a renamed
21+
// workflow degrades to the slower covered path instead of losing coverage.
22+
// - The suite runs on the lane whose --shard=N/M has N === M (or an unsharded
23+
// invocation), so exactly one PR lane carries it.
24+
import { spawnSync } from "node:child_process";
25+
import { dirname, resolve } from "node:path";
26+
import { fileURLToPath } from "node:url";
27+
28+
const packageRoot = resolve(dirname(fileURLToPath(import.meta.url)), "..");
29+
const workspaceRoot = resolve(packageRoot, "../..");
30+
const args = process.argv.slice(2).filter((value) => value !== "--dry-run");
31+
const dryRun = process.argv.includes("--dry-run");
32+
33+
const shardArg = args.find((value) => value.startsWith("--shard="));
34+
const shardMatch = shardArg ? /^--shard=(\d+)\/(\d+)$/.exec(shardArg) : null;
35+
if (shardArg && !shardMatch) {
36+
console.error(`[pr-vitest-lane] unrecognized shard argument: ${shardArg}`);
37+
process.exit(1);
38+
}
39+
const isFinalShard = !shardArg || shardMatch[1] === shardMatch[2];
40+
const isPrWorkflow = process.env.GITHUB_WORKFLOW === "PR";
41+
const runNativeRunnerGroup = isPrWorkflow && isFinalShard;
42+
43+
const plannedCommands = [
44+
{ command: "pnpm", args: ["run", "ensure:eval-build-deps"], cwd: packageRoot },
45+
{ command: "pnpm", args: ["run", "build:rust"], cwd: packageRoot },
46+
{ command: "pnpm", args: ["exec", "vitest", "run", ...args], cwd: packageRoot },
47+
...(runNativeRunnerGroup
48+
? [{
49+
command: "pnpm",
50+
args: ["test:run:general", "--", "--group", "general-server-native-runner"],
51+
cwd: workspaceRoot,
52+
}]
53+
: []),
54+
];
55+
56+
if (dryRun) {
57+
console.log(JSON.stringify({ isPrWorkflow, isFinalShard, runNativeRunnerGroup, plannedCommands }, null, 2));
58+
process.exit(0);
59+
}
60+
61+
for (const planned of plannedCommands) {
62+
const result = spawnSync(planned.command, planned.args, {
63+
cwd: planned.cwd,
64+
stdio: "inherit",
65+
});
66+
if (result.status !== 0) {
67+
process.exit(result.status ?? 1);
68+
}
69+
}

scripts/__tests__/run-vitest-stable-shard.test.mjs

Lines changed: 149 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import assert from "node:assert/strict";
22
import { spawnSync } from "node:child_process";
3+
import { readFileSync } from "node:fs";
34
import path from "node:path";
45
import { fileURLToPath } from "node:url";
56
import test from "node:test";
@@ -21,16 +22,29 @@ const serializedDurationsManifest = path.join(
2122
"serialized-shard-durations.json",
2223
);
2324

24-
function dryRun(args) {
25+
// Membership of general-server-without-chat depends on GITHUB_WORKFLOW (see
26+
// prWorkflowName in run-vitest-stable.mjs), so strip the ambient value and
27+
// make every test pin the caller it mirrors explicitly — these tests
28+
// themselves run inside a workflow on CI.
29+
function workflowEnv(envOverrides = {}) {
30+
const env = { ...process.env, ...envOverrides };
31+
if (!("GITHUB_WORKFLOW" in envOverrides)) {
32+
delete env.GITHUB_WORKFLOW;
33+
}
34+
return env;
35+
}
36+
37+
function dryRun(args, envOverrides = {}) {
2538
const result = spawnSync(process.execPath, [script, ...args, "--dry-run"], {
2639
cwd: repoRoot,
2740
encoding: "utf8",
41+
env: workflowEnv(envOverrides),
2842
});
2943
return result;
3044
}
3145

32-
function dryRunJson(args) {
33-
const result = dryRun(args);
46+
function dryRunJson(args, envOverrides = {}) {
47+
const result = dryRun(args, envOverrides);
3448
assert.equal(result.status, 0, `expected success for ${args.join(" ")}: ${result.stderr}`);
3549
return JSON.parse(result.stdout);
3650
}
@@ -222,11 +236,15 @@ test("the real serialized shard partition is duration-balanced", () => {
222236

223237
test("the real shard partition is duration-balanced", () => {
224238
// Mirrors the PR matrix: general-server-without-chat across SHARD_COUNT
225-
// runners, with the chat suite carried by the dedicated general-chat lanes.
239+
// runners, with the chat suite carried by the dedicated general-chat lanes
240+
// and the native-runner suite by the Rust-cached PR vitest lane.
226241
const durations = loadShardDurations(durationsManifest);
227242
const fallback = defaultSuiteWeight(durations);
228243
const shards = Array.from({ length: SHARD_COUNT }, (_, index) =>
229-
dryRunJson(["--mode", "general", "--group", "general-server-without-chat", "--shard-index", String(index), "--shard-count", String(SHARD_COUNT)]),
244+
dryRunJson(
245+
["--mode", "general", "--group", "general-server-without-chat", "--shard-index", String(index), "--shard-count", String(SHARD_COUNT)],
246+
{ GITHUB_WORKFLOW: "PR" },
247+
),
230248
);
231249

232250
const totals = shards.map((shard) =>
@@ -235,10 +253,15 @@ test("the real shard partition is duration-balanced", () => {
235253
const maxTotal = Math.max(...totals);
236254
const minTotal = Math.min(...totals);
237255
// LPT keeps the spread within the heaviest single suite; use that as the
238-
// bound. The chat suite runs in its own lanes, so exclude it here.
256+
// bound. The chat and native-runner suites run in their own lanes, so
257+
// exclude them here.
239258
const chat = "server/src/__tests__/chat-channels.integration.test.ts";
259+
const nativeRunner =
260+
"server/src/services/native-runtime/native-codex-runner.integration.test.ts";
240261
const heaviest = Math.max(
241-
...Object.entries(durations).filter(([file]) => file !== chat).map(([, ms]) => ms),
262+
...Object.entries(durations)
263+
.filter(([file]) => file !== chat && file !== nativeRunner)
264+
.map(([, ms]) => ms),
242265
);
243266
assert.ok(
244267
maxTotal - minTotal <= heaviest,
@@ -247,24 +270,131 @@ test("the real shard partition is duration-balanced", () => {
247270
});
248271

249272

250-
// 12 mirrors pr-trusted.yml, 10 mirrors release-verify.yml.
251-
for (const withoutChatShardCount of [10, 12]) {
252-
test(`${withoutChatShardCount} without-chat shards plus the dedicated chat file cover the original server group exactly`, () => {
253-
const full = dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", "0", "--shard-count", "1"]);
254-
const shards = Array.from({ length: withoutChatShardCount }, (_, index) => dryRunJson([
273+
const chatSuitePath = "server/src/__tests__/chat-channels.integration.test.ts";
274+
const nativeRunnerSuitePath =
275+
"server/src/services/native-runtime/native-codex-runner.integration.test.ts";
276+
277+
// Mirrors pr-trusted.yml (12 shards, called by pr.yml so GITHUB_WORKFLOW is
278+
// "PR"): the chat suite runs in its dedicated lanes and the cargo-dependent
279+
// native-runner suite in the Rust-cached final Verify Paperclip Runner vitest
280+
// shard, so together the three cover the full server group exactly.
281+
test("12 PR without-chat shards plus the dedicated chat and native-runner lanes cover the original server group exactly", () => {
282+
const prEnv = { GITHUB_WORKFLOW: "PR" };
283+
const full = dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", "0", "--shard-count", "1"], prEnv);
284+
const shards = Array.from({ length: 12 }, (_, index) => dryRunJson([
285+
"--mode", "general", "--group", "general-server-without-chat",
286+
"--shard-index", String(index), "--shard-count", "12",
287+
], prEnv));
288+
const files = shards.flatMap((shard) => shard.selectedGeneralServerSuites);
289+
assert.ok(!files.includes(chatSuitePath));
290+
assert.ok(!files.includes(nativeRunnerSuitePath));
291+
assert.deepEqual([...files, chatSuitePath, nativeRunnerSuitePath].sort(), full.selectedGeneralServerSuites.sort());
292+
assert.equal(new Set(files).size, files.length);
293+
const defaultRun = dryRunJson([], prEnv);
294+
assert.ok(defaultRun.generalServerSuiteCount === full.generalServerSuiteCount);
295+
});
296+
297+
// Mirrors release-verify.yml (10 shards, called by the Release and Cloud
298+
// readiness workflows) and local runs: no Rust-cached vitest lane exists
299+
// there, so the native-runner suite must stay in the server shards.
300+
for (const [caller, envOverrides] of [["Release", { GITHUB_WORKFLOW: "Release" }], ["no ambient workflow", {}]]) {
301+
test(`10 without-chat shards under ${caller} keep the native-runner suite and cover the server group with chat alone`, () => {
302+
const full = dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", "0", "--shard-count", "1"], envOverrides);
303+
const shards = Array.from({ length: 10 }, (_, index) => dryRunJson([
255304
"--mode", "general", "--group", "general-server-without-chat",
256-
"--shard-index", String(index), "--shard-count", String(withoutChatShardCount),
257-
]));
305+
"--shard-index", String(index), "--shard-count", "10",
306+
], envOverrides));
258307
const files = shards.flatMap((shard) => shard.selectedGeneralServerSuites);
259-
const chat = "server/src/__tests__/chat-channels.integration.test.ts";
260-
assert.ok(!files.includes(chat));
261-
assert.deepEqual([...files, chat].sort(), full.selectedGeneralServerSuites.sort());
308+
assert.ok(!files.includes(chatSuitePath));
309+
assert.ok(files.includes(nativeRunnerSuitePath));
310+
assert.deepEqual([...files, chatSuitePath].sort(), full.selectedGeneralServerSuites.sort());
262311
assert.equal(new Set(files).size, files.length);
263-
const defaultRun = dryRunJson([]);
264-
assert.ok(defaultRun.generalServerSuiteCount === full.generalServerSuiteCount);
265312
});
266313
}
267314

315+
test("the native-runner lane runs exactly the cargo-dependent vertical-slice suite", () => {
316+
const lane = dryRunJson(["--mode", "general", "--group", "general-server-native-runner"]);
317+
assert.deepEqual(lane.selectedGeneralServerSuites, [
318+
"server/src/services/native-runtime/native-codex-runner.integration.test.ts",
319+
]);
320+
});
321+
322+
test("shard flags are rejected for the native-runner group", () => {
323+
const result = dryRun(["--mode", "general", "--group", "general-server-native-runner", "--shard-index", "0", "--shard-count", "2"]);
324+
assert.notEqual(result.status, 0, "the native-runner lane is a single suite and must not accept shard flags");
325+
});
326+
327+
// The PR-side exclusion above is safe only while the wiring it assumes holds:
328+
// pr.yml (the caller whose name reusable pr-trusted.yml jobs see as
329+
// GITHUB_WORKFLOW) is named PR, the sharded vitest lanes partition cleanly
330+
// with exactly one final shard, and that lane's package script routes through
331+
// the wrapper that runs the native-runner group.
332+
test("the PR workflow wiring for the native-runner lane holds", () => {
333+
const prWorkflow = readFileSync(path.join(repoRoot, ".github/workflows/pr.yml"), "utf8");
334+
assert.match(prWorkflow, /^name: PR$/m,
335+
"renaming pr.yml silently moves the native-runner suite back into the uncached server shards");
336+
337+
const trustedWorkflow = readFileSync(path.join(repoRoot, ".github/workflows/pr-trusted.yml"), "utf8");
338+
const lanes = [...trustedWorkflow.matchAll(/command: test:typescript:vitest --shard=(\d+)\/(\d+)/g)]
339+
.map((match) => [Number(match[1]), Number(match[2])]);
340+
assert.ok(lanes.length > 0, "expected sharded test:typescript:vitest lanes in pr-trusted.yml");
341+
assert.equal(new Set(lanes.map(([, count]) => count)).size, 1, "vitest lanes must agree on the shard count");
342+
const shardCount = lanes[0][1];
343+
assert.deepEqual(
344+
lanes.map(([index]) => index).sort((left, right) => left - right),
345+
Array.from({ length: shardCount }, (_, index) => index + 1),
346+
"vitest lanes must cover every shard exactly once",
347+
);
348+
assert.equal(lanes.filter(([index, count]) => index === count).length, 1,
349+
"exactly one final vitest shard carries the native-runner group");
350+
351+
const runnerPackage = JSON.parse(
352+
readFileSync(path.join(repoRoot, "packages/paperclip-runner/package.json"), "utf8"),
353+
);
354+
assert.equal(runnerPackage.scripts["test:typescript:vitest"], "node ./scripts/run-pr-vitest-lane.mjs");
355+
});
356+
357+
const laneWrapper = path.join(repoRoot, "packages/paperclip-runner/scripts/run-pr-vitest-lane.mjs");
358+
359+
function wrapperPlan(args, envOverrides = {}) {
360+
const result = spawnSync(process.execPath, [laneWrapper, ...args, "--dry-run"], {
361+
cwd: repoRoot,
362+
encoding: "utf8",
363+
env: workflowEnv(envOverrides),
364+
});
365+
assert.equal(result.status, 0, `expected wrapper dry run to succeed for ${args.join(" ")}: ${result.stderr}`);
366+
return JSON.parse(result.stdout);
367+
}
368+
369+
test("the PR vitest lane wrapper runs the native-runner group exactly on the final PR shard", () => {
370+
for (const [args, envOverrides, expected] of [
371+
[["--shard=1/2"], { GITHUB_WORKFLOW: "PR" }, false],
372+
[["--shard=2/2"], { GITHUB_WORKFLOW: "PR" }, true],
373+
[[], { GITHUB_WORKFLOW: "PR" }, true],
374+
[["--shard=2/2"], { GITHUB_WORKFLOW: "Release" }, false],
375+
[["--shard=2/2"], {}, false],
376+
[[], {}, false],
377+
]) {
378+
const plan = wrapperPlan(args, envOverrides);
379+
assert.equal(plan.runNativeRunnerGroup, expected,
380+
`args ${JSON.stringify(args)} env ${JSON.stringify(envOverrides)}`);
381+
const commands = plan.plannedCommands.map((planned) => planned.args.join(" "));
382+
assert.ok(commands.some((command) => command.startsWith(`exec vitest run${args.length ? ` ${args.join(" ")}` : ""}`)),
383+
"the wrapper must pass shard flags through to vitest");
384+
assert.equal(
385+
commands.some((command) => command.includes("--group general-server-native-runner")),
386+
expected,
387+
);
388+
}
389+
390+
const malformed = spawnSync(process.execPath, [laneWrapper, "--shard=nonsense", "--dry-run"], {
391+
cwd: repoRoot,
392+
encoding: "utf8",
393+
env: workflowEnv({ GITHUB_WORKFLOW: "PR" }),
394+
});
395+
assert.notEqual(malformed.status, 0, "a malformed shard flag must fail rather than guess a lane");
396+
});
397+
268398
const lineShardFile = path.join(repoRoot, "server/src/__tests__/chat-channels.integration.test.ts");
269399
const caseAt = (line, name) => ({ name, file: lineShardFile, projectName: "@paperclipai/server", location: { line, column: 3 } });
270400

0 commit comments

Comments
 (0)