diff --git a/AGENTS.md b/AGENTS.md index cc9a5479..2c750959 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -92,11 +92,11 @@ The reasoning behind each of these, and what breaks when it is ignored, is the - **A package that moves to the root moves its `vitest.shared.mts` pin with it.** Left pointing at `/node_modules` a pin resolves to a directory that no longer exists — or, where a transitive copy happens to sit there (`chokidar` under `vite`, `react` as a peer of `react-dom` and `ink`), to the very duplicate the pin list exists to prevent. **`react` and `react-dom` are the deliberate exception** and stay pinned per client, so a client's renderer and the React it calls into come from one install; every other root-owned pin resolves from the repo root. - **`dependencies` vs `devDependencies` follows from who consumes it at runtime**, not from where it is declared. Anything `core/` imports at runtime must be a root **`dependency`** — the client builds externalize npm packages and a published install resolves them from the root manifest, where devDependencies are absent. - **The shared toolchain is declared once, at the repo root, and in no client manifest.** `eslint`, `@eslint/js`, `typescript-eslint`, `globals`, `prettier`, `typescript`, `vitest`, `@vitest/coverage-v8` and `@types/node` are used by every client's own scripts, and a client that declares none of them still resolves the root copy by walk-up — `npm run` puts each ancestor `node_modules/.bin` on `PATH`, and Node and TypeScript walk parent `node_modules` / `node_modules/@types` the same way. `clients/launcher` declares no `devDependencies` at all and its `validate` is unchanged. A client-side declaration buys nothing and installs a second copy free to drift, as `globals` (`^17.7.0` root / `^17.4.0` clients) and `typescript-eslint` (`^8.65.0` / `^8.56.1`) had before #2196. These stay **`devDependencies`** — none is consumed at runtime and the tarball ships only each client's `build/`. The boundary is **used by every client**, not "used by one": anything narrower stays where it is, whether one client declares it (`tsx`, `playwright`, `storybook`, `happy-dom`, `ink-testing-library`, `vite-node`, each client's own `@types/*`) or several do — `tsup` is declared in web, cli and tui, and `vite` in web and tui on top of the root **runtime** `dependency` that `--web --dev` needs. Those are out of scope here; consolidating them is a different call with a different rationale. - - ⚠️ **Deleting the declaration does not always delete the copy, and the local copy still wins.** npm auto-installs an unmet **peer** into the install that needs it, and it has no visibility into the root's tree — so a client-only ESLint plugin drags a client-local `eslint` in (`eslint-plugin-react-refresh`/`-storybook` in web, `eslint-plugin-react-hooks` in tui), and web's Storybook/Vitest stack drags in a local `typescript` and `vitest`. A hoisted transitive does the same: `@types/express` puts an `@types/node` in web and cli. Those copies sit *nearer* than the root's and take precedence. The consolidation is therefore about **one declaration and one place to bump**, not about a single copy on disk. ⚠️ **Nothing keeps the surviving copies aligned, and nothing gates them.** A **peer** copy is at least constrained by its holder's peer range — tightly for `vitest` (an exact peer, hence the pin below), loosely for `eslint` (`^9 || ^10`), where the copies agree only because npm resolves the same latest in both installs. A **transitive** copy is constrained by nothing of ours at all, and cli's `@types/node` (`24.13.1` against the root's `24.13.3`) has already diverged on exactly that. `verify:dep-lockstep` does not catch either: it compares only packages that one `tsc` **program** loads from two installs, so a stray `eslint`, `prettier` or `vitest` binary is outside its candidate set entirely, and the cli `@types/node` difference goes unreported because no one program sees both copies. Check a tool copy by hand — `npm exec -- which eslint` from the client — when you change what a client declares. + - ⚠️ **Deleting the declaration does not always delete the copy, and the local copy still wins.** npm auto-installs an unmet **peer** into the install that needs it, and it has no visibility into the root's tree — so a client-only ESLint plugin drags a client-local `eslint` in (`eslint-plugin-react-refresh`/`-storybook` in web, `eslint-plugin-react-hooks` in tui), and web's Storybook/Vitest stack drags in a local `typescript` and `vitest`. A hoisted transitive does the same: `@types/express` puts an `@types/node` in web and cli. Those copies sit *nearer* than the root's and take precedence. The consolidation is therefore about **one declaration and one place to bump**, not about a single copy on disk. ⚠️ **Nothing keeps the surviving copies aligned, and nothing gates them.** A **peer** copy is at least constrained by its holder's peer range — tightly for `vitest` (an exact peer, hence the pin below), loosely for `eslint` (`^9 || ^10`), where the copies agree only because npm resolves the same latest in both installs. A **transitive** copy is constrained by nothing of ours at all, and cli's `@types/node` (`24.13.1` against the root's `24.13.3`) diverged on exactly that. **Since #2226 `verify:dep-lockstep` does catch this class** — its second tier compares every package any install *declares* against every top-level copy across all five installs, independent of what a `tsc` program loads, so a transitive drift and a peer shadow (`eslint`, `typescript`, `vitest`) are both in scope now. Two limits remain: the tier reads lockfiles, so a tool binary you installed by hand and never committed is still invisible; and it only compares names some manifest declares, so a purely transitive package no manifest names is out of scope in both tiers unless a `tsc` program loads both copies. Aligning a stale install is `npm update ` there; a transitive copy that will not move takes an `overrides` entry in that install (`clients/cli` pins `@types/node` this way). - ⚠️ **`vitest`, `@vitest/coverage-v8` and web's `@vitest/browser-playwright` are pinned exactly, and move together.** `@vitest/browser-playwright` declares an **exact** peer on `vitest`, so it — not the root range — decides which `vitest` web installs. Left to float, the root resolves a newer patch and web's tests then run on one `vitest` while loading a coverage provider built against another. Bumping means editing all three in one change, the same discipline the exact `prettier` pin (#1790) exists for. - **A root-declared package that `core/` imports at runtime must also be named in all three bundler `external` lists** (`clients/{cli,tui}/tsup.config.ts`, `clients/web/tsup.runner.config.ts`), since which client reaches it is a function of what `core/` imports rather than of what the client's own code names. `npm run verify:bundle-externals` enforces this against the **built output**. - **A dependency that renders React components must be bundled** into the client that uses it (`noExternal`) and declared only there — an externalized one resolves its own `react` and splits the tree. `ink` is the single exemption, on cost, and it is only safe while the root `react` range stays open to the whole major (`^19.0.0`). -- **One version per install-crossing dependency.** When bumping a dependency the shared sources pull in, bump it in every install that declares it. Consolidating to the root is what makes most of these unbumpable in two places at once, but it does not retire the rule — a client's `devDependencies`, and any package that arrives transitively into a client install, can still skew against the root. Never raise the tsc heap to work around one. `npm run verify:dep-lockstep` enforces this. +- **One version per install-crossing dependency.** When bumping a dependency the shared sources pull in, bump it in every install that declares it. Consolidating to the root is what makes most of these unbumpable in two places at once, but it does not retire the rule — a client's `devDependencies`, and any package that arrives transitively into a client install, can still skew against the root. Never raise the tsc heap to work around one. `npm run verify:dep-lockstep` enforces this in two tiers: packages that reach one `tsc` **program** from two installs (the #1896 heap-exhaustion class), and — since #2226 — every package any install **declares** that more than one install holds a top-level copy of, whether or not a program ever sees both. - **Pin a transitive dependency with an `overrides` entry**, not with `npm audit fix` — which "resolves" an advisory with no upward escape by silently downgrading. ### Dependency updates are issue-driven, like everything else diff --git a/clients/cli/package-lock.json b/clients/cli/package-lock.json index 17dab282..a8e46a61 100644 --- a/clients/cli/package-lock.json +++ b/clients/cli/package-lock.json @@ -907,9 +907,9 @@ "license": "MIT" }, "node_modules/@types/node": { - "version": "24.13.1", - "resolved": "https://registry.npmjs.org/@types/node/-/node-24.13.1.tgz", - "integrity": "sha512-RSpUJGmvsJ1ZeBehQZFhIdpsz+bIpES0nIQXko4Ybq+N+kX6XvOq3Jo+iJ82FWLdblFq85AsMikd3m35jgezYg==", + "version": "24.13.3", + "resolved": "https://registry.npmjs.org/@types/node/-/node-24.13.3.tgz", + "integrity": "sha512-Dh8vAsV36ig5wa9OX4pXvMc9D3Veibfw2wix0CUwYODLD8nkj9UsLjASr49nPg+2eKzxhBV+v7L8pXvT4e639Q==", "dev": true, "license": "MIT", "dependencies": { diff --git a/clients/cli/package.json b/clients/cli/package.json index 70f320a6..11ef4342 100644 --- a/clients/cli/package.json +++ b/clients/cli/package.json @@ -36,6 +36,7 @@ "tsup": "^8.5.0" }, "overrides": { + "@types/node": "^24.12.4", "esbuild": "^0.28.2" } } diff --git a/clients/tui/package-lock.json b/clients/tui/package-lock.json index 9b4ebc83..5c105f2f 100644 --- a/clients/tui/package-lock.json +++ b/clients/tui/package-lock.json @@ -1713,9 +1713,9 @@ "peer": true }, "node_modules/@types/react": { - "version": "19.2.17", - "resolved": "https://registry.npmjs.org/@types/react/-/react-19.2.17.tgz", - "integrity": "sha512-MXfmqaVPEVgkBT/aY0aGCkRWWtByiYQXo3xdQ8r5RzuFrPiRn8Gar2tQdXSUQ2GKV3bkXckek89V8wQBY2Q/Aw==", + "version": "19.2.18", + "resolved": "https://registry.npmjs.org/@types/react/-/react-19.2.18.tgz", + "integrity": "sha512-AnzbBERsrLKtk2XSfTbYRLjQPdy116Sty4q+T+Bp3IC4l6jNBvreVPAHmpq9qhXQM7CXZPjLVmGMw9sy+hxQ3w==", "devOptional": true, "license": "MIT", "dependencies": { @@ -3642,9 +3642,9 @@ } }, "node_modules/react": { - "version": "19.2.7", - "resolved": "https://registry.npmjs.org/react/-/react-19.2.7.tgz", - "integrity": "sha512-HNe9WslTbXmFK8o8cmwgAeJFSBvt1bPdHCVKtaaV+WlAN36mpT4hcRpwbf3fY56ar2oIXzsBpOAiIRHAdY0OlQ==", + "version": "19.2.8", + "resolved": "https://registry.npmjs.org/react/-/react-19.2.8.tgz", + "integrity": "sha512-PWaYA1L/q9u2u7xYQi+Y3L3Yfnie7XyLeaJICV1MGD6LprsBxcAqGjYyr0eY3p+QdsA+x/Irkt4Qif8D63+Sbw==", "license": "MIT", "peer": true, "engines": { diff --git a/clients/web/package-lock.json b/clients/web/package-lock.json index 68b47d54..17a1e846 100644 --- a/clients/web/package-lock.json +++ b/clients/web/package-lock.json @@ -3717,9 +3717,9 @@ "license": "ISC" }, "node_modules/@vitejs/plugin-react": { - "version": "6.0.5", - "resolved": "https://registry.npmjs.org/@vitejs/plugin-react/-/plugin-react-6.0.5.tgz", - "integrity": "sha512-BOVzne/NL162sMdResB25mUv+vWMF5NoAjNf09TeGlE7ZpszZWSD3winycicLJw72yeVsoCn/2kOhEuCvEShMA==", + "version": "6.1.1", + "resolved": "https://registry.npmjs.org/@vitejs/plugin-react/-/plugin-react-6.1.1.tgz", + "integrity": "sha512-yxLaQV9gkhS8ezJqCM6+ndU7mDY6gqAg75NQ+0IjwEI8IYOmQCgkRwHKVSfWXW076DsqMo0Dk+0FK1U+M5RgFw==", "dev": true, "license": "MIT", "dependencies": { @@ -3731,6 +3731,7 @@ "peerDependencies": { "@rolldown/plugin-babel": "^0.1.7 || ^0.2.0", "babel-plugin-react-compiler": "^1.0.0", + "oxc-transform-react": "^0.145.0", "vite": "^8.0.0" }, "peerDependenciesMeta": { @@ -3739,6 +3740,9 @@ }, "babel-plugin-react-compiler": { "optional": true + }, + "oxc-transform-react": { + "optional": true } } }, diff --git a/package-lock.json b/package-lock.json index ba8b251e..ee754a1d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1596,12 +1596,12 @@ } }, "node_modules/@vitejs/plugin-react": { - "version": "6.0.2", - "resolved": "https://registry.npmjs.org/@vitejs/plugin-react/-/plugin-react-6.0.2.tgz", - "integrity": "sha512-DlSMqo4WhThw4vB8Mpn0Woe9J+Jfq1geJ61AKW0QEgLzGMNwtIMdxbDUzLxcun8W7NbJO0e2Jg/Nxm3cCSVzzg==", + "version": "6.1.1", + "resolved": "https://registry.npmjs.org/@vitejs/plugin-react/-/plugin-react-6.1.1.tgz", + "integrity": "sha512-yxLaQV9gkhS8ezJqCM6+ndU7mDY6gqAg75NQ+0IjwEI8IYOmQCgkRwHKVSfWXW076DsqMo0Dk+0FK1U+M5RgFw==", "license": "MIT", "dependencies": { - "@rolldown/pluginutils": "^1.0.0" + "@rolldown/pluginutils": "^1.0.1" }, "engines": { "node": "^20.19.0 || >=22.12.0" @@ -1609,6 +1609,7 @@ "peerDependencies": { "@rolldown/plugin-babel": "^0.1.7 || ^0.2.0", "babel-plugin-react-compiler": "^1.0.0", + "oxc-transform-react": "^0.145.0", "vite": "^8.0.0" }, "peerDependenciesMeta": { @@ -1617,6 +1618,9 @@ }, "babel-plugin-react-compiler": { "optional": true + }, + "oxc-transform-react": { + "optional": true } } }, @@ -4429,9 +4433,9 @@ } }, "node_modules/react": { - "version": "19.2.7", - "resolved": "https://registry.npmjs.org/react/-/react-19.2.7.tgz", - "integrity": "sha512-HNe9WslTbXmFK8o8cmwgAeJFSBvt1bPdHCVKtaaV+WlAN36mpT4hcRpwbf3fY56ar2oIXzsBpOAiIRHAdY0OlQ==", + "version": "19.2.8", + "resolved": "https://registry.npmjs.org/react/-/react-19.2.8.tgz", + "integrity": "sha512-PWaYA1L/q9u2u7xYQi+Y3L3Yfnie7XyLeaJICV1MGD6LprsBxcAqGjYyr0eY3p+QdsA+x/Irkt4Qif8D63+Sbw==", "license": "MIT", "engines": { "node": ">=0.10.0" diff --git a/scripts/verify-dep-lockstep.main.test.mjs b/scripts/verify-dep-lockstep.main.test.mjs index 810bff6d..9295b5cd 100644 --- a/scripts/verify-dep-lockstep.main.test.mjs +++ b/scripts/verify-dep-lockstep.main.test.mjs @@ -82,6 +82,9 @@ function makeFixture({ webTsconfig, rootNestedInner, cliDeps, + rootManifestDeps, + webManifestDeps, + webSolo, } = {}) { // realpath matters: on macOS `tmpdir()` is `/var/...`, a symlink to // `/private/var/...`. The guard only runs `main()` when `import.meta.url` @@ -101,6 +104,7 @@ function makeFixture({ write("package.json", { name: "fixture", + ...(rootManifestDeps ? { dependencies: rootManifestDeps } : {}), scripts: scripts ?? { validate: "npm run verify:format-coverage && npm run verify:dep-lockstep", "verify:format-coverage": "node scripts/verify-format-coverage.mjs", @@ -121,6 +125,7 @@ function makeFixture({ ); write("clients/web/package.json", { name: "web", + ...(webManifestDeps ? { devDependencies: webManifestDeps } : {}), scripts: webScripts ?? { typecheck: "tsc --noEmit -p tsconfig.json" }, }); write( @@ -166,6 +171,20 @@ function makeFixture({ stub(installRoot, versions, "inner", "export type Inner = number;\n"); } stub("", root, "solo", "export type Solo = string;\n"); + // `solo` under the CLIENT install as well. Nothing in `clients/web/src` + // imports it, so the program still resolves exactly one copy (the root's, via + // the shared `core/`) — the declared tier's whole point is that it sees this + // second copy anyway (#2226). + const webLockDeps = { ...web }; + if (webSolo) { + stub( + "clients/web/", + { solo: webSolo }, + "solo", + "export type Solo = string;\n", + ); + webLockDeps.solo = webSolo; + } // The root reaches `inner` only through a copy NESTED under `outer`, at a // version its top-level entry does not carry. npm's own conflict resolution, @@ -182,7 +201,7 @@ function makeFixture({ } write("package-lock.json", lock(rootLockDeps)); - write("clients/web/package-lock.json", rawWebLock ?? lock(web)); + write("clients/web/package-lock.json", rawWebLock ?? lock(webLockDeps)); // A second client, to prove a third install's copy is not dragged into a // comparison it never took part in. Its program spans only its own install. @@ -503,3 +522,75 @@ test("main: exits 1 when the root validate no longer runs the sibling guard", () }, ); }); + +// --------------------------------------------------------------------------- +// The declared tier (#2226) — skew across installs that no ONE program sees. +// --------------------------------------------------------------------------- + +test("main: a DECLARED package skewed across installs fails even when no program holds both copies (#2226)", () => { + // The exact shape the issue reported. `solo` is installed under the root and + // under `clients/web`, and both manifests name it — but only the shared + // `core/` imports it, so the client's program resolves the root's copy alone + // and the program tier is silent, correctly. The declared tier is what sees + // it, which is the whole reason it exists: cli's `@types/node` was hoisted in + // via `@types/express` and no program ever met the root's copy. + withFixture( + { + rootManifestDeps: { solo: "^7.0.0" }, + webManifestDeps: { solo: "^7.0.0" }, + webSolo: "7.0.0", + }, + (dir) => { + const { status, out } = runGuard(dir); + assert.equal(status, 1, out); + assert.match(out, /1 declared dependency resolves/); + assert.match(out, /7\.8\.9\s+\(\.\/node_modules\/solo\)/); + assert.match(out, /7\.0\.0\s+\(clients\/web\/node_modules\/solo\)/); + // The program tier must NOT have claimed it — its message names a program. + assert.doesNotMatch(out, /in clients\/web\/tsconfig\.json/); + }, + ); +}); + +test("main: a declared package held at the same version in both installs passes and is counted", () => { + withFixture( + { + rootManifestDeps: { solo: "^7.0.0" }, + webManifestDeps: { solo: "^7.0.0" }, + webSolo: "7.8.9", + }, + (dir) => { + const { status, out } = runGuard(dir); + assert.equal(status, 0, out); + assert.match(out, /1 declared dependencies agree/); + }, + ); +}); + +test("main: an installed package NO manifest declares is not a declared-tier candidate", () => { + // `outer` and `inner` sit in both installs in every fixture, but no manifest + // names them. The declared tier's boundary is what the repo declares, so its + // candidate count is zero here — the program tier is what covers those. + withFixture({}, (dir) => { + const { status, out } = runGuard(dir); + assert.equal(status, 0, out); + assert.match(out, /0 declared dependencies agree/); + }); +}); + +test("main: a declared package only ONE install holds cannot skew", () => { + // `solo` is declared at the root and listed in the root lockfile only. One + // holder is not an install-crossing dependency, so its version is nobody + // else's business. + withFixture( + { + rootManifestDeps: { solo: "^7.0.0" }, + webDeps: { outer: "1.2.3", inner: "4.5.6" }, + }, + (dir) => { + const { status, out } = runGuard(dir); + assert.equal(status, 0, out); + assert.match(out, /0 declared dependencies agree/); + }, + ); +}); diff --git a/scripts/verify-dep-lockstep.mjs b/scripts/verify-dep-lockstep.mjs index 05d9277a..26eebe9d 100644 --- a/scripts/verify-dep-lockstep.mjs +++ b/scripts/verify-dep-lockstep.mjs @@ -44,6 +44,40 @@ // packages verified to tolerate it (below). A dependency that starts skewing // fails `validate` and forces a decision, rather than surfacing months later as // an unexplained OOM. +// +// ## The second tier: declared packages, whatever any program loads (#2226) +// +// The program-derived tier above is deliberately narrow — it asks only "can two +// structurally-distinct copies of a type meet inside ONE `tsc` program", because +// that is the question the #1896 heap exhaustion turned on. That narrowness is a +// real blind spot, and `AGENTS.md` documented it before anything measured it: +// `clients/cli` resolved `@types/node` 24.13.1 against the root's 24.13.3, and +// this guard reported OK — correctly, since no one program sees both copies. +// +// So a SECOND tier runs alongside it, asking the weaker but broader question +// `AGENTS.md`'s "one version per install-crossing dependency" rule actually +// states: does a package that this repo DECLARES somewhere resolve to two +// different versions across our installs at all? The candidate set is every name +// in any install's `dependencies`/`devDependencies` (root or client) that more +// than one install holds a top-level copy of — 17 packages today, against the +// program tier's much smaller set. +// +// The two tiers are complementary and neither subsumes the other: +// +// • The program tier sees a copy NO manifest names — the `@modelcontextprotocol/sdk` +// case, whose `.d.ts` files arrive only through another package's types. +// • The declared tier sees a copy that arrives TRANSITIVELY into one install +// and is therefore constrained by no range of ours: cli's `@types/node` is +// hoisted via `@types/express`, so nothing pulled it forward when the root +// moved. It also sees two CLIENTS disagreeing (`@types/react` in web against +// tui), which no single program can, and the tool copies `AGENTS.md` calls +// out as ungated — `eslint`, `typescript`, `vitest` — which are peer shadows +// that no `tsc` program loads at all. +// +// The declared tier is the weaker signal, so it is the one to reach for an +// allowlist entry on. It is still deny-by-default: the cost of a false alarm is +// one `npm update ` in the stale install, and the cost of a miss is the +// class of bug that reproduces on one machine and not another. import { readFileSync, existsSync, readdirSync } from "node:fs"; import path from "node:path"; @@ -97,6 +131,17 @@ const repoRoot = path.resolve( // a better basis for a rationale than a pre-emptive entry. const TOLERATED_SKEW = new Map(); +// The declared tier's allowlist, same shape and same within-a-major rule as +// TOLERATED_SKEW above, and empty for the same reason: an entry is worth writing +// only with a real version pair in hand. +// +// Note what is deliberately NOT listed. `react` and `react-dom` are pinned per +// client on purpose (`AGENTS.md`, so a client's renderer and the React it calls +// into come from one install) — but "two copies by design" is not "two versions +// by design", and the rule that survives is still one version everywhere. The +// same holds for `vite` and `tsup`, declared by several clients each. +const TOLERATED_DECLARED_SKEW = new Map(); + /** * Installed versions in a parsed lockfile, keyed by the **install path** npm * writes — `node_modules/zod`, `node_modules/a/node_modules/zod`. @@ -210,11 +255,99 @@ export function majorOf(version) { * listed package whose holders disagree on major is still a failure. */ export function partitionSkew(skewed, tolerated = TOLERATED_SKEW) { - const isTolerated = (s) => { - if (!tolerated.has(s.name)) return false; - const majors = new Set(skewHolders(s).map((h) => majorOf(h.version))); - return majors.size === 1 && !majors.has(null); + const isTolerated = (s) => toleratesSkew(s.name, skewHolders(s), tolerated); + return { + failures: skewed.filter((s) => !isTolerated(s)), + ignored: skewed.filter(isTolerated), }; +} + +/** + * Whether `tolerated` excuses this package's skew: it must be listed, AND its + * holders must agree on major. Shared by both tiers so the within-a-major rule + * cannot be stated twice and drift — a listed package that splits across a major + * fails in either tier. + */ +export function toleratesSkew(name, holders, tolerated) { + if (!tolerated.has(name)) return false; + const majors = new Set(holders.map((h) => majorOf(h.version))); + return majors.size === 1 && !majors.has(null); +} + +/** + * Every package name any install declares, across `dependencies` and + * `devDependencies`. + * + * `manifests` is `[{ dir, manifest }]`. Both fields count because the boundary + * this tier polices is "does the repo name it", not "does it ship": a + * devDependency skew is exactly the `@types/*` and toolchain case #2226 is + * about. `peerDependencies` and `optionalDependencies` are NOT declarations of + * what this repo installs — a peer range is a constraint on the consumer, and + * the copy npm auto-installs to satisfy one is caught anyway, because it lands + * top-level in an install whose sibling declares the same name. + */ +export function declaredPackages(manifests) { + const names = new Set(); + for (const { manifest } of manifests) + for (const field of ["dependencies", "devDependencies"]) + for (const name of Object.keys(manifest?.[field] ?? {})) names.add(name); + return names; +} + +/** + * An install's TOP-LEVEL installed versions — `node_modules/`, with + * nested copies (`node_modules/a/node_modules/`) excluded. + * + * Top-level is the right depth here: it is what the install's own code resolves, + * and it is the single copy a `` range in that manifest governs. A nested + * copy exists precisely because some dependency asked for a different version, + * so it is constrained by that dependency's range rather than by anything we + * own — reporting it would fail the gate on a decision that is not ours to make. + * (The program tier prices nested copies, because a program that actually loads + * one is a different question.) + */ +export function topLevelVersions(lock) { + const versions = new Map(); + for (const [entryPath, version] of lockVersionsByPath(lock)) { + const rest = entryPath.slice("node_modules/".length); + if (rest.includes("node_modules/")) continue; + versions.set(rest, version); + } + return versions; +} + +/** + * Declared packages whose top-level copies disagree across installs. + * + * `declared` is {@link declaredPackages}' output and `versions` maps an install + * dir to that install's {@link topLevelVersions}. An install that holds no + * top-level copy of a name simply is not a holder — absence is not skew, and a + * package only one install holds cannot cross an install boundary at all. + * + * Returns `[{ name, holders: [{ dir, version }] }]` sorted by name, holders in + * install order. + */ +export function findDeclaredSkew(declared, versions) { + const skewed = []; + for (const name of [...declared].sort()) { + const holders = []; + for (const [dir, byName] of versions) { + const version = byName.get(name); + if (version !== undefined) holders.push({ dir, version }); + } + if (holders.length < 2) continue; + if (new Set(holders.map((h) => h.version)).size > 1) + skewed.push({ name, holders }); + } + return skewed; +} + +/** {@link partitionSkew} for the declared tier's `{ name, holders }` entries. */ +export function partitionDeclaredSkew( + skewed, + tolerated = TOLERATED_DECLARED_SKEW, +) { + const isTolerated = (s) => toleratesSkew(s.name, s.holders, tolerated); return { failures: skewed.filter((s) => !isTolerated(s)), ignored: skewed.filter(isTolerated), @@ -495,13 +628,86 @@ export function main() { process.exit(1); } + // ---- Second tier: every DECLARED package held by more than one install. + // Independent of what any `tsc` program loads, so it sees the transitive and + // peer-shadow copies the program tier structurally cannot (#2226). + const manifests = dirs.map((dir) => { + const file = path.join(repoRoot, dir, "package.json"); + try { + return { dir, manifest: JSON.parse(readFileSync(file, "utf8")) }; + } catch (cause) { + throw new Error( + `verify:dep-lockstep — could not parse ${dir}/package.json.`, + { cause }, + ); + } + }); + const declaredVersions = new Map( + locks.map(({ dir, lock }) => [dir, topLevelVersions(lock)]), + ); + const declared = declaredPackages(manifests); + const declaredSkew = findDeclaredSkew(declared, declaredVersions); + const { failures: declaredFailures, ignored: declaredIgnored } = + partitionDeclaredSkew(declaredSkew); + + if (declaredFailures.length > 0) { + console.error( + `verify:dep-lockstep — ${declaredFailures.length} declared ${declaredFailures.length === 1 ? "dependency resolves" : "dependencies resolve"} to different versions across installs:\n`, + ); + let anyListed = false; + for (const failure of declaredFailures) { + const listed = TOLERATED_DECLARED_SKEW.has(failure.name); + anyListed ||= listed; + console.error( + ` ${failure.name}${listed ? " (allowlisted — but this is a MAJOR skew)" : ""}`, + ); + for (const { dir, version } of failure.holders) + console.error( + ` ${version} (${dir}/node_modules/${failure.name})`, + ); + } + console.error( + "\nThese are packages this repo declares somewhere, held at two versions across our installs —" + + "\nthe `one version per install-crossing dependency` rule in AGENTS.md. Unlike the check above," + + "\nthis tier does not require the copies to meet in one `tsc` program, so it catches a copy that" + + "\narrives TRANSITIVELY into one install (cli's `@types/node`, hoisted via `@types/express`, #2226)" + + "\nor as a peer shadow no program loads at all.", + ); + console.error( + "\nAlign them with `npm update ` in whichever install is behind — that moves the lockfile" + + "\nwithin the declared range, without widening a range as `npm install @` would. If a" + + "\ntransitive copy will not move, pin it with an `overrides` entry in that install (see" + + "\nclients/cli). If the skew is genuinely benign, add it to TOLERATED_DECLARED_SKEW in" + + "\nscripts/verify-dep-lockstep.mjs with the reason.", + ); + if (anyListed) + console.error( + "\nNote: an allowlisted package is tolerated only WITHIN a major version. Align the major.", + ); + process.exit(1); + } + const note = ignored.length > 0 ? `, ${ignored.length} tolerated` : ""; + const declaredNote = + declaredIgnored.length > 0 ? `, ${declaredIgnored.length} tolerated` : ""; console.log( `verify:dep-lockstep — OK: ${found.size} install-crossing dependencies agree across ${dirs.length} installs${note} ` + - `(derived from ${programs.length} tsc programs).`, + `(derived from ${programs.length} tsc programs); ` + + `${countDeclaredHeld(declared, declaredVersions)} declared dependencies agree across those installs${declaredNote}.`, ); } +/** How many declared packages more than one install holds — the declared tier's candidate count. */ +export function countDeclaredHeld(declared, versions) { + let n = 0; + for (const name of declared) { + let holders = 0; + for (const byName of versions.values()) if (byName.has(name)) holders += 1; + if (holders > 1) n += 1; + } + return n; +} + // Run only when executed directly (`node scripts/verify-dep-lockstep.mjs`); // importing this file (tests) exposes the pure helpers without running the guard. if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) diff --git a/scripts/verify-dep-lockstep.test.mjs b/scripts/verify-dep-lockstep.test.mjs index edcbbc4f..b648c276 100644 --- a/scripts/verify-dep-lockstep.test.mjs +++ b/scripts/verify-dep-lockstep.test.mjs @@ -13,11 +13,17 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { clientProjects, + countDeclaredHeld, + declaredPackages, + findDeclaredSkew, findSkew, hasReadableLockShape, lockVersionsByPath, majorOf, + partitionDeclaredSkew, partitionSkew, + toleratesSkew, + topLevelVersions, } from "./verify-dep-lockstep.mjs"; test("clientProjects: a `typecheck` script's projects win over references", () => { @@ -402,3 +408,247 @@ test("majorOf: prerelease and build metadata are irrelevant", () => { for (const bad of ["next", "", undefined, null, "v4.4.3"]) assert.equal(majorOf(bad), null, JSON.stringify(bad)); }); + +// --------------------------------------------------------------------------- +// The declared tier (#2226). One case per rule, same convention as above. +// --------------------------------------------------------------------------- + +test("declaredPackages: dependencies and devDependencies from every install", () => { + // devDependencies count because the boundary is "does the repo name it", not + // "does it ship" — `@types/node` and the toolchain are exactly the case #2226 + // is about, and all of them are devDependencies. + assert.deepEqual( + [ + ...declaredPackages([ + { dir: ".", manifest: { dependencies: { react: "^19.0.0" } } }, + { + dir: "clients/web", + manifest: { + dependencies: { "react-dom": "^19.2.4" }, + devDependencies: { "@types/react": "^19.2.14" }, + }, + }, + ]), + ].sort(), + ["@types/react", "react", "react-dom"], + ); +}); + +test("declaredPackages: peer and optional ranges are not declarations", () => { + // A peer range constrains the consumer rather than naming what we install; + // the copy npm auto-installs to satisfy one is still caught, because it lands + // top-level in an install whose sibling declares the same name. + assert.deepEqual( + [ + ...declaredPackages([ + { + dir: ".", + manifest: { + peerDependencies: { react: "^19.0.0" }, + optionalDependencies: { fsevents: "^2.3.3" }, + }, + }, + ]), + ], + [], + ); +}); + +test("declaredPackages: a manifest with no dependency fields contributes nothing", () => { + assert.equal( + declaredPackages([{ dir: "clients/launcher", manifest: { name: "l" } }]) + .size, + 0, + ); +}); + +test("topLevelVersions: nested copies are excluded", () => { + // Top-level is what the install's own code resolves and what a range in that + // manifest governs. A nested copy exists because some dependency asked for a + // different version, so it is constrained by that dependency, not by us. + const versions = topLevelVersions({ + lockfileVersion: 3, + packages: { + "": { name: "fixture" }, + "node_modules/zod": { version: "4.4.3" }, + "node_modules/outer/node_modules/zod": { version: "3.0.0" }, + "clients/web/node_modules/zod": { version: "9.9.9" }, + }, + }); + assert.deepEqual([...versions], [["zod", "4.4.3"]]); +}); + +test("findDeclaredSkew: two installs disagreeing is skew, and every holder is named", () => { + const skewed = findDeclaredSkew( + new Set(["@types/node"]), + new Map([ + [".", new Map([["@types/node", "24.13.3"]])], + ["clients/cli", new Map([["@types/node", "24.13.1"]])], + ["clients/web", new Map([["@types/node", "24.13.3"]])], + ]), + ); + assert.deepEqual(skewed, [ + { + name: "@types/node", + holders: [ + { dir: ".", version: "24.13.3" }, + { dir: "clients/cli", version: "24.13.1" }, + { dir: "clients/web", version: "24.13.3" }, + ], + }, + ]); +}); + +test("findDeclaredSkew: two CLIENTS disagreeing is skew, with no root copy involved", () => { + // No single `tsc` program can see this pair, which is why the program tier + // structurally cannot report it (`@types/react`, web against tui). + assert.deepEqual( + findDeclaredSkew( + new Set(["@types/react"]), + new Map([ + [".", new Map()], + ["clients/web", new Map([["@types/react", "19.2.18"]])], + ["clients/tui", new Map([["@types/react", "19.2.17"]])], + ]), + ).map((s) => s.name), + ["@types/react"], + ); +}); + +test("findDeclaredSkew: one holder cannot skew, and agreement is not skew", () => { + assert.deepEqual( + findDeclaredSkew( + new Set(["only-root", "agreed"]), + new Map([ + [ + ".", + new Map([ + ["only-root", "1.0.0"], + ["agreed", "2.0.0"], + ]), + ], + ["clients/web", new Map([["agreed", "2.0.0"]])], + ]), + ), + [], + ); +}); + +test("findDeclaredSkew: an installed package no manifest declares is not a candidate", () => { + assert.deepEqual( + findDeclaredSkew( + new Set(), + new Map([ + [".", new Map([["undeclared", "1.0.0"]])], + ["clients/web", new Map([["undeclared", "2.0.0"]])], + ]), + ), + [], + ); +}); + +test("findDeclaredSkew: results are sorted by package name", () => { + const two = new Map([ + [ + ".", + new Map([ + ["zod", "1.0.0"], + ["ajv", "1.0.0"], + ]), + ], + [ + "clients/web", + new Map([ + ["zod", "2.0.0"], + ["ajv", "2.0.0"], + ]), + ], + ]); + assert.deepEqual( + findDeclaredSkew(new Set(["zod", "ajv"]), two).map((s) => s.name), + ["ajv", "zod"], + ); +}); + +test("partitionDeclaredSkew: deny by default", () => { + const skewed = [ + { + name: "zod", + holders: [ + { dir: ".", version: "4.4.3" }, + { dir: "w", version: "4.3.6" }, + ], + }, + ]; + const { failures, ignored } = partitionDeclaredSkew(skewed, new Map()); + assert.deepEqual( + failures.map((f) => f.name), + ["zod"], + ); + assert.deepEqual(ignored, []); +}); + +test("partitionDeclaredSkew: an allowlisted package tolerates only a within-major skew", () => { + const within = { + name: "hono", + holders: [ + { dir: ".", version: "4.1.0" }, + { dir: "w", version: "4.2.0" }, + ], + }; + const across = { + name: "hono", + holders: [ + { dir: ".", version: "4.1.0" }, + { dir: "w", version: "5.0.0" }, + ], + }; + const allow = new Map([["hono", "reason"]]); + assert.deepEqual(partitionDeclaredSkew([within], allow).ignored.length, 1); + assert.deepEqual(partitionDeclaredSkew([across], allow).failures.length, 1); +}); + +test("toleratesSkew: the within-a-major rule is stated once for both tiers", () => { + const allow = new Map([["p", "reason"]]); + assert.equal( + toleratesSkew("p", [{ version: "1.1.0" }, { version: "1.2.0" }], allow), + true, + ); + assert.equal( + toleratesSkew("p", [{ version: "1.1.0" }, { version: "2.0.0" }], allow), + false, + ); + // Unparseable can't be proven same-major, so it fails rather than passes. + assert.equal( + toleratesSkew("p", [{ version: "1.1.0" }, { version: "next" }], allow), + false, + ); + assert.equal( + toleratesSkew("other", [{ version: "1.1.0" }, { version: "1.2.0" }], allow), + false, + ); +}); + +test("countDeclaredHeld: counts declared names more than one install holds", () => { + const versions = new Map([ + [ + ".", + new Map([ + ["shared", "1.0.0"], + ["root-only", "1.0.0"], + ]), + ], + [ + "clients/web", + new Map([ + ["shared", "1.0.0"], + ["undeclared", "1.0.0"], + ]), + ], + ]); + assert.equal( + countDeclaredHeld(new Set(["shared", "root-only", "undeclared"]), versions), + // `shared` alone: the other two are held by one install each. + 1, + ); +});