chore: bump mise/pnpm/lockfiles, enable pnpm global store - #6424
chore: bump mise/pnpm/lockfiles, enable pnpm global store#6424kanadgupta wants to merge 14 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 AI Review
The reviews agree on one confirmed contributor-documentation mismatch. A second minor performance concern is confirmed: the broad supported-architecture matrix makes all eight new pnpm executable packages eligible for cold-install downloads. Two tooling interactions remain uncertain because the relevant third-party implementations are not present in the checkout. The CI cache concern is refuted. No critical or major issues were verified.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | mise.toml:1 |
documentation |
claude+codex | The enforced minimum mise version is 2026.9.0, but the contributor guide still says 2026.7.0 or newer is sufficient. |
| 🟡 MINOR | pnpm-workspace.yaml:80 |
build-tooling |
claude | The root prepare script may mutate dependency contents now shared through pnpm's global virtual store, potentially affecting other checkouts on the same machine. |
| 🟡 MINOR | package.json:45 |
build-tooling |
claude | Turbo 2.10.11 may fail to derive correct workspace and cache hashes after removal of the root packageManager field and introduction of a two-document pnpm lockfile. |
| 🟡 MINOR | pnpm-lock.yaml:91 |
performance |
claude | The new pnpm package-manager dependency makes eight platform-specific pnpm executables eligible for download during a cold install because the workspace requests dependencies for every represented OS, CPU, and libc combination. |
Refuted findings (kept for transparency, not posted as review comments)
pnpm-workspace.yaml:80(ci): The CI cache may omit pnpm's global virtual store or reuse an unchanged cache key for the new layout.
Refuted: pnpm's global virtual store is maintained beneath its store path, which the action caches at .github/actions/setup/action.yml:30-37. The exact cache key also includes hashFiles('pnpm-lock.yaml'), and this PR materially changes that lockfile, so the new layout does not reuse the previous exact key.
Stats
Claude findings: 5 · Codex findings: 1 · Confirmed: 2 · Refuted: 1 · Uncertain: 2
Models: claude-opus-5 + gpt-5.6-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
| @@ -1,4 +1,4 @@ | |||
| min_version = '2026.7.0' | |||
| min_version = '2026.9.0' | |||
There was a problem hiding this comment.
🟡 MINOR · documentation · source: claude+codex
The enforced minimum mise version is 2026.9.0, but the contributor guide still says 2026.7.0 or newer is sufficient.
Evidence: mise.toml:1 sets min_version = '2026.9.0', while CONTRIBUTING.md:37 still directs contributors to use mise 2026.7.0 or newer.
Suggested fix: Update CONTRIBUTING.md:37 to require mise 2026.9.0 or refer directly to the minimum declared in mise.toml.
There was a problem hiding this comment.
Fixed. CONTRIBUTING.md no longer hardcodes a mise version; it points contributors at the pin in mise.toml (min_version = '2026.9.0') instead, so the two can't drift again.
| patchedDependencies: | ||
| "@libpg-query/parser@17.6.10": patches/@libpg-query__parser@17.6.10.patch | ||
|
|
||
| virtualStoreType: global |
There was a problem hiding this comment.
🟡 MINOR · build-tooling · source: claude
The root prepare script may mutate dependency contents now shared through pnpm's global virtual store, potentially affecting other checkouts on the same machine.
Evidence: pnpm-workspace.yaml:80 enables virtualStoreType: global; package.json:5 runs effect-tsgo patch --no-typescript --oxlint during prepare; .oxlintrc.effect.json:2-3 consumes assets from the installed @effect/tsgo package.
Suggested fix: Verify which files effect-tsgo patch --oxlint writes and ensure any mutated dependency receives a checkout-private copy or avoid the global virtual store.
Adjudication (uncertain): The repository confirms that the patch command runs, but the @effect/tsgo patch implementation is neither checked in nor installed here. The configuration references demonstrate reads from node_modules, not that the command writes into shared package directories.
There was a problem hiding this comment.
Verified, and this one is real. Findings:
What the prepare script writes. effect-tsgo patch --oxlint replaces files inside three installed packages — oxlint/dist/index.d.ts, the current platform's @oxlint/binding-* native module, and the @oxlint-tsgolint/* binary — leaving .original backups beside them. Under virtualStoreType: global those writes land in the store's links/ directories, not in the checkout. I found the mutated files and backups there on my machine.
Blast radius. Each links/ directory is keyed only by the hash of that package's own dependency graph, so every checkout on the machine that resolves the same oxlint/binding/tsgolint versions — other worktrees of this repo or unrelated projects — shares the directory and would run the Effect-patched binaries. It does not corrupt pnpm's content-addressable store: the patcher renames the original and writes a new file rather than mutating in place, and the link dirs are clone/hardlink copies, so the CAS entries stay pristine.
No knob exists. effect-tsgo patch has no destination flag, pnpm has no per-package opt-out of the global virtual store, and I didn't find an upstream issue on Effect-TS/tsgo tracking this — worth filing.
Fix in this PR. packageExtensions in pnpm-workspace.yaml now declares @effect/tsgo as a dependency of all 15 packages the patcher mutates (oxlint, the 8 @oxlint/binding-*, the 6 @oxlint-tsgolint/*). That folds the patcher's version into each package's graph hash, so these packages get store directories private to identically-configured checkouts, and they're re-keyed to fresh directories on every @effect/tsgo bump. Verified end-to-end: after reinstall all three patch targets moved to new hash directories and prepare re-patched them there; lint:effect:check and check:all pass. The remaining mutation is confined to directories only this repo's checkouts resolve, which all run the same prepare.
(The same packageExtensions block also declares the undeclared type deps — undici-types for bun-types, @types/pg/@types/debug for @supabase/pg-delta — that the global store's lack of a hidden hoist directory broke for tsc in apps/cli.)
| "packageManager": { | ||
| "name": "pnpm", | ||
| "version": "12.2.1", | ||
| "onFail": "error" | ||
| }, | ||
| "runtime": { | ||
| "name": "node", | ||
| "version": "24" |
There was a problem hiding this comment.
🟡 MINOR · build-tooling · source: claude
Turbo 2.10.11 may fail to derive correct workspace and cache hashes after removal of the root packageManager field and introduction of a two-document pnpm lockfile.
Evidence: package.json:44-53 contains only devEngines.packageManager; pnpm-lock.yaml:1-101 places the package-manager dependency and workspace lock data in separate YAML documents; pnpm-workspace.yaml:60 pins Turbo 2.10.11; turbo.json:7-8 enables task caching.
Suggested fix: Run Turbo's JSON dry run and a repeated cacheable task in CI, checking for package-manager or lockfile parsing warnings and a cache hit on the second run.
Adjudication (uncertain): The checked-out repository establishes the new manifest and lockfile shapes, but it does not contain Turbo's parser implementation or an execution result showing whether Turbo 2.10.11 supports devEngines.packageManager and this multi-document lockfile.
There was a problem hiding this comment.
Checked empirically — no issue. With pnpm 12.2.1, the devEngines-only manifest, and the two-document lockfile, Turbo resolves the workspace graph without warnings, check:all (13 tasks) passes, and re-running a cacheable task hits the cache (FULL TURBO). No change needed.
| optionalDependencies: | ||
| '@pnpm/exe.darwin-arm64': 12.2.1 | ||
| '@pnpm/exe.darwin-x64': 12.2.1 | ||
| '@pnpm/exe.linux-arm64': 12.2.1 | ||
| '@pnpm/exe.linux-arm64-musl': 12.2.1 | ||
| '@pnpm/exe.linux-x64': 12.2.1 | ||
| '@pnpm/exe.linux-x64-musl': 12.2.1 | ||
| '@pnpm/exe.win32-arm64': 12.2.1 | ||
| '@pnpm/exe.win32-x64': 12.2.1 |
There was a problem hiding this comment.
🟡 MINOR · performance · source: claude
The new pnpm package-manager dependency makes eight platform-specific pnpm executables eligible for download during a cold install because the workspace requests dependencies for every represented OS, CPU, and libc combination.
Evidence: pnpm-lock.yaml:15-57 declares eight platform-specific @pnpm/exe packages and lines 91-99 attach all eight as optional dependencies; pnpm-workspace.yaml:62-75 requests arm64 and x64 across Darwin, Linux, Windows, glibc, and musl.
Suggested fix: Measure cold-install and cache sizes; if the added downloads are material, narrow supportedArchitectures or exclude unnecessary package-manager executable variants.
There was a problem hiding this comment.
Partially valid but not observed in practice. The eight @pnpm/exe.* entries come from pnpm 12's self-managed devEngines.packageManager (pmOnFail: download is the default), and pnpm only downloads its declared version when the running pnpm doesn't match. Since mise provides the pinned 12.2.1 both locally and in CI, that path isn't triggered: after several installs, none of the eight packages exist in the store on my machine. supportedArchitectures is unchanged from develop (it was added earlier for the compiled-CLI e2e runtime), so there's nothing to narrow here. If someone did run a mismatched pnpm, the download would happen — I haven't verified whether pnpm fetches only the current platform in that case.
The global virtual store (virtualStoreType: global) has no hidden hoist directory (node_modules/.pnpm/node_modules), so packages that reach for dependencies they never declared stopped type-resolving in apps/cli: - bun-types imports undici-types in its fetch overrides but only declares @types/node, which broke the global Response/Request/Headers types - @supabase/pg-delta is consumed as raw TypeScript source via the bun exports condition and imports pg and debug without their @types packages Declare the missing dependencies via pnpm packageExtensions, keeping the global store for everything else. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The root prepare script (effect-tsgo patch --oxlint) replaces files inside the installed oxlint, @oxlint/binding-*, and @oxlint-tsgolint/* packages. Under virtualStoreType: global those packages live in store directories keyed only by each package's dependency-graph hash and shared with every checkout on the machine that resolves the same graph, so the in-place patches leaked into unrelated projects using the same oxlint version. Declare @effect/tsgo as a dependency of every package it mutates via packageExtensions. That folds the patcher's version into each package's graph hash, giving these packages store directories private to identically-patched checkouts and re-keyed whenever @effect/tsgo bumps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Anchor the @effect/tsgo catalog version and alias it from the oxlint packageExtensions so the pin lives in one place, and trim the section comments. No resolution change; the lockfile is untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d-2026-09-01/mise-and-pnpm-upgrades
1 participant
Summary
mise lock --upgrade(and bumpsmiseminimum version + github action accordingly)pnpmto v12pnpm-workspacefile so types resolve properly)this PR was inspired by these changes
Checklist
fix(cli): …).pnpm check:allpasses; relevant package tests pass for every touched workspace, andpnpm types:checkpasses for each touched TypeScript workspace (or workspace declaring it).