Conversation
…pnpm 12.4.2 pnpm 11 stopped reading the pnpm block in package.json and now consults .npmrc for registry and auth only, so overrides, peerDependencyRules, ignoredOptionalDependencies, engineStrict, strictPeerDependencies and fetchTimeout move into the new core-web/pnpm-workspace.yaml. onlyBuiltDependencies was removed along with its siblings; its ten entries become an allowBuilds map. less joins them as an explicit false: pnpm 10 only printed "Ignored build scripts: less", while pnpm 12's strictDepBuilds turns that into ERR_PNPM_IGNORED_BUILDS and fails the install. Declares the supply-chain posture explicitly rather than inheriting it: trustPolicy no-downgrade, blockExoticSubdeps, strictDepBuilds, and verifyDepsBeforeRun warn (pnpm 11 changed that default to install, which would inject a dependency install into Maven and CI builds). Eight older versions fail no-downgrade because newer releases of the same packages carry provenance they do not; each is listed in trustPolicyExclude with its publish date and reason, with trustPolicyExcludePrune on so the list cannot rot. minimumReleaseAge is left at its default and documented as inert: the registry mirror returns no time field, so pnpm cannot evaluate the maturity check through it. Tracked in #37562. The dead stylus override is dropped: it pointed at a git source for an optional peer that is never installed, and blockExoticSubdeps is now on. Refs #37553
…build pnpm 12 ships as a native binary through @pnpm/exe.* optional dependencies. Installing it with npm runs the wrapper's install.js, which links the platform binary in place, so everything lands inside installs/ where CI's node-binary cache already picks it up. Corepack runs no lifecycle scripts: measured on a clean environment, its shim downloads the native binary on first use into ~/.cache/node/corepack, outside that cache, which would mean a fetch and a signature check on every job. npm exposes the binary at <prefix>/bin/pnpm rather than at the path corepack's shim used to occupy, so nodejs-parent publishes a pnpm.executable property and the consuming modules invoke it instead of hardcoding the old location. Refs #37553
v6.1.0 is the first release that understands pnpm 12. Both call sites read the version from core-web/package.json, so the action version is the only manual part of the bump. The version-sync guard compared core-web/package.json against nodejs-parent/pom.xml and left dotcms-postman/package.json, which carries its own packageManager pin, with no safety net. It now checks all three. Refs #37553
From pnpm 11 the lockfile is a two-document YAML: the second document carries the environment and config-dependency data. lockfileVersion stays at 9.0 and no dependency version changes; the churn is pnpm 12 re-keying the peer variants of cyclic packages once, which it does deterministically from now on. Refs #37553
…12 move Points at pnpm-workspace.yaml and names the two installs that now fail where they used to warn, so a failing install is diagnosable without reading the pnpm changelog. Refs #37553
github-actions
Bot
added
Area : Backend
|
Claude finished @nicobytes's task in 2m 39s —— View job Review: pnpm 10 → 12 migrationI reviewed the diff against Resolved
New Issues
Existing (non-blocking, already discussed in-thread)
Everything else in the CI/pom/workspace changes reads correctly. The remaining items from prior reviews (Windows PATH separator, • |
![semgrep-dotcms[bot]](https://avatars.githubusercontent.com/u/1005263?s=60&v=4){kind=small}
| # From pnpm 11 on this file is read for registry and auth only; every other setting | ||
| # is ignored in silence. See pnpm-workspace.yaml for the rest of the configuration. | ||
| registry=https://dotcms-npm.b-cdn.net |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
This .npmrc does not set a minimum release age or sets it too low. Newly published packages can be malicious or unstable. Add min-release-age = 7 to wait 7 days before resolving newly published package versions. Added in: v11.10 Reference: https://github.blog/changelog/2026-02-18-npm-bulk-trusted-publishing-config-and-script-security-now-generally-available/
Why this might be safe to ignore:
The rule matched the registry line because its broad pattern treats any .npmrc content without min-release-age as a finding. This file explicitly states that pnpm 11 reads it only for registry and authentication, so adding min-release-age here would be ineffective and the relevant setting belongs in pnpm-workspace.yaml.
To resolve this comment:
🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by npm-missing-minimum-release-age.
If this is a critical or high severity finding, please also link this issue in the #security channel in Slack.
You can view more details about this finding in the Semgrep AppSec Platform.
There was a problem hiding this comment.
/fp From pnpm 11 onward .npmrc is read for registry and authentication only; every other setting is ignored there, so adding min-release-age to this file would have no effect. The control the rule is asking for is in place, in the file pnpm actually reads: core-web/pnpm-workspace.yaml sets minimumReleaseAge: 10080 (7 days, against pnpm's 1-day default) plus minimumReleaseAgeIgnoreMissingTime: false so the check fails loudly rather than skipping when publish dates are unavailable.
Verified live: raising the window to a year fails resolution with ERR_PNPM_MINIMUM_RELEASE_AGE_VIOLATION naming the computed cutoff, so the gate reads real publish dates rather than passing vacuously.
Worth flagging to whoever maintains the rule: it will report this on every pnpm 11+ repository, since the setting can no longer live where it looks.
There was a problem hiding this comment.
/fp The rule asks for a 7-day minimum release age. This repository already enforces exactly that — in the file pnpm actually reads.
min-release-age is npm's setting. This project is on pnpm 12, where .npmrc is consulted for registry and authentication only; every other key there is ignored in silence, which is why this PR moved the rest of the configuration out of it. Adding min-release-age to core-web/.npmrc would be a no-op that reads as protection — strictly worse than not having it.
The equivalent control lives in core-web/pnpm-workspace.yaml:
minimumReleaseAge: 10080 # 7 days, the value this rule asks for
minimumReleaseAgeIgnoreMissingTime: falseThe second line matters as much as the first: it closes the silent hole where pnpm skips the check without saying so when the registry omits publish timestamps.
Verified live rather than assumed. With the window raised to a year, pnpm add vite through our registry resolves 7.1.5 (published 2025-09-08) instead of latest 8.3.0 (published 2026-09-10); and an exact pin inside the window fails with ERR_PNPM_MINIMUM_RELEASE_AGE_VIOLATION, naming the computed cutoff. The gate is live and reading real publish dates.
nicobytes
left a comment
nicobytes
left a comment
There was a problem hiding this comment.
Reviewed across correctness / readability / architecture / security / performance, and additionally against the four pnpm guides this migration should be aligned with: docker, supply-chain-security, continuous-integration#github-actions and git-worktrees.
Overall this is a careful and unusually well-documented migration. The configuration move to pnpm-workspace.yaml is correct and complete, dropping corepack is the right call and matches upstream (the pnpm CI docs no longer recommend it), the trustPolicyExclude entries are each justified with a publish date rather than waved through with trustPolicyIgnoreAfter, and trustPolicyExcludePrune: true is a good touch because it stops that list from rotting.
Two things I think have to change before merge, then five alignments with the docs above.
Blocking
1. pnpm is no longer on PATH for anything the Maven build spawns
Full detail inline on nodejs-parent/pom.xml. Short version: the corepack shim used to sit at ${node.install.dir}/pnpm, and ${node.install.dir} is exactly what every pom exports as PATH. npm puts the binary at ${node.install.dir}/bin/pnpm, and ${node.install.dir}/bin is not on that PATH. The new ${pnpm.executable} property fixes the five places that invoke pnpm directly, but not the places that resolve it from PATH:
core-web/libs/dotcms-webcomponents/project.json:34—"command": "pnpm exec stencil build ..."undernx:run-commands. The project has notags, so--exclude='tag:skip:build'does not skip it; it runs inside thenx run-many -t buildthat Maven drives.core-web/libs/sdk/create-app/scripts/verify-package.sh:61—for candidate in pnpm npm; do ... command -v "$candidate". Its comment on line 53 still reads "pnpm— the corepack shim the Maven build puts on PATH", which this PR makes false.
I verified pnpm does not paper over this. With pnpm 12, pnpm exec prepends only node_modules/.bin to the child environment — it does not add the directory holding the pnpm executable:
$ env PATH="$NODE_DIR:/usr/bin:/bin" "$PNPM_DIR/pnpm" exec node -e '...command -v pnpm...'
PATH= .../node_modules/.bin:.../fakenode:/usr/bin:/bin
which pnpm -> NOT FOUND
Both verification environments hide this: your dev shell has a global pnpm, and CI gets one from pnpm/action-setup before Maven runs. So the nested pnpm exec silently resolves to that pnpm — a version nothing in this repo pins — instead of failing. That is a correctness regression against the self-contained toolchain the previous setup actually had, and it will only surface somewhere without an ambient pnpm.
2. dotcms-postman is moved to pnpm 12 with none of this PR's posture
dotcms-postman/package.json is bumped to pnpm@12.4.2, but there is no dotcms-postman/pnpm-workspace.yaml and no dotcms-postman/.npmrc. Detail inline.
Alignment with the pnpm docs
3. devEngines instead of .nvmrc (+ packageManager)
Inline on core-web/package.json. This is the change that makes the Node pin a verified pin rather than a string three files agree on.
4. Git worktrees — virtualStoreType: global is missing
Inline on pnpm-workspace.yaml. Straight out of https://pnpm.io/git-worktrees, and this repo is routinely worked on in several worktrees at once.
5. minimumReleaseAge is inert, and the fix may be in this PR rather than in a follow-up
Inline on core-web/.npmrc. Shipping trustPolicy while pnpm's strongest defence against a freshly compromised release silently does nothing is the weaker half of the posture, and the PR description already contains the evidence that the mirror may be droppable today.
6. pnpm/setup@v2 — optional, but it collapses ~25 lines of CI
Inline on maven-job/action.yml.
7. Docker — nothing to do, and that is worth recording
I checked: no Dockerfile in this repo installs pnpm or builds the frontend (the image consumes the already-built WAR). So ghcr.io/pnpm/pnpm:12, RUN --mount=type=cache,id=pnpm,target=/pnpm/store and pnpm fetch from https://pnpm.io/docker have no surface to apply to here. Noting it so the question is closed rather than left open.
Verified, no action needed
pnpm/action-setupis not deprecated. Its README states it supports pnpm v11 and v12 and that continuing to use it withactions/setup-nodeis supported. So the bump is a legitimate choice, not debt.- The v6.1.0 pin is correct: the annotated tag
v6.1.0dereferences toea17c68df8912ef543352723c149a84f56e3d413. - The AC about
.github/copilot-instructions.mdand.github/instructions/frontend.instructions.mdis satisfied by "nothing to change" — neither file pins a pnpm version or mentionspackage.json#pnpm. Worth saying so explicitly in the PR body so the checklist can be closed honestly. - The
stylusoverride removal is right, and consistent with turning onblockExoticSubdeps— it would have been the only exotic source in the tree.
Stale comments to sweep
.github/actions/core-cicd/maven-job/action.yml:208-210— "the pnpm-era layout (install-node-and-npm + corepack shim)". Corepack is gone; the reason to hashnodejs-parent/pom.xmlis now that<pnpm.version>lives there, which is a better reason and worth stating.core-web/libs/sdk/create-app/scripts/verify-package.sh:53— see blocker 1.core-web/pom.xml:126-130andcore-web/apps/dotcms-ui-e2e/pom.xml— see inline.
Verdict
Request changes — items 1 and 2. Items 3-6 are alignments I'd like a decision on (accept, or reply with why not) rather than silent deferral; 4 in particular is a small diff with a real payoff. Everything else is FYI.
The corepack shim used to sit at ${node.install.dir}/pnpm, which is exactly what
every pom exports as PATH, so pnpm was on PATH for everything the build spawned.
npm puts the binary at ${node.install.dir}/bin/pnpm, and that directory was not
exported: with the old PATH, `command -v pnpm` finds nothing.
Two callers on the build's critical path resolve pnpm from PATH rather than
through ${pnpm.executable} — libs/dotcms-webcomponents/project.json runs
`pnpm exec stencil build` and carries no tags, so --exclude='tag:skip:build'
does not skip it, and libs/sdk/create-app/scripts/verify-package.sh looks pnpm up
with command -v. Both silently fell through to whatever ambient pnpm the machine
happened to have: a developer shell locally, pnpm/action-setup on CI. That is how
this passed verification while being wrong, and it would have failed anywhere
without an ambient pnpm.
A new pnpm.bin.dir property is exported first in all six PATH blocks, so the pnpm
the build installed wins over any ambient one:
PATH=${pnpm.bin.dir}:${node.install.dir}:${env.PATH}
command -v pnpm -> installs/node/bin/pnpm, 12.4.2
npm install also gains --prefer-offline: with the version already installed it
takes 0.82s against 1.30s, and still succeeds with a warm npm cache and no
network at all.
Sweeps the comments the mechanism change made false: the PATH rationale in
core-web/pom.xml described a shim shebang that no longer exists, and
verify-package.sh still described the corepack shim.
Refs #37553
…ting it The module has its own package.json, lockfile and pom, so nothing in core-web reaches it. Bumping it to pnpm 12 without a config of its own left it on the bare defaults while its larger sibling declared a posture — including verifyDepsBeforeRun: install, the one default core-web deliberately overrides because this module's pom also runs its install and then execs through pnpm inside a Maven build that expects an already-installed tree. Refs #37553
.nvmrc records a string that nothing verifies. devEngines.runtime makes pnpm resolve the version and record it in the lockfile with a checksum per platform, so the runtime becomes a verified pin rather than three files agreeing by convention, and scripts run against that pinned runtime. Added alongside .nvmrc and packageManager rather than replacing them: .nvmrc has seven consumers across the workflows and two cache keys, and the CI version-sync guard compares packageManager. Retiring those belongs in its own change. Refs #37553
pnpm/setup installs pnpm's standalone binary and, with cache: true, restores and saves the pnpm store itself keyed on the lockfile. That removes the `pnpm store path` probe and the paired restore/save cache steps, about 25 lines. It reads the version from devEngines.packageManager or packageManager, so the pin stays where the sync guard already checks it. pnpm/action-setup is not deprecated and the v6.1.0 bump it replaces was legitimate; this is the action the pnpm CI docs now recommend, not a fix for anything broken. Refs #37553
With three sources the guard exited on the first mismatch, so a contributor who forgot to bump both manifests would fix one, re-run CI, and only then learn about the other. It now collects them, annotates each offending file, and fails once. Refs #37553
Installing pnpm with npm moved the binary from ${node.install.dir}/pnpm, where
corepack's shim used to sit, to ${node.install.dir}/bin/pnpm. ${pnpm.executable}
covers the direct invocations, but every pom exports ${node.install.dir} as PATH
and nothing put the new bin directory there, so anything resolving pnpm from PATH
stopped finding the pnpm this repo pins:
- core-web/libs/dotcms-webcomponents/project.json runs `pnpm exec stencil build`
through nx:run-commands, and the project carries no tags, so it is not skipped
by --exclude='tag:skip:build'
- core-web/libs/sdk/create-app/scripts/verify-package.sh probes `command -v pnpm`
pnpm does not compensate: it prepends only node_modules/.bin to a child process's
environment, never the directory holding its own executable. The failure stayed
invisible because both verification environments supply an ambient pnpm — a dev
shell locally, pnpm/action-setup on CI — so the nested calls silently resolved to
a pnpm that nothing here pins.
Add ${pnpm.bin.dir} next to ${pnpm.executable} and prepend it in all six PATH
blocks. Verified that the property resolves in dotcms-nodejs-parent and inherits
into dotcms-core-web, dotcms-postman and dotcms-ui-e2e.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dotcms-postman/package.json was bumped to pnpm@12.4.2 but the module has its own package.json and lockfile and no pnpm-workspace.yaml, so pnpm read the pnpm 12 defaults there rather than the posture declared for core-web. The one that bites is verifyDepsBeforeRun, whose default changed to `install` in pnpm 11. pom.xml runs `pnpm install --frozen-lockfile` at generate-resources behind a skippable flag, then `pnpm run start` at integration-test. Reproduced on a copy of this module's manifest and lockfile: with the default, a `pnpm exec`/`pnpm run` on a drifted tree aborts with ERR_PNPM_OUTDATED_LOCKFILE and never runs the command, so a build using -Dskip.npm.install=true fails in the integration-test phase. With verifyDepsBeforeRun: warn it reports the drift and proceeds. Also declares trustPolicy, blockExoticSubdeps and strictDepBuilds so the two modules cannot drift apart silently. `pnpm install --frozen-lockfile` passes all supply-chain policies here with no trustPolicyExclude entries needed (134 entries, 5.5s). No overrides, peerDependencyRules or allowBuilds: this is a three-dependency Newman runner with no peer conflicts and nothing that builds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…smatch Comments left over from the corepack era described the current mechanism incorrectly: the node-binary cache key no longer guards a "corepack shim" (it guards <pnpm.version> and the installs/ layout), and pnpm 12 is a native binary with no `#!/usr/bin/env node` shebang to resolve. The version-sync check grew a third source in this branch but still exited on the first mismatch, so a contributor who forgot to bump both package.json files would fix one, re-run CI, and only then learn about the second. It now loops over the manifests, annotates each offending file, and fails once. Also records why allowBuilds keeps `canvas` even though ignoredOptionalDependencies means it is never installed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
Area : SDK
…dotCMS/core into issue-37553-migrate-core-web-to-pnpm-12
…ns publish dates minimumReleaseAge refuses to resolve a version until it has been public for a given period. It is pnpm's defence against a freshly compromised release, which is usually detected and pulled within hours. It was documented here as inert: the mirror dotcms-npm.b-cdn.net served whichever metadata variant its CDN cache happened to hold, ignoring the Accept header, and the abbreviated variant carries no `time` field. pnpm's default is to skip the check when it cannot date a release, silently, so the setting read as enabled while applying to an arbitrary and shifting subset of packages. The Cloud team has since fixed the pull zone to vary on Accept (#37562), and the field now comes back consistently. So the gate is turned on for real: 7 days rather than the 1-day default, and minimumReleaseAgeIgnoreMissingTime: false so that losing the data fails loudly instead of disabling the gate in silence. Declaring the window explicitly also enables minimumReleaseAgeStrict, so a range with no mature version fails rather than falling back to a young one. Verified: a full resolution passes (16s, nothing in the tree is younger than 7 days), and raising the window to a year fails with ERR_PNPM_MINIMUM_RELEASE_AGE_VIOLATION naming the computed cutoff, which proves the check is live and reading real publish dates rather than passing vacuously. Refs #37553
|
Our fixes for blockers 1 and 2 crossed on the wire — you pushed yours at 10:27, I pushed mine a few minutes earlier, and the merge kept both. I read yours against mine and kept yours in all three places where they overlap: the On your item 5, the situation changed underneath us today. The mirror was not simply stripping So the gate is enforced in this PR rather than deferred, which was your point: Items 3 and 6 are in: Item 4 is the one I am not shipping. I tried it and the payoff is exactly what you said: From the bot's review: measured |
…ore-web-to-pnpm-12
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved Windows PATH, bootstrap verification, runtime ownership, cache invalidation, and maturity-gate issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Migrates core-web and dotcms-postman from pnpm 10.17.1 to 12.4.2, updating configuration, lockfiles, Maven integration, and CI setup.
Changes:
- Moves pnpm configuration into workspace YAML and adds supply-chain policies.
- Pins pnpm and Node versions and regenerates lockfiles.
- Updates npm-based installation, PATH handling, CI setup, caching, and documentation.
File summaries
| File | Reviewed changes |
|---|---|
nodejs-parent/pom.xml |
Installs pnpm via npm and exports its binary path. Critical (1 vote): Windows PATH handling uses the wrong separator and lacks pnpm.cmd. Moderate (1 vote): Corepack signature verification is removed. |
dotcms-postman/pom.xml |
Uses the shared pnpm executable. |
dotcms-postman/pnpm-workspace.yaml |
Adds Postman pnpm configuration. Moderate (1 vote): The seven-day maturity gate is not inherited. |
dotcms-postman/pnpm-lock.yaml |
Regenerates the pnpm 12 lockfile. |
dotcms-postman/package.json |
Pins pnpm 12.4.2. |
core-web/pom.xml |
Updates pnpm execution and PATH handling. |
core-web/pnpm-workspace.yaml |
Adds migrated and supply-chain settings. Moderate (1 vote): minimumReleaseAgeStrict is not explicitly enabled. |
core-web/package.json |
Adds pnpm and Node development pins. |
core-web/libs/sdk/create-app/scripts/verify-package.sh |
Updates pnpm path documentation. |
core-web/CLAUDE.md |
Documents the new pnpm configuration model. |
core-web/apps/dotcms-ui-e2e/pom.xml |
Updates pnpm execution paths. |
core-web/.npmrc |
Retains registry configuration. |
.github/actions/core-cicd/maven-job/action.yml |
Updates setup, version checks, and caching. Moderate (3 votes): the version guard checks only packageManager; Moderate (2 votes): setup installs a second runtime; Moderate (1 vote): the cache key does not cover the Postman lockfile. |
.github/actions/core-cicd/deployment/deploy-javascript-sdk/action.yml |
Uses pnpm/setup. Moderate (1 vote): it installs a second Node runtime instead of using the existing setup-node runtime. |
Review details
Files not reviewed (1)
- dotcms-postman/pnpm-lock.yaml: Generated file
Suppressed comments (5)
.github/actions/core-cicd/deployment/deploy-javascript-sdk/action.yml:167
- With no
runtimeinput,pnpm/setupinstalls every runtime declared indevEngines.runtime. This action already runsactions/setup-nodeearlier, so the new step downloads and prepends a second Node installation; subsequent publish commands use the PNPM_HOME runtime rather than thesetup-nodeinstallation. Remove one runtime owner or configure the setup action not to install the runtime.
uses: pnpm/setup@703c52620218391530e48b9e8870d5c0082e1b9b # v2.1.0
with:
working-directory: core-web
install: false
cache: true
.github/actions/core-cicd/maven-job/action.yml:251
- This composite action is also used by the Postman matrix, but
working-directory: core-webmakes the new cache key depend only oncore-web/pnpm-lock.yaml. Maven runs the Postman install from its separatedotcms-postman/pnpm-lock.yaml, so Postman dependency changes never invalidate this store cache and new packages repeatedly miss the intended cache. Use a module-specific lockfile key or include both lockfiles.
working-directory: core-web
install: false
cache: true
core-web/pnpm-workspace.yaml:116
minimumReleaseAgeStrictis a separate pnpm setting; declaringminimumReleaseAgedoes not enable it. As written, an exact-version update can bypass the seven-day gate, so the security posture described here is not enforced for all installs. SetminimumReleaseAgeStrict: trueexplicitly and update this comment accordingly.
# Declaring this explicitly also turns on minimumReleaseAgeStrict (pnpm 12.3.0+): when no
# version in a range is old enough, resolution fails instead of quietly falling back to a
# young one. `pnpm audit --fix` records a security update that must skip the wait in
# minimumReleaseAgeExclude.
dotcms-postman/pnpm-workspace.yaml:23
- Because
dotcms-postmanhas its ownpnpm-workspace.yaml, it does not inherit core-web'sminimumReleaseAge: 10080orminimumReleaseAgeIgnoreMissingTime: false. Its install therefore only gets pnpm's default maturity behavior, so the 7-day supply-chain gate described by this PR is not applied to Postman dependencies. Add the maturity settings here, or explicitly document and test this module's exemption.
trustPolicy: no-downgrade
blockExoticSubdeps: true
strictDepBuilds: true
nodejs-parent/pom.xml:99
- Replacing Corepack's
preparewithnpm install -g pnpm@12.4.2removes the signature verification that previously protected the pnpm bootstrap. npm resolves the exact version from registry metadata and immediately runs its install script, while the newtrustPolicyonly covers project dependencies, not this package-manager binary. Preserve signature verification or verify the exact pnpm tarball/signature before executing it.
<argument>${node.install.dir}/node_modules/npm/bin/npm-cli.js</argument>
<argument>install</argument>
<argument>-g</argument>
<argument>--prefix</argument>
<argument>${node.install.dir}</argument>
<!-- Measured with the version already installed: 1.30s plain, 0.82s
with this flag, and it still succeeds with a warm npm cache and no
network. Falls back to the registry when the cache cannot answer. -->
<argument>--prefer-offline</argument>
<argument>pnpm@${pnpm.version}</argument>
- Files reviewed: 13/15 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
pnpm/setup resolves its version from devEngines.packageManager before falling back
to packageManager, but the guard compared only packageManager. A bump to the
devEngines value alone would pass the check while the action installed a different
pnpm than the one Maven installs from ${pnpm.version} — no step would fail, and the
build would run on two different pnpm versions depending on who invoked it.
Now checks both pnpm declarations, and devEngines.runtime against <node.js.version>
for the same reason. An absent declaration is still fine; a present one must agree.
The comparison is a plain `if` rather than a `&&` chain: under `set -e` a
short-circuited chain returns non-zero and would abort the step instead of
recording the mismatch and continuing to the next.
Verified by running the script against the current tree (passes, reporting
"pnpm 12.4.2, node 22.22.3") and against copies with devEngines.packageManager
bumped to 12.5.0 and devEngines.runtime to 24.0.0 — both are caught, and both
mismatches are reported in a single run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Moves
core-webfrom pnpm 10.17.1 to pnpm 12.4.2, crossing two majors. The version bump is the small part; what changes is where pnpm configuration lives and what pnpm does by default.package.json#pnpmand reads.npmrcfor registry and auth only, so everything now lives incore-web/pnpm-workspace.yaml, withdotcms-postmandeclaring its own rather than inheriting bare defaults.onlyBuiltDependenciesis gone, replaced by theallowBuildsmap.lessneeded an explicit entry: pnpm 10 printedIgnored build scripts: lessand carried on, while pnpm 12'sstrictDepBuildsraisesERR_PNPM_IGNORED_BUILDSand fails the install.trustPolicy: no-downgrade,blockExoticSubdeps,strictDepBuilds,verifyDepsBeforeRun: warn, and a maturity gate at 7 days (see below).@pnpm/exe.*optional dependencies; npm runs the wrapper'sinstall.jsand links the platform binary insideinstalls/, which CI's node-binary cache already restores. Corepack runs no lifecycle scripts and downloads that binary on first use into~/.cache/node/corepack, outside the cache — a fetch plus a signature check on every job.devEngines. The lockfile now recordsnode@runtime:22.22.3with a sha256 per platform, so the runtime pin is verified rather than three files agreeing by convention..nvmrcstays; it has seven consumers.pnpm/setupv2.1.0, which installs pnpm and handles the store cache itself, replacing the manualpnpm store pathprobe and the paired restore/save steps.Closes #37553
Why pnpm 12 rather than staying on 10 or stopping at 11
pnpm 12 breaks dependency cycles canonically during peer resolution, so the lockfile becomes a pure function of the dependency graph: repeated installs and reordered importers produce byte-identical lockfiles, and peer resolution is 2-3x faster on cycle-heavy trees like this one (33k-line lockfile, 3,011 packages, Angular 22 + PrimeNG + NgRx). Install-side there is a SQLite store index, direct-to-CAS writes, undici, and hardlink-before-clone on Linux. pnpm 11 alone would have been a toll booth, since 12 keeps 11's configuration model.
The maturity gate, and the mirror fix behind it
minimumReleaseAgerefuses to resolve a version until it has been public for a given period. It is pnpm's defence against a freshly compromised release, which is usually detected and pulled within hours, and it needs the registry'stimefield.This PR originally shipped it documented as inert. The registry mirror
dotcms-npm.b-cdn.netwas ignoring theAcceptheader and serving whichever of npm's two metadata documents its CDN cache happened to hold — the abbreviated one carries notime— so the check applied to an arbitrary and shifting subset of packages while reading as enabled. The Cloud team has since fixed the pull zone to vary onAccept, verified across eight packages againstregistry.npmjs.org(#37562, closed).With the data reliably there, the gate is enforced rather than deferred:
minimumReleaseAge: 10080(7 days, against pnpm's 1-day default) andminimumReleaseAgeIgnoreMissingTime: false, so if the publish dates ever stop coming back the install fails loudly instead of silently disabling the check. Declaring the window explicitly also enablesminimumReleaseAgeStrict.trustPolicy exclusions
trustPolicy: no-downgradefails an install when a version carries weaker publish-trust evidence than earlier releases of the same package. Eight lockfile entries trip it. None is a takeover; each is an older release of a package whose other releases carry provenance, and the check compares against the package's whole history regardless of semver line. Every one is listed intrustPolicyExcludewith its publish date and reason, andtrustPolicyExcludePrunedrops entries once the lockfile stops resolving them.semver@5.7.2,semver@6.3.1css-declaration-sorter@6.4.1undici-types@6.21.0detect-port@1.6.1chokidar@4.0.3@modern-js/node-bundle-require@2.68.2,@modern-js/utils@2.68.2A regression this PR introduced and then fixed
The corepack shim used to sit at
${node.install.dir}/pnpm, which is exactly what every pom exports asPATH. npm puts the binary at${node.install.dir}/bin/pnpm, which was not exported, so anything resolvingpnpmfromPATH—dotcms-webcomponents' stencil build,verify-package.sh— silently fell through to whatever ambient pnpm the machine had: a developer shell locally, the CI action's pnpm on runners. It passed verification while being wrong. Apnpm.bin.dirproperty is now exported first in all six<PATH>blocks:Verified locally
pnpm install(core-web, full resolution)pnpm install --frozen-lockfile(what CI and Maven run)pnpm install+--frozen-lockfile(dotcms-postman)nx run-many -t build --exclude='tag:skip:build' --skip-nx-cache./mvnw test -pl :dotcms-core-web -Pvalidate -Dmaven.build.cache.enabled=false, from a deletedinstalls/pnpm install207ms, lint, format:check, build, build-analytics and 43 test suites greenERR_PNPM_MINIMUM_RELEASE_AGE_VIOLATIONnaming the computed cutoff; at 7 days a full resolution passes in 16sstrictDepBuildsis livelessfromallowBuildsfails a forced install withERR_PNPM_IGNORED_BUILDSlockfileVersion: 9.0, no dependency version driftOne task failed in the Maven run,
sdk-create-app:verify-compose-static, for an unrelated local reason: its script callspython3 -c, which a wrapper on that machine refuses in favour ofuv run python3. It never executes the check, and no such wrapper exists on CI.Only CI can verify these
@pnpm/exe.*.deploy-javascript-sdk, which matters because pnpm 11 replaced the npm-delegating publish flow with a native one.Considered and not shipped
virtualStoreType: global. The payoff is real —core-web/node_modulesdrops from 2.1 GB to 10 MB per worktree — but Nx cannot build its project graph with it:chokidar@5.0.0importingReaddirpStreamfromreaddirpresolves to the CommonJS copy through the store links and throws, andnx's own postinstall fails the same way (TypeError: chalk.blue is not a function) without pnpm surfacing it as an install failure. Reverting restores both. Worth its own change rather than riding along here.core-web/apps/ai-evals'prune-lockfiletarget is the only thing in the repo that parsespnpm-lock.yamlrather than hashing it. It fails today onmainfor an unrelated reason —apps/ai-evals/package.jsondoes not exist — and nothing in CI invokes it, so whether Nx 23.1.1 reads the two-document lockfile correctly is untested. Pre-existing.Documentation.
.github/copilot-instructions.mdand.github/instructions/frontend.instructions.mdneeded no change: neither pins a pnpm version nor mentionspackage.json#pnpm.core-web/CLAUDE.mdgains a short section on where configuration now lives and the two installs that now fail where they used to warn.