Files
PaperClipAI/scripts/__tests__/e2e-shard.test.mjs
T
b226336762 ci: skip full CI for release-notes-only pull requests (#16207)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Each beta release ends with a pull request that adds one release
notes file under `releases/`
> - PR CI runs the full build, test, and e2e matrix for that pull
request
> - No build, test, or e2e lane reads `releases/v*.md` or
`releases/beta/v*.md`
> - The full matrix takes about 30 minutes, and lost runners or flaky
e2e specs can make it run again
> - This pull request lets the existing CI scope gate skip the heavy
lanes when a pull request changes only release notes
> - The benefit is that release notes pull requests get a green result
in minutes, and code pull requests keep full CI

## Linked Issues or Issue Description

**Issue type**

CI efficiency enhancement.

**Where is the issue?**

The `Select stacked PR CI scope` step in
`.github/workflows/ci-routing.yml`. This step sets the `full_ci` output
for PR CI.

**What's wrong?**

The step sets `full_ci=false` only for middle pull requests in a stack.
A pull request that changes only one release notes markdown file runs
the full matrix. For example, #16053 changed one file in
`releases/beta/`. Its CI ran two times: on the first run, 8 shards lost
their runners, and on the second run, one Playwright spec flaked and
needed a rerun.

**Suggested fix**

Read the pull request diff from the GitHub compare API in the scope
step. When every changed path is release notes, set `full_ci=false`. The
callers already support this value. I found no open pull request or
issue for this change.

## What Changed

- `ci-routing.yml`: the scope step (renamed to `Select PR CI scope`)
also sets `full_ci=false` when every path in the diff matches
`releases/v*.md` or `releases/beta/v*.md`.
- The step reads the diff from `compare/<event base SHA>...<event head
SHA>`. The SHAs come from the event, so a later push cannot change the
decision for this run.
- For a renamed file, the step checks the old path and the new path.
- The step keeps full CI when the event is not `pull_request`, a SHA is
malformed, the API call fails, the diff is empty, or the file list has
300 entries. The compare API can truncate a list at 300 entries.
- The step does not change the stacked pull request rules. It checks the
paths only when the stack rules select full CI.
- `ci-routing.test.mjs`: the GitHub stub now returns `files` for compare
calls. 17 new cases cover release notes paths, mixed diffs, renames,
non-markdown files, nested folders, empty and truncated lists, API
failure, non-PR events, malformed SHAs, and stack interaction.
- `e2e-shard.test.mjs`: the test now uses the new step name.
- `doc/DEVELOPING.md`: added a short description of the CI scope rules.

## Verification

- `node --test .github/scripts/tests/ci-routing.test.mjs
scripts/__tests__/e2e-shard.test.mjs`: 73 pass, 0 fail.
- `node --test '.github/scripts/tests/*.test.mjs'`: all pass.
- I ran the jq filter on real compare API responses. #16053 (one file in
`releases/beta/`) returns `true`. #16082 (10 files) returns `false`.
- After merge: open a pull request that changes only a
`releases/beta/v*.md` file. The `Select trusted runner` job shows the
notice `release notes only; full_ci=false`. The heavy lanes show as
skipped. The `policy`, `verify`, and `e2e` checks pass.

Note: `pr.yml` calls `pr-trusted.yml@master`, and that workflow calls
`ci-routing.yml@master`. This pull request does not change its own CI
run. The change applies after merge.

## Risks

- Low risk. The skip uses the existing `full_ci=false` path. The
`verify` and `e2e` aggregate checks already accept that value, and the
`policy` job always runs.
- A future job that reads release notes files would not run for
notes-only pull requests. Today, only the release workflows read these
files, and those workflows do not run on pull requests.
- The step now uses `GH_TOKEN` to call one more read-only API endpoint.
The workflow still does not check out or run pull request code, and its
permissions do not change.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic. Agentic coding with
tool use (shell, file edits, GitHub CLI).

## 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
- [ ] 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

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-14 13:16:34 -07:00

355 lines
15 KiB
JavaScript

