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): derive dep-lockstep candidates from what enters each tsc program

Browse source 
`verify:dep-lockstep` derived its candidate set from the packages the shared
first-party sources named *directly*, so a package whose declarations reach a
program only through another package's `.d.ts` was invisible to it — it could
skew across installs and recreate the two-copy blow-up #1896 was about while
the guard stayed green. `@modelcontextprotocol/sdk` was the live example: never
written in first-party code, yet 16 of its `.d.ts` files land in clients/web's
test program.

Replace the derivation with the measured one: list every client tsconfig
project with `tsc --listFilesOnly`, map each resolved `node_modules` file to
its owning install and package, and keep the packages that reach ONE program
from TWO installs. That is exactly the set that can put two structurally
distinct copies of a type in front of one checker. The rest of the guard is
unchanged — top-level lockfile comparison, deny-by-default, `TOLERATED_SKEW` as
a name allowlist scoped to within-a-major.

The listing machinery moves to `scripts/lib/tsc-program.mjs`, shared with
`verify:typecheck-coverage`, so the two guards can't disagree about what a
program contains. It is deliberately not cached to disk between the two
processes: a stale fingerprint would make a guard measure a program that no
longer exists and pass on a real miss. The second pass costs ~14s.

`TOLERATED_SKEW` is now empty. `jose` and `@modelcontextprotocol/ext-apps` are
root-only since #1970 and cannot skew; `react` (whose types are `@types/react`,
resolved from one install) and `hono` never reach one program from two
installs. Each would be a rationale for a skew that cannot occur.

Closes #1965

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-16 12:17:15 -04:00
commit 7f9be24bf3
1 changed files with 1069 additions and 933 deletions
Download patch file Download diff file Expand all files Collapse all files

281
scripts/verify-dep-lockstep.test.mjs
View file

@ -3,258 +3,61 @@
// change that relaxes one is visible as a deleted assertion rather than a quiet
// behavior shift. Run via `npm run test:scripts` (node:test; the root has no
// vitest harness).
//
// The candidate derivation itself lives in `lib/tsc-program.mjs` (shared with
// `verify:typecheck-coverage` since #1965) and is covered by
// `lib/tsc-program.test.mjs`; what stays here is the lockfile comparison and the
// client-enrollment rule this guard owns.
import { test } from "node:test";
import assert from "node:assert/strict";
import {
clientProjects,
findSkew,
hasReadableLockShape,
importedPackageNames,
isSharedSourceFile,
majorOf,
packageNameOf,
partitionSkew,
sourcesWithNoFiles,
topLevelLockVersions,
typeReferencePackageNames,
} from "./verify-dep-lockstep.mjs";
test("isSharedSourceFile: all four TS extensions, not just .ts/.tsx", () => {
// `.mts`/`.cts` are gated by `verify:format-coverage` and
// `verify:typecheck-coverage` too. None exist under the shared trees today,
// so dropping them here would go unnoticed until a new shared dependency
// arrived through one — the skew this guard exists to catch (Copilot, #1962).
for (const ext of [".ts", ".tsx", ".mts", ".cts"]) {
assert.equal(isSharedSourceFile(`core/mcp/thing${ext}`), true, ext);
assert.equal(
isSharedSourceFile(`test-servers/src/thing${ext}`),
true,
`test-servers ${ext}`,
);
}
});
test("isSharedSourceFile: non-TS files and other trees are excluded", () => {
const rejected = [
"core/README.md", // not TypeScript
"core/mcp/data.json",
"clients/web/src/App.tsx", // a client's own sources resolve from the client
"scripts/verify-dep-lockstep.mjs",
"test-servers/configs/modern-http.json",
// Path-boundary anchoring: a sibling dir whose name merely starts with a
// shared dir's name must not be swept in.
"core-internal/thing.ts",
"test-servers/src-legacy/thing.ts",
];
for (const file of rejected)
assert.equal(isSharedSourceFile(file), false, file);
});
test("sourcesWithNoFiles: each configured source must contribute (Copilot, #1962)", () => {
const dirs = ["core", "test-servers/src"];
const named = ["vitest.shared.mts"];
const complete = [
"core/mcp/a.ts",
"test-servers/src/b.ts",
"vitest.shared.mts",
];
assert.deepEqual(sourcesWithNoFiles(complete, dirs, named), []);
// The failure an aggregate count can't see: `core/` moved, but the other two
// sources keep `files.length` nonzero, so the guard would derive candidates
// from an incomplete set and pass on skew it should catch.
test("clientProjects: a `typecheck` script's projects win over references", () => {
// Both enrollment paths exist (cli/tui/launcher declare `typecheck`,
// `clients/web` is a `tsc -b` solution) and this guard must measure the same
// programs as `verify:typecheck-coverage`, which prefers the script.
assert.deepEqual(
sourcesWithNoFiles(
["test-servers/src/b.ts", "vitest.shared.mts"],
dirs,
named,
clientProjects(
{
typecheck: "tsc --noEmit -p tsconfig.json && tsc -p tsconfig.test.json",
},
["./ignored.json"],
),
["core"],
["tsconfig.json", "tsconfig.test.json"],
);
});
test("clientProjects: a reference client is measured through its references", () => {
assert.deepEqual(
sourcesWithNoFiles(["core/mcp/a.ts", "test-servers/src/b.ts"], dirs, named),
["vitest.shared.mts"],
clientProjects({ build: "tsc -b && vite build" }, ["./a.json"]),
["./a.json"],
);
assert.deepEqual(sourcesWithNoFiles([], dirs, named), [
"core",
"test-servers/src",
"vitest.shared.mts",
]);
// Neither path available — the caller reports it rather than measuring nothing.
assert.deepEqual(clientProjects({ build: "vite build" }, []), []);
});
test("sourcesWithNoFiles: a prefix sibling does not vouch for a dir", () => {
// `core-internal/` starts with `core` but is not it — the boundary check has
// to be on a path separator, or a renamed dir would look present.
assert.deepEqual(sourcesWithNoFiles(["core-internal/a.ts"], ["core"], []), [
"core",
]);
});
test("packageNameOf: bare names, scopes, and subpaths", () => {
const cases = [
["zod", "zod"],
["zod/v4", "zod"], // subpath dropped — one package, one version
["@modelcontextprotocol/client", "@modelcontextprotocol/client"],
["@modelcontextprotocol/client/core", "@modelcontextprotocol/client"],
["react-dom/client", "react-dom"],
];
for (const [input, expected] of cases)
assert.equal(packageNameOf(input), expected, input);
});
test("packageNameOf: non-packages are rejected", () => {
// Relative/absolute paths, built-ins with and without the `node:` prefix,
// protocol specifiers, and prose that follows the word `from` in a comment.
const rejected = [
"./foo",
"../core/mcp",
"/abs/path",
"fs",
"path",
"node:crypto",
"node:test",
"file:",
"data:text/plain,x",
"cwd omitted",
"",
];
for (const input of rejected)
assert.equal(packageNameOf(input), null, JSON.stringify(input));
assert.equal(packageNameOf(undefined), null);
});
test("importedPackageNames: CommonJS and awkward dynamic-import forms (Copilot, #1962)", () => {
// Under-approximating is the dangerous direction: a package the scan misses
// never enters the candidate set, so its skew passes the guard silently.
// `.cts` sources in the shared trees use `import x = require(…)` as ordinary
// syntax, and a dynamic import may carry import attributes or a static
// template literal — none of which the original three patterns matched.
const source = `
import express = require("express");
const yaml = require("yaml");
const a = await import("undici", { with: { type: "json" } });
const b = await import(\`jose\`);
`;
assert.deepEqual([...importedPackageNames(source)].sort(), [
"express",
"jose",
"undici",
"yaml",
]);
});
test("importedPackageNames: comment trivia between tokens (Copilot, #1962)", () => {
// TypeScript allows a comment anywhere whitespace is legal, so all of these
// are valid imports. Missing one is the dangerous direction: the package
// never enters the candidate set and its skew passes the guard silently.
const source = `
import { a } from /* explanation */ "express";
const b = await import(/* webpackIgnore: true */ "undici");
const c = require(/* lazy */ "yaml");
import /* side effect */ "pino";
`;
assert.deepEqual([...importedPackageNames(source)].sort(), [
"express",
"pino",
"undici",
"yaml",
]);
});
test("importedPackageNames: line-comment trivia, not just block (Copilot, #1962)", () => {
// `//` runs to end-of-line and is legal in every position a block comment is,
// so a specifier can sit on the next line and these are still valid imports.
const source = [
"import { a } from // reason",
' "express";',
"const b = await import(// lazy",
' "undici");',
"const c = require(// lazy",
' "yaml");',
].join("\n");
assert.deepEqual([...importedPackageNames(source)].sort(), [
"express",
"undici",
"yaml",
]);
});
test("importedPackageNames: static, side-effect, and dynamic forms; builtins and relatives dropped", () => {
const source = `
import { z } from "zod/v4";
export type { Foo } from '@modelcontextprotocol/core';
import "./side-effect.css";
import "pino";
const mod = await import("chokidar");
import fs from "node:fs";
import { helper } from "../local/helper";
`;
assert.deepEqual([...importedPackageNames(source)].sort(), [
"@modelcontextprotocol/core",
"chokidar",
"pino",
"zod",
]);
});
test("importedPackageNames: triple-slash type references count (Copilot, #1962)", () => {
// A `/// <reference types="x" />` pulls in declarations exactly like an
// import, but TypeScript reports it in `typeReferenceDirectives`, not
// `importedFiles` — so reading only the latter let a referenced package skew
// unseen. `path` references name a file, not a package, and are ignored.
const source = [
'/// <reference types="node" />',
'/// <reference types="express" />',
'/// <reference path="./local.d.ts" />',
'import { z } from "zod";',
].join("\n");
assert.deepEqual([...importedPackageNames(source)].sort(), [
"@types/express",
"@types/node",
"express",
"node",
"zod",
]);
});
test("typeReferencePackageNames: both the bare and the @types form (Copilot, #1962)", () => {
// The directive names a *type*, not a package: `node` resolves to
// `@types/node`, while a package shipping its own declarations resolves to
// itself. Returning both over-approximates, the safe direction — whichever
// isn't installed drops out downstream.
assert.deepEqual(typeReferencePackageNames("node"), ["node", "@types/node"]);
// Scoped names mangle with a double underscore, TypeScript's convention.
assert.deepEqual(typeReferencePackageNames("@scope/pkg"), [
"@scope/pkg",
"@types/scope__pkg",
]);
assert.deepEqual(typeReferencePackageNames("./relative"), []);
assert.deepEqual(typeReferencePackageNames(""), []);
});
test("importedPackageNames: prose in comments never becomes a package (Copilot, #1962)", () => {
// The regex scan this replaced could not tell code from a comment, so
// `// adapted from "react"` added `react` to the candidate set — and if that
// installed package were skewed, an unrelated comment would fail `validate`.
// These use REAL package names, which is the case the old prose test missed:
// it only passed because `cwd omitted` isn't a valid package name.
const source = `
// adapted from "react"
/** Mirrors the behavior of "express", see require("yaml") below. */
/** The excluded set derived from \\\`hono\\\`-style paths. */
// const disabled = await import("undici");
import { z } from "zod";
`;
assert.deepEqual([...importedPackageNames(source)], ["zod"]);
});
test("importedPackageNames: a specifier inside a string literal is not an import", () => {
const source = `
const msg = 'run require("chokidar") to load it';
const re = /"jose"/;
import { z } from "zod";
`;
assert.deepEqual([...importedPackageNames(source)], ["zod"]);
test("clientProjects: a NEUTERED project still contributes its program (#1965)", () => {
// `--noCheck` stops that pass type-checking, which is the sibling guard's
// complaint; the program still resolves its imports, and dropping it here
// would shrink what THIS guard measures on the strength of that other defect.
assert.deepEqual(
clientProjects(
{
typecheck:
"tsc -p tsconfig.json --noCheck && tsc -p tsconfig.test.json",
},
[],
).sort(),
["tsconfig.json", "tsconfig.test.json"],
);
});
test("topLevelLockVersions: nested duplicates are ignored", () => {
@ -479,13 +282,3 @@ 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));
});
test("isSharedSourceFile: individually-named shared files are included (Copilot, #1962)", () => {
// `vitest.shared.mts` is root-owned, imported by every client's vitest
// config, and already treated as shared by `verify:typecheck-coverage`. It
// imports only Node built-ins today, which is why omitting it would go
// unnoticed until a third-party import appeared there and skewed.
assert.equal(isSharedSourceFile("vitest.shared.mts"), true);
// Still anchored: a same-named file nested elsewhere is not the shared one.
assert.equal(isSharedSourceFile("clients/web/vitest.shared.mts"), false);
});