From 19c76bfc3feed589639084efa8d3c780753d7fd3 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 11 Sep 2026 14:35:28 -0700 Subject: [PATCH] fix(ci): avoid empty pnpm caches from lockfile refresh (#13267) 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. > - CI installs dependencies before it verifies and builds cloud artifacts. > - Install jobs share a pnpm package-store cache with lockfile refresh. > - Lockfile refresh resolves versions without downloading packages. > - That job saved an empty cache before full install jobs could save theirs. > - This PR prevents lockfile refresh from publishing that empty entry. > - Full install jobs can then populate the cache and reuse dependencies. ## Linked Issues or Issue Description Refs #13259 for the related cloud verification cache work. No duplicate empty-cache fix was found. **What happened?** Refresh Lockfile run 34517514932 saved a 216-byte default-branch pnpm cache at 18:58:08 UTC on September 10. Full install jobs still restore that empty entry. The cache API reports 216 bytes for master and about 703 MB for populated entries with the same key and cache version in PR scopes. **Expected behavior** A job that installs dependencies should populate the shared package-store cache. **Steps to reproduce** 1. Run lockfile refresh with a new lockfile cache key. 2. Its resolution-only command leaves the package store empty. 3. The Node action saves the empty archive before a full install finishes. 4. Later jobs report a cache hit but download packages again. **Paperclip version or commit** Observed on master 6728e133f88efc5282decfaf8dd71c0fa0531fd4 and still present at a23ae894a5a267c3917b6f5201cf2aea5245fe80. **Deployment mode** GitHub Actions cloud verification and release workflows. ## What Changed - Disable package-manager caching in Refresh Lockfile. - Document how to remove the existing empty default-branch entry and verify a populated replacement. - Add regression coverage for explicit and automatic package-manager cache selection in a resolution-only job. ## Verification - actionlint and git diff checks pass. - All 175 existing workflow-script tests pass. Both new regression cases pass and fail against the original workflow, covering the explicit pnpm cache and automatic npm cache paths. This change adds no application behavior. - [The cache creator job](https://github.com/paperclipai/paperclip/actions/runs/34517514932/job/103006542158) logs a 216-byte upload under the same key still used by cloud verification. - The batch-wide local full typecheck and build passed. The local full test run reported 10,600 passed, 65 skipped, and 13 permission failures in unchanged runtime-skill suites. These checks were not repeated in this dependency-free worktree. Current-head Linux CI passes. The unchanged chat and browser suites passed on their single retry; all final checks are green. Greptile is 5/5 with all threads resolved. - After merge, delete only the existing empty master cache entry. Verify that a master install saves a populated archive and subsequent jobs reuse packages. Measure the net install-time change before claiming a latency gain. ## Risks - Lockfile resolution can require fresh registry metadata. It does not need a cached package store. - The existing empty cache must be removed once; this change prevents its recreation by this workflow. - Cache benefits vary with download speed and archive extraction time. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository tools, and code execution. The exact serving model ID and context window are not exposed by this environment. ## 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 — focused workflow checks pass; the batch-wide local test limitation is disclosed above - [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 --------- Co-authored-by: Paperclip --- .../tests/lockfile-refresh-cache.test.mjs | 29 +++++++++++++++ .github/workflows/refresh-lockfile.yml | 4 ++- doc/cloud-build-readiness.md | 36 +++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 .github/scripts/tests/lockfile-refresh-cache.test.mjs diff --git a/.github/scripts/tests/lockfile-refresh-cache.test.mjs b/.github/scripts/tests/lockfile-refresh-cache.test.mjs new file mode 100644 index 0000000000..2af8a57df0 --- /dev/null +++ b/.github/scripts/tests/lockfile-refresh-cache.test.mjs @@ -0,0 +1,29 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; + +const workflow = readFileSync(new URL("../../workflows/refresh-lockfile.yml", import.meta.url), "utf8"); +const nodeStep = workflow.split(" - name: Setup Node.js\n")[1]?.split(" - name:")[0]; +assert.ok(nodeStep, "the refresh workflow must set up Node"); +const input = (name) => nodeStep.match(new RegExp(`^ ${name}: (.+)$`, "m"))?.[1].trim(); + +// setup-node's explicit cache input enables a store cache independently of its +// automatic npm detection. Disabling only automatic detection is insufficient. +function cacheProvider(explicitCache, automaticCache, packageManager) { + if (explicitCache) return explicitCache; + if (automaticCache !== "false" && packageManager.startsWith("npm@")) return "npm"; + return undefined; +} + +for (const packageManager of ["pnpm@9.15.4", "npm@11.0.0"]) { + test(`resolution-only refresh cannot write a package-store cache (${packageManager})`, () => { + assert.match(workflow, /run: pnpm install --resolution-only --ignore-scripts --no-frozen-lockfile/); + assert.equal( + cacheProvider(input("cache"), input("package-manager-cache"), packageManager), + undefined, + "a metadata-only job must not claim the shared cache key with an empty store", + ); + // This is the original failure mode, even with automatic caching disabled. + assert.equal(cacheProvider("pnpm", "false", packageManager), "pnpm"); + }); +} diff --git a/.github/workflows/refresh-lockfile.yml b/.github/workflows/refresh-lockfile.yml index df9bd7abef..ecc0c73ebe 100644 --- a/.github/workflows/refresh-lockfile.yml +++ b/.github/workflows/refresh-lockfile.yml @@ -32,7 +32,9 @@ jobs: uses: actions/setup-node@v7 with: node-version: 24 - cache: pnpm + # Resolution-only installs do not populate the package store. Do not + # claim the shared cache key with an empty archive before full installs. + package-manager-cache: false - name: Refresh pnpm lockfile run: pnpm install --resolution-only --ignore-scripts --no-frozen-lockfile diff --git a/doc/cloud-build-readiness.md b/doc/cloud-build-readiness.md index 5c1b421a15..e6197ed595 100644 --- a/doc/cloud-build-readiness.md +++ b/doc/cloud-build-readiness.md @@ -186,3 +186,39 @@ The `release-typecheck-v1` cache is separate from Runner verification because those jobs compile different profiles. The pinned toolchain is selected before cache lookup. Workspace crates and installed cargo binaries are excluded, and all typechecks still execute. A missing or invalidated cache triggers compilation. + +### pnpm dependency store cache + +The Refresh Lockfile workflow does not cache the pnpm store. Its resolution-only +command does not download packages and can save an empty default-branch cache +before full install jobs finish. Jobs that install dependencies retain caching. + +After deploying this correction, remove any existing empty default-branch entry +for the current lockfile key. List cache IDs, branches, and archive sizes first: + +```sh +gh api --paginate 'repos/paperclipai/paperclip/actions/caches?ref=refs/heads/master&key=node-cache-Linux-x64-pnpm-&per_page=100' \ + --jq '.actions_caches[] | {id, ref, key, size_in_bytes}' +``` + +Match the key and upload size against the cache-creation job's logs. The +September 11 incident was cache ID `7559920987`, a 216-byte archive. This guarded +command deletes only that observed entry. It leaves a populated replacement or +an entry on another branch untouched, and does nothing if the old ID is absent: + +```sh +bad_cache_id=7559920987 +bad_cache_key=node-cache-Linux-x64-pnpm-c3096ecb02a34aaa9782baaadafcb731510e1dba10dd661618c3a2ee91e58fa5 +entries="$(gh api --paginate --slurp 'repos/paperclipai/paperclip/actions/caches?ref=refs/heads/master&per_page=100')" +if printf '%s\n' "$entries" | jq -e --argjson id "$bad_cache_id" --arg key "$bad_cache_key" ' + [.[].actions_caches[] | select(.id == $id)] | + length == 1 and .[0].ref == "refs/heads/master" and + .[0].key == $key and .[0].size_in_bytes == 216 +' >/dev/null; then + gh api --method DELETE "repos/paperclipai/paperclip/actions/caches/$bad_cache_id" +fi +``` + +A subsequent master install can populate the missing entry. Check the saved +archive size and package reuse in install logs; a cache hit alone does not prove +that the entry contains dependencies.