import assert from "node:assert/strict";
import { spawnSync } from "node:child_process";
import { mkdtempSync, readFileSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
import { fileURLToPath } from "node:url";
import test from "node:test";
import { defaultSuiteWeight, loadShardDurations } from "../general-server-shard.mjs";
import { IGNORED_SPECS, listE2eSpecs, selectE2eShard } from "../e2e-shard.mjs";
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", "..");
const script = path.join(repoRoot, "scripts", "e2e-shard.mjs");
const durationsManifest = path.join(repoRoot, "scripts", "e2e-shard-durations.json");
const playwrightConfig = path.join(repoRoot, "tests", "e2e", "playwright.config.ts");
const prCallerWorkflow = path.join(repoRoot, ".github", "workflows", "pr.yml");
const trustedPrWorkflowPath = ".github/workflows/pr-trusted.yml";
const trustedPrWorkflow = path.join(repoRoot, trustedPrWorkflowPath);
const routingWorkflow = path.join(repoRoot, ".github", "workflows", "ci-routing.yml");
const SHARD_COUNT = 8;
function runShard(args) {
const result = spawnSync(process.execPath, [script, ...args], { cwd: repoRoot, encoding: "utf8" });
assert.equal(result.status, 0, `expected success for ${args.join(" ")}: ${result.stderr}`);
return result.stdout.trim().split(/\s+/).filter(Boolean);
}
function readTrustedPrWorkflow() {
const caller = readFileSync(prCallerWorkflow, "utf8");
assert.match(
caller,
/^\s+uses: paperclipai\/paperclip\/\.github\/workflows\/pr-trusted\.yml@master\s*$/m,
"pr.yml must call the trusted workflow from CODEOWNERS-protected master",
);
// Validate proposed workflow changes locally; CI executes the merged master version.
return readFileSync(trustedPrWorkflow, "utf8");
}
function readWorkflowJobs(workflow) {
const jobs = new Map();
let current = null;
for (const line of workflow.split("\n")) {
const header = /^ {2}([A-Za-z0-9_-]+):\s*$/.exec(line);
if (header) {
current = header[1];
jobs.set(current, []);
continue;
}
if (current && /^\S/.test(line)) current = null;
if (current) jobs.get(current).push(line);
}
for (const [id, lines] of jobs) jobs.set(id, lines.join("\n"));
return jobs;
}
function runStackScope(stack, prBaseRef) {
const workflow = readFileSync(routingWorkflow, "utf8");
const match = workflow.match(
/ - name: Select PR CI scope[\s\S]*? run: \|\n([\s\S]*?)\s*$/,
);
assert.ok(match, "routing workflow must define the PR CI scope script");
const script = match[1]
.split("\n")
.map((line) => line.replace(/^ {10}/, ""))
.join("\n");
const scratch = mkdtempSync(path.join(tmpdir(), "paperclip-stack-scope-"));
const output = path.join(scratch, "github-output");
try {
const result = spawnSync("bash", ["-c", script], {
encoding: "utf8",
env: {
...process.env,
GITHUB_OUTPUT: output,
PR_BASE_REF: prBaseRef,
STACK_JSON: JSON.stringify(stack),
},
});
assert.equal(result.status, 0, result.stderr);
return Object.fromEntries(
readFileSync(output, "utf8")
.trim()
.split("\n")
.map((line) => line.split("=")),
);
} finally {
rmSync(scratch, { recursive: true, force: true });
}
}
test("the e2e shards form a complete, non-overlapping partition", () => {
const specs = listE2eSpecs();
assert.ok(specs.length > 0, "expected a non-empty e2e spec set");
const shards = Array.from({ length: SHARD_COUNT }, (_, index) =>
runShard(["--shard-index", String(index), "--shard-count", String(SHARD_COUNT)]),
);
const combined = shards.flat();
assert.equal(combined.length, specs.length, "every spec must land on exactly one shard");
assert.deepEqual([...combined].sort(), [...specs].sort());
for (const shard of shards) {
assert.ok(shard.length > 0, "no shard may be empty — Playwright fails a run with no matching specs");
}
});
test("the ignored spec list matches playwright.config.ts testIgnore", () => {
const config = readFileSync(playwrightConfig, "utf8");
const match = config.match(/testIgnore:\s*\[([^\]]*)\]/);
assert.ok(match, "expected a testIgnore array in playwright.config.ts");
const configured = [...match[1].matchAll(/"([^"]+)"/g)].map((entry) => entry[1]);
assert.deepEqual([...configured].sort(), [...IGNORED_SPECS].sort());
});
test("the duration manifest only names specs that still exist", () => {
const durations = loadShardDurations(durationsManifest);
assert.ok(Object.keys(durations).length > 0, "expected a populated duration manifest");
const specs = new Set(listE2eSpecs());
for (const file of Object.keys(durations)) {
assert.ok(specs.has(file), `duration manifest names a spec that no longer runs: ${file}`);
}
});
test("the weighted partition keeps the shards close to balanced", () => {
const durations = loadShardDurations(durationsManifest);
const specs = listE2eSpecs();
// New specs use the scheduler's median estimate until measured durations exist.
const fallbackWeight = defaultSuiteWeight(durations);
const weights = Array.from({ length: SHARD_COUNT }, (_, index) =>
selectE2eShard(specs, index, SHARD_COUNT, durations).reduce((sum, file) => sum + (durations[file] ?? fallbackWeight), 0),
);
const heaviest = Math.max(...weights);
const total = weights.reduce((sum, weight) => sum + weight, 0);
// Round-robin/count-based sharding would strand the ~168s smoke-lab spec on
// one runner alongside other specs. Assert the weighted split stays within
// 15% of an even cut so a future spec-time regression surfaces here instead
// of on the PR critical path. A single indivisible spec (smoke-lab) can
// legitimately exceed the even cut on its own, so the bound is floored at
// the largest per-spec weight — the best any file-level partition can do.
const largestSpec = Math.max(...specs.map((file) => durations[file] ?? fallbackWeight));
const bound = Math.max((total / SHARD_COUNT) * 1.15, largestSpec);
assert.ok(
heaviest <= bound,
`heaviest shard ${heaviest}ms exceeds the balance bound (${bound}ms)`,
);
});
test("shard arguments are validated", () => {
for (const args of [
["--shard-index", "2", "--shard-count", "2"],
["--shard-index", "-1", "--shard-count", "2"],
["--shard-index", "0", "--shard-count", "0"],
]) {
const result = spawnSync(process.execPath, [script, ...args], { cwd: repoRoot, encoding: "utf8" });
assert.notEqual(result.status, 0, `expected failure for ${args.join(" ")}`);
}
});
test("pr.yml calls the trusted PR workflow from master", () => {
assert.ok(readTrustedPrWorkflow().length > 0);
});
test("the trusted PR workflow keeps a stable aggregate check named e2e over the shard matrix", () => {
// Branch protection requires a check literally named `e2e`. The shards run
// as `e2e shard (n/3)`, so the aggregate job below is what keeps the
// required-check contract intact — same pattern as the `verify` aggregate.
const workflow = readTrustedPrWorkflow();
const jobs = readWorkflowJobs(workflow);
const aggregate = jobs.get("e2e");
assert.ok(aggregate, "pr-trusted.yml must define an `e2e` job to satisfy branch protection");
assert.match(aggregate, /^ {4}name: e2e$/m, "the aggregate job must be named exactly `e2e`");
assert.match(aggregate, /^ {4}if: \$\{\{ always\(\) \}\}$/m, "the aggregate must run even when a shard fails");
assert.match(
aggregate,
/^ {4}needs: \[gate, (?:policy, )?e2e_shards\]$/m,
"the aggregate must depend on the runner gate, optional policy gate, and shard matrix",
);
assert.match(
aggregate,
/test "\$E2E_SHARDS_RESULT" = "success"/,
"the aggregate must fail unless every shard succeeded",
);
const shards = jobs.get("e2e_shards");
assert.ok(shards, "pr-trusted.yml must define the `e2e_shards` matrix job");
const matrixEntries = [
...shards.matchAll(
/^ {10}- shard_index: (?<shardIndex>\d+)\n {12}shard_count: (?<shardCount>\d+)\n {12}shard_label: (?<shardLabel>\d+\/\d+)$/gm,
),
].map((match) => ({
shardIndex: Number(match.groups.shardIndex),
shardCount: Number(match.groups.shardCount),
shardLabel: match.groups.shardLabel,
}));
assert.equal(matrixEntries.length, SHARD_COUNT, "the shard matrix must define exactly SHARD_COUNT entries");
assert.deepEqual(
matrixEntries.map((entry) => entry.shardIndex).sort((a, b) => a - b),
Array.from({ length: SHARD_COUNT }, (_, index) => index),
"the shard matrix must define each shard index exactly once",
);
for (const entry of matrixEntries) {
assert.equal(entry.shardCount, SHARD_COUNT, "each shard matrix entry must use the same SHARD_COUNT");
assert.equal(entry.shardLabel, `${entry.shardIndex + 1}/${SHARD_COUNT}`, "each shard label must match its index");
}
});
test("runner selection has reserved AWS capacity and executes metadata checks only", () => {
const workflow = readFileSync(routingWorkflow, "utf8");
const jobs = readWorkflowJobs(workflow.split(/^jobs:\n/m)[1]);
assert.deepEqual([...jobs.keys()], ["gate"]);
assert.match(jobs.get("gate"), /^ {4}runs-on: \$\{\{ vars\.AWS_CI_ENABLED[\s\S]*paperclip-ci-routing-x64/m);
assert.doesNotMatch(jobs.get("gate"), /uses:|actions\/checkout|secrets\./);
assert.match(workflow, /^ {6}runner:\n {8}value: \$\{\{ jobs\.gate\.outputs\.runner \}\}$/m);
assert.match(workflow, /^ {6}full_ci:\n {8}value: \$\{\{ jobs\.gate\.outputs\.full_ci \}\}$/m);
assert.doesNotMatch(workflow, /^ {2}(?:actions|contents|pull-requests): write$/m);
});
test("the trusted PR workflow limits full CI to merge-relevant stack layers", () => {
const workflow = readFileSync(trustedPrWorkflow, "utf8");
const jobs = readWorkflowJobs(workflow);
assert.match(jobs.get("gate"), /uses: paperclipai\/paperclip\/\.github\/workflows\/ci-routing\.yml@master/);
const gate = readWorkflowJobs(readFileSync(routingWorkflow, "utf8")).get("gate");
assert.match(gate, /^ {6}full_ci: \$\{\{ steps\.scope\.outputs\.full_ci \}\}$/m);
assert.match(gate, /STACK_JSON: \$\{\{ toJSON\(github\.event\.pull_request\.stack\) \}\}/);
assert.match(gate, /stack_position == stack_size/);
assert.match(gate, /"\$stack_base_ref" == "\$PR_BASE_REF"/);
for (const jobId of [
"typecheck_release_registry",
"general_tests",
"verify_paperclip_runner",
"build",
"verify_serialized_server",
"canary_dry_run",
"e2e_shards",
]) {
assert.match(
jobs.get(jobId),
/^ {4}if: \$\{\{ needs\.gate\.outputs\.full_ci == 'true' \}\}$/m,
`${jobId} must run only when the gate selects full CI`,
);
}
assert.doesNotMatch(
jobs.get("policy"),
/needs\.gate\.outputs\.full_ci/,
"the policy job must run on every PR layer",
);
const verify = jobs.get("verify");
assert.match(
verify,
/^ {4}needs: \[gate, policy, typecheck_release_registry, general_tests, verify_paperclip_runner, build, docker_context_integrity\]$/m,
);
assert.match(verify, /POLICY_RESULT: \$\{\{ needs\.policy\.result \}\}/);
assert.match(verify, /test "\$TYPECHECK_RELEASE_REGISTRY_RESULT" = "skipped"/);
assert.match(verify, /test "\$GENERAL_TESTS_RESULT" = "skipped"/);
// Runner verification must participate in the legacy aggregate required
// check, or a runner regression could be merged while `verify` succeeds.
assert.match(verify, /RUNNER_VERIFICATION_RESULT: \$\{\{ needs\.verify_paperclip_runner\.result \}\}/);
assert.match(verify, /test "\$RUNNER_VERIFICATION_RESULT" = "success"/);
assert.match(verify, /test "\$RUNNER_VERIFICATION_RESULT" = "skipped"/);
assert.match(verify, /test "\$BUILD_RESULT" = "skipped"/);
// Both halves of the docker-context lane's gating: the result must be
// wired into the aggregate's env AND asserted successful on full CI —
// dropping either would let `verify` pass after the lane fails.
assert.match(verify, /DOCKER_CONTEXT_INTEGRITY_RESULT: \$\{\{ needs\.docker_context_integrity\.result \}\}/);
assert.match(verify, /test "\$DOCKER_CONTEXT_INTEGRITY_RESULT" = "success"/);
assert.match(verify, /test "\$DOCKER_CONTEXT_INTEGRITY_RESULT" = "skipped"/);
const e2e = jobs.get("e2e");
assert.match(e2e, /^ {4}needs: \[gate, policy, e2e_shards\]$/m);
assert.match(e2e, /POLICY_RESULT: \$\{\{ needs\.policy\.result \}\}/);
assert.match(e2e, /false\) test "\$E2E_SHARDS_RESULT" = "skipped"/);
});
test("the stacked PR scope selector runs full CI only where intended", () => {
assert.equal(runStackScope(null, "master").full_ci, "true");
assert.equal(
runStackScope({ position: 11, size: 11, base: { ref: "master" } }, "stack-10").full_ci,
"true",
);
assert.equal(
runStackScope({ position: 1, size: 11, base: { ref: "master" } }, "master").full_ci,
"true",
);
assert.equal(
runStackScope({ position: 6, size: 11, base: { ref: "master" } }, "stack-5").full_ci,
"false",
);
assert.equal(
runStackScope({ position: "invalid", size: 11, base: { ref: "master" } }, "stack-5").full_ci,
"true",
);
});
test("the trusted PR workflow passes the shard's spec filter to Playwright without a literal --", () => {
// `pnpm run test:e2e -- $specs` forwards the literal separator to Playwright,
// so the specs after it are not applied as file filters.
const workflow = readTrustedPrWorkflow();
assert.ok(
!/pnpm run test:e2e --\s/.test(workflow),
"pr-trusted.yml must not insert a literal `--` between `pnpm run test:e2e` and the spec filter",
);
assert.match(
workflow,
/pnpm run test:e2e \$specs/,
"pr-trusted.yml e2e_shards must invoke `pnpm run test:e2e $specs`",
);
});
test("the trusted PR workflow regenerates stale stacked lockfiles", () => {
// Validate the proposed workflow here. The caller executes the merged master
// workflow; edits to this workflow take effect after code-owner review and merge.
const workflow = readFileSync(trustedPrWorkflow, "utf8");
assert.match(
workflow,
/policy:\n needs: \[gate\][\s\S]{0,160}timeout-minutes: 10/,
"the unconditional resolution step needs the same timeout headroom as the lockfile refresh workflow",
);
const policy = workflow.split(" policy:\n")[1].split(" typecheck_release_registry:\n")[0];
assert.doesNotMatch(
policy,
/cache: pnpm|uses: actions\/cache/,
"resolution-only policy must not restore or save a dependency store",
);
assert.match(
policy,
/pnpm install --resolution-only --ignore-scripts --no-frozen-lockfile/,
"the policy job must resolve the complete merge tree without rewriting platform metadata",
);
// Test lanes no longer wait on a policy-job artifact: each install step
// resolves a stale lockfile inline, so a manifest-changing or stacked PR
// still installs while the policy job validates resolution in parallel.
const fallbackInstalls = workflow.match(
/if ! pnpm install --frozen-lockfile; then\n[\s\S]{0,240}?pnpm install --resolution-only --ignore-scripts --no-frozen-lockfile\n\s+pnpm install --frozen-lockfile\n\s+fi/g,
) ?? [];
assert.equal(
fallbackInstalls.length,
7,
"every downstream install job must resolve a stale lockfile inline",
);
assert.doesNotMatch(
workflow,
/Restore regenerated PR lockfile|lockfile_regenerated|name: pr-lockfile/,
"the policy lockfile artifact chain must stay removed; it put the policy job on every lane's critical path",
);
});