Repository navigation
build: upgrade to pnpm 12 - #17156
Conversation
📝 WalkthroughWalkthroughThe repository upgrades its package manager from pnpm 10 to pnpm 12. pnpm settings move from package manifests and Merge Risk: 🟡 Moderate · up to The workflows can select older refs whose package-manager metadata is incompatible with the new setup behavior, causing CI or release jobs to fail before dependencies are installed. Merge should wait for a compatibility path for those refs or explicit owner acceptance of the bounded release risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 10 files. (26 skipped: 26 unsupported.) Full details: Description checkExplanation The description is comprehensive and covers the required sections, including scope, rationale, testing, cross-platform impact, and checklist completion. However, the Linked Issue section contains "N/A" even though the template requires an issue link.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR upgrades the desktop and mobile workspaces to pnpm 12 while migrating package-manager policy and CI setup to pnpm 12-compatible locations and interfaces.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains within the eligible follow-up-review scope. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| package.json | Pins pnpm 12 and removes package-level pnpm configuration now represented by the root workspace manifest. |
| pnpm-workspace.yaml | Centralizes desktop dependency overrides, patch declarations, architecture targets, hoisting, release-age, and build policies. |
| mobile/pnpm-workspace.yaml | Adds pnpm 12-compatible mobile overrides, patch declarations, release-age policy, and build permissions. |
| .github/actions/install-node-dependencies/action.yml | Migrates the shared dependency-install action to the new pnpm setup action and extends the clean-install guard to the workspace manifest. |
| config/scripts/regenerate-xterm-patches.mjs | Updates patch generation and lockfile hash handling for pnpm 12’s patch and lockfile formats. |
| pnpm-lock.yaml | Regenerates the desktop lockfile with pnpm 12’s flattened patched-dependency hashes. |
| mobile/pnpm-lock.yaml | Regenerates the independent mobile lockfile for pnpm 12. |
Reviews (2): Last reviewed commit: "build: upgrade to pnpm 12" | Re-trigger Greptile
e1bc769 to
5ab025b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2ec5d2ba-99d3-4110-bcaa-3733a7549b39
⛔ Files ignored due to path filters (2)
mobile/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (40)
.github/actions/install-node-dependencies/action.yml.github/workflows/adhoc-mac-build.yml.github/workflows/daemon-relocation-spike.yml.github/workflows/daily-mac-build.yml.github/workflows/dev-channel-win-build.yml.github/workflows/golden-e2e-experiment.yml.github/workflows/hourly-mac-build.yml.github/workflows/linux-wayland-gpu-sandbox.yml.github/workflows/mobile-android-release.yml.github/workflows/mobile-ios-release.yml.github/workflows/mobile.yml.github/workflows/release-cut.yml.github/workflows/release-mac-build.yml.github/workflows/terminal-ime-e2e.yml.github/workflows/terminal-perf.yml.github/workflows/win-crash-survival-e2e.yml.github/workflows/win-update-e2e.yml.github/workflows/win-update-survival-e2e.yml.github/workflows/windows-signing-rehearsal.yml.github/workflows/windows-terminal-restart-e2e.yml.npmrcconfig/docker/headless-pairing/Dockerfile.buildconfig/patches/@vscode__windows-process-tree@0.8.0.patchconfig/patches/@xterm__addon-webgl@0.20.0-beta.286.patchconfig/scripts/computer-e2e-workflow.test.mjsconfig/scripts/ensure-native-runtime.mjsconfig/scripts/install-node-dependencies-action.test.mjsconfig/scripts/package-electron-runtime-contract.test.mjsconfig/scripts/pr-workflow-parallelism.test.mjsconfig/scripts/regenerate-xterm-patches.mjsconfig/scripts/regenerate-xterm-patches.test.mjsconfig/scripts/verify-telemetry-constants.mjsconfig/scripts/windows-process-tree-patch-contract.test.mjsdocs/reference/windows-process-enumeration.mdmobile/.npmrcmobile/package.jsonmobile/pnpm-workspace.yamlpackage.jsonpnpm-workspace.yamltests/e2e/terminal-windows-codex-multiline-paste.spec.ts
💤 Files with no reviewable changes (3)
- mobile/.npmrc
- .npmrc
- mobile/package.json
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| uses: pnpm/setup@v2 | ||
| with: | ||
| version: 10.24.0 | ||
| run_install: false | ||
| install: false |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Make package-manager setup compatible with selected refs.
These workflows check out caller-selected tags, branches, or SHAs, then invoke pnpm/setup@v2 without a version input. A selected tree that declares pnpm 10.24.0 or lacks package-manager metadata can fail during setup, before dependency installation. Add a compatibility path for pre-pnpm-11 refs at all affected setup sites, or select setup behavior from the checked-out manifest.
📍 Affects 2 files
.github/workflows/release-cut.yml#L1196-L1198(this comment).github/workflows/adhoc-mac-build.yml#L174-L176
The root moved to pnpm 12 in #17156 but these two isolated workspaces were left on pnpm 10, so CI and deploys carried a second pnpm and pnpm/action-setup. - Pin the root's integrity-checked pnpm 12.14.0 and regenerate both lockfiles (only the pnpm 12 package-manager document is added; the dependency graph is unchanged). - Move settings pnpm 12 ignores out of .npmrc and package.json#pnpm into each pnpm-workspace.yaml; onlyBuiltDependencies becomes allowBuilds, and cloud keeps esbuild's script skipped as pnpm 10 did. package-manager-strict is gone in pnpm 12, whose own version pinning supersedes it. - docs/site drops --ignore-workspace: under pnpm 12 it skips the workspace file, losing the overrides and failing the frozen install. vercel.json now pins install/build so a stale dashboard command cannot reintroduce it. - Cloud and docs workflows use pnpm/setup@v2 like the root; the relay unit-test step uses the root's pnpm instead of npx pnpm@10.
ELI5
Upgrade Orca's package manager from pnpm 10 to pnpm 12 while preserving the dependency, patch, architecture, hoisting, release-age, and build-script policies we already rely on.
What Changed
package.json#pnpmand.npmrclocations into eachpnpm-workspace.yaml, including explicit build allow/deny policy.pnpm/action-setupaction topnpm/setup@v2, with automatic installs disabled and the integrity-pinnedpackageManagerdeclaration as the single version source.pnpm.exeas well as Corepack'spnpm.cmd.Why
pnpm 11+ no longer reads project pnpm settings from
package.json#pnpmor non-registry settings from.npmrc, and pnpm 12 uses a stricter native patch parser. Moving the settings and normalizing the patches keeps installs reproducible instead of silently dropping policy or failing on a clean install.CI Performance
Measured against the median of five adjacent successful pnpm 10 PR runs (33274209872, 33274471001, 33274584159, 33274625294, and 33274749051) versus the green pnpm 12 run 33274652961:
Across the 12 directly comparable jobs, aggregate runner time fell from a 3,101s median to 2,819s: 282 runner-seconds saved per PR run (~4m42s, 9%). Jobs fan out, so aggregate runner time is not the same as workflow wall time. Hosted-runner/cache noise is material; the macOS package job shows that build/sign/package work can dominate the faster dependency phase. This is a CI/install improvement, not an application-runtime performance change.
Linked Issue
N/A — direct package-manager maintenance upgrade.
Visual Proof
N/A — toolchain and dependency-install behavior only; no UI or interaction changes.
Testing
Verified on macOS arm64 with Node 24:
corepack pnpm --version→ 12.0.0pnpm tcpnpm --dir mobile typecheckpnpm run check:code-quality:changed→ 0 findingsnode config/scripts/regenerate-xterm-patches.mjs --check→ patches in syncpnpm/setup@v2workflow steps parse with automatic install disabled and no duplicated version pinConcerns / Follow-ups
.0release: pnpm 12.0.0 has less ecosystem soak than a later 12.x. The exact version plus integrity pin prevents surprise drift, but follow-up point-release upgrades remain manual.patchedDependencieshashes. Known repository consumers now handle this and the full matrix is green; future custom lockfile readers must parse all YAML documents rather than assuming one document.pnpm/setup@v2installs nativepnpm.exe, while Corepack commonly exposespnpm.cmd. The affected native rebuild path now supports both and Windows smoke/package pass; future direct Windows child-process calls should not assume only.cmdexists.minimumReleaseAge, hoisting, architecture targets, overrides, patches, and explicit build allow/deny policy are preserved in the canonical workspace files.Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
actions/setup-nodeordering/cache behavior intact.5ab025b; GitHub reports the PR clean and mergeable.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)