Skip to content

ci: unpin pnpm and upgrade pnpm/action-setup to v6.1.0 - #844

Merged
cb1kenobi merged 1 commit into
mainfrom
ci/unpin-pnpm
Sep 14, 2026
Merged

cb1kenobi merged 1 commit into
mainfrom
ci/unpin-pnpm

Conversation

@cb1kenobi

@cb1kenobi cb1kenobi commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

What

Reverts the packageManager: "pnpm@11.25.0" pin added in Make a WriteBufferManager write stall observable (#824) and restores version: latest on all 12 pnpm/action-setup steps, while bumping the action itself from v6.0.9 → v6.1.0.

Why — the pin treated a symptom

The pin was added in response to a CI failure, but the failure was not caused by pnpm 12 being broken. It was caused by pnpm/action-setup@v6.0.9 not supporting pnpm 12.

Root cause, from the pre-pin run on codex/issue-831-phase0-lease (job log):

Checking for updates...
Switching pnpm from v11.7.0 to v12.3.4...
[WARN] Detected a pnpm v10 installation layout at PNPM_HOME...
Successfully updated pnpm to v12.3.4
...
[command] C:\...\cmd.exe /D /S /C "C:\Users\runneradmin\setup-pnpm\node_modules\.bin\bin\pnpm.CMD store path --silent"
'"C:\Users\runneradmin\setup-pnpm\node_modules\.bin\bin\\..\global\v11\1a04-1a07dcab9dc\node_modules\pnpm\pnpm"'
is not recognized as an internal or external command,

action-setup@v6.0.9 bootstraps pnpm 11 via npm, then runs pnpm self-update <target>. pnpm 12 ships a native executable at node_modules/pnpm/pnpm[.exe] instead of the old bin/pnpm.mjs JS entry point, so the CMD shim that pnpm 11's self-update wrote points at an extensionless file that cmd.exe cannot execute. actions/setup-node with cache: 'pnpm' then dies calling pnpm store path, failing every Windows job.

This was Windows-only — Linux and macOS were fine on pnpm 12 the whole time.

The actual fix

pnpm/action-setup@v6.1.0 (released 2026-09-05, pnpm/action-setup#288) adds a native pnpm 12 bootstrap path — installing pnpm 12's plain package directly instead of self-updating across the v11→v12 binary boundary — with new smoke coverage on Linux, macOS, and Windows. The repo was pinned to v6.0.9 (June 15), two releases behind.

Verification done locally

  • Ran pnpm install --ignore-scripts --no-frozen-lockfile under both 11.25.0 and 12.3.4 against this repo's manifest. Both succeed and produce byte-identical pnpm-lock.yaml. lockfileVersion stays at 9.0, so there is no lockfile-format hazard (and CI uses --no-frozen-lockfile everywhere regardless).
  • All 12 workflow files re-validated as parseable YAML.
  • pnpm 12 honours the pnpm-workspace.yaml allowBuilds / minimumReleaseAge settings unchanged.

CI result — confirmed green

Full matrix passed on the first run, including all six Windows jobs (Node 22/24/26, Bun, Deno, Native C++). The Install pnpm and Use Node.js steps — the exact pair that failed before — are green on every Windows runner.

I had expected Windows to still fail here: action-setup@v6.1.0 routes to its new native path via semver.subset(validRange(version), '>=12.0.0 <13.0.0'), and validRange('latest') is null, so version: latest takes the legacy bootstrap-then-self-update path. That prediction was wrong — v6.1.0 also fixes the legacy path's Windows shim, so plain latest is fine and no version: 12 fallback is needed.

Longer term, pnpm/setup@v2 is the documented successor to action-setup, but it also replaces actions/setup-node and auto-runs pnpm install, so migrating it is a larger change and out of scope here.


🤖 Generated with Claude Code

@cb1kenobi
cb1kenobi requested a review from kriszyp as a code owner September 9, 2026 22:37

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes the packageManager field from package.json and adds the @harperfast/rocksdb-js-linux-x64-musl dependency to pnpm-lock.yaml. The reviewer recommends retaining and updating the packageManager field to a v12 version (such as pnpm@12.3.4) instead of removing it entirely, to ensure deterministic builds across different environments.

Comment thread package.json
@github-actions

github-actions Bot commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

📊 Benchmark Results

get-sync.bench.ts

getSync() > random keys - small key size (100 records)

Implementation Rank Operations/sec Mean (ms) Min (ms) Max (ms) RME (%) Samples
🥇 lmdb 1 24.29K ops/sec 41.17 39.86 507.467 0.104 121,438
🥈 rocksdb 2 11.12K ops/sec 89.90 86.85 24,010.254 0.973 55,621

getSync() > sequential keys - small key size (100 records)

Implementation Rank Operations/sec Mean (ms) Min (ms) Max (ms) RME (%) Samples
🥇 lmdb 1 28.43K ops/sec 35.18 34.00 494.488 0.103 142,136
🥈 rocksdb 2 10.79K ops/sec 92.70 88.93 614.597 0.051 53,940

ranges.bench.ts

getRange() > small range (100 records, 50 range)

Implementation Rank Operations/sec Mean (ms) Min (ms) Max (ms) RME (%) Samples
🥇 lmdb 1 25.27K ops/sec 39.57 36.23 1,782.664 0.298 126,362
🥈 rocksdb 2 16.07K ops/sec 62.24 52.93 1,091.818 0.122 80,331

realistic-load.bench.ts

Realistic write load with workers > write variable records with transaction log

Implementation Rank Operations/sec Mean (ms) Min (ms) Max (ms) RME (%) Samples
🥇 rocksdb 1 356.28 ops/sec 2,806.782 51.44 66,645.475 16.86 729
🥈 lmdb 2 26.44 ops/sec 37,815.128 410.371 1,190,803.123 136.274 64.00

transaction-log.bench.ts

Transaction log > read 100 iterators while write log with 100 byte records

Implementation Rank Operations/sec Mean (ms) Min (ms) Max (ms) RME (%) Samples
🥇 rocksdb 1 38.82K ops/sec 25.76 12.00 20,940.09 0.851 194,109
🥈 lmdb 2 439.76 ops/sec 2,273.95 186.696 13,604.024 1.32 2,199

Transaction log > read one entry from random position from log with 1000 100 byte records

Implementation Rank Operations/sec Mean (ms) Min (ms) Max (ms) RME (%) Samples
🥇 rocksdb 1 746.43K ops/sec 1.34 1.16 4,682.54 0.196 3,732,134
🥈 lmdb 2 458.15K ops/sec 2.18 1.15 5,627.837 0.786 2,290,757

worker-put-sync.bench.ts

putSync() > random keys - small key size (100 records, 10 workers)

Implementation Rank Operations/sec Mean (ms) Min (ms) Max (ms) RME (%) Samples
🥇 rocksdb 1 820.41 ops/sec 1,218.901 1,046.99 1,831.057 0.336 1,641
🥈 lmdb 2 1.15 ops/sec 870,801.63 830,288.904 948,492.584 2.85 10.00

worker-transaction-log.bench.ts

Transaction log with workers > write log with 100 byte records

Implementation Rank Operations/sec Mean (ms) Min (ms) Max (ms) RME (%) Samples
🥇 rocksdb 1 22.41K ops/sec 44.62 29.30 20,270.902 2.07 44,823
🥈 lmdb 2 820.45 ops/sec 1,218.84 215.478 12,080.333 5.41 1,641

Results from commit 9f2b658

kriszyp added a commit that referenced this pull request Sep 11, 2026
Remove the twelve workflow-local pnpm pins so this PR stays focused on the AI-review callers and leaves the package-manager policy to PR #844.

Co-Authored-By: GPT-5 Codex <noreply@openai.com>

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Reviewed with Codex

uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6.0.9
uses: pnpm/action-setup@ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0
with:
version: latest

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latest makes the release toolchain mutable: identical release commits can use different pnpm versions across runs—or even across preflight, build, and publish jobs. A newly tagged incompatible version can change install/build/publish behavior without a repository change, and a compromised release would eventually execute as pnpm publish with NPM_TOKEN. This is the same class of unreviewed package-manager drift the previous pin guarded against; upgrading pnpm/action-setup fixes the current bootstrap issue but does not require unpinning pnpm. Please retain a single exact, tested packageManager version in package.json (for example, the verified pnpm@12.3.4) and omit the duplicated version inputs so every workflow reads that central pin.

cb1kenobi added a commit that referenced this pull request Sep 12, 2026
Reverts the `packageManager: pnpm@11.25.0` pin added in #824 and restores
`version: latest` on every `pnpm/action-setup` step.

The pin was a workaround for a Windows-only CI failure, but the root cause
was not pnpm 12 itself — it was `pnpm/action-setup@v6.0.9` not supporting
pnpm 12. That version bootstraps pnpm 11 via npm and then runs
`pnpm self-update <target>`. pnpm 12 ships a native executable
(`node_modules/pnpm/pnpm[.exe]`) instead of the old JS entry point, so the
CMD shim pnpm 11's self-update wrote pointed at an extensionless file:

    '"C:\...\setup-pnpm\node_modules\.bin\bin\\..\global\v11\...\node_modules\pnpm\pnpm"'
    is not recognized as an internal or external command

`actions/setup-node` with `cache: pnpm` then died invoking
`pnpm store path`, which failed every Windows job.

`pnpm/action-setup@v6.1.0` (released 2026-09-05, pnpm/action-setup#288)
adds a native pnpm 12 bootstrap path with Linux/macOS/Windows smoke
coverage, so the action can install pnpm 12 without the self-update hop.

Verified locally that pnpm 11.25.0 and 12.3.4 produce byte-identical
lockfiles for this repo and that `lockfileVersion` stays at 9.0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cb1kenobi
cb1kenobi merged commit d78f36e into main Sep 14, 2026
44 of 45 checks passed
@cb1kenobi
cb1kenobi deleted the ci/unpin-pnpm branch September 14, 2026 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants