mirror of
https://github.com/deepseek-ai/deepseek-harness.git
synced 2026-08-29 04:26:38 +00:00
docs: add ADR 0016 for the pnpm migration
Records the why behind the Yarn 4 → pnpm move (PR #39): ecosystem alignment, strict-linker phantom-dependency safety, build-script allowlisting, and the package-manager-independent constraints script. Captures migration-time install benchmarks and notes the gate suite passes unchanged on pnpm.
This commit is contained in:
@@ -0,0 +1,37 @@
|
||||
# ADR 0016: pnpm as the package manager instead of Yarn 4
|
||||
|
||||
Status: accepted (2026-06-16)
|
||||
|
||||
## Context
|
||||
|
||||
The repo shipped on **Yarn 4** with the `node-modules` linker — a deliberately conservative choice that behaves like npm's flat layout while giving us Yarn's workspaces and `yarn constraints`. It worked. But Yarn 4's Plug'n'Play heritage makes the `node-modules` linker the off-the-beaten-path mode, and the broader JS ecosystem — tooling defaults, CI actions, Corepack examples, contributor familiarity — increasingly centers on pnpm. For a repo that is built primarily by agents and read by occasional human contributors, "the package manager most tools and people expect" has real value: fewer surprises, better-trodden failure paths, more copy-pasteable answers.
|
||||
|
||||
The switching cost is at its lowest right now. Nothing publishes from this repo yet (every package is `private: true`); dev/test/demo all run **unbuilt** via tsx, so the package manager only has to (a) resolve and link `node_modules`, (b) run the workspace scripts, and (c) enforce the workspace constraints. The one Yarn-specific asset is `yarn.config.cjs` (the `@yarnpkg/types` constraints engine), which is small and mechanical to re-express. This mirrors the reasoning in [ADR 0008](0008-tsdown-over-dumble.md): swap a load-bearing tool for the healthier-ecosystem option while the blast radius is still small.
|
||||
|
||||
## Decision
|
||||
|
||||
Adopt **pnpm 11.7.0**, pinned via the `packageManager` field and installed through Corepack (same mechanism Yarn used):
|
||||
|
||||
- **Workspaces** move from the `package.json` `workspaces` array + `.yarnrc.yml` to `pnpm-workspace.yaml` (`vendor/*`, `packages/*` — the same globs; `examples/*` stay non-workspace, matching the prior setup and tsdown's explicit globs).
|
||||
- **Strict symlinked linker** (pnpm's default) replaces Yarn's hoisted `node-modules` linker. We deliberately add **no** `node-linker=hoisted` / `shamefully-hoist` escape hatch: pnpm's non-flat `node_modules` makes phantom dependencies (importing an undeclared transitive dep) fail loudly, which is a *feature* for a repo whose whole quality story is mechanical gates ([ADR 0007](0007-quality-gates.md)). The gate suite — typecheck, lint, test, build, knip — is the safety net that proves no such phantom imports exist.
|
||||
- **Build-script allowlist.** pnpm 10+ does not run dependency lifecycle scripts unless allowlisted. `pnpm-workspace.yaml` carries an explicit `allowBuilds` map (`esbuild`, `lefthook`, `@google/genai`, `protobufjs`) — the same supply-chain-hardening posture the repo already takes toward model/tool output, now applied to install-time code execution. `peerDependencyRules.allowedVersions.typescript: '>=5 <7'` silences benign peer-range warnings for the in-repo TypeScript.
|
||||
- **Constraints become package-manager-independent.** `yarn.config.cjs` (which imported `@yarnpkg/types` and used `Yarn.workspaces()` / `workspace.set()`) is replaced by `scripts/check-workspace-constraints.ts`, a plain tsx script run as `pnpm run constraints`. It enforces the identical invariants — every package `private: true`; `@deepseek-ai/dsh-*` packages declare `cordis` as both a peer- and dev-dependency with matching ranges, `version: 0.0.1`, `type: module`; vendored packages checked for privacy only — over the same `vendor` + `packages` scope.
|
||||
- All `yarn …` verbs across CI, lefthook hooks, `package.json` scripts, and docs become `pnpm …` / `pnpm run …`. `yarn.lock` → `pnpm-lock.yaml` (lockfile v9). `.gitignore` swaps `.yarn/` for `.pnpm-store/`. Vendored READMEs (e.g. `vendor/cordis/README.md`) keep their upstream `yarn` examples untouched per the Vendoring Policy.
|
||||
|
||||
Alternatives considered: **keep Yarn 4** (zero churn, but bets on the less-traveled linker mode and a constraints engine tied to one package manager); **npm workspaces** (ubiquitous, but no constraints story and weaker monorepo ergonomics); **pnpm with hoisted linker** (smoother migration, but throws away the phantom-dependency safety that is the main correctness reason to move).
|
||||
|
||||
## Consequences
|
||||
|
||||
The constraints check loses Yarn's auto-**fix** (`workspace.set()` could rewrite a manifest in place); the tsx script is check-only and exits non-zero with a message instead. This is acceptable — CI never ran `--fix`, and a one-line manual edit is rare. Contributors now `corepack enable` for pnpm rather than Yarn; `pnpm exec lefthook install` replaces `yarn lefthook install` (the `postinstall` hook still runs `lefthook install`).
|
||||
|
||||
Performance (measured at migration time on the dev NFS filesystem; single-digit-run samples, high variance — directional, not a benchmark suite):
|
||||
|
||||
| Scenario | Yarn 4 | pnpm 11 |
|
||||
|---|---|---|
|
||||
| Cold (empty cache/store, no `node_modules`) | ~14 s | ~16 s |
|
||||
| Warm relink (cache/store warm, `node_modules` removed) | ~12–14 s | ~15–22 s |
|
||||
| Frozen, `node_modules` present (no-op revalidate) | ~2–8 s | ~0.5–7 s |
|
||||
|
||||
On a fast local disk pnpm's content-addressed store typically wins on cold/warm installs and, especially, on **disk footprint** across multiple checkouts (one global store hardlinked into every `node_modules` vs Yarn copying ~279 MB per worktree — some devs regularly keep ~10 or more worktrees for this repo). That dedup advantage did **not** show in the migration-time numbers above because the test store and `node_modules` sat on different filesystems, defeating hardlinks; on a single-filesystem dev box or CI cache it applies. The honest summary: install speed on our NFS dev filesystem is a wash within noise; the move is justified by ecosystem alignment, phantom-dependency safety, and cross-checkout disk dedup — not by a raw install-time win.
|
||||
|
||||
All quality gates (constraints, typecheck, lint, doc-sync, test:coverage at 100%, build, knip, publint, echo-agent demo smoke) pass unchanged on pnpm, which is the correctness proof that the linker swap introduced no phantom-dependency breakage.
|
||||
@@ -27,3 +27,4 @@ Do NOT write an ADR for: a mechanical or local choice (a variable name, a one-fi
|
||||
| [0013](0013-property-based-testing.md) | Property-based testing for protocol-shaped code | accepted |
|
||||
| [0014](0014-doc-sync-enforcement.md) | Doc-sync enforcement (doc code blocks + event taxonomy) | accepted |
|
||||
| [0015](0015-structured-error-taxonomy.md) | Structured error taxonomy (HarnessError base) | accepted |
|
||||
| [0016](0016-pnpm-over-yarn.md) | pnpm as the package manager instead of Yarn 4 | accepted |
|
||||
|
||||
Reference in New Issue
Block a user