Skip to content

ci(setup-pnpm): split the Corepack cache into restore + guarded save so no job can cache an unpinned pnpm - #16213

Merged
baozhoutao merged 2 commits into
mainfrom
claude/issue-16012-setup-pnpm-corepack-store-pin
Sep 6, 2026
Merged

ci(setup-pnpm): split the Corepack cache into restore + guarded save so no job can cache an unpinned pnpm#16213
baozhoutao merged 2 commits into
mainfrom
claude/issue-16012-setup-pnpm-corepack-store-pin

Conversation

@claude

@claude claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #16012

Closes the channel half of the 2026-09-05 incident. .github/actions/setup-pnpm cached COREPACK_HOME with the full actions/cache@v6, whose post-job save writes back whatever any step of the job left in that store — including a package manager nobody pinned — under the pinned key, where every later job restores it.

The trigger half is already gone from main: PR #16002 (merged 2026-09-05T19:14:38Z) pins the regen self-test fixture to the root manifest's packageManager, held there by its own self-test case at scripts/check-regen-pending.mjs:1107 (re-verified on origin/main @ 159dbad). Sibling card #15990 is no longer open. So this PR is not chasing a live trigger: it removes the write path that turned one job's mistake into every later job's problem, and makes the store's contents audible when it happens again.

The shape, and why this one

Three shapes were on the table; I took the third. Plain cache/restore closes the channel by making the store unwritable — but then nothing ever populates it either, so every job is cold forever and the action's whole reason for existing goes with it (its header, lines 26-36: the remedy for the undici/ECONNRESET crashes was to stop calling the registry on the happy path, with the retry as the cold-cache backstop only). Measured against pnpm check:pnpm-acquisition: 27 of 28 pnpm-acquiring CI jobs come through this composite, so "always cold" means 27 registry fetches per build — precisely the exposure the action was written to delete. An assertion with cache@v6 left in place does not satisfy the ruling either, for the ordering reason below. So: cache/restore at the top, cache/save at the bottom, and an assertion between them that fails loudly and, by being the step immediately before the save, also guards it.

The ordering fact

actions/cache's save is a post-job step: it runs after every step of the job. An assertion placed after setup therefore observes only the store the job restored — it catches the previous job's poisoning and can do nothing about this job's, because the save happens after it has already passed. Moving the save into the step sequence, right after this action materialised the pin, is what makes a job structurally unable to donate an unpinned manager: at that point the store's contents are still deterministic, and the save carries no always(), so a store that failed the assertion is never written back.

The assertion still runs on a cache hit, where nothing is saved. That is deliberate and it is the half triage asked for: a restored store holding an unpinned manager means the channel is armed again somewhere, and the job that restored it now says so by name instead of failing something innocent three jobs later.

Measured (corepack 0.34.6 on Node 22.22.2 — the Corepack Node 22 ships, which is what the runners pin via .nvmrc)

  • corepack install in this repo against a scratch COREPACK_HOME produces exactly v1/pnpm/10.31.0/ and no lastKnownGood.json. This is the card's A2 assumption, re-measured and confirmed.
  • One pnpm invocation from a directory carrying no packageManager, against that same store, adds v1/pnpm/12.3.4/ — the registry's latest — and writes lastKnownGood.json naming 12.3.4. Store grows 22 MB to 71 MB. The poisoning is reproduced end to end, not assumed.
  • On that pnpm 12.3.4: pnpm -s SOME_SCRIPT prints error: unexpected argument '-s' found and exits 2, the card's claim, reproduced. (Worth recording precisely: pnpm -s --version exits 0 — the version flag short-circuits — so the script form is the discriminator, not -s alone.)
  • Store layout is COREPACK_HOME/v1/MANAGER/VERSION/; lastKnownGood.json sits at the store root. The assertion reads that layout rather than guessing it.

The assertion, exercised

The run: body was extracted from action.yml by parsing the YAML, so the bytes tested are the bytes shipped, then run against scratch stores. Exit codes captured before any pipe:

leg store exit says
green pinned version only 0 Corepack store holds exactly pnpm@10.31.0, and no lastKnownGood.json.
red pinned + the real Corepack-resolved 12.3.4 + lastKnownGood.json 1 names pnpm@12.3.4 and the sticky file
red pinned + lastKnownGood.json only 1 names the file
red empty store — pin not locatable 1 "Corepack's layout changed and this assertion is no longer measuring anything — fix the assertion, do not delete it"
red COREPACK_HOME diverged from the cached path 1 names both paths

The fourth leg is the point of the design, not padding: the assertion must positively locate the pin. A future Corepack that moves its layout would otherwise make the scan enumerate nothing and report clean — a check that has retired itself while still reading green. Not finding the pin is a failure here.

The -v2- key salt

A one-time rotation, not a widening: it is a literal, not a restore-keys prefix, and nothing new can satisfy the pin. Entries written by the old post-job save may already hold an unpinned manager — that store has been restored by every CI job since 09-05, so it is kept alive by use and would not have aged out — and the new assertion would then, correctly, red every job that restored one. Rotating makes this fix independent of what is in the cache today, at the cost of exactly one cold build: the same cost as a packageManager bump, which the retry in "Materialise pnpm" already exists to cover.

Not measured, and it needs a human with the Actions cache view: whether the pre-rotation entry is in fact poisoned. GET /repos/OWNER/REPO/actions/caches is refused by this container's egress proxy (HTTP 403, proxy policy — not a credential problem), so I could not look. If you want the answer, list the repo's caches for keys beginning Linux-corepack- without -v2-: a clean store is ~22 MB on disk, a store carrying a second pnpm ~71 MB. Either way those entries are now unreachable and can be deleted.

Structural checks on the shipped YAML (parsed, not grepped)

Assertion step is index 5, save is index 6 — assertion strictly before save · save carries no always() · no actions/cache@v6 remains · restore and save keys are byte-identical · no restore-keys · the packageManager pin check at the top of the action is untouched. That last one is explicit in the card's ruling 3, along with not broadening the key; neither is touched.

Gates

22 commands, every one exit 0 — the 20 derived by node scripts/pm/dispatch-gates.mjs --changed --commands --repo objectstack-ai/objectstack at this branch's head, plus two I added because they read this file's neighbourhood: pnpm check:pnpm-acquisition (the composite is still recognised as the cached mechanism — 27 sites) and node scripts/pr-labels.mjs --self-test (its roster lives under .github, which the derivation flagged as neither pass nor fail for my paths). Full list and verdict lines are in the os-dev-report comment on #16012.

No .changeset/ entry: this ships nothing from any package. Labelled skip-changeset.

Left for the maintainer's veto window, per the PM's disposition on the card: this is bug/p2 on the channel's realised cost — main's own push build plus four unrelated PRs on 09-05 — rather than on a live trigger.


Generated by Claude Code

`actions/cache@v6` registers a post-job save that writes back whatever any
step of the job left in COREPACK_HOME. A step that runs a package manager
from a directory carrying no `packageManager` field makes Corepack resolve
`latest` off the registry into that shared store and write a sticky
`lastKnownGood.json`, and the post-job save then hands that store to every
later job under the PINNED key.

Split the step into `cache/restore` at the top and `cache/save` at the
bottom, with an assertion between them that enumerates the store and fails
with `::error::` naming any manager that is not the pin. The save carries no
`always()`, so a store that failed the assertion is never written back, and
the save now happens at a point in the job where the store's contents are
still deterministic rather than after arbitrary later steps.

The `-v2-` key salt is a one-time rotation so the fix does not depend on
whether entries written by the old post-job save are already poisoned; it is
not a restore-keys prefix and does not widen what can satisfy the pin.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 6, 2026
@baozhoutao
baozhoutao marked this pull request as ready for review September 6, 2026 06:29
@baozhoutao
baozhoutao enabled auto-merge September 6, 2026 06:29
@baozhoutao
baozhoutao added this pull request to the merge queue Sep 6, 2026
Merged via the queue into main with commit 1ebde49 Sep 6, 2026
31 checks passed
@baozhoutao
baozhoutao deleted the claude/issue-16012-setup-pnpm-corepack-store-pin branch September 6, 2026 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants