diff --git a/AGENTS.md b/AGENTS.md index eac73b7f..d07e84f4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -694,7 +694,7 @@ The ⚠️ option-deletion hazard, the snapshot rule, and the recovery recipe ab - **cli and tui now typecheck their `src` (#1689).** Their `build`/`test` run through esbuild (no type check), so each has a `typecheck` script folded into `validate`. Their `tsconfig.json` matches `clients/web/tsconfig.app.json`'s module/lib _resolution_ options — DOM lib, `moduleResolution: bundler`, and **no** `noUncheckedIndexedAccess` (web's app config does not extend `tsconfig.base`, so re-enabling it would surface `core/` issues web never gates) — so the imported `core/` sources are validated the same way web validates them. It does **not** mirror web's extra strictness flags (`noUnusedLocals`, `verbatimModuleSyntax`, ES2023 target, …), so cli/tui's own `src` is checked slightly more loosely than web's. `core/` itself still typechecks through web's `tsc -b`. - **The `__tests__` dirs are typechecked too (#1791).** The src-only `tsconfig.json` excludes `**/*.test.*`, so each of cli, tui, and launcher carries a **`tsconfig.test.json`** — extending the build config, `noEmit`, including `__tests__/**/*` (only the tests root the project; tsc pulls in the `src` they import, and the src-only config already validates all of `src` without the test-only aliases) and adding the test-only path aliases that resolve what vitest resolves via `vitest.shared.mts`. The alias set differs per client: **cli's is the widest** (`@modelcontextprotocol/inspector-test-server` → `test-servers/src`, the `@inspector/core/*` deep paths, express/vitest — cli is the only one importing the test-server package); **tui's** carries only the `@inspector/core/*` + react/vitest redirects; **launcher's** has **no** `paths` at all — it's a plain `rootDir: "."` sibling of the build config (whose `rootDir: ./src` is what rejects the tests). Each client's `typecheck` script runs **both** projects (`tsc -p tsconfig.json && tsc -p tsconfig.test.json`) so running it standalone means the same thing everywhere (launcher's `build` also `tsc`s `src`, but `typecheck` doesn't rely on that). cli additionally carries `@types/express` (devDep) so the transitively-aliased test-server source typechecks, mirroring `clients/web` (cli's `tsconfig.test.json` also names `test-servers/src/server-composable.ts` explicitly — a bin entry the barrel doesn't import, so nothing else gives it a tsc pass). The client **config files** are typechecked too: cli's/tui's (`vitest.config.ts`, `tsup.config.ts`, tui `dev.ts`) are folded into each src `tsconfig.json`'s `include`; launcher's `vitest.config.ts` goes in its `tsconfig.test.json` instead (again the `rootDir: ./src` reason). Note the gate checks mock **implementations and return types** (typing a `vi.fn()` against a real signature keeps its `mockResolvedValue`/impl in sync) but **not** `toHaveBeenCalledWith(...)` arguments — vitest types those to accept anything regardless of the mock's type parameter. **`npm run verify:typecheck-coverage`** (`scripts/verify-typecheck-coverage.mjs`, run as the second step of `validate` right after `verify:format-coverage`) is the durable guard for this invariant: it runs each client's `typecheck` projects with `tsc --listFilesOnly`, unions them, and fails on any tracked `.ts`/`.tsx`/`.mts`/`.cts` that lands in no project — for every gated Node client, which it discovers from disk (each `clients/*` is enrolled through its `typecheck` script's projects, or — for a `tsc -b` client like `clients/web` with no `typecheck` script — through its `tsconfig.json` `references`), so a new client is covered without editing the guard — the typecheck analog of `verify:format-coverage`, since a project only reaches the files its `include` names plus their transitive imports, so a new top-level file (launcher especially, whose build `rootDir: ./src` rejects package-root files) can otherwise fall out silently. Like its sibling it also asserts the gate is _wired_ (each client's typecheck pass is reachable from its `validate` — its `typecheck` script for cli/tui/launcher, or a real `tsc -b` for web — and the root chain runs each client's `validate`), so it can't stay green while measuring a pass nothing invokes. It asserts the same of **`test:scripts`** — its own parser tests — on three axes: reachable from the root `validate`, a **non-empty** tracked `scripts/**/*.{test,spec}.*` set, and **every one of those files matched by a glob harvested across the scripts reachable from `test:scripts`** (so a delegating `test:scripts` still measures correctly). The third axis exists because `node --test` silently _skips_ a file its glob misses and still exits 0 — a rename to `*.spec.mjs` would shrink the suite with a green run. Beyond the clients it also covers, **deny-by-default**, the first-party TS no client owns — everything tracked outside `clients/*` (`test-servers/src/**`, the root `vitest.shared.mts`, **all of `core/`**, and any new top-level TS location) must land in the _global_ union of client projects (cli aliases the test-server source; web's enrolled projects include `core/`). So a `core` `*.tsx` web's `include` doesn't reach, or an unimported `test-servers/src` bin entry, can't ship uncompiled-but-unchecked. The one "listed but unchecked" tier the guard structurally can't see — a per-file `// @ts-nocheck` — is owned by a different gate: `@typescript-eslint/ban-ts-comment` rejects it across every surface (`lint:core`, `lint:shared`, and each client's `eslint .`). The guard's own pure parsers (`scripts/lib/npm-scripts.mjs` + the exported helpers of `verify-typecheck-coverage.mjs`, whose execution is behind a `main()` so importing it for tests doesn't run it) are **unit-tested** — `npm run test:scripts` (node's built-in `node --test`, in `validate`; the root has no vitest harness by design) runs table-driven cases, one per rule the guard's parsers encode, and the guard itself enforces that this stays wired (above). - The one CLI nuance: `clients/cli`'s out-of-process `e2e.test.ts` spawns the built binary, so its `test` **builds first** via `pretest` (`test-servers:build && build`). To avoid building it twice, `clients/cli`'s `validate` folds that in — it is `format:check && lint && typecheck && test` with **no** separate `build` step (the other clients, whose tests don't spawn their bundle, keep an explicit `build`). `validate:web`/`validate:tui`/`validate:launcher` are the uniform `format:check && lint && (typecheck &&) build && test`. (#1778, #1789, #1792) `clients/web`'s `format`/`format:check` covers `src`, `server`, `.storybook`, and its top-level configs (the uniform `*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` glob — `vite.config.ts`, `tsup.runner.config.ts`, `eslint.config.js`, …), not just `src`, so the Node backend, Storybook config, and Vite/build config are prettier-gated too; `clients/launcher`'s covers `src`, `__tests__`, `scripts`, and its top-level configs (the `*.` top-level glob is non-recursive, so each nested dir — `.storybook`, `scripts` — is named explicitly). The `verify:format-coverage` guard (#1792) enforces that this coverage stays complete. - - **One version per install-crossing dependency (#1896).** Because v2 is not a workspace, the root and each `clients/*` carry their own `node_modules` — and a client's `tsconfig.test.json` compiles first-party sources that live *outside* the client (`test-servers/src`, `core/`), which resolve their dependencies from the **root** install while the client's own sources resolve from the client install. So the same package can appear **twice in one `tsc` program**. At the same version that duplication is harmless; on a skew, TypeScript must relate two structurally-distinct declarations of the same type. For a deeply recursive-generic surface that is exponential: zod `4.3.6` (root) against zod `4.4.3` (`clients/web`) made `clients/web`'s `tsc -b` exhaust the 4GB default heap outright via `TS2589 Type instantiation is excessively deep`, because every `@modelcontextprotocol/*` schema is built out of zod generics. **Raising the heap with `--max-old-space-size` hides this class rather than fixing it — align the versions instead.** `npm run verify:dep-lockstep` (`scripts/verify-dep-lockstep.mjs`, in `validate`) is the durable guard: it **derives** the candidate set from **what actually enters each `tsc` program** (#1965) — every client tsconfig project is listed with `tsc --listFilesOnly` through the shared `scripts/lib/tsc-program.mjs` helper (the same machinery `verify:typecheck-coverage` reads a program through, so the two guards can't disagree about what one contains), each resolved `node_modules` file is mapped to its owning install and package, and a package reaching **one** program from **two** installs is a candidate. That is precisely the set that can put two structurally-distinct copies of a type in front of one checker — and it needs no editing when a new dependency arrives. It replaced a derivation that read the packages the shared sources named *directly*, which could not see one whose declarations arrive only through another package's `.d.ts`: `@modelcontextprotocol/sdk` is never written in first-party code (the shared sources import the split `@modelcontextprotocol/client|core|…`) yet 16 of its `.d.ts` files land in `clients/web`'s test program, so a second copy under `clients/web/node_modules` skewed unseen. Two properties are worth knowing: a package present in two installs but reached from only one in a given program is correctly **not** a candidate, and TypeScript's package-identity redirect collapses two copies at the *same* name@version (so an aligned package's own transitive dependencies load once) — the redirect stops applying the moment they skew, which is exactly when the guard needs to see both. Versions themselves still come from the committed lockfiles' **top-level** `node_modules/` entries — a *nested* transitive duplicate inside one install is routine and deliberately ignored — and any candidate held at two versions fails, **deny-by-default**. The escape hatch is `TOLERATED_SKEW` in that file, an allowlist of *names* (not version pairs, so an ordinary patch float doesn't churn it), each entry carrying why that package's types can't blow up; it is **empty today** — the four names it used to carry (`react`, `hono`, `jose`, `@modelcontextprotocol/ext-apps`) were admitted under the old derivation and none is a candidate under this one, so each would be a rationale for a skew that cannot occur. **Being listed is not a blanket exemption** — it tolerates skew only *within a major version*, since a rationale about patch-level differences says nothing about a React 18-vs-19 split, where the type surface itself changes; a cross-major skew fails even for a listed package. **When bumping a dependency that the shared sources pull in, bump it in every install that declares it** — that's the root plus whichever clients list it, not all four unconditionally (launcher declares no zod, for instance, and a package absent from an install can't skew, so the guard ignores it there). Don't add a dependency to a client just to satisfy this. Its pure helpers are unit-tested via `test:scripts` (its own, plus `scripts/lib/tsc-program.test.mjs` for the shared derivation), and it vouches — with `verify:format-coverage` and `verify:typecheck-coverage` — that its siblings are still wired into `validate`. It runs the same `tsc --listFilesOnly` pass its sibling does, in its own process, costing ~14s: the listing is deliberately **not** cached to disk between the two, because a fingerprint that missed an input would make a guard measure a program that no longer exists and pass on a real miss. + - **One version per install-crossing dependency (#1896).** Because v2 is not a workspace, the root and each `clients/*` carry their own `node_modules` — and a client's `tsconfig.test.json` compiles first-party sources that live *outside* the client (`test-servers/src`, `core/`), which resolve their dependencies from the **root** install while the client's own sources resolve from the client install. So the same package can appear **twice in one `tsc` program**. At the same version that duplication is harmless; on a skew, TypeScript must relate two structurally-distinct declarations of the same type. For a deeply recursive-generic surface that is exponential: zod `4.3.6` (root) against zod `4.4.3` (`clients/web`) made `clients/web`'s `tsc -b` exhaust the 4GB default heap outright via `TS2589 Type instantiation is excessively deep`, because every `@modelcontextprotocol/*` schema is built out of zod generics. **Raising the heap with `--max-old-space-size` hides this class rather than fixing it — align the versions instead.** `npm run verify:dep-lockstep` (`scripts/verify-dep-lockstep.mjs`, in `validate`) is the durable guard: it **derives** the candidate set from **what actually enters each `tsc` program** (#1965) — every client tsconfig project is listed with `tsc --listFilesOnly` through the shared `scripts/lib/tsc-program.mjs` helper (the same machinery `verify:typecheck-coverage` reads a program through, so the two guards can't disagree about what one contains), each resolved `node_modules` file is mapped to its owning install and package, and a package reaching **one** program from **two** installs is a candidate. That is precisely the set that can put two structurally-distinct copies of a type in front of one checker — and it needs no editing when a new dependency arrives. It replaced a derivation that read the packages the shared sources named *directly*, which could not see one whose declarations arrive only through another package's `.d.ts`: `@modelcontextprotocol/sdk` is never written in first-party code (the shared sources import the split `@modelcontextprotocol/client|core|…`) yet 16 of its `.d.ts` files land in `clients/web`'s test program, so a second copy under `clients/web/node_modules` skewed unseen. Two properties are worth knowing: a package present in two installs but reached from only one in a given program is correctly **not** a candidate, and TypeScript's package-identity redirect collapses two copies at the *same* name@version (so an aligned package's own transitive dependencies load once) — the redirect stops applying the moment they skew, which is exactly when the guard needs to see both. Versions still come from the committed lockfiles, but from the entry for the **exact install path the program resolved** (`node_modules/zod`, `node_modules/a/node_modules/zod`) rather than from the install's top-level entry: a *nested* duplicate inside one install is still not a candidate on its own — folding it onto its outermost install is what keeps the set small — but once a program has loaded one, pricing it from a top-level entry that may be absent or differently versioned would let a real pair pass (Copilot). Only the installs that actually **met in one program** are compared, so a third install's copy that no program loads beside another is not evidence of anything. Any co-occurrence whose copies disagree fails, **deny-by-default**; a resolved copy with no lockfile entry fails too, since the tree and the lockfile then disagree about what was loaded. The escape hatch is `TOLERATED_SKEW` in that file, an allowlist of *names* (not version pairs, so an ordinary patch float doesn't churn it), each entry carrying why that package's types can't blow up; it is **empty today** — the four names it used to carry (`react`, `hono`, `jose`, `@modelcontextprotocol/ext-apps`) were admitted under the old derivation and none is a candidate under this one, so each would be a rationale for a skew that cannot occur. **Being listed is not a blanket exemption** — it tolerates skew only *within a major version*, since a rationale about patch-level differences says nothing about a React 18-vs-19 split, where the type surface itself changes; a cross-major skew fails even for a listed package. **When bumping a dependency that the shared sources pull in, bump it in every install that declares it** — that's the root plus whichever clients list it, not all four unconditionally (launcher declares no zod, for instance, and a package absent from an install can't skew, so the guard ignores it there). Don't add a dependency to a client just to satisfy this. Its pure helpers are unit-tested via `test:scripts` (its own, plus `scripts/lib/tsc-program.test.mjs` for the shared derivation), and it vouches — with `verify:format-coverage` and `verify:typecheck-coverage` — that its siblings are still wired into `validate`. It runs the same `tsc --listFilesOnly` pass its sibling does, in its own process, costing ~14s: the listing is deliberately **not** cached to disk between the two, because a fingerprint that missed an input would make a guard measure a program that no longer exists and pass on a real miss. - **`npm run coverage`** is the per-file ≥90 gate and is now part of `npm run ci` — never treat it as optional before a push. It supersedes the old standalone `test:integration` step: web's `test:coverage` runs the `unit` **and** `integration` projects under v8 instrumentation, so `coverage` both enforces the ≥90 gate and exercises the same web integration paths CI covers. - **`smoke` is NOT part of `validate`** — it is included in `npm run ci`. It runs `smoke:launcher` (`--help` dispatch) plus the prod `smoke:cli` / `smoke:tui` / `smoke:web` / `smoke:web:browser` / `smoke:web:app`, and contains **no build commands** — it assumes the cli/tui/launcher bundles already exist (a full `validate` builds them; `smoke:web` builds `clients/web/dist` on demand). CI runs `validate`, then the `coverage` gate (which also covers the web integration project), then `verify:build-gate` (the #1769 build gate — see below), then `smoke` (with Playwright chromium installed just before it, since `smoke:web:browser` needs it). GitHub CI runs this same chain as separate workflow steps, with the Storybook play-function tests last (see below). - `smoke:launcher` (`scripts/smoke-launcher.mjs`) runs the built launcher with `--help`, `--cli --help`, and `--tui --help`, asserting each exits 0 and prints that mode's usage banner (which also proves the launcher resolved and loaded the right client build). It's the cheap dispatch check before the heavier prod smokes below. diff --git a/README.md b/README.md index 0bca3291..cb57dd35 100644 --- a/README.md +++ b/README.md @@ -301,7 +301,7 @@ Each client self-validates from its own folder; the root scripts chain them. The | `npm run verify:format-coverage` | Parses the `format:check` globs out of every `package.json` (only those reachable from `validate`), enumerates all tracked source files, and **fails** listing any not covered by a glob — the durable guard for the "every first-party source file is format-gated" invariant (#1792). Runs first in `validate`. | | `npm run test:scripts` | Table-driven unit tests (`node --test`) for the guard's own pure parsers (`scripts/lib/npm-scripts.mjs`, `scripts/lib/tsc-program.mjs` + the exported helpers of `verify-typecheck-coverage.mjs` and `verify-dep-lockstep.mjs`), one case per rule they encode. Runs in `validate` — and `verify:typecheck-coverage` guards *this* gate in turn (reachable from `validate`, non-empty test set, every test file matched by the `test:scripts` glob), since `node --test` silently skips a file its glob misses and still exits 0. | | `npm run verify:typecheck-coverage` | The typecheck-coverage analog of the above (#1791): for each Node client (auto-discovered from disk — enrolled via its `typecheck` script's projects, or for a `tsc -b` client like `clients/web` via its `tsconfig.json` `references`) it runs those projects with `tsc --listFilesOnly`, unions them, and **fails** listing any tracked `.ts`/`.tsx`/`.mts`/`.cts` under the client that lands in no project (so a new top-level config/helper can't silently go untypechecked). It also requires, deny-by-default, the first-party TS no client owns (`test-servers/src`, the root `vitest.shared.mts`, all of `core/`, and any new top-level location) to land in some client project's tsc pass — so a `core` `*.tsx` web's projects don't reach is caught too. Also asserts the gate is wired (each client's typecheck pass — its `typecheck` script, or web's `tsc -b` — is reachable from its `validate`, and the root chain runs each client's `validate`). Runs in `validate`. | -| `npm run verify:dep-lockstep` | Guards the "one version per install-crossing dependency" invariant (#1896). v2 is not a workspace, so a client's test project compiles the shared first-party TypeScript — `core/`, `test-servers/src`, and the root-owned `vitest.shared.mts`, all of which resolve their dependencies from the **root** install — alongside the client's own sources, putting the same package in one `tsc` program twice. At the same version that's harmless; skewed, TypeScript must relate two structurally-distinct copies of every type, which for a recursive-generic surface is exponential (zod `4.3.6` vs `4.4.3` exhausted the 4GB tsc heap in `clients/web`). Derives its candidate set from **what actually enters each program** (#1965) — every client tsconfig project listed with `tsc --listFilesOnly` via the shared `scripts/lib/tsc-program.mjs`, each resolved `node_modules` file mapped to its owning install, keeping the packages that reach one program from two installs (a package whose declarations arrive only through another package's `.d.ts`, as `@modelcontextprotocol/sdk`'s do, is invisible to a scan of first-party imports). Compares those against the committed lockfiles' top-level entries and **fails deny-by-default** on any skew not in the annotated `TOLERATED_SKEW` allowlist — empty today — and an allowlisted package is tolerated only *within a major version*. Runs in `validate`. +| `npm run verify:dep-lockstep` | Guards the "one version per install-crossing dependency" invariant (#1896). v2 is not a workspace, so a client's test project compiles the shared first-party TypeScript — `core/`, `test-servers/src`, and the root-owned `vitest.shared.mts`, all of which resolve their dependencies from the **root** install — alongside the client's own sources, putting the same package in one `tsc` program twice. At the same version that's harmless; skewed, TypeScript must relate two structurally-distinct copies of every type, which for a recursive-generic surface is exponential (zod `4.3.6` vs `4.4.3` exhausted the 4GB tsc heap in `clients/web`). Derives its candidate set from **what actually enters each program** (#1965) — every client tsconfig project listed with `tsc --listFilesOnly` via the shared `scripts/lib/tsc-program.mjs`, each resolved `node_modules` file mapped to its owning install, keeping the packages that reach one program from two installs (a package whose declarations arrive only through another package's `.d.ts`, as `@modelcontextprotocol/sdk`'s do, is invisible to a scan of first-party imports). Prices each copy from the lockfile entry for the exact install path the program resolved, compares only the installs that met in one program, and **fails deny-by-default** on any disagreement not in the annotated `TOLERATED_SKEW` allowlist — empty today — with an allowlisted package tolerated only *within a major version*. Runs in `validate`. | `npm run ci` | **Mandatory pre-push command.** `validate` → `coverage` → `verify:build-gate` → `smoke` → Storybook. A true superset of GitHub CI. | | `npm run pack:verify` | Publish smoke — see [Publishing](#publishing). | diff --git a/scripts/lib/tsc-program.mjs b/scripts/lib/tsc-program.mjs index 189c13df..98a678e6 100644 --- a/scripts/lib/tsc-program.mjs +++ b/scripts/lib/tsc-program.mjs @@ -302,18 +302,26 @@ export function projectPackageFiles(clientDir, project) { } /** - * The install a repo-relative `node_modules` path belongs to, and the package it - * holds — `{ installRoot, name }`, or null for a path that names neither. + * Where a repo-relative `node_modules` path came from — `{ installRoot, name, + * entryPath }`, or null for a path that names no package. * - * The install root is the prefix before the **first** `node_modules` segment - * (`.` for the repo root, `clients/web` for a client install), while the package - * name comes from after the **last** one. That asymmetry is deliberate: a - * *nested* `node_modules/a/node_modules/b` is npm resolving a transitive - * conflict **inside one install**, which is routine and not a cross-install - * skew — the same reason the lockfile comparison reads only top-level entries. - * Folding it onto its outermost install is what keeps the candidate set the - * handful of genuinely install-crossing packages instead of every transitive - * duplicate in the tree. + * - `installRoot` is the prefix before the **first** `node_modules` segment + * (`.` for the repo root, `clients/web` for a client install). A *nested* + * `node_modules/a/node_modules/b` folds onto its outermost install, because it + * is npm resolving a transitive conflict **inside one install** — routine, and + * not the cross-install skew this feeds. Folding it is what keeps the + * candidate set the handful of genuinely install-crossing packages instead of + * every transitive duplicate in the tree. + * - `name` comes from after the **last** `node_modules`, so the nested copy is + * still attributed to the package it actually is. + * - `entryPath` is the package directory relative to that install — exactly the + * key npm writes in that install's lockfile (`node_modules/zod`, + * `node_modules/a/node_modules/zod`). Folding tells you *which install* loaded + * it; this tells you *which copy*, so the version can be read from the entry + * the program actually resolved rather than from whatever sits at the + * install's top level (Copilot, #1965 r1 — a nested copy at a different + * version would otherwise be compared against a top-level entry that is + * absent, or worse, present and aligned, and the real pair would pass). */ export function classifyModulePath(rel) { const parts = rel.split("/"); @@ -324,13 +332,13 @@ export function classifyModulePath(rel) { // `node_modules/.bin/…`, `node_modules/.package-lock.json`: npm's own // bookkeeping, not a package. if (!head || head.startsWith(".")) return null; - const name = head.startsWith("@") - ? parts[last + 2] && `${head}/${parts[last + 2]}` - : head; + const scoped = head.startsWith("@"); + const name = scoped ? parts[last + 2] && `${head}/${parts[last + 2]}` : head; if (!name) return null; // a bare `node_modules/@scope` path names no package return { installRoot: first === 0 ? "." : parts.slice(0, first).join("/"), name, + entryPath: parts.slice(first, last + (scoped ? 3 : 2)).join("/"), }; } @@ -340,9 +348,15 @@ export function classifyModulePath(rel) { * type in front of the type checker. * * `programs` is an iterable of `{ label, files }`, one per leaf tsconfig project, - * `files` being repo-relative `node_modules` paths. Returns a Map keyed by - * package name, each value `{ installRoots, programs }` naming where it was seen - * — the diagnostic a failure needs. + * `files` being repo-relative `node_modules` paths. Returns + * `Map>>>` — the + * co-occurrences themselves, not a flattened name list. + * + * Keeping the structure is what lets the caller compare only the copies that met + * (Copilot, #1965 r1). Flattening to names hands the comparison a package that + * is aligned wherever it co-occurs, and a third install holding a different + * version of it then fails the guard even though no program ever loads that + * copy — with a diagnostic naming an install that was never involved. * * The two-install test is applied **within one program**, not across the run: a * package reached from the root install in web's program and from the client @@ -362,20 +376,20 @@ export function classifyModulePath(rel) { export function crossInstallPackages(programs) { const found = new Map(); for (const { label, files } of programs) { - const roots = new Map(); // name -> Set(installRoot), within THIS program + // Within THIS program: name -> installRoot -> the entry paths loaded from it. + const seen = new Map(); for (const rel of files) { const hit = classifyModulePath(rel); if (!hit) continue; - if (!roots.has(hit.name)) roots.set(hit.name, new Set()); - roots.get(hit.name).add(hit.installRoot); + if (!seen.has(hit.name)) seen.set(hit.name, new Map()); + const byRoot = seen.get(hit.name); + if (!byRoot.has(hit.installRoot)) byRoot.set(hit.installRoot, new Set()); + byRoot.get(hit.installRoot).add(hit.entryPath); } - for (const [name, installRoots] of roots) { - if (installRoots.size < 2) continue; - if (!found.has(name)) - found.set(name, { installRoots: new Set(), programs: new Set() }); - const entry = found.get(name); - for (const root of installRoots) entry.installRoots.add(root); - entry.programs.add(label); + for (const [name, byRoot] of seen) { + if (byRoot.size < 2) continue; // one install — nothing to relate + if (!found.has(name)) found.set(name, new Map()); + found.get(name).set(label, byRoot); } } return found; diff --git a/scripts/lib/tsc-program.test.mjs b/scripts/lib/tsc-program.test.mjs index fa9bb9e1..c76825ff 100644 --- a/scripts/lib/tsc-program.test.mjs +++ b/scripts/lib/tsc-program.test.mjs @@ -155,28 +155,46 @@ test("classifyModulePath: install root is the OUTERMOST node_modules, package th assert.deepEqual(classifyModulePath("node_modules/zod/index.d.ts"), { installRoot: ".", name: "zod", + entryPath: "node_modules/zod", }); assert.deepEqual( classifyModulePath("clients/web/node_modules/zod/index.d.ts"), - { installRoot: "clients/web", name: "zod" }, + { + installRoot: "clients/web", + name: "zod", + entryPath: "node_modules/zod", + }, ); // A scoped package keeps both segments. assert.deepEqual( classifyModulePath("node_modules/@modelcontextprotocol/sdk/dist/x.d.ts"), - { installRoot: ".", name: "@modelcontextprotocol/sdk" }, + { + installRoot: ".", + name: "@modelcontextprotocol/sdk", + entryPath: "node_modules/@modelcontextprotocol/sdk", + }, ); // A NESTED copy folds onto its outermost install: npm resolving a transitive - // conflict inside one install is routine, not a cross-install skew, and the - // lockfile comparison ignores it for the same reason. + // conflict inside one install is routine, not a cross-install skew. Its + // `entryPath` still names the copy itself, so the caller can price the version + // the program actually loaded (Copilot, #1965 r1). assert.deepEqual( classifyModulePath("node_modules/a/node_modules/zod/index.d.ts"), - { installRoot: ".", name: "zod" }, + { + installRoot: ".", + name: "zod", + entryPath: "node_modules/a/node_modules/zod", + }, ); assert.deepEqual( classifyModulePath( "clients/cli/node_modules/a/node_modules/@scope/b/index.d.ts", ), - { installRoot: "clients/cli", name: "@scope/b" }, + { + installRoot: "clients/cli", + name: "@scope/b", + entryPath: "node_modules/a/node_modules/@scope/b", + }, ); }); @@ -210,14 +228,11 @@ test("crossInstallPackages: two installs in ONE program is the whole test (#1965 }, ]); assert.deepEqual([...found.keys()], ["zod"]); - assert.deepEqual([...found.get("zod").installRoots].sort(), [ - ".", - "clients/web", - ]); - assert.deepEqual( - [...found.get("zod").programs], - ["clients/web/tsconfig.test.json"], - ); + const byRoot = found.get("zod").get("clients/web/tsconfig.test.json"); + assert.deepEqual([...byRoot.keys()].sort(), [".", "clients/web"]); + // The entry paths ride along so the caller can price each copy from its own + // lockfile entry (Copilot, #1965 r1). + assert.deepEqual([...byRoot.get(".")], ["node_modules/zod"]); }); test("crossInstallPackages: two installs across SEPARATE programs is not a candidate", () => { @@ -252,8 +267,26 @@ test("crossInstallPackages: every program that saw both copies is recorded", () { label: "web/test", files: both }, { label: "cli/src", files: ["node_modules/zod/index.d.ts"] }, ]); - assert.deepEqual([...found.get("zod").programs].sort(), [ + assert.deepEqual([...found.get("zod").keys()].sort(), [ "web/app", "web/test", ]); }); + +test("crossInstallPackages: a nested copy is kept as its own entry path (Copilot, #1965 r1)", () => { + // Folded onto the root install for candidacy, but recorded at the path it was + // loaded from so its real version can be read. + const found = crossInstallPackages([ + { + label: "p", + files: [ + "node_modules/a/node_modules/zod/index.d.ts", + "clients/web/node_modules/zod/index.d.ts", + ], + }, + ]); + assert.deepEqual( + [...found.get("zod").get("p").get(".")], + ["node_modules/a/node_modules/zod"], + ); +}); diff --git a/scripts/verify-dep-lockstep.main.test.mjs b/scripts/verify-dep-lockstep.main.test.mjs index eb410167..32819af9 100644 --- a/scripts/verify-dep-lockstep.main.test.mjs +++ b/scripts/verify-dep-lockstep.main.test.mjs @@ -44,14 +44,18 @@ const typescriptDir = path.dirname( ), ); -/** A lockfileVersion 3 lockfile, including the `""` root entry npm always writes. */ +/** + * A lockfileVersion 3 lockfile, including the `""` root entry npm always writes. + * A key already containing `node_modules/` is used verbatim, so a nested install + * path (`node_modules/outer/node_modules/inner`) can be expressed. + */ const lock = (deps) => ({ lockfileVersion: 3, packages: { "": { name: "fixture" }, ...Object.fromEntries( Object.entries(deps).map(([name, version]) => [ - `node_modules/${name}`, + name.includes("node_modules/") ? name : `node_modules/${name}`, { version }, ]), ), @@ -76,6 +80,8 @@ function makeFixture({ webScripts, rawWebLock, webTsconfig, + rootNestedInner, + cliDeps, } = {}) { // realpath matters: on macOS `tmpdir()` is `/var/...`, a symlink to // `/private/var/...`. The guard only runs `main()` when `import.meta.url` @@ -161,9 +167,54 @@ function makeFixture({ } stub("", root, "solo", "export type Solo = string;\n"); - write("package-lock.json", lock(root)); + // 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, + // and the case that must still be priced from the entry the program resolved. + const rootLockDeps = { ...root }; + if (rootNestedInner) { + stub( + "node_modules/outer/", + { inner: rootNestedInner }, + "inner", + "export type Inner = number;\n", + ); + rootLockDeps["node_modules/outer/node_modules/inner"] = rootNestedInner; + } + + write("package-lock.json", lock(rootLockDeps)); write("clients/web/package-lock.json", rawWebLock ?? lock(web)); + // 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. + if (cliDeps) { + write("clients/cli/package.json", { + name: "cli", + scripts: { typecheck: "tsc --noEmit -p tsconfig.json" }, + }); + write("clients/cli/tsconfig.json", { + compilerOptions: { + noEmit: true, + module: "esnext", + target: "esnext", + moduleResolution: "bundler", + types: [], + }, + include: ["src/**/*.ts"], + }); + write( + "clients/cli/src/main.ts", + 'import type { Outer } from "outer";\nexport type FromCli = Outer;\n', + ); + stub( + "clients/cli/", + cliDeps, + "outer", + 'import type { Inner } from "inner";\nexport type Outer = Inner;\n', + ); + stub("clients/cli/", cliDeps, "inner", "export type Inner = number;\n"); + write("clients/cli/package-lock.json", lock(cliDeps)); + } + // The guard resolves its repo root from its own location, so it has to live // inside the fixture; its `lib/` imports come along. mkdirSync(path.join(dir, "scripts", "lib"), { recursive: true }); @@ -236,10 +287,10 @@ test("main: exits 1 and names the skewed package and every holder", () => { const { status, out } = runGuard(dir); assert.equal(status, 1, out); assert.match(out, /\bouter\b/); - assert.match(out, /1\.0\.0\s+\(\.\)/); - assert.match(out, /1\.2\.3\s+\(clients\/web\)/); + assert.match(out, /1\.0\.0\s+\(\.\/node_modules\/outer\)/); + assert.match(out, /1\.2\.3\s+\(clients\/web\/node_modules\/outer\)/); // The program that saw both copies is named, so the claim is checkable. - assert.match(out, /seen in clients\/web\/tsconfig\.json/); + assert.match(out, /in clients\/web\/tsconfig\.json/); // `inner` agrees, so it must not be reported. assert.doesNotMatch(out, /^\s+inner$/m); }, @@ -263,8 +314,8 @@ test("main: a package reached only through another package's .d.ts is caught (#1 const { status, out } = runGuard(dir); assert.equal(status, 1, out); assert.match(out, /^\s+inner$/m); - assert.match(out, /4\.0\.0\s+\(\.\)/); - assert.match(out, /4\.5\.6\s+\(clients\/web\)/); + assert.match(out, /4\.0\.0\s+\(\.\/node_modules\/inner\)/); + assert.match(out, /4\.5\.6\s+\(clients\/web\/node_modules\/inner\)/); }, ); }); @@ -284,6 +335,45 @@ test("main: a package that reaches the program from ONE install is not a candida ); }); +test("main: a third install's copy is not dragged into a comparison it never joined (Copilot, #1965 r1)", () => { + // `clients/cli` holds `outer` at a different version, but its own program is + // the only one that loads that copy and no second install meets it there. + // Flattening the candidates to names would compare cli against web's aligned + // pair and fail — naming an install that never took part. + withFixture( + { cliDeps: { ...ALIGNED, outer: "9.9.9", inner: "9.9.9" } }, + (dir) => { + const { status, out } = runGuard(dir); + assert.equal(status, 0, out); + assert.doesNotMatch(out, /clients\/cli/); + }, + ); +}); + +test("main: a nested copy is priced from its own lockfile entry (Copilot, #1965 r1)", () => { + // The root reaches `inner` only through `node_modules/outer/node_modules/inner` + // at 9.9.9, while its top-level entry still reads 4.5.6 — the version web + // holds. Pricing the copy from the top-level entry would report the pair as + // agreeing and let a real 9.9.9-vs-4.5.6 program through. + withFixture( + { + rootDeps: { ...ALIGNED, outer: "1.0.0" }, + webDeps: ALIGNED, + rootNestedInner: "9.9.9", + }, + (dir) => { + const { status, out } = runGuard(dir); + assert.equal(status, 1, out); + assert.match(out, /^\s+inner$/m); + assert.match( + out, + /9\.9\.9\s+\(\.\/node_modules\/outer\/node_modules\/inner\)/, + ); + assert.match(out, /4\.5\.6\s+\(clients\/web\/node_modules\/inner\)/); + }, + ); +}); + test("main: exits 1 when a client names no tsconfig project", () => { // No `typecheck` script and no `tsconfig.json` references: none of that // client's programs is measured, so a skew reaching only them would pass. diff --git a/scripts/verify-dep-lockstep.mjs b/scripts/verify-dep-lockstep.mjs index 3fe3a40e..05d9277a 100644 --- a/scripts/verify-dep-lockstep.mjs +++ b/scripts/verify-dep-lockstep.mjs @@ -98,18 +98,23 @@ const repoRoot = path.resolve( const TOLERATED_SKEW = new Map(); /** - * Top-level installed versions of every package in a parsed lockfile, keyed by - * package name. Only `node_modules/` entries count — a *nested* - * `node_modules/a/node_modules/b` is npm resolving a transitive conflict inside - * one install, which is routine and not what this guard is about (the same - * asymmetry `classifyModulePath` applies to a resolved path). + * Installed versions in a parsed lockfile, keyed by the **install path** npm + * writes — `node_modules/zod`, `node_modules/a/node_modules/zod`. + * + * Keyed by path rather than by package name so a version is read from the exact + * copy a program resolved (Copilot, #1965 r1). Reading only top-level entries + * would mean a nested copy that entered the program got compared against + * whatever sits at the install's top level — a different version, or nothing at + * all — and a real pair could pass. Nested copies are still not a *candidate* on + * their own (`classifyModulePath` folds them onto their outermost install); this + * is about pricing a copy correctly once a program has loaded it. */ -export function topLevelLockVersions(lock) { +export function lockVersionsByPath(lock) { const versions = new Map(); for (const [entryPath, entry] of Object.entries(lock?.packages ?? {})) { - const m = /^node_modules\/(@[^/]+\/[^/]+|[^@/][^/]*)$/.exec(entryPath); - if (!m || typeof entry?.version !== "string") continue; - versions.set(m[1], entry.version); + if (!entryPath.startsWith("node_modules/")) continue; + if (typeof entry?.version !== "string") continue; + versions.set(entryPath, entry.version); } return versions; } @@ -120,7 +125,7 @@ export function topLevelLockVersions(lock) { * the root project. * * This is checked rather than tolerated because the gate is deny-by-default and - * `topLevelLockVersions` returns an empty map for anything else. An unreadable + * `lockVersionsByPath` returns an empty map for anything else. An unreadable * lockfile would otherwise contribute no holders, and a real skew among the * remaining installs would be reported as aligned — the gate failing *open*, * which is the one way it must never fail (Copilot, #1962). A v1 lockfile @@ -141,22 +146,48 @@ export function hasReadableLockShape(lock) { } /** - * Find candidate packages that resolve to more than one version across the - * installs. `installs` is an array of `{ dir, versions }`. Returns one entry per - * skewed package, sorted by name, each listing the version each install holds. - * Packages present in fewer than two installs cannot skew and are skipped. + * Price each co-occurrence and keep the ones whose copies disagree. + * + * `found` is {@link crossInstallPackages}' output — + * `Map>>>` — and `versions` + * maps an install dir to that install's {@link lockVersionsByPath}. A package is + * skewed when the copies **one program** loaded, from two different installs, + * carry more than one version. Only the installs that actually met in that + * program are compared: a third install's copy that no program loads is not + * evidence of anything (Copilot, #1965 r1). + * + * Returns `{ skewed, unresolved }`, sorted by name. `unresolved` names a copy + * whose lockfile entry is missing — the tree and the lockfile disagree, so the + * comparison cannot be trusted and the caller must fail rather than skip it. */ -export function findSkew(candidates, installs) { +export function findSkew(found, versions) { const skewed = []; - for (const name of [...candidates].sort()) { - const holders = installs - .filter(({ versions }) => versions.has(name)) - .map(({ dir, versions }) => ({ dir, version: versions.get(name) })); - if (holders.length < 2) continue; - const distinct = new Set(holders.map((h) => h.version)); - if (distinct.size > 1) skewed.push({ name, holders }); + const unresolved = []; + for (const name of [...found.keys()].sort()) { + const occurrences = []; + for (const [program, byRoot] of found.get(name)) { + const holders = []; + for (const [dir, entryPaths] of byRoot) + for (const entryPath of entryPaths) { + const version = versions.get(dir)?.get(entryPath); + if (version === undefined) unresolved.push({ name, dir, entryPath }); + else holders.push({ dir, entryPath, version }); + } + if (new Set(holders.map((h) => h.version)).size > 1) + occurrences.push({ program, holders }); + } + if (occurrences.length > 0) skewed.push({ name, occurrences }); } - return skewed; + return { skewed, unresolved }; +} + +/** Every holder across a skewed package's occurrences, deduped by install + path. */ +export function skewHolders(entry) { + const byKey = new Map(); + for (const { holders } of entry.occurrences) + for (const holder of holders) + byKey.set(`${holder.dir}|${holder.entryPath}`, holder); + return [...byKey.values()]; } /** @@ -181,7 +212,7 @@ export function majorOf(version) { export function partitionSkew(skewed, tolerated = TOLERATED_SKEW) { const isTolerated = (s) => { if (!tolerated.has(s.name)) return false; - const majors = new Set(s.holders.map((h) => majorOf(h.version))); + const majors = new Set(skewHolders(s).map((h) => majorOf(h.version))); return majors.size === 1 && !majors.has(null); }; return { @@ -343,8 +374,6 @@ export function main() { ); process.exit(1); } - const candidates = new Set(found.keys()); - const dirs = installDirs(); // A missing lockfile is a failure, not a skipped install: dropping one would @@ -398,32 +427,53 @@ export function main() { process.exit(1); } - const installs = locks.map(({ dir, lock }) => ({ - dir, - versions: topLevelLockVersions(lock), - })); + const versions = new Map( + locks.map(({ dir, lock }) => [dir, lockVersionsByPath(lock)]), + ); - const { failures, ignored } = partitionSkew(findSkew(candidates, installs)); + const { skewed, unresolved } = findSkew(found, versions); + + // A copy the program loaded but the lockfile doesn't list means the installed + // tree and the lockfile disagree — the comparison would be reading a version + // that isn't the one on disk, so refuse it rather than skip the copy. + if (unresolved.length > 0) { + console.error( + `verify:dep-lockstep — ${unresolved.length} resolved package(s) have no lockfile entry:\n`, + ); + for (const { name, dir, entryPath } of unresolved) + console.error(` ${name} ${dir}/${entryPath}`); + console.error( + "\nThe program loaded these, so their versions decide the comparison — but the install's lockfile" + + "\ndoesn't list them, which means the tree and the lockfile disagree. Run `npm install` at the repo" + + "\nroot to re-sync every install.", + ); + process.exit(1); + } + + const { failures, ignored } = partitionSkew(skewed); if (failures.length > 0) { console.error( `verify:dep-lockstep — ${failures.length} ${failures.length === 1 ? "dependency resolves" : "dependencies resolve"} to different versions across installs:\n`, ); let anyListed = false; - for (const { name, holders } of failures) { + for (const failure of failures) { // A package already on the allowlist reached here only by skewing across // a MAJOR boundary, so say that rather than advising an entry that exists. - const listed = TOLERATED_SKEW.has(name); + const listed = TOLERATED_SKEW.has(failure.name); anyListed ||= listed; console.error( - ` ${name}${listed ? " (allowlisted — but this is a MAJOR skew)" : ""}`, + ` ${failure.name}${listed ? " (allowlisted — but this is a MAJOR skew)" : ""}`, ); - for (const { dir, version } of holders) - console.error(` ${version} (${dir})`); - // Which program saw both copies is the whole reason the package is a - // candidate — without it the reader has no way to check the claim. - for (const label of [...found.get(name).programs].sort()) - console.error(` seen in ${label}`); + // Report per program: which copies met, and where. The program is the + // whole reason the package is a candidate, and naming only the versions + // would leave the reader unable to check the claim — or to tell a nested + // copy from the install's top-level one. + for (const { program, holders } of failure.occurrences) { + console.error(` in ${program}`); + for (const { dir, entryPath, version } of holders) + console.error(` ${version} (${dir}/${entryPath})`); + } } console.error( "\nEach of these reaches a single `tsc` program from two installs (a client's own sources resolve" + @@ -447,7 +497,7 @@ export function main() { const note = ignored.length > 0 ? `, ${ignored.length} tolerated` : ""; console.log( - `verify:dep-lockstep — OK: ${candidates.size} install-crossing dependencies agree across ${installs.length} installs${note} ` + + `verify:dep-lockstep — OK: ${found.size} install-crossing dependencies agree across ${dirs.length} installs${note} ` + `(derived from ${programs.length} tsc programs).`, ); } diff --git a/scripts/verify-dep-lockstep.test.mjs b/scripts/verify-dep-lockstep.test.mjs index 25a643ab..edcbbc4f 100644 --- a/scripts/verify-dep-lockstep.test.mjs +++ b/scripts/verify-dep-lockstep.test.mjs @@ -15,9 +15,9 @@ import { clientProjects, findSkew, hasReadableLockShape, + lockVersionsByPath, majorOf, partitionSkew, - topLevelLockVersions, } from "./verify-dep-lockstep.mjs"; test("clientProjects: a `typecheck` script's projects win over references", () => { @@ -60,11 +60,11 @@ test("clientProjects: a NEUTERED project still contributes its program (#1965)", ); }); -test("topLevelLockVersions: nested duplicates are ignored", () => { - // A nested `node_modules/a/node_modules/b` is npm resolving a transitive - // conflict *inside* one install — routine, and not the cross-install skew - // this guard is about (`cosmiconfig`'s yaml@1 alongside the top-level yaml@2 - // is the live example). +test("lockVersionsByPath: keyed by install path, nested entries included (Copilot, #1965 r1)", () => { + // Keyed by PATH, not by package name: a nested copy that entered a program has + // to be priced from its own entry. Reading only `node_modules/` would + // compare it against the install's top-level copy — a different version, or + // none — and a real pair could pass. const lock = { packages: { "": { name: "root" }, @@ -75,18 +75,173 @@ test("topLevelLockVersions: nested duplicates are ignored", () => { "node_modules/no-version": { resolved: "https://example.test/x.tgz" }, }, }; - assert.deepEqual([...topLevelLockVersions(lock)].sort(), [ - ["@modelcontextprotocol/client", "2.0.0-beta.5"], - ["yaml", "2.9.0"], - ["zod", "4.4.3"], + assert.deepEqual([...lockVersionsByPath(lock)].sort(), [ + ["node_modules/@modelcontextprotocol/client", "2.0.0-beta.5"], + ["node_modules/cosmiconfig/node_modules/yaml", "1.10.3"], + ["node_modules/yaml", "2.9.0"], + ["node_modules/zod", "4.4.3"], ]); }); -test("topLevelLockVersions: a malformed or empty lockfile yields nothing", () => { +test("lockVersionsByPath: a malformed or empty lockfile yields nothing", () => { // Safe as a pure helper *because* `hasReadableLockShape` rejects these before // any comparison — an empty map reaching `findSkew` is the fail-open path. for (const lock of [undefined, null, {}, { packages: {} }]) - assert.equal(topLevelLockVersions(lock).size, 0); + assert.equal(lockVersionsByPath(lock).size, 0); +}); + +/** `crossInstallPackages`-shaped input: name → program → install → entry paths. */ +const occurrence = (name, program, byRoot) => + new Map([ + [ + name, + new Map([ + [ + program, + new Map( + Object.entries(byRoot).map(([dir, paths]) => [dir, new Set(paths)]), + ), + ], + ]), + ], + ]); + +const lockPaths = (byDir) => + new Map( + Object.entries(byDir).map(([dir, entries]) => [ + dir, + new Map(Object.entries(entries)), + ]), + ); + +test("findSkew: reports the copies one program loaded, with their paths", () => { + const { skewed, unresolved } = findSkew( + occurrence("zod", "clients/web/tsconfig.test.json", { + ".": ["node_modules/zod"], + "clients/web": ["node_modules/zod"], + }), + lockPaths({ + ".": { "node_modules/zod": "4.3.6" }, + "clients/web": { "node_modules/zod": "4.4.3" }, + }), + ); + assert.deepEqual(unresolved, []); + assert.deepEqual(skewed, [ + { + name: "zod", + occurrences: [ + { + program: "clients/web/tsconfig.test.json", + holders: [ + { dir: ".", entryPath: "node_modules/zod", version: "4.3.6" }, + { + dir: "clients/web", + entryPath: "node_modules/zod", + version: "4.4.3", + }, + ], + }, + ], + }, + ]); +}); + +test("findSkew: only the installs that MET in a program are compared (Copilot, #1965 r1)", () => { + // `clients/cli` holds a different zod, but no program loads it beside another + // copy — nothing has to relate the two, so it is not a finding, and naming cli + // in a diagnostic about web's program would be wrong as well as noisy. + const { skewed } = findSkew( + occurrence("zod", "clients/web/tsconfig.test.json", { + ".": ["node_modules/zod"], + "clients/web": ["node_modules/zod"], + }), + lockPaths({ + ".": { "node_modules/zod": "4.4.3" }, + "clients/web": { "node_modules/zod": "4.4.3" }, + "clients/cli": { "node_modules/zod": "4.3.6" }, + }), + ); + assert.deepEqual(skewed, []); +}); + +test("findSkew: a nested copy is priced from its own entry (Copilot, #1965 r1)", () => { + // The root loaded zod through `a`'s nested copy. Pricing it from the root's + // TOP-LEVEL entry (4.4.3, aligned with web) would report the pair as agreeing. + const { skewed } = findSkew( + occurrence("zod", "clients/web/tsconfig.test.json", { + ".": ["node_modules/a/node_modules/zod"], + "clients/web": ["node_modules/zod"], + }), + lockPaths({ + ".": { + "node_modules/zod": "4.4.3", + "node_modules/a/node_modules/zod": "3.1.0", + }, + "clients/web": { "node_modules/zod": "4.4.3" }, + }), + ); + assert.deepEqual( + skewed[0].occurrences[0].holders.map((h) => `${h.dir}:${h.version}`), + [".:3.1.0", "clients/web:4.4.3"], + ); +}); + +test("findSkew: agreement is not skew", () => { + const { skewed, unresolved } = findSkew( + occurrence("zod", "p", { + ".": ["node_modules/zod"], + "clients/web": ["node_modules/zod"], + }), + lockPaths({ + ".": { "node_modules/zod": "4.4.3" }, + "clients/web": { "node_modules/zod": "4.4.3" }, + }), + ); + assert.deepEqual(skewed, []); + assert.deepEqual(unresolved, []); +}); + +test("findSkew: a copy with no lockfile entry is reported, not skipped", () => { + // Skipping it would drop a holder from the comparison and could report a real + // skew as agreement — the gate failing open. + const { skewed, unresolved } = findSkew( + occurrence("zod", "p", { + ".": ["node_modules/zod"], + "clients/web": ["node_modules/zod"], + }), + lockPaths({ ".": { "node_modules/zod": "4.4.3" }, "clients/web": {} }), + ); + assert.deepEqual(unresolved, [ + { name: "zod", dir: "clients/web", entryPath: "node_modules/zod" }, + ]); + assert.deepEqual(skewed, []); +}); + +test("findSkew: results are sorted by package name", () => { + const found = new Map([ + ...occurrence("zod", "p", { + ".": ["node_modules/zod"], + "clients/web": ["node_modules/zod"], + }), + ...occurrence("hono", "p", { + ".": ["node_modules/hono"], + "clients/web": ["node_modules/hono"], + }), + ]); + const { skewed } = findSkew( + found, + lockPaths({ + ".": { "node_modules/zod": "1.0.0", "node_modules/hono": "1.0.0" }, + "clients/web": { + "node_modules/zod": "2.0.0", + "node_modules/hono": "2.0.0", + }, + }), + ); + assert.deepEqual( + skewed.map((s) => s.name), + ["hono", "zod"], + ); }); test("hasReadableLockShape: only a v2+ packages table with a root entry (Copilot, #1962)", () => { @@ -124,89 +279,32 @@ test("hasReadableLockShape: only a v2+ packages table with a root entry (Copilot assert.equal(hasReadableLockShape(lock), false, JSON.stringify(lock)); }); -test("findSkew: reports a package held at two versions", () => { - const installs = [ - { dir: ".", versions: new Map([["zod", "4.3.6"]]) }, - { dir: "clients/web", versions: new Map([["zod", "4.4.3"]]) }, - { dir: "clients/cli", versions: new Map([["zod", "4.4.3"]]) }, - ]; - assert.deepEqual(findSkew(new Set(["zod"]), installs), [ - { - name: "zod", - holders: [ - { dir: ".", version: "4.3.6" }, - { dir: "clients/web", version: "4.4.3" }, - { dir: "clients/cli", version: "4.4.3" }, - ], - }, - ]); -}); - -test("findSkew: agreement and single-install packages are not skew", () => { - const installs = [ - { - dir: ".", - versions: new Map([ - ["zod", "4.4.3"], - ["express", "5.2.1"], - ]), - }, - { dir: "clients/web", versions: new Map([["zod", "4.4.3"]]) }, - ]; - // `express` lives in one install only, so it cannot skew — a package absent - // from a client is not a finding. - assert.deepEqual(findSkew(new Set(["zod", "express"]), installs), []); -}); - -test("findSkew: a candidate in no lockfile is inert", () => { - // `@inspector/core` is a build-time alias, not a package; the scan picks it - // up and it must drop out here rather than error. - const installs = [ - { dir: ".", versions: new Map([["zod", "4.4.3"]]) }, - { dir: "clients/web", versions: new Map([["zod", "4.4.3"]]) }, - ]; - assert.deepEqual(findSkew(new Set(["@inspector/core"]), installs), []); -}); - -test("findSkew: results are sorted by package name", () => { - const installs = [ - { - dir: ".", - versions: new Map([ - ["zod", "1.0.0"], - ["hono", "1.0.0"], - ]), - }, - { - dir: "clients/web", - versions: new Map([ - ["zod", "2.0.0"], - ["hono", "2.0.0"], - ]), - }, - ]; - assert.deepEqual( - findSkew(new Set(["zod", "hono"]), installs).map((s) => s.name), - ["hono", "zod"], - ); -}); - test("partitionSkew: the allowlist is by name, not by version pair", () => { // So an ordinary patch float within a tolerated package does not churn the // allowlist, while any *unlisted* package that starts skewing still fails. const skewed = [ { name: "react", - holders: [ - { dir: ".", version: "19.2.7" }, - { dir: "clients/web", version: "19.2.8" }, + occurrences: [ + { + program: "p", + holders: [ + { dir: ".", version: "19.2.7" }, + { dir: "clients/web", version: "19.2.8" }, + ], + }, ], }, { name: "zod", - holders: [ - { dir: ".", version: "4.3.6" }, - { dir: "clients/web", version: "4.4.3" }, + occurrences: [ + { + program: "p", + holders: [ + { dir: ".", version: "4.3.6" }, + { dir: "clients/web", version: "4.4.3" }, + ], + }, ], }, ]; @@ -223,7 +321,14 @@ test("partitionSkew: the allowlist is by name, not by version pair", () => { }); test("partitionSkew: deny by default — nothing tolerated fails everything", () => { - const skewed = [{ name: "zod", holders: [{ dir: ".", version: "1.0.0" }] }]; + const skewed = [ + { + name: "zod", + occurrences: [ + { program: "p", holders: [{ dir: ".", version: "1.0.0" }] }, + ], + }, + ]; assert.equal(partitionSkew(skewed, new Map()).failures.length, 1); }); @@ -235,18 +340,28 @@ test("partitionSkew: the allowlist tolerates skew only within a major (Copilot, const withinMajor = [ { name: "react", - holders: [ - { dir: ".", version: "19.2.7" }, - { dir: "clients/web", version: "19.2.8" }, + occurrences: [ + { + program: "p", + holders: [ + { dir: ".", version: "19.2.7" }, + { dir: "clients/web", version: "19.2.8" }, + ], + }, ], }, ]; const acrossMajor = [ { name: "react", - holders: [ - { dir: ".", version: "18.3.1" }, - { dir: "clients/web", version: "19.2.8" }, + occurrences: [ + { + program: "p", + holders: [ + { dir: ".", version: "18.3.1" }, + { dir: "clients/web", version: "19.2.8" }, + ], + }, ], }, ]; @@ -261,9 +376,14 @@ test("partitionSkew: an unparseable version can't be proven same-major, so it fa const skewed = [ { name: "react", - holders: [ - { dir: ".", version: "19.2.7" }, - { dir: "clients/web", version: "next" }, + occurrences: [ + { + program: "p", + holders: [ + { dir: ".", version: "19.2.7" }, + { dir: "clients/web", version: "next" }, + ], + }, ], }, ];