modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

chore(scripts): compare only the copies a program actually met

Browse source 
Two findings from the Copilot review of #1965, both real.

1. The candidates were flattened to package names before the version
   comparison, so a third install holding a different copy failed the guard
   even when no program loads that copy beside another — and the diagnostic
   named an install that never took part. `crossInstallPackages` now returns
   the co-occurrences themselves (name → program → install → entry paths) and
   `findSkew` compares per program, across only the installs that met there.

2. Folding a nested copy onto its outermost install lost the path that
   identifies which copy was loaded, and the version was then read from the
   install's top-level entry — absent or differently versioned, either way
   letting a real pair pass. `classifyModulePath` now also returns the
   `entryPath` npm keys the lockfile by, `topLevelLockVersions` becomes
   `lockVersionsByPath`, and each holder is priced from the entry the program
   resolved. A resolved copy with no lockfile entry is now a hard failure
   rather than a silently dropped holder.

The failure output prints `in <program>` with each holder's install path
beside its version, so a nested copy is distinguishable from a top-level one.

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-16 12:35:38 -04:00
commit a9b9a917e3
2 changed files with 495 additions and 188 deletions
Download patch file Download diff file Expand all files Collapse all files

68
scripts/lib/tsc-program.mjs
View file

@ -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<name, Map<programLabel, Map<installRoot, Set<entryPath>>>>` — 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;

63
scripts/lib/tsc-program.test.mjs
View file

@ -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"],
);
});