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. > - GitHub Actions builds the Docker images that ship Paperclip, and downstream deployments consume the `-cloud` image variant on every master merge. > - PR #12769 slimmed the Docker build context with a broad `.dockerignore` block for `packages/paperclip-runner`, and the block also removed three files the image build itself reads. > - The image build re-runs the runner's generated-file drift checks, so it found no committed capability contract in the context and failed on every master commit after the merge. > - PR CI never runs those checks against the Docker context, so the pull request stayed green and the breakage only appeared post-merge, on every image build. > - This pull request restores the three files with narrow `.dockerignore` exceptions and adds a PR CI job that runs the drift checks against the exact Docker build context. > - The benefit is that image publishing works again now, and the next context-slimming regression fails the pull request instead of every post-merge image build. ## Linked Issues or Issue Description Refs #12769 (the context-slimming change that exposed this) and #12608 (which committed the generated contract outputs the image build checks). **What happened?** Every `Docker` workflow run on master failed from 2026-09-04 12:58Z onward, in both the `build-and-push` and `build-and-push-cloud` jobs. The failing step reported `Generated contract drift: generated/capability/capability-contract.md` from `check:capability-contract` inside `pnpm --filter @paperclipai/server build`. The committed contract file is current — regeneration on a full checkout is a no-op. The file was simply absent from the build context: the new `packages/paperclip-runner/**/*.md` ignore rule strips the committed drift-check outputs (`generated/capability/capability-contract.md`, `generated/capability/downstream-handoff.md`), and the `packages/paperclip-runner/docs` rule also strips `docs/capability-contract.md`, which `check:capability-inventory` reads next in the chain. No cloud image published for eight hours, which stalled every downstream deployment that consumes the canary images. **Expected behavior** The Docker build context must contain every file the image build reads, and a change that removes one must fail the pull request that introduces it, not every image build after the merge. **Steps to reproduce** 1. Check out master at any commit from `af3023f1` onward. 2. Run `docker buildx build -f .github/docker-context-checks.Dockerfile .` (the probe added by this PR), or start the real `Docker` workflow build. 3. Observe `Generated contract drift: generated/capability/capability-contract.md` — while `node packages/paperclip-runner/scripts/generate-capability-contract.mjs --check` passes on the same checkout outside Docker. **Paperclip version or commit** `d593463ab` (master tip at diagnosis time; first failing commit `af3023f1`). **Deployment mode** GitHub Actions image builds (`docker.yml`), consumed by managed cloud deployments. ## What Changed - `.dockerignore`: narrow exceptions (last match wins) re-include the committed drift-check outputs (`!packages/paperclip-runner/generated/**`) and the inventory check's documentation input (`!packages/paperclip-runner/docs/capability-contract.md`). Every other exclusion from #12769 stays: no crate declares an explicit `[[test]]` target, so cargo builds without the `tests` directories, and the image build chain never runs the excluded smoke scripts. - `.github/docker-context-checks.Dockerfile` (new): a small probe that COPYs the real build context — identical `.dockerignore` semantics — and runs the dependency-independent drift checks inside it (`generate-capability-contract.mjs --check`, `check-capability-inventory.mjs`). ajv installs in an isolated directory for schema validation only; codegen checks such as `generate-protocol-schema-module` stay out because their emitted bytes vary with the ajv release and would raise false drift alarms outside the locked dependency tree. - `.github/workflows/pr-trusted.yml`: new `docker_context_integrity` job builds the probe on every full-CI pull request, and the existing `verify` aggregate now requires its result, so the guard gates merges through the same required check as the other lanes. - Activation note: `pr.yml` pins `pr-trusted.yml` by commit SHA, so the new job starts gating pull requests after the usual follow-up `ci: activate ...` pin bump once this merges. The `.dockerignore` fix needs no activation — `docker.yml` reads it directly, so image builds recover on the first master commit after this merges. ## Verification - `docker buildx build -f .github/docker-context-checks.Dockerfile .` on master (before the `.dockerignore` fix): fails with the exact production error, `Generated contract drift: generated/capability/capability-contract.md`. - Same command with the `.dockerignore` exceptions applied: passes, which also proves BuildKit honors the `!` exceptions, including the file inside the excluded `docs` directory. - `node scripts/generate-capability-contract.mjs --check` on a full checkout: passes both before and after, which confirms the committed contract was never stale — only missing from the context. - Static sweep of every script in the image build chain (`build`, `build:typescript` and their `check:*` steps) against the ignore rules: the three restored files are the only build inputs the #12769 block strips. - YAML for `pr-trusted.yml` lints clean. ## Risks - Low. The `.dockerignore` exceptions only re-add three committed files to the build context; image contents do not change otherwise. - The probe job adds one context transfer and two Node scripts per full-CI pull request run (about one to two minutes, no dependency install beyond one isolated ajv package). - The `verify` aggregate now also requires the new job, mirroring the existing pattern for the other lanes; on non-full-CI runs the job skips and `verify` asserts the skip, unchanged from how the other lanes behave. - The new job only takes effect for pull requests after a follow-up pin bump in `pr.yml` (same two-step flow as every `pr-trusted.yml` change). > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used Claude Fable 5 (Anthropic, model id `claude-fable-5`), extended thinking, agentic tool use in Claude Code: GitHub Actions log forensics to isolate the failing check, static analysis of the build-chain scripts against the ignore rules, and local docker buildx runs to reproduce the failure and verify the fix. ## 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 (the docker probe, both failing-before and passing-after; the drift checks themselves on a full checkout) - [x] I have added or updated tests where applicable (the probe IS the regression test for this class) - [x] I have updated relevant documentation to reflect my changes (inline comments in `.dockerignore` and the probe explain the invariant) - [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
337 lines
14 KiB
JavaScript
337 lines
14 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 { 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 SHARD_COUNT = 3;
|
|
|
|
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 readPinnedTrustedPrWorkflow() {
|
|
const caller = readFileSync(prCallerWorkflow, "utf8");
|
|
const pin = caller.match(
|
|
/uses: paperclipai\/paperclip\/\.github\/workflows\/pr-trusted\.yml@([0-9a-f]{40})/,
|
|
);
|
|
assert.ok(pin, "pr.yml must call the trusted workflow at a full commit SHA");
|
|
|
|
const result = spawnSync("git", ["show", `${pin[1]}:${trustedPrWorkflowPath}`], {
|
|
cwd: repoRoot,
|
|
encoding: "utf8",
|
|
});
|
|
assert.equal(result.status, 0, `cannot read the pinned trusted workflow: ${result.stderr}`);
|
|
return result.stdout;
|
|
}
|
|
|
|
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(trustedPrWorkflow, "utf8");
|
|
const match = workflow.match(
|
|
/ - name: Select stacked PR CI scope[\s\S]*? run: \|\n([\s\S]*?)\n\n policy:/,
|
|
);
|
|
assert.ok(match, "trusted workflow must define the stacked PR 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();
|
|
const weights = Array.from({ length: SHARD_COUNT }, (_, index) =>
|
|
selectE2eShard(specs, index, SHARD_COUNT, durations).reduce((sum, file) => sum + (durations[file] ?? 0), 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] ?? 0));
|
|
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 at an immutable SHA", () => {
|
|
assert.ok(readPinnedTrustedPrWorkflow().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 = readPinnedTrustedPrWorkflow();
|
|
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("the trusted PR workflow limits full CI to merge-relevant stack layers", () => {
|
|
const workflow = readFileSync(trustedPrWorkflow, "utf8");
|
|
const jobs = readWorkflowJobs(workflow);
|
|
const gate = jobs.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",
|
|
"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, 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"/);
|
|
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 = readPinnedTrustedPrWorkflow();
|
|
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", () => {
|
|
// Implementation PRs validate the workflow under development here. The
|
|
// caller remains pinned to the last merged trusted SHA until a separate
|
|
// activation PR advances it, so unmerged PR code never runs on trusted
|
|
// infrastructure.
|
|
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",
|
|
);
|
|
assert.match(
|
|
workflow,
|
|
/- name: Setup Node\.js\n uses: actions\/setup-node@[0-9a-f]+[^\n]*\n with:\n node-version: 24\n cache: pnpm/,
|
|
"the policy job must restore the pnpm cache before dependency resolution",
|
|
);
|
|
assert.match(
|
|
workflow,
|
|
/pnpm install --resolution-only --ignore-scripts --no-frozen-lockfile/,
|
|
"the policy job must resolve the complete merge tree without rewriting platform metadata",
|
|
);
|
|
assert.match(
|
|
workflow,
|
|
/cmp -s "\$RUNNER_TEMP\/pnpm-lock\.before\.yaml" pnpm-lock\.yaml/,
|
|
"the policy job must upload a lockfile only when regeneration changed it",
|
|
);
|
|
|
|
const restoreSteps = workflow.match(
|
|
/- name: Restore regenerated PR lockfile \(if policy uploaded one\)\n if: needs\.policy\.outputs\.lockfile_regenerated == '1'/g,
|
|
) ?? [];
|
|
assert.equal(restoreSteps.length, 6, "every downstream install job must restore a required regenerated artifact");
|
|
assert.doesNotMatch(
|
|
workflow,
|
|
/- name: Restore regenerated PR lockfile \(if policy uploaded one\)[\s\S]{0,220}continue-on-error:/,
|
|
"a missing artifact must fail after the policy job says it uploaded one",
|
|
);
|
|
});
|