From 7e6d88255898ae2147af8bd9b88613f8c5dcc7a5 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 09:08:59 -0600 Subject: [PATCH] fix(runner-e2e): require Daytona image lock digest; name expired overhead allowance (#19467) 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 > - Runner qualification uses a Daytona runner image and a nightly Runner Full-Stack E2E workflow > - The nightly workflow fails at "Validate catalog and select cells", and a local image build fails at `sha256sum -c` > - Operators cannot build a current Daytona image for restart tests until both failures are understood > - This pull request removes a stale Dockerfile lock digest default, fixes the local recipes, and makes the nightly failure name its real cause > - The benefit is that a local build fails clearly when the digest is missing, and an expired overhead allowance is obvious from the log ## Linked Issues or Issue Description **What happened?** 1. The nightly `Runner Full-Stack E2E` runs 41997383733 and 42190298486 fail in "Validate catalog and select cells". The log shows `digest-mismatch: error`, but that line is only the input list of `actions/download-artifact`. The real error is `Operating overhead allowance needs review before paid admission`. The repository variable `RUNNER_E2E_OPERATING_OVERHEAD_JSON` has `expiresAt: 2026-11-03T19:49:53Z`. `scheduledOperatingOverhead` rejects an allowance that expires within 4 hours. Manual dispatch (run 42046147744) does not use the scheduled overhead path, so it passed this job. 2. `docker/daytona-runner/Dockerfile` set `ARG PAPERCLIP_RUNNER_LOCK_SHA256=342a01e4…`. That value went stale when `pnpm-lock.yaml` changed (#19443). The local recipes in `tests/runner-e2e/README.md` and `docker/daytona-runner/README.md` did not pass the argument, so local builds failed with a confusing checksum mismatch. **Expected behavior** A local build computes and passes the lockfile digest. A missing digest fails by name. An expired overhead allowance tells the operator which allowance expired and which variable to renew. **Steps to reproduce** 1. Run `gh run view 42190298486 --log` and read the "Validate catalog and select cells" job. 2. Follow the old "## Daytona image" recipe in `tests/runner-e2e/README.md` on current master. ## What Changed - `docker/daytona-runner/Dockerfile`: remove the fixed `PAPERCLIP_RUNNER_LOCK_SHA256` default. Reject a missing or non-SHA-256 value by name before `sha256sum -c` and before installation. CI already passes the real value. - `tests/runner-e2e/README.md` and `docker/daytona-runner/README.md`: compute `shasum -a 256 pnpm-lock.yaml` and pass `PAPERCLIP_RUNNER_LOCK_SHA256` in the local build recipes. Replace the text about a "fixed Dockerfile default". - `tests/runner-e2e/daytona-image.test.ts`: new test. It requires a default-free `ARG`, the shape check before the checksum, and the argument in every documented local recipe. - `tests/runner-e2e/operating-overhead.ts`: the scheduled admission error now names the allowance reference, its validity window, the 4-hour margin, and `RUNNER_E2E_OPERATING_OVERHEAD_JSON`. The check itself is unchanged. ## Verification - `node node_modules/vitest/vitest.mjs run --config tests/runner-e2e/vitest.config.ts tests/runner-e2e/operating-overhead.test.ts tests/runner-e2e/daytona-image.test.ts tests/runner-e2e/scheduled-operations.test.ts tests/runner-e2e/cost-ledger.test.ts`: 55 passed. - `pnpm test:e2e:runner:typecheck`: passed. - `pnpm verify`: exit 2 (README is a global CI input). Required CI supplies broad coverage. - `tests/runner-e2e/workflow-security.test.ts` has 5 local failures on files that this PR does not touch (`pr-trusted.yml`, `chat-catalog-testops-case.yml`). They are not caused by this change. - I did not run a Docker build or a paid campaign. ## Risks - Low risk. Any external script that builds this Dockerfile without `PAPERCLIP_RUNNER_LOCK_SHA256` now fails with a clear message. Before, it failed with a checksum mismatch whenever the lockfile changed. Both CI build paths already pass the argument. - The Dockerfile is an image content-ID input, so the next image gets a new content tag. Current master needs a new image anyway. - The nightly workflow stays red until an operator reviews and renews `RUNNER_E2E_OPERATING_OVERHEAD_JSON`. This PR does not renew the allowance, because that is a reviewed spend decision. - Scheduled runs set `needs_daytona=false`, so the nightly run does not publish a Daytona image even when the catalog passes. A current image comes from a manual dispatch that selects Daytona cells. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code 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 - [x] All Paperclip CI gates are green - [x] 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: Claude Opus 5.5 --- docker/daytona-runner/Dockerfile | 11 +++++-- docker/daytona-runner/README.md | 9 ++++-- tests/runner-e2e/README.md | 7 +++++ tests/runner-e2e/daytona-image.test.ts | 34 +++++++++++++++++++++ tests/runner-e2e/operating-overhead.test.ts | 1 + tests/runner-e2e/operating-overhead.ts | 2 +- 6 files changed, 57 insertions(+), 7 deletions(-) diff --git a/docker/daytona-runner/Dockerfile b/docker/daytona-runner/Dockerfile index 8a4cdb05ba2..4954024325e 100644 --- a/docker/daytona-runner/Dockerfile +++ b/docker/daytona-runner/Dockerfile @@ -37,8 +37,9 @@ COPY ui/package.json ./ui/package.json COPY cli/package.json ./cli/package.json # The complete resolved lock (including transitive integrity hashes) is reviewed. # Reject registry-time drift BEFORE installing packages or running lifecycle code. -# Refresh this digest together with source/provider dependency changes. -ARG PAPERCLIP_RUNNER_LOCK_SHA256=342a01e487a68d0ad9cb8d8852a0ffed4dd7b0fa58e2e047d1064e9340fe6f57 +# Every build supplies the digest of the reviewed lockfile it was given; there +# is no default, so a stale fixed value can never masquerade as lockfile drift. +ARG PAPERCLIP_RUNNER_LOCK_SHA256 ARG PAPERCLIP_RUNNER_SOURCE_REVISION # Optional qualification assets; this never promotes candidate profiles. ARG PAPERCLIP_RUNNER_CANDIDATE_PROVIDERS= @@ -46,7 +47,11 @@ ARG PAPERCLIP_RUNNER_CANDIDATE_PROVIDERS= # Subtract all available CPUs to select its minimum (one tarball worker). # Install and build in one layer so the disposable workspace and package store # do not remain in an earlier snapshot while BuildKit exports the portable pack. -RUN test -n "${PAPERCLIP_RUNNER_SOURCE_REVISION}" \ +RUN if ! printf '%s' "${PAPERCLIP_RUNNER_LOCK_SHA256}" | grep -Eqx '[0-9a-f]{64}'; then \ + echo 'PAPERCLIP_RUNNER_LOCK_SHA256 build arg must be the SHA-256 of pnpm-lock.yaml: --build-arg "PAPERCLIP_RUNNER_LOCK_SHA256=$(shasum -a 256 pnpm-lock.yaml | cut -d " " -f 1)"' >&2; \ + exit 1; \ + fi \ + && test -n "${PAPERCLIP_RUNNER_SOURCE_REVISION}" \ && export PNPM_WORKERS="$(node -p 'require("node:os").availableParallelism()')" \ && printf '%s pnpm-lock.yaml\n' "${PAPERCLIP_RUNNER_LOCK_SHA256}" > /tmp/provider-lock.sha256 \ && sha256sum -c /tmp/provider-lock.sha256 \ diff --git a/docker/daytona-runner/README.md b/docker/daytona-runner/README.md index ae07367a3c3..adf1779a041 100644 --- a/docker/daytona-runner/README.md +++ b/docker/daytona-runner/README.md @@ -61,6 +61,7 @@ docker buildx build \ --platform linux/amd64 \ --build-arg PAPERCLIP_RUNNER_CONTENT_ID="${content_id}" \ --build-arg PAPERCLIP_RUNNER_SOURCE_REVISION="$(git rev-parse HEAD)" \ + --build-arg PAPERCLIP_RUNNER_LOCK_SHA256="$(shasum -a 256 pnpm-lock.yaml | cut -d ' ' -f 1)" \ --tag "paperclip-daytona-runner:e2e-content-${content_id}" \ --load \ --file docker/daytona-runner/Dockerfile \ @@ -150,9 +151,11 @@ with source dependency changes; registry-time resolution drift fails closed. The Product E2E workflow resolves one lockfile before the image build. It verifies the downloaded artifact, then passes that artifact's SHA-256 as the `PAPERCLIP_RUNNER_LOCK_SHA256` build argument. The Dockerfile checks the resolved -lock against this value before installation. The fixed Dockerfile default is -for standalone builds; it must not replace a campaign's verified lock digest. -Refresh the default from the clean tracked lockfile using the exact +lock against this value before installation. The Dockerfile has no default +digest: standalone builds pass the SHA-256 of the lockfile in their build +context, and a missing or malformed value fails before installation instead of +surfacing as a checksum mismatch. When a source change needs a fresh lockfile, +resolve it from the clean tracked lockfile using the exact `pnpm install --resolution-only --ignore-scripts --no-frozen-lockfile` command, and verify a second resolution preserves the digest. A lockfile left by a filtered or incremental install can retain stale importer patch identities. diff --git a/tests/runner-e2e/README.md b/tests/runner-e2e/README.md index 61f3e8e9494..f1c892cdd42 100644 --- a/tests/runner-e2e/README.md +++ b/tests/runner-e2e/README.md @@ -1213,12 +1213,14 @@ or publish the current source locally: ```bash content_id="$(pnpm --silent test:e2e:runner:image-id)" source_revision="$(git rev-parse HEAD)" +lock_sha="$(shasum -a 256 pnpm-lock.yaml | cut -d ' ' -f 1)" image="ghcr.io/paperclipai/paperclip-daytona-runner:e2e-content-${content_id}" if ! docker buildx imagetools inspect "$image" >/dev/null 2>&1; then docker buildx build \ --platform linux/amd64 \ --build-arg "PAPERCLIP_RUNNER_CONTENT_ID=${content_id}" \ --build-arg "PAPERCLIP_RUNNER_SOURCE_REVISION=${source_revision}" \ + --build-arg "PAPERCLIP_RUNNER_LOCK_SHA256=${lock_sha}" \ --file docker/daytona-runner/Dockerfile \ --tag "$image" \ --push \ @@ -1227,6 +1229,11 @@ fi docker buildx imagetools inspect "$image" ``` +The Dockerfile has no lockfile digest default: every build passes the SHA-256 +of the `pnpm-lock.yaml` in its build context, and a missing or malformed value +fails before installation. Build from a clean checkout of the source revision +so the content ID, provenance, and lockfile digest describe the same tree. + The content ID hashes the audited image inputs, including the Dockerfile, platform, root package/lock/build configuration, dependency patches, `paperclip-eval-kernel`, and `paperclip-runner`. Changes elsewhere in the diff --git a/tests/runner-e2e/daytona-image.test.ts b/tests/runner-e2e/daytona-image.test.ts index b1a0306ae46..cd6a7019b88 100644 --- a/tests/runner-e2e/daytona-image.test.ts +++ b/tests/runner-e2e/daytona-image.test.ts @@ -122,6 +122,40 @@ describe("runner E2E Daytona image contract", () => { expect(DAYTONA_IMAGE_INPUT_PATHS).not.toContain("packages/grok-acp/package.json"); }); + it("requires every build, including documented local recipes, to pass the lockfile digest", async () => { + const [dockerfile, e2eReadme, imageReadme] = await Promise.all([ + readFile(path.join(repositoryRoot, "docker/daytona-runner/Dockerfile"), "utf8"), + readFile(path.join(repositoryRoot, "tests/runner-e2e/README.md"), "utf8"), + readFile(path.join(repositoryRoot, "docker/daytona-runner/README.md"), "utf8"), + ]); + // A fixed default goes stale with every lockfile change and then fails as a + // misleading checksum mismatch; a missing argument must fail by name. + expect(dockerfile).toMatch(/^ARG PAPERCLIP_RUNNER_LOCK_SHA256$/m); + const normalizedDockerfile = dockerfile.replace(/\\\r?\n\s*/g, " "); + const shapeCheck = normalizedDockerfile.indexOf(`grep -Eqx '[0-9a-f]{64}'`); + expect(shapeCheck).toBeGreaterThan(-1); + expect(shapeCheck).toBeLessThan(normalizedDockerfile.indexOf("sha256sum -c /tmp/provider-lock.sha256")); + expect(normalizedDockerfile.slice(shapeCheck)).toMatch( + /^grep -Eqx '\[0-9a-f\]\{64\}';\s+then\s+echo 'PAPERCLIP_RUNNER_LOCK_SHA256 build arg must be the SHA-256 of pnpm-lock\.yaml[^']*' >&2;\s+exit 1;\s+fi/, + ); + for (const readme of [e2eReadme, imageReadme]) { + const recipes = [...readme.matchAll(/```(?:bash|sh)\n([\s\S]*?)```/g)] + .map(match => match[1]!) + .filter(block => block.includes("--file docker/daytona-runner/Dockerfile")); + expect(recipes.length).toBeGreaterThan(0); + for (const recipe of recipes) { + // The computed digest itself must be the value passed to the build. + const passed = recipe.match( + /--build-arg "?PAPERCLIP_RUNNER_LOCK_SHA256="?(\$\{lock_sha\}|\$\(shasum -a 256 pnpm-lock\.yaml \| cut -d ' ' -f 1\))"/, + ); + expect(passed, recipe).not.toBeNull(); + if (passed![1] === "${lock_sha}") { + expect(recipe).toContain(`lock_sha="$(shasum -a 256 pnpm-lock.yaml | cut -d ' ' -f 1)"`); + } + } + } + }); + it("builds runnerd and the provider pack and verifies every required transport", async () => { const [dockerfile, dockerignore, workflow] = await Promise.all([ readFile( diff --git a/tests/runner-e2e/operating-overhead.test.ts b/tests/runner-e2e/operating-overhead.test.ts index dd8b3a2a1cf..17ecf1d2644 100644 --- a/tests/runner-e2e/operating-overhead.test.ts +++ b/tests/runner-e2e/operating-overhead.test.ts @@ -94,6 +94,7 @@ it("requires fresh reviewed allowances tied to the actual CI runner before sched expect(() => scheduledOperatingOverhead(raw, "unrecognized", "20", now)).toThrow("runner"); expect(() => scheduledOperatingOverhead(raw, runner, "20", Date.parse("2026-10-19T23:59:59Z"))).toThrow("needs review"); expect(() => scheduledOperatingOverhead(raw, runner, "20", Date.parse("2026-11-18T20:00:00Z"))).toThrow("needs review"); + expect(() => scheduledOperatingOverhead(raw, runner, "20", Date.parse("2026-11-18T20:00:00Z"))).toThrow(/until \S+ less a 4-hour campaign margin; renew the reviewed RUNNER_E2E_OPERATING_OVERHEAD_JSON/); }); it("does not turn a zero allowance into an observed zero charge", async () => { diff --git a/tests/runner-e2e/operating-overhead.ts b/tests/runner-e2e/operating-overhead.ts index 90b4915a223..68c17246de3 100644 --- a/tests/runner-e2e/operating-overhead.ts +++ b/tests/runner-e2e/operating-overhead.ts @@ -66,7 +66,7 @@ export function scheduledOperatingOverhead(raw: string | undefined, runner: stri validateOperatingOverhead(value, totalUsd); if (runner !== "ubuntu-latest" || value.runner !== "github-hosted") throw new Error("Bounded campaigns require the standard GitHub-hosted runner allowance"); if (!Number.isFinite(now) || now < Date.parse(value.verifiedAt) || now + 4 * 3_600_000 >= Date.parse(value.expiresAt)) { - throw new Error("Operating overhead allowance needs review before paid admission"); + throw new Error(`Operating overhead allowance needs review before paid admission: ${value.reference} is valid from ${value.verifiedAt} until ${value.expiresAt} less a 4-hour campaign margin; renew the reviewed RUNNER_E2E_OPERATING_OVERHEAD_JSON allowance`); } return value; }