[osprey-ui] migrate from npm to pnpm via Corepack#252
Open
haileyok wants to merge 13 commits into
Open
Conversation
Phase 1 of pnpm migration. Adds the packageManager field so Corepack materializes pnpm 10.x identically across local dev, CI, and Docker. Pinning pnpm 10 (not 11) because pnpm 11 drops Node 18 and moves peer-dep config out of .npmrc — both conflict with this migration's scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace legacy-peer-deps=true (npm) with auto-install-peers=true and strict-peer-dependencies=false (pnpm equivalent). Preserves the permissive peer-dep behavior CRA's outdated peer ranges require. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each entry in pnpm.overrides represents a transitive whose pnpm-resolved version drifted from the original npm package-lock.json resolution. Pinning to the npm baseline so this migration is a pure tooling swap with no transitive version bumps. Drift identified by osprey_ui/scripts/lockfile_parity.py (committed here) and driven to zero across N override rounds. The harness lives in the tree through Phase 6's final re-verification and gets deleted at the end of Phase 6. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Strict-parity has been achieved via pnpm.overrides — every transitive resolves to the same version as npm did. The npm lockfile is no longer authoritative. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Final lockfile after the strict-parity override loop. pnpm resolves every transitive to the same version as the previous npm package-lock.json. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 3 of pnpm migration. Replaces `npm install --legacy-peer-deps` with the Corepack-on-Alpine pattern: upgrade bundled Corepack to dodge the 2025 npm registry signature-key rotation, then `pnpm install --frozen-lockfile` reads the lockfile committed in Phase 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 4 of pnpm migration. The ui-quality job now uses pnpm/action-setup@v6 (ordered before actions/setup-node@v4) and pnpm install --frozen-lockfile / pnpm run format:check. setup-node caches the pnpm store keyed off osprey_ui/pnpm-lock.yaml. Python and Rust jobs in the same workflow are untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 5 of pnpm migration. Updates AGENTS.md (Build, Testing, CI, Dependencies, Human-approval-required sections), docs/DEVELOPMENT.md (prerequisites), docs/UI.md (dev setup snippets), and osprey_ui/README.md (prerequisites and dev commands) to reference pnpm via Corepack. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pnpm's default isolated layout creates two distinct module identities for cytoscape: the bundled types in cytoscape@3.33.1 and @types/cytoscape@3.21.9 pulled in transitively by @types/cytoscape-dagre and @types/cytoscape-popper. TypeScript then sees two incompatible Ext / CytoscapeOptions definitions and the production build fails. Under npm flat hoisting this is silently resolved. node-linker=hoisted gives pnpm an npm-compatible flat node_modules without giving up Corepack-pinned pnpm, the frozen lockfile, or the strict-parity override contract. Single-package repo so isolation guarantees matter less than build parity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The lockfile_parity.py harness served its purpose during the Phase 2 strict-parity loop and the Phase 6 final re-verification. Removing it now keeps the migration self-contained — future dependency changes are reviewed via the pnpm.overrides block in package.json, not by re-running a one-shot diff tool. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A future engineer pruning .npmrc would otherwise have to git blame the line to recover the cytoscape duplicate-types context. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 262-entry pnpm.overrides block was needed during the initial strict-parity convergence in Phase 2. Once pnpm-lock.yaml was committed, the lockfile itself locks every resolved version — the overrides were belt-and-suspenders, not a separate enforcement mechanism. CI and Docker already use `pnpm install --frozen-lockfile`, which reads the lockfile verbatim and refuses to update it. One incidental drift: `@types/express-serve-static-core@5.0.7` no longer gets installed as a duplicate alongside `4.19.6`. It only existed under npm/flat hoisting via a parent-scoped pin (`@types/express > @types/...`) and is dead weight — `@types/express` actually needs 4.19.6, not 5.0.7. All other 1,417 resolved name@version pairs match the original npm baseline exactly. Trade-off: a future `pnpm add some-pkg` or non-frozen `pnpm install` can now re-resolve transitives within their `^` ranges. Mitigation lives in the workflow — direct-dep changes go through review and surface in the pnpm-lock.yaml diff there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The dependencies-management callout previously pointed reviewers at the overrides block as the parity-enforcement mechanism. With overrides gone, the lockfile is the contract — reviewers should check the pnpm-lock.yaml diff on each pnpm add. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
haileyok
requested review from
a team,
EXBreder,
ayubun and
vinaysrao1
as code owners
haileyok
commented
May 13, 2026
| - **[Git](https://git-scm.com/)** for version control | ||
| - **[uv](https://docs.astral.sh/uv/)** for Python package management | ||
| - **[npm](https://nodejs.org/en/download)** | ||
| - **[Node.js](https://nodejs.org/en/download/) 18+** for the UI (Corepack ships with Node and auto-resolves pnpm from `osprey_ui/package.json`'s `packageManager` field — no separate pnpm install needed) |
Member
Author
There was a problem hiding this comment.
is corepack what we want? im unfamiliar with it, though it does seem useful from my brief understanding
haileyok
commented
May 13, 2026
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: "18" |
Member
Author
There was a problem hiding this comment.
should probably update this at some point, but that can happen later
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #260; related to #231
Summary
Migrates
osprey_ui/from npm to pnpm 10 via Corepack. Pure tooling swap — no application code changed, no React/Antd/Highcharts version bumps. 1,417 of 1,418 resolved transitives match the original npm baseline exactly; the single drop is a duplicate type def that npm's flat hoisting had silently kept alive.Related Issues/Tasks
N/A
Changes Made
packageManager: "pnpm@10.33.4+sha512.…"toosprey_ui/package.json; replacedlegacy-peer-deps=truewithauto-install-peers=true+strict-peer-dependencies=falseinosprey_ui/.npmrc.osprey_ui/pnpm-lock.yamlviapnpm import; iteratively pinned 262 transitives inpnpm.overridesacross 9 rounds until the parity harness reported zero drift against the npm baseline. Once the lockfile was committed, the overrides became redundant with the lockfile itself — dropped the entirepnpm.overridesblock in a follow-up. Deletedosprey_ui/package-lock.json.osprey_ui/Dockerfileto the Corepack-on-Alpine pattern (npm install -g corepack@latest && corepack enable, thenpnpm install --frozen-lockfile,CMD ["pnpm", "start"]).ui-qualityin.github/workflows/code-quality.ymltopnpm/action-setup@v6+actions/setup-node@v4withcache: pnpmandcache-dependency-path: osprey_ui/pnpm-lock.yaml. Python and Rust jobs untouched.AGENTS.md(7 sections),docs/DEVELOPMENT.md,docs/UI.md,osprey_ui/README.md.node-linker=hoisted. Addednode-linker=hoistedtoosprey_ui/.npmrcafter discovering that pnpm's default isolated layout exposed two distinct cytoscape module identities to TypeScript (bundled types in cytoscape@3.33.1 vs@types/cytoscape@3.21.9pulled in transitively by@types/cytoscape-dagre), breakingpnpm run build. Verified npm-based build at the base commit also installed both packages but npm's flat hoisting silently hid one — so the hoisted linker is the minimum-change restoration of build parity. Layout-only fix; resolved versions unchanged.Deliberate deviation from design — pinned pnpm 10, not pnpm latest
The design plan said
corepack use pnpm@latest. Todaypnpm@latestis 11.1.1, which drops Node 18 support and moves.npmrcsettings topnpm-workspace.yaml. Both conflict with the design's DoD ("replacing the Dockerfile base image" is explicitly out of scope) and AC3 (.npmrccarries the peer-dep flags). Pinned pnpm 10.33.4 instead — the latest version that satisfies the existing constraints. If you want the Node 22 + pnpm 11 path, that's a separate scope change to the design.Parity review checklist
pnpm-lock.yamlis the parity contract and the new source of truth. 1,417 of 1,418 resolved transitives match the original npm baseline exactly (the one drop:@types/express-serve-static-core@5.0.7was a duplicate type def kept alive only by a parent-scoped override; the4.19.6that@types/expressactually uses is still there). CI and Docker usepnpm install --frozen-lockfile, which refuses to update the lockfile.node-linker=hoistedgives an npm-compatible flatnode_moduleslayout. Single-package repo, so the isolation guarantee we give up matters less than build parity.pnpm addor non-frozenpnpm installcan re-resolve transitives within^ranges; thepnpm-lock.yamldiff in the PR is the review surface for catching unintended drift.Confidence Level
Confidence Level: Claude
Testing
Verified locally on
hailey.coder:corepack pnpm install --frozen-lockfilefrom a cleannode_modulesexits 0 (Done in 2.6s using pnpm v10.33.4).corepack pnpm run format:checkexits 0 — "All matched files use Prettier code style!"corepack pnpm run buildexits 0 —build/index.htmlpresent, "Compiled successfully".corepack pnpm startboots the dev server on:5002;curl http://127.0.0.1:5002returns HTTP 200.npm run buildat base commit13e62d7was verified to succeed in a temp worktree before declaring the node-linker fix necessary.Deferred to operator / this PR's CI
docker compose build osprey-uianddocker compose up osprey-ui(AC4.2 / AC4.3) — Dockerfile is on AGENTS.md's human-approval list; image build deferred to local docker run or to CI/staging.A human test plan with step-by-step instructions for each deferred item lives at
docs/test-plans/2026-05-13-pnpm-migration.md(untracked, per planning-docs policy).Rollback
git revertthe merge commit (or the squash). Devs would then needrm -rf osprey_ui/node_modules && npm cito re-bootstrap. Single squash → clean revert.Out of scope
Per the design DoD: no CRA / react-scripts upgrade, no React/Antd/Highcharts version bumps, no workspaces, no dependabot ecosystem changes, no base-image change.