Support Node 25#7747
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Review Summary by QodoSupport Node 25 with dynamic pnpm resolution and Corepack fallback
WalkthroughsDescription• Support Node 25 by resolving pnpm dynamically via Corepack fallback • Add unified pnpm resolver in src/node/updater/pnpm.ts for updater and plugin code • Auto-bootstrap Corepack in bin/functions.sh when neither pnpm nor Corepack present • Update all CI workflows to Node 25 as default PR target, full matrix on push • Use corepack pnpm in root package.json scripts to honor pinned version Diagramflowchart LR
A["Node 25<br/>no Corepack"] -->|resolvePnpmCommandSync| B["pnpm resolver<br/>src/node/updater/pnpm.ts"]
B -->|prefers| C["pnpm on PATH"]
B -->|fallback| D["corepack pnpm"]
C -->|used by| E["Updater<br/>RollbackHandler<br/>UpdateExecutor"]
D -->|used by| E
C -->|used by| F["Plugin installer<br/>pluginfw"]
D -->|used by| F
G["bin/functions.sh<br/>ensure_pnpm"] -->|bootstrap| D
H["package.json scripts"] -->|corepack pnpm| D
File Changes1. src/node/updater/pnpm.ts
|
Code Review by Qodo
1. Scripts require corepack
|
| "lint": "corepack pnpm --filter ep_etherpad-lite run lint", | ||
| "test": "corepack pnpm --filter ep_etherpad-lite run test", | ||
| "test-utils": "corepack pnpm --filter ep_etherpad-lite run test-utils", | ||
| "test-container": "corepack pnpm --filter ep_etherpad-lite run test-container", | ||
| "dev": "corepack pnpm --filter ep_etherpad-lite run dev", | ||
| "prod": "corepack pnpm --filter ep_etherpad-lite run prod", | ||
| "ts-check": "corepack pnpm --filter ep_etherpad-lite run ts-check", | ||
| "ts-check:watch": "corepack pnpm --filter ep_etherpad-lite run ts-check:watch", | ||
| "test-ui": "corepack pnpm --filter ep_etherpad-lite run test-ui", | ||
| "test-ui:ui": "corepack pnpm --filter ep_etherpad-lite run test-ui:ui", | ||
| "test-admin": "corepack pnpm --filter ep_etherpad-lite run test-admin", | ||
| "test-admin:ui": "corepack pnpm --filter ep_etherpad-lite run test-admin:ui", | ||
| "plugins": "corepack pnpm --filter bin run plugins", | ||
| "install-plugins": "corepack pnpm --filter bin run plugins i", | ||
| "remove-plugins": "corepack pnpm --filter bin run remove-plugins", | ||
| "list-plugins": "corepack pnpm --filter bin run list-plugins", | ||
| "build:etherpad": "corepack pnpm --filter admin run build-copy && corepack pnpm --filter ui run build-copy", | ||
| "build:ui": "corepack pnpm --filter ui run build-copy && corepack pnpm --filter admin run build-copy", | ||
| "makeDocs": "corepack pnpm --filter bin run makeDocs" |
There was a problem hiding this comment.
1. Scripts require corepack 🐞 Bug ≡ Correctness
Root package.json scripts now invoke corepack pnpm ..., so pnpm run prod / `pnpm run build:etherpad will fail on systems that have pnpm installed but no corepack` binary. This contradicts the README’s documented “install pnpm directly” path and the shell wrappers (run_pnpm/exec_pnpm) that are designed to work without corepack.
Agent Prompt
### Issue description
Root npm scripts hardcode `corepack pnpm`, but the repo explicitly supports environments that have `pnpm` on PATH without Corepack. This makes `pnpm run <script>` fail when Corepack is missing.
### Issue Context
- README documents `npm install -g pnpm` as a supported setup.
- `bin/functions.sh` implements `run_pnpm`/`exec_pnpm` to use `pnpm` when available and fall back to `corepack pnpm`.
- Root `package.json` scripts bypass that fallback and always call `corepack`.
### Fix Focus Areas
- package.json[15-35]
### Suggested implementation direction
- Replace `corepack pnpm ...` with a small cross-platform wrapper (Node script) that:
1) prefers `pnpm` if it is runnable,
2) otherwise uses `corepack pnpm` if available,
3) errors with a clear message if neither works.
- Update scripts to call `node ./<wrapper> <pnpm args>` so both Windows and POSIX behave consistently.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
CI failures here are unrelated to Node 25 — all cascade from a develop-side bugEvery red job on this PR fails at the same step in
This is exactly the bug PR #7746 fixes. Once #7746 merges into develop I'll rebase this PR and CI should go fully green — every Node-25-only check (perform type check, shellcheck, Playwright, build, dependency-review, the Linux with Plugins (25) variant on internal-PR path) is already passing. Verified Node-25 paths green:
|
32a86c0 to
20082bc
Compare
Bumps the workflow Node version (PR matrix → [25], full push matrix stays at [22, 24, 25]) and the pinned pnpm to 11.1.2 with a matching `engines.pnpm` minimum. End-users install pnpm the same way they always have (`npm install -g pnpm` works on Node 25 — only Corepack was dropped from the official Node 25 distribution). Also includes two workflow fixes that were entangled with the Node-version edits in the same files: - `upgrade-from-latest-release.yml` now actually checks out the latest release tag instead of `ref: develop #FIXME`, so the job finally exercises what its name implies. - `installer-test.yml` resolves `ETHERPAD_REPO` / `ETHERPAD_BRANCH` from the PR head when running on a fork, so the smoke test exercises the PR branch rather than the base. Verified end-to-end against `node:25-bookworm-slim` (no corepack): `npm install -g pnpm` → `pnpm i` → `pnpm run build:etherpad` → `pnpm run prod` boots and listens on 9001. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
20082bc to
7e7ccd9
Compare
ether#7747 added Node 25 *support* but left the floor at Node 22. This commit completes the cutover so the runtime requirement matches the documentation bumped in the previous commit. - package.json: engines.node ">=22.13.0" → ">=25.0.0" - bin/functions.sh, bin/installer.sh, bin/installer.ps1: REQUIRED_NODE bumped to 25 (controls the error message users see when they invoke the installer or pnpm scripts on an older Node) - Dockerfile: base image node:22-alpine → node:25-alpine (×2). Corepack comment updated: Node 25 no longer ships corepack at all, so we install it from npm rather than refreshing a stale signing-key list - snap/snapcraft.yaml: pinned NODE_VERSION 22.22.2 → 25.9.0 and the surrounding design notes rewritten to reflect Node 25 instead of 22 - .github/workflows/*.yml: matrix dropped from [22, 24, 25] to just [25] (anything older now fails engines anyway). Stale comments in build-and-deploy-docs.yml referencing vite 8's 22.12 floor cleaned up - bin/plugins/lib/npmpublish.yml: setup-node 22 → 25 so the plugin template propagated to every ether/* plugin matches the new minimum Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
node:25-alpine doesn't ship corepack but does pre-install yarn at /usr/local/bin/yarn, so `npm install -g corepack@latest` fails with EEXIST trying to register its yarn shim. Per ether#7747, end-users install pnpm via plain `npm install -g pnpm` on Node 25 — use the same flow in the Dockerfile (and remove the unused yarn binary so it doesn't sit on PATH inside the image). Drops COREPACK_HOME and the related issue-7687 cache-sharing tweak since there's no corepack shim to share. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s) (#7749) * docs: bump documented Node.js minimum to 25 Etherpad is moving its supported Node.js floor to >= 25 (CI matrix is already pinned to 25 across all workflows on the node25-corepack-pnpm11 work). Sync the user-facing documentation so the install instructions, requirements section, and plugin metadata example all reflect the new minimum instead of Node 22 / 12.17. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: require Node.js >= 25 (engines, installers, Dockerfile, snap, CI) #7747 added Node 25 *support* but left the floor at Node 22. This commit completes the cutover so the runtime requirement matches the documentation bumped in the previous commit. - package.json: engines.node ">=22.13.0" → ">=25.0.0" - bin/functions.sh, bin/installer.sh, bin/installer.ps1: REQUIRED_NODE bumped to 25 (controls the error message users see when they invoke the installer or pnpm scripts on an older Node) - Dockerfile: base image node:22-alpine → node:25-alpine (×2). Corepack comment updated: Node 25 no longer ships corepack at all, so we install it from npm rather than refreshing a stale signing-key list - snap/snapcraft.yaml: pinned NODE_VERSION 22.22.2 → 25.9.0 and the surrounding design notes rewritten to reflect Node 25 instead of 22 - .github/workflows/*.yml: matrix dropped from [22, 24, 25] to just [25] (anything older now fails engines anyway). Stale comments in build-and-deploy-docs.yml referencing vite 8's 22.12 floor cleaned up - bin/plugins/lib/npmpublish.yml: setup-node 22 → 25 so the plugin template propagated to every ether/* plugin matches the new minimum Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(docker): install pnpm directly on Node 25 (no corepack) node:25-alpine doesn't ship corepack but does pre-install yarn at /usr/local/bin/yarn, so `npm install -g corepack@latest` fails with EEXIST trying to register its yarn shim. Per #7747, end-users install pnpm via plain `npm install -g pnpm` on Node 25 — use the same flow in the Dockerfile (and remove the unused yarn binary so it doesn't sit on PATH inside the image). Drops COREPACK_HOME and the related issue-7687 cache-sharing tweak since there's no corepack shim to share. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Bumps the workflow Node version to 25 and the pinned pnpm to 11.1.2.
Corepack is gone from the diff. Node 25 stopped distributing Corepack, and the earlier approach in #7741 (
corepack pnpmeverywhere + dynamic resolver + bootstrap helpers) was over-engineering. End-users install pnpm withnpm install -g pnpmexactly as before — that's all that's needed.What this changes (16 files, +42/-38)
[22, 24, 25]matrix.packageManager→pnpm@11.1.2,engines.pnpm→>=11.1.2.upgrade-from-latest-release.ymlnow checks out the latest release tag instead ofref: develop #FIXME, finally exercising what its name implies.installer-test.ymlresolvesETHERPAD_REPO/ETHERPAD_BRANCHfrom the PR head when running on a fork, so the installer smoke test runs against the PR branch.What this does NOT change
src/changes. No new files. No corepack shimming. The runtime updater, plugin migration,bin/installDeps.sh,bin/run.sh, package.json scripts — all untouched vs develop.Verification
End-to-end test in real
node:25-bookworm-slim(confirmed nocorepackon PATH):Local tests:
ts-check,test-utils(mocha) 20/20, vitest 598/598,updater-integration5/5,updater-scheduler-integration2/2.Closes #7741.