From bb920fb859f650fb0abb208c06d988c0d62e8d8b Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 4 Sep 2026 15:35:10 -0700 Subject: [PATCH] ci: keep the Docker build context complete and guard it on every PR (#12855) 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. > - 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 --- .dockerignore | 8 +++++ .github/docker-context-checks.Dockerfile | 43 ++++++++++++++++++++++++ .github/workflows/pr-trusted.yml | 34 ++++++++++++++++++- scripts/__tests__/e2e-shard.test.mjs | 11 +++++- 4 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 .github/docker-context-checks.Dockerfile diff --git a/.dockerignore b/.dockerignore index b8cad4152a..1b9aa11ca5 100644 --- a/.dockerignore +++ b/.dockerignore @@ -29,3 +29,11 @@ packages/paperclip-runner/**/*.test.ts packages/paperclip-runner/**/*.test.tsx packages/paperclip-runner/runner/crates/*/tests packages/paperclip-runner/scripts/*-smoke.mjs +# Exceptions (last match wins): the image build re-runs the runner's +# generated-file drift checks, so their committed outputs and inputs must +# survive the slimming above. 2026-09-04: the *.md rule stripped the +# committed capability contract out of the context and every image build +# on master failed its drift check — .github/docker-context-checks.Dockerfile +# now guards this in PR CI. +!packages/paperclip-runner/generated/** +!packages/paperclip-runner/docs/capability-contract.md diff --git a/.github/docker-context-checks.Dockerfile b/.github/docker-context-checks.Dockerfile new file mode 100644 index 0000000000..48f5921d9c --- /dev/null +++ b/.github/docker-context-checks.Dockerfile @@ -0,0 +1,43 @@ +# Runs the runner's generated-file drift checks against the EXACT build +# context the image builds see — same .dockerignore semantics — so a +# context-slimming change that strips a committed build input fails the +# pull request instead of every post-merge image build. (2026-09-04: a new +# `packages/paperclip-runner/**/*.md` ignore rule stripped the committed +# capability contract out of the context; every Docker build on master then +# failed its drift check, and no cloud image published for eight hours +# while PR CI stayed green.) +# +# Only checks whose compared output is independent of dependency versions +# run here: ajv is installed for schema VALIDATION only (pinned to the +# runner's declared range), while codegen checks like +# generate-protocol-schema-module stay out — their emitted bytes vary with +# the ajv release, so running them against a fresh install would raise +# false drift alarms. Those still run inside the real image build, which +# installs the locked dependency tree; the existence assertions below keep +# their committed inputs and outputs covered by this probe regardless. +# +# node:24-slim — the runner requires Node >= 24.11 and the production +# image builds on Node 24; the digest pin keeps the security gate's own +# runtime immutable. +FROM node:24-slim@sha256:ba849c60be29959425b8734d57b8b4b7d56f98edd9504c9af091d5281095a71e +WORKDIR /context +COPY . . +# Committed artifacts the image build reads whose drift checks cannot run +# here (they need the locked dependency tree or compiled dist/). Existence +# in the context is the property this probe guards; content correctness is +# the real build's job. If a path is intentionally removed from the repo, +# update this list in the same PR. +RUN test -f packages/paperclip-runner/generated/capability/semantic-tool-contracts.json \ + && test -f packages/paperclip-runner/generated/semantic-action-catalog.json \ + && test -f packages/paperclip-runner/spec/evals/stress-workflow-traceability.json \ + && test -d packages/paperclip-runner/protocol/fixtures/replay +# ajv is installed in an isolated directory (the runner's own package.json +# uses workspace: ranges npm cannot install from) and symlinked in so ESM +# resolution finds it from the scripts' location. +RUN AJV_RANGE="$(node -p "require('/context/packages/paperclip-runner/package.json').dependencies.ajv")" \ + && mkdir /probe-deps && cd /probe-deps && npm init -y >/dev/null \ + && npm install --ignore-scripts --no-audit --no-fund "ajv@${AJV_RANGE}" \ + && ln -s /probe-deps/node_modules /context/packages/paperclip-runner/node_modules \ + && cd /context/packages/paperclip-runner \ + && node scripts/generate-capability-contract.mjs --check \ + && node scripts/check-capability-inventory.mjs diff --git a/.github/workflows/pr-trusted.yml b/.github/workflows/pr-trusted.yml index cc04e18fc4..4bcaaa279a 100644 --- a/.github/workflows/pr-trusted.yml +++ b/.github/workflows/pr-trusted.yml @@ -513,11 +513,40 @@ jobs: pnpm test:run:general -- --group '${{ matrix.group }}' fi + docker_context_integrity: + name: Docker context integrity + needs: gate + if: ${{ needs.gate.outputs.full_ci == 'true' }} + runs-on: ${{ needs.gate.outputs.runner }} + timeout-minutes: 15 + + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + + # Not every runner the gate can select ships the Buildx plugin — + # the image-build workflows set it up explicitly, so this lane does + # too rather than failing before it checks anything. + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4 + + # Same .dockerignore semantics as the real image builds: a + # context-slimming change that strips a committed build input must + # fail here, on the pull request, instead of failing every + # post-merge image build. (2026-09-04: a new **/*.md ignore rule + # stripped the committed capability contract out of the context; + # every Docker build on master failed its drift check and no cloud + # image published for eight hours while PR CI stayed green.) + - name: Run generated-file drift checks against the Docker build context + run: docker buildx build --file .github/docker-context-checks.Dockerfile . + verify: # Preserve the legacy required-check name while the underlying work runs in parallel. name: verify if: ${{ always() }} - needs: [gate, policy, typecheck_release_registry, general_tests, build] + needs: [gate, policy, typecheck_release_registry, general_tests, build, docker_context_integrity] runs-on: ${{ needs.gate.outputs.runner }} timeout-minutes: 5 @@ -529,6 +558,7 @@ jobs: TYPECHECK_RELEASE_REGISTRY_RESULT: ${{ needs.typecheck_release_registry.result }} GENERAL_TESTS_RESULT: ${{ needs.general_tests.result }} BUILD_RESULT: ${{ needs.build.result }} + DOCKER_CONTEXT_INTEGRITY_RESULT: ${{ needs.docker_context_integrity.result }} run: | test "$POLICY_RESULT" = "success" case "$FULL_CI" in @@ -536,11 +566,13 @@ jobs: test "$TYPECHECK_RELEASE_REGISTRY_RESULT" = "success" test "$GENERAL_TESTS_RESULT" = "success" test "$BUILD_RESULT" = "success" + test "$DOCKER_CONTEXT_INTEGRITY_RESULT" = "success" ;; false) test "$TYPECHECK_RELEASE_REGISTRY_RESULT" = "skipped" test "$GENERAL_TESTS_RESULT" = "skipped" test "$BUILD_RESULT" = "skipped" + test "$DOCKER_CONTEXT_INTEGRITY_RESULT" = "skipped" ;; *) echo "Invalid full_ci decision: $FULL_CI" >&2 diff --git a/scripts/__tests__/e2e-shard.test.mjs b/scripts/__tests__/e2e-shard.test.mjs index ae56c9c411..1fed746a27 100644 --- a/scripts/__tests__/e2e-shard.test.mjs +++ b/scripts/__tests__/e2e-shard.test.mjs @@ -241,11 +241,20 @@ test("the trusted PR workflow limits full CI to merge-relevant stack layers", () ); const verify = jobs.get("verify"); - assert.match(verify, /^ {4}needs: \[gate, policy, typecheck_release_registry, general_tests, build\]$/m); + 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);