Migrate client and client-v3 from npm to pnpm

[Tim020]

Summary

Switches client/ and client-v3/ from npm to pnpm 12.5.1 (each keeps its own independent pnpm-lock.yaml — no shared workspace). Electron's removal (#1416) cleared the way for this: Electron Forge needed pnpm's node-linker=hoisted, which would have meant giving up pnpm's strict node_modules for at least one project. With Electron gone, both remaining projects use pnpm's default strict linker.

Why

Recurring npm lockfile problems recorded against this repo:

• npm only records the optional platform/CPU package variants it actually installed locally, so a lockfile regenerated on macOS silently drops the Linux CI variants (npm/cli#7961, #8320).
• Separately, targeted npm install runs have been observed to non-deterministically drop unrelated optional dependency entries from the lockfile.

pnpm resolves from registry metadata rather than local node_modules, so the lockfile is platform-independent by construction. Verified by spot-checking the generated lockfile for @rolldown/binding-* entries across all target platforms, not just darwin-arm64 (the machine that generated it).

What changed

• packageManager: "pnpm@12.5.1" pinned in both package.json; engines.npm removed.
• pnpm-workspace.yaml added per project (pnpm 10+ moved overrides/onlyBuiltDependencies out of package.json's pnpm field into this file — a real breaking change I hit while doing this migration). Each allowlists only the install/postinstall scripts actually needed, with a comment on each explaining what it does:
  • @parcel/watcher (both) — native file-watcher binding used by chokidar for HMR/watch mode
  • vue-demi (client-v3) — selects the Vue 2 vs 3 interop shim; functionally required
  • esbuild (client) — downloads its native binary; required to run at all
  • bootstrap-vue, core-js (both) — harmless donation-message no-ops, allowed for completeness
• client/.npmrc: dropped legacy-peer-deps=true (npm-specific). pnpm warns rather than hard-fails on the pre-existing @vue/test-utils@2.5.1 peer mismatch against vue@2.7.14 — pnpm peers check shows the warning, but it's cosmetic; the full test suite already exercises this pairing and passes.
• CI (nodelint.yml, client-test.yml, playwright-test.yml, build-server.yml) and both Dockerfile frontend build stages switched to pnpm/action-setup@v4 + actions/setup-node (cache: pnpm) + pnpm install --frozen-lockfile. client-test.yml also picked up a pre-existing actions/checkout@v3 → v4 bump for consistency (unrelated drift, fixed while touching the file). dependabot.yml needs no change — its npm ecosystem key already covers pnpm-lock.yaml.
• scripts/update-version.sh, README.md, client-v3/README.md, documentation/development.md updated to reference pnpm. Also fixed two pre-existing doc inaccuracies noticed while editing: the root README's "Client" build instructions pointed at client while describing client-v3's output path, and development.md's "Project Structure" section had dropped client-v3 entirely (introduced by the Electron-removal pass in Remove the Electron desktop app (web-only again) #1416, which turned "three main components: server/client/electron" into "two: server/client" without noticing client-v3 was the actual third).

Testing

Both projects, from a clean pnpm install --frozen-lockfile:

• typecheck, ci-lint, full unit test suite, and production build all pass.
• The full 221-test Playwright suite against client-v3 passes cleanly (confirmed twice — once with a temporary, since-reverted port/tmpfile override to rule out interference from an unrelated concurrent E2E run on the dev machine, and once with no override needed at all).

Not covered here

• client/ still carries an @vue/test-utils peer-dependency warning (see above) — pre-existing, not introduced by this change, and not a functional problem.
• PR Make E2E test server port and cleanup safe for parallel runs #1420 (E2E parallel-isolation) is still open; this PR doesn't depend on it and doesn't touch any of the same files.

🤖 Generated with Claude Code

[github-actions]

Client V3 Test Results

209 tests   209 ✅  0s ⏱️
 14 suites    0 💤
  1 files      0 ❌

Results for commit 0bbb13b.

♻️ This comment has been updated with latest results.

[github-actions]

Client Test Results

95 tests   95 ✅  0s ⏱️
 6 suites   0 💤
 1 files     0 ❌

Results for commit 0bbb13b.

♻️ This comment has been updated with latest results.

[github-actions]

Playwright E2E Results (chromium)

222 tests   222 ✅  2m 9s ⏱️
 14 suites    0 💤
  1 files      0 ❌

Results for commit 0bbb13b.

♻️ This comment has been updated with latest results.

[github-actions]

Playwright E2E Results (firefox)

222 tests   222 ✅  2m 10s ⏱️
 14 suites    0 💤
  1 files      0 ❌

Results for commit 0bbb13b.

♻️ This comment has been updated with latest results.

[github-actions]

Python Test Results

  1 files    1 suites   46s ⏱️
934 tests 934 ✅ 0 💤 0 ❌
939 runs  939 ✅ 0 💤 0 ❌

Results for commit 0bbb13b.

♻️ This comment has been updated with latest results.

[Tim020]

Review round 1

Reviewed head: b3fd9bc1899809e0a2444389ab7cb569c6837ac6

Verdict: changes needed. The migration itself is solid: CI is fully green (unit, lint, typecheck, Playwright on both browsers, Docker, and server builds on all 3 OSes), and pnpm install --frozen-lockfile works cleanly with pnpm 12.5.1. One real regression needs fixing before merge: scripts/update-version.sh breaks under pnpm 12. Engine-strict enforcement is also silently lost.

Previous round follow-up

First round, so there are no previous threads.

SonarQube

Quality gate passed. Data came from the anonymous SonarCloud API and matches the analysed commit for this head SHA.

• All 5 conditions pass: new reliability, security and maintainability ratings are all A; new duplication 0.0% (limit 3%); hotspots reviewed 100%.
• 0 issues and 0 security hotspots.
• Coverage isn't a gate condition here, and none was computed because there are no new coverable lines.

Nothing needed to pass.

New findings

• Critical: 0
• Important: 3
  • update-version.sh fails on the second bump (reproduced).
  • engine-strict is no longer honoured.
  • There are no corepack enable instructions.
• Suggestion: 3
  • The onlyBuiltDependencies blocks do nothing.
  • The Dockerfile duplicates the pnpm version.
  • A devEngines guard would stop accidental npm install.

Findings that couldn't be anchored to diff lines:

• Suggestion: some npm/npx references are left over outside the diff hunks:

  • the lint and ci-lint scripts in both package.json files (npm run format && ...);
  • client/scripts/eslint-filter.sh:83;
  • client/README.md (still npm ci / npm run ...);
  • client-v3/e2e/README.md;
  • comments in client-v3/e2e/global-setup.ts:34, client-v3/src/js/yjs/draftWriterFixture.test.ts:19 and server/test/utils/script/test_draft_writer_interop.py:11.

  The scripts still work because npm ships with Node, but mixing package managers is confusing, and npm run skips pnpm's .npmrc/workspace settings.

• Note: the root CLAUDE.md is gitignored, so this PR can't change it, but the local copy still documents npm ci / npm 11.x throughout. Worth updating locally after merge.

• Note: Dependabot's npm ecosystem does cover pnpm, but its support for pnpm 12 / lockfile v9 hasn't been verified here. Watch its first run after merge.

Strengths

• pnpm/action-setup is pinned by SHA and placed before setup-node with cache: pnpm in every workflow. package_json_file keeps the version in one place.
• The glob override moved correctly to pnpm-workspace.yaml; the lockfile resolves only glob@13.0.6.
• The allowBuilds entries each come with a comment explaining why the build is allowed.
• Dockerfile layer order is right: manifest, lockfile and workspace yaml are copied before install, then the source.
• documentation/development.md now correctly describes the three components and the ui-old/ output path. The old README text wrongly said client builds to server/static/.
• client-v3's prebuild hook still runs under pnpm 12 (verified).

🤖 Generated with Claude Code

[Tim020]

Review round 1: response

All three Important findings and all three Suggestions are fixed in 65d2c7d, and each inline thread has a reply. I reproduced every claim before changing anything (details in the threads); the two that mattered most were real regressions from my original change (pnpm version half-applying a bump, and engine-strict being silently ignored).

Unanchored findings

• Leftover npm/npx references: all converted.
  • lint / ci-lint scripts in both package.json files now use pnpm run. With the new devEngines guard, npm refuses npm run inside these scripts too, so leaving them would have broken linting rather than just being untidy.
  • client/scripts/eslint-filter.sh, client/README.md, client-v3/e2e/README.md.
  • Comments in client-v3/e2e/global-setup.ts, draftWriterFixture.test.ts and test_draft_writer_interop.py (npx vitest is now pnpm exec vitest).
• Local CLAUDE.md: updated locally (npm to pnpm throughout, plus a note on corepack enable, devEngines and where pnpm settings live). It's gitignored, so it isn't part of this diff.
• Dependabot on pnpm 12 / lockfile v9: not verified, agreed. I'll watch its first run after merge rather than guess.

Verification of the fix commit

• Both projects: frozen install, typecheck, ci-lint (running the new nested pnpm run scripts), unit tests (209 + 95) and build.
• Full Playwright suite: 222/222 passing.
• scripts/update-version.sh run end to end on a dirty tree, then reverted.
• Both Dockerfile frontend stages built locally, each resolving pnpm 12.5.1 from packageManager.

One thing I noticed but did not change: pnpm 12.6.0 is already out, and this PR pins 12.5.1. That seems like a separate bump rather than something to fold into this PR.

🤖 Generated with Claude Code

[Tim020]

Review round 2

Reviewed head: 65d2c7decb55d0e6612282c95d99f4002119f306. The incremental diff is b3fd9bc..65d2c7d, a fast-forward with no rebase.

Verdict: ready to merge. Every round 1 finding is fixed and verified against the code, CI is fully green on 65d2c7d (unit, lint, typecheck, Playwright 222/222 on both browsers, docker-build-and-run, server builds on 3 OSes), and the one new item below is an optional Suggestion.

Previous round follow-up

• Verified fixed: 6 of 6. Each thread has a verification reply and is resolved:
  • update-version.sh --no-git-checks
  • engineStrict moved to the workspace file
  • corepack enable docs
  • onlyBuiltDependencies removed, no-op builds set to false
  • Dockerfile version duplication
  • devEngines guard
• Still open: 0. Fix incomplete: 0. Needs human decision: 0.
• Unanchored round 1 items: the leftover npm/npx references are all converted. Converting the nested npm run calls inside the lint/ci-lint scripts turned out to be required, not just tidy, because devEngines makes npm refuse them.

SonarQube

Quality gate passed. Data came from the anonymous SonarCloud API, and the analysed commit matches 65d2c7d.

• All 5 conditions pass: new reliability, security and maintainability ratings are all A; new duplication 0.0% (limit 3%); hotspots reviewed 100%.
• 0 issues and 0 security hotspots.

Nothing needed to pass.

New findings (in the round 2 changes)

• Critical: 0
• Important: 0
• Suggestion: 1. engines.node (>=24.0.0) is lower than the real floor the dependencies need (24.15.0), and with engineStrict that gap now shows up as a confusing transitive-dependency error. This is not a regression; see the inline comment.

Unanchored notes:

• Dependabot on pnpm 12 / lockfile v9 is still unverified; you've said you'll watch its first run after merge.
• A pnpm 12.6.0 bump is reasonably left for a separate PR.

Strengths

• Each fix was reproduced before it was changed and verified afterwards: engine enforcement was confirmed back in the real project, both Docker stages were built locally, and update-version.sh was run end to end on a dirty tree. The replies record that evidence, which made this round quick to verify.
• The allowBuilds comments now say, for every entry, why it runs or is skipped.
• documentation/development.md explains why npm install is refused on purpose, which heads off confusion for new contributors.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud