Merge pull request #1962 from modelcontextprotocol/v2/chore/zod-version-lockstep
chore(deps): align zod across installs and guard against version skew
This commit is contained in:
commit
16b9eff8f4
16 changed files with 1453 additions and 24 deletions
1
.github/copilot-instructions.md
vendored
1
.github/copilot-instructions.md
vendored
|
|
@ -92,6 +92,7 @@ Both exist and do different jobs. Theme files (`src/theme/<Component>.ts`) custo
|
|||
## Gates and PR hygiene
|
||||
|
||||
- `npm run format` before committing; **`npm run ci` before pushing** (`validate` → `coverage` → `verify:build-gate` → `smoke` → Storybook). `npm run validate` is the fast inner-loop check and is **not** a substitute — it runs `test`, not `test:coverage`, so it does zero coverage gating.
|
||||
- **A dependency bump must land in every install that declares it.** v2 is not a workspace — the root and each `clients/*` have their own `node_modules`, and a client's test project compiles `core/` and `test-servers/src` (which resolve from the **root**) alongside the client's own sources. Bumping a shared dependency in one manifest only puts two versions of it in one `tsc` program; for a recursive-generic surface like zod that exhausts the tsc heap (#1896). `verify:dep-lockstep` fails the build on this, so a PR bumping a package that the shared sources import should update the root **and every client that already lists it** — not every client unconditionally, since a package absent from an install can't skew and adding it there would be a spurious dependency.
|
||||
- **Every PR references an issue**, first body line `Closes #<ISSUE_NUMBER>`.
|
||||
- **Every PR carries exactly one version label**, `v1` or `v2`, matching its base branch.
|
||||
- Update the relevant `README.md` / `AGENTS.md` when a change adds, removes, renames, or repurposes a file or folder, changes the structure or tech stack, or introduces a command, dependency, or architectural pattern.
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
|
@ -37,7 +37,7 @@ inspector/
|
|||
│ ├── react/ # React hooks over the state stores
|
||||
│ └── storage/ # File I/O helpers for the OAuth persist backends
|
||||
├── test-servers/ # Composable MCP test servers + fixtures used by integration tests
|
||||
├── scripts/ # Root build/verify tooling (install cascade, smokes, verify-build-gate, verify-format-coverage, pack:verify)
|
||||
├── scripts/ # Root build/verify tooling (install cascade, smokes, verify-build-gate, verify-format-coverage, verify-dep-lockstep, pack:verify)
|
||||
├── docs/ # Task-oriented guides (server configuration, MCP App review, launcher/config plan)
|
||||
├── specification/ # Design/build specifications
|
||||
├── AGENTS.md # Contribution rules for agents AND humans (see below)
|
||||
|
|
@ -279,13 +279,14 @@ Each client self-validates from its own folder; the root scripts chain them. The
|
|||
|
||||
| Script | What it does |
|
||||
| --------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `npm run validate` | Runs `verify:format-coverage` (asserts every tracked source file is format-gated) first, then `validate:core` (the shared `core/` `format:check` + `lint` gate), then per client: `format:check` + `lint` + **`typecheck`** (cli/tui only) + `build` + fast unit tests. The quick inner-loop check. |
|
||||
| `npm run validate` | Runs the three durable guards first — `verify:format-coverage` (every tracked source file is format-gated), `verify:typecheck-coverage` (every one lands in a tsconfig project), `verify:dep-lockstep` (no dependency the shared sources directly import skews across installs) — then `test:scripts` (the guards' own parser unit tests), then `validate:core` (the shared `core/` `format:check` + `lint` gate), then per client: `format:check` + `lint` + **`typecheck`** (cli/tui/launcher; web typechecks via `tsc -b` inside its `build`) + `build` + fast unit tests. The quick inner-loop check. |
|
||||
| `npm run coverage` | The **per-file ≥90% gate** (lines/statements/functions/branches) under v8 instrumentation, per client. CI-enforced. For web this also runs the integration project and covers the shared `core/` runtime (including `core/json` and `core/client`). |
|
||||
| `npm run smoke` | End-to-end smokes through the built launcher (`--help` dispatch + prod cli/tui/web), plus two headless-Chromium smokes: a boot smoke that runs the prod web bundle and asserts a clean first render (no uncaught error — sync exception or unhandled rejection, how a Node built-in reaching the browser bundle manifests), and an **MCP Apps** smoke (`smoke:web:app`) that drives connect → open app → `data-app-status="ready"` against a composable App server, covering the sandbox proxy and UI-protocol bridge. |
|
||||
| `npm run verify:build-gate` | Runs a real `vite build` with a Node built-in forced into the browser graph and asserts the build **fails** via the #1769 gate (which turns Vite's browser-externalization warning into a hard error). Guards against the warning phrasing drifting in a Vite bump and silently disabling the gate. Part of `npm run ci`. |
|
||||
| `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` + the exported helpers of `verify-typecheck-coverage.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 test:scripts` | Table-driven unit tests (`node --test`) for the guard's own pure parsers (`scripts/lib/npm-scripts.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 those shared sources import, compares the committed lockfiles' top-level entries, and **fails deny-by-default** on any skew not in the annotated `TOLERATED_SKEW` allowlist — and an allowlisted package is 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). |
|
||||
|
||||
|
|
|
|||
2
clients/cli/package-lock.json
generated
2
clients/cli/package-lock.json
generated
|
|
@ -19,7 +19,7 @@
|
|||
"open": "^10.2.0",
|
||||
"pino": "^9.14.0",
|
||||
"undici": "^8.5.0",
|
||||
"zod": "^4.3.6"
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"bin": {
|
||||
"mcp-inspector-cli": "build/index.js"
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@
|
|||
"open": "^10.2.0",
|
||||
"pino": "^9.14.0",
|
||||
"undici": "^8.5.0",
|
||||
"zod": "^4.3.6"
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@eslint/js": "^10.0.1",
|
||||
|
|
|
|||
2
clients/tui/package-lock.json
generated
2
clients/tui/package-lock.json
generated
|
|
@ -20,7 +20,7 @@
|
|||
"open": "^10.2.0",
|
||||
"pino": "^9.14.0",
|
||||
"react": "^19.2.4",
|
||||
"zod": "^4.3.6"
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"bin": {
|
||||
"mcp-inspector-tui": "build/index.js"
|
||||
|
|
|
|||
|
|
@ -39,7 +39,7 @@
|
|||
"open": "^10.2.0",
|
||||
"pino": "^9.14.0",
|
||||
"react": "^19.2.4",
|
||||
"zod": "^4.3.6"
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"overrides": {
|
||||
"ink-select-input": "^6.2.0"
|
||||
|
|
|
|||
8
clients/web/package-lock.json
generated
8
clients/web/package-lock.json
generated
|
|
@ -35,7 +35,7 @@
|
|||
"react-markdown": "^10.1.0",
|
||||
"react-syntax-highlighter": "^16.1.1",
|
||||
"remark-gfm": "^4.0.1",
|
||||
"zod": "~4.3.6"
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"bin": {
|
||||
"mcp-inspector-web": "build/index.js"
|
||||
|
|
@ -11677,9 +11677,9 @@
|
|||
}
|
||||
},
|
||||
"node_modules/zod": {
|
||||
"version": "4.3.6",
|
||||
"resolved": "https://registry.npmjs.org/zod/-/zod-4.3.6.tgz",
|
||||
"integrity": "sha512-rftlrkhHZOcjDwkGlnUtZZkvaPHCsDATp4pGpuOOMDaTdDDXF91wuVDJoWoPsKX/3YPQ5fHuF3STjcYyKr+Qhg==",
|
||||
"version": "4.4.3",
|
||||
"resolved": "https://registry.npmjs.org/zod/-/zod-4.4.3.tgz",
|
||||
"integrity": "sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==",
|
||||
"license": "MIT",
|
||||
"funding": {
|
||||
"url": "https://github.com/sponsors/colinhacks"
|
||||
|
|
|
|||
|
|
@ -61,7 +61,7 @@
|
|||
"react-markdown": "^10.1.0",
|
||||
"react-syntax-highlighter": "^16.1.1",
|
||||
"remark-gfm": "^4.0.1",
|
||||
"zod": "~4.3.6"
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@chromatic-com/storybook": "^5.2.1",
|
||||
|
|
|
|||
8
package-lock.json
generated
8
package-lock.json
generated
|
|
@ -32,7 +32,7 @@
|
|||
"undici": "^8.5.0",
|
||||
"vite": "^8.1.5",
|
||||
"yaml": "^2.9.0",
|
||||
"zod": "^4.3.6"
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"bin": {
|
||||
"mcp-inspector": "clients/launcher/build/index.js"
|
||||
|
|
@ -4534,9 +4534,9 @@
|
|||
"license": "MIT"
|
||||
},
|
||||
"node_modules/zod": {
|
||||
"version": "4.3.6",
|
||||
"resolved": "https://registry.npmjs.org/zod/-/zod-4.3.6.tgz",
|
||||
"integrity": "sha512-rftlrkhHZOcjDwkGlnUtZZkvaPHCsDATp4pGpuOOMDaTdDDXF91wuVDJoWoPsKX/3YPQ5fHuF3STjcYyKr+Qhg==",
|
||||
"version": "4.4.3",
|
||||
"resolved": "https://registry.npmjs.org/zod/-/zod-4.4.3.tgz",
|
||||
"integrity": "sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==",
|
||||
"license": "MIT",
|
||||
"funding": {
|
||||
"url": "https://github.com/sponsors/colinhacks"
|
||||
|
|
|
|||
|
|
@ -43,8 +43,9 @@
|
|||
"verify:build-gate": "node scripts/verify-build-gate.mjs",
|
||||
"verify:typecheck-coverage": "node scripts/verify-typecheck-coverage.mjs",
|
||||
"test:scripts": "node --test \"scripts/**/*.test.mjs\"",
|
||||
"validate": "npm run verify:format-coverage && npm run verify:typecheck-coverage && npm run test:scripts && npm run validate:core && npm run validate:web && npm run validate:cli && npm run validate:tui && npm run validate:launcher",
|
||||
"validate": "npm run verify:format-coverage && npm run verify:typecheck-coverage && npm run verify:dep-lockstep && npm run test:scripts && npm run validate:core && npm run validate:web && npm run validate:cli && npm run validate:tui && npm run validate:launcher",
|
||||
"verify:format-coverage": "node scripts/verify-format-coverage.mjs",
|
||||
"verify:dep-lockstep": "node scripts/verify-dep-lockstep.mjs",
|
||||
"validate:core": "npm run format:check:core && npm run format:check:scripts && npm run format:check:shared && npm run lint:core && npm run lint:shared",
|
||||
"lint:core": "eslint \"core/**/*.{ts,tsx}\"",
|
||||
"lint:shared": "eslint \"test-servers/src/**/*.{ts,tsx,mts,cts}\" vitest.shared.mts eslint.config.js",
|
||||
|
|
@ -98,7 +99,7 @@
|
|||
"undici": "^8.5.0",
|
||||
"vite": "^8.1.5",
|
||||
"yaml": "^2.9.0",
|
||||
"zod": "^4.3.6"
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"overrides": {
|
||||
"ink-select-input": "^6.2.0"
|
||||
|
|
|
|||
290
scripts/verify-dep-lockstep.main.test.mjs
Normal file
290
scripts/verify-dep-lockstep.main.test.mjs
Normal file
|
|
@ -0,0 +1,290 @@
|
|||
// End-to-end tests for the dep-lockstep guard's executable path (Copilot,
|
||||
// #1962). The sibling tests cover the pure helpers; nothing exercised `main()`,
|
||||
// so a regression in source enumeration, install discovery, lockfile loading,
|
||||
// the sibling-guard vouch, or the nonzero exit on real skew would have left the
|
||||
// whole suite green.
|
||||
//
|
||||
// Each case builds a throwaway repo — the guard derives its root from its own
|
||||
// file location, so the script is copied into the fixture rather than pointed
|
||||
// at one — `git add`s it (the enumeration is `git ls-files`, which reads the
|
||||
// index), and runs the guard as a subprocess to assert the exit status and
|
||||
// message. Run via `npm run test:scripts`.
|
||||
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { execFileSync, spawnSync } from "node:child_process";
|
||||
import {
|
||||
cpSync,
|
||||
mkdirSync,
|
||||
mkdtempSync,
|
||||
realpathSync,
|
||||
rmSync,
|
||||
symlinkSync,
|
||||
writeFileSync,
|
||||
} from "node:fs";
|
||||
import { createRequire } from "node:module";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
const scriptsDir = path.dirname(fileURLToPath(import.meta.url));
|
||||
const repoRoot = path.join(scriptsDir, "..");
|
||||
|
||||
/** The real typescript install, symlinked into each fixture so the guard resolves it. */
|
||||
const typescriptDir = path.dirname(
|
||||
createRequire(path.join(repoRoot, "clients", "web", "package.json")).resolve(
|
||||
"typescript/package.json",
|
||||
),
|
||||
);
|
||||
|
||||
/** A lockfileVersion 3 lockfile, including the `""` root entry npm always writes. */
|
||||
const lock = (deps) => ({
|
||||
lockfileVersion: 3,
|
||||
packages: {
|
||||
"": { name: "fixture" },
|
||||
...Object.fromEntries(
|
||||
Object.entries(deps).map(([name, version]) => [
|
||||
`node_modules/${name}`,
|
||||
{ version },
|
||||
]),
|
||||
),
|
||||
},
|
||||
});
|
||||
|
||||
/**
|
||||
* Build a fixture repo: shared sources importing `zod` and `express`, a root
|
||||
* install and one client install, and the guard itself. `rootDeps`/`webDeps`
|
||||
* decide whether the two installs agree.
|
||||
*/
|
||||
function makeFixture({ rootDeps, webDeps, scripts, rawWebLock }) {
|
||||
// realpath matters: on macOS `tmpdir()` is `/var/...`, a symlink to
|
||||
// `/private/var/...`. The guard only runs `main()` when `import.meta.url`
|
||||
// (always the resolved path) matches `process.argv[1]`, so launching it via
|
||||
// the unresolved path would load the module and silently do nothing —
|
||||
// exiting 0 with no output, which every assertion here would misread.
|
||||
const dir = realpathSync(mkdtempSync(path.join(tmpdir(), "dep-lockstep-")));
|
||||
const write = (rel, contents) => {
|
||||
mkdirSync(path.join(dir, path.dirname(rel)), { recursive: true });
|
||||
writeFileSync(
|
||||
path.join(dir, rel),
|
||||
typeof contents === "string"
|
||||
? contents
|
||||
: JSON.stringify(contents, null, 2),
|
||||
);
|
||||
};
|
||||
|
||||
write("package.json", {
|
||||
name: "fixture",
|
||||
scripts: scripts ?? {
|
||||
validate: "npm run verify:format-coverage && npm run verify:dep-lockstep",
|
||||
"verify:format-coverage": "node scripts/verify-format-coverage.mjs",
|
||||
"verify:dep-lockstep": "node scripts/verify-dep-lockstep.mjs",
|
||||
},
|
||||
});
|
||||
write("core/client.ts", 'import { z } from "zod";\nexport const s = z;\n');
|
||||
write(
|
||||
"test-servers/src/server.ts",
|
||||
'import express from "express";\nexport const app = express;\n',
|
||||
);
|
||||
write(
|
||||
"vitest.shared.mts",
|
||||
'import path from "node:path";\nexport default path;\n',
|
||||
);
|
||||
write("package-lock.json", lock(rootDeps));
|
||||
write("clients/web/package.json", { name: "web" });
|
||||
write("clients/web/package-lock.json", rawWebLock ?? lock(webDeps));
|
||||
|
||||
// The guard resolves its repo root from its own location, so it has to live
|
||||
// inside the fixture; `lib/npm-scripts.mjs` comes along as its import.
|
||||
mkdirSync(path.join(dir, "scripts", "lib"), { recursive: true });
|
||||
for (const rel of [
|
||||
"verify-dep-lockstep.mjs",
|
||||
path.join("lib", "npm-scripts.mjs"),
|
||||
])
|
||||
cpSync(path.join(scriptsDir, rel), path.join(dir, "scripts", rel));
|
||||
|
||||
mkdirSync(path.join(dir, "node_modules"), { recursive: true });
|
||||
symlinkSync(
|
||||
typescriptDir,
|
||||
path.join(dir, "node_modules", "typescript"),
|
||||
"dir",
|
||||
);
|
||||
|
||||
execFileSync("git", ["init", "-q"], { cwd: dir });
|
||||
execFileSync("git", ["add", "-A"], { cwd: dir });
|
||||
return dir;
|
||||
}
|
||||
|
||||
function runGuard(dir) {
|
||||
const r = spawnSync(
|
||||
process.execPath,
|
||||
[path.join(dir, "scripts", "verify-dep-lockstep.mjs")],
|
||||
{ cwd: dir, encoding: "utf8" },
|
||||
);
|
||||
return { status: r.status, out: `${r.stdout}${r.stderr}` };
|
||||
}
|
||||
|
||||
/** Run `fn` against a fresh fixture, always cleaning the temp dir up. */
|
||||
function withFixture(options, fn) {
|
||||
const dir = makeFixture(options);
|
||||
try {
|
||||
return fn(dir);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
const ALIGNED = { zod: "4.4.3", express: "5.2.1" };
|
||||
|
||||
test("main: exits 0 when every install agrees", () => {
|
||||
withFixture({ rootDeps: ALIGNED, webDeps: ALIGNED }, (dir) => {
|
||||
const { status, out } = runGuard(dir);
|
||||
assert.equal(status, 0, out);
|
||||
assert.match(out, /verify:dep-lockstep — OK/);
|
||||
// Both shared trees and the named shared file contributed their imports.
|
||||
assert.match(out, /2 install-crossing dependencies/);
|
||||
});
|
||||
});
|
||||
|
||||
test("main: exits 1 and names the skewed package and every holder", () => {
|
||||
withFixture(
|
||||
{ rootDeps: { ...ALIGNED, zod: "4.3.6" }, webDeps: ALIGNED },
|
||||
(dir) => {
|
||||
const { status, out } = runGuard(dir);
|
||||
assert.equal(status, 1, out);
|
||||
assert.match(out, /\bzod\b/);
|
||||
assert.match(out, /4\.3\.6\s+\(\.\)/);
|
||||
assert.match(out, /4\.4\.3\s+\(clients\/web\)/);
|
||||
// `express` agrees, so it must not be reported.
|
||||
assert.doesNotMatch(out, /^\s+express$/m);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("main: a package imported only by test-servers/src is still checked", () => {
|
||||
// Guards the enumeration of the *second* shared tree: if only `core/` were
|
||||
// scanned, this skew would pass.
|
||||
withFixture(
|
||||
{ rootDeps: { ...ALIGNED, express: "5.0.0" }, webDeps: ALIGNED },
|
||||
(dir) => {
|
||||
const { status, out } = runGuard(dir);
|
||||
assert.equal(status, 1, out);
|
||||
assert.match(out, /\bexpress\b/);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("main: exits 1 when a configured shared source matches no file", () => {
|
||||
withFixture({ rootDeps: ALIGNED, webDeps: ALIGNED }, (dir) => {
|
||||
// Drop `core/` from the index — the other sources keep the file count
|
||||
// nonzero, which is exactly what the old aggregate check missed.
|
||||
execFileSync("git", ["rm", "-r", "-q", "--cached", "core"], { cwd: dir });
|
||||
const { status, out } = runGuard(dir);
|
||||
assert.equal(status, 1, out);
|
||||
assert.match(out, /matched no tracked file/);
|
||||
assert.match(out, /^\s+core$/m);
|
||||
});
|
||||
});
|
||||
|
||||
test("main: exits 1 on a lockfile it cannot read, rather than failing open (Copilot, #1962)", () => {
|
||||
// A v1 lockfile has `dependencies` and no `packages` table. Treating it as an
|
||||
// install that simply holds nothing would leave the *root's* zod unopposed
|
||||
// and the skew below reported as aligned — the gate failing open.
|
||||
withFixture(
|
||||
{
|
||||
rootDeps: { ...ALIGNED, zod: "4.3.6" },
|
||||
webDeps: ALIGNED,
|
||||
rawWebLock: {
|
||||
lockfileVersion: 1,
|
||||
dependencies: { zod: { version: "4.4.3" } },
|
||||
},
|
||||
},
|
||||
(dir) => {
|
||||
const { status, out } = runGuard(dir);
|
||||
assert.equal(status, 1, out);
|
||||
assert.match(out, /not in a readable format/);
|
||||
assert.match(out, /clients\/web\/package-lock\.json/);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("main: exits 1 on a lockfile with a `packages` table but no root entry", () => {
|
||||
// The shape check is not just "has a packages key" — a table without the
|
||||
// `""` root npm always writes is not a lockfile this guard can trust.
|
||||
withFixture(
|
||||
{
|
||||
rootDeps: ALIGNED,
|
||||
webDeps: ALIGNED,
|
||||
rawWebLock: {
|
||||
lockfileVersion: 3,
|
||||
packages: { "node_modules/zod": { version: "4.4.3" } },
|
||||
},
|
||||
},
|
||||
(dir) => {
|
||||
const { status, out } = runGuard(dir);
|
||||
assert.equal(status, 1, out);
|
||||
assert.match(out, /not in a readable format/);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("main: exits 1 on an install whose lockfile is missing (Copilot, #1962)", () => {
|
||||
// Enrolment is by `package.json`, so a missing lockfile is loud rather than a
|
||||
// silently absent row. The fixture IS skewed, so dropping the install instead
|
||||
// would leave the remaining holder unopposed and report aligned.
|
||||
withFixture(
|
||||
{ rootDeps: { ...ALIGNED, zod: "4.3.6" }, webDeps: ALIGNED },
|
||||
(dir) => {
|
||||
rmSync(path.join(dir, "clients", "web", "package-lock.json"));
|
||||
const { status, out } = runGuard(dir);
|
||||
assert.equal(status, 1, out);
|
||||
assert.match(out, /no lockfile/);
|
||||
assert.match(out, /clients\/web\/package-lock\.json/);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("main: exits 1 when the ROOT lockfile is missing", () => {
|
||||
// The worst variant: the root is the install every shared source resolves
|
||||
// from, so omitting it could let the guard pass on client locks alone.
|
||||
withFixture({ rootDeps: ALIGNED, webDeps: ALIGNED }, (dir) => {
|
||||
rmSync(path.join(dir, "package-lock.json"));
|
||||
const { status, out } = runGuard(dir);
|
||||
assert.equal(status, 1, out);
|
||||
assert.match(out, /no lockfile/);
|
||||
assert.match(out, /^\s+\.\/package-lock\.json$/m);
|
||||
});
|
||||
});
|
||||
|
||||
test("main: a clients/ dir with no package.json is not an install", () => {
|
||||
// Enrolling a stray directory would demand a lockfile it should never have.
|
||||
withFixture({ rootDeps: ALIGNED, webDeps: ALIGNED }, (dir) => {
|
||||
mkdirSync(path.join(dir, "clients", "scratch"), { recursive: true });
|
||||
writeFileSync(
|
||||
path.join(dir, "clients", "scratch", "notes.md"),
|
||||
"scratch\n",
|
||||
);
|
||||
const { status, out } = runGuard(dir);
|
||||
assert.equal(status, 0, out);
|
||||
});
|
||||
});
|
||||
|
||||
test("main: exits 1 when the root validate no longer runs the sibling guard", () => {
|
||||
withFixture(
|
||||
{
|
||||
rootDeps: ALIGNED,
|
||||
webDeps: ALIGNED,
|
||||
// `verify:format-coverage` dropped from the chain: the vouch must fail
|
||||
// even though the dependency versions themselves are fine.
|
||||
scripts: {
|
||||
validate: "npm run verify:dep-lockstep",
|
||||
"verify:dep-lockstep": "node scripts/verify-dep-lockstep.mjs",
|
||||
},
|
||||
},
|
||||
(dir) => {
|
||||
const { status, out } = runGuard(dir);
|
||||
assert.equal(status, 1, out);
|
||||
assert.match(out, /no longer runs `verify:format-coverage`/);
|
||||
},
|
||||
);
|
||||
});
|
||||
544
scripts/verify-dep-lockstep.mjs
Normal file
544
scripts/verify-dep-lockstep.mjs
Normal file
|
|
@ -0,0 +1,544 @@
|
|||
#!/usr/bin/env node
|
||||
// Durable guard for the "one version per install-crossing dependency" invariant
|
||||
// (#1896). v2 is not an npm workspace: the root and each `clients/*` carry their
|
||||
// own `node_modules`, so the *same* package can resolve to two different
|
||||
// versions in one process — or, worse, in one `tsc` program.
|
||||
//
|
||||
// That second case is what this guard exists for. A client's
|
||||
// `tsconfig.test.json` compiles first-party sources that live *outside* the
|
||||
// client (`test-servers/src`, `core/`), and those files resolve their
|
||||
// dependencies from the **root** install while the client's own sources resolve
|
||||
// from the client install. When the two copies are the same version the
|
||||
// duplication is harmless; when they skew, TypeScript must relate two
|
||||
// structurally-distinct declarations of the same type.
|
||||
//
|
||||
// For most packages that is merely redundant work. For a deeply
|
||||
// recursive-generic type surface it is exponential: zod `4.3.6` (root) against
|
||||
// zod `4.4.3` (clients/web) made `tsc -b` in `clients/web` exhaust the 4GB
|
||||
// default heap outright (`FATAL ERROR: Ineffective mark-compacts near heap
|
||||
// limit`) via `TS2589 Type instantiation is excessively deep`, because every
|
||||
// `@modelcontextprotocol/*` schema is built out of zod generics. Aligning the
|
||||
// two copies — changing nothing else — returned the build to its baseline cost.
|
||||
//
|
||||
// The candidate set is DERIVED, not hand-listed: it is the packages imported by
|
||||
// the shared first-party TypeScript — `core/`, `test-servers/src`, and the
|
||||
// root-owned `vitest.shared.mts` — the surfaces compiled into more than one
|
||||
// client's program. Skew is then denied by default, with a small
|
||||
// allowlist of packages verified to tolerate it (below). A dependency that
|
||||
// starts skewing therefore fails `validate` and forces a decision, rather than
|
||||
// surfacing months later as an unexplained OOM.
|
||||
//
|
||||
// KNOWN BOUNDARY (#1965): the candidate set covers packages the shared sources
|
||||
// name *directly*. A package whose declarations reach the program only through
|
||||
// another package's `.d.ts` is invisible here — `@modelcontextprotocol/sdk` is
|
||||
// the live example, skewed root 1.29.0 vs `clients/web` 1.30.0 and present in
|
||||
// web's program from both installs, yet never written in first-party code
|
||||
// (the shared sources import the split `@modelcontextprotocol/client|core|…`).
|
||||
// Two derivations were measured for closing this. A lockfile dependency
|
||||
// closure is unusable — 155 packages, 25 of them skewed, nearly all irrelevant
|
||||
// tooling (`chai`, `qs`, `iconv-lite`) — and it misses the SDK anyway. Reading
|
||||
// what actually lands in each program (`tsc --listFilesOnly`, keeping packages
|
||||
// present under two install roots) is both correct and small: 15 for
|
||||
// `clients/web`, ~10 once nested duplicates are dropped. That is the right
|
||||
// derivation and is tracked separately, since it changes what the guard
|
||||
// measures and surfaces skews needing their own decisions.
|
||||
|
||||
import { readFileSync, existsSync, readdirSync } from "node:fs";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { builtinModules, createRequire } from "node:module";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
import { rootReachesScript } from "./lib/npm-scripts.mjs";
|
||||
|
||||
const repoRoot = path.resolve(
|
||||
path.dirname(fileURLToPath(import.meta.url)),
|
||||
"..",
|
||||
);
|
||||
|
||||
// The first-party source trees that are compiled into more than one install's
|
||||
// `tsc` program, and so define which dependencies can appear twice in one
|
||||
// program. `core/` is consumed by every client via the `@inspector/core` alias;
|
||||
// `test-servers/src` is pulled into the web and cli test projects.
|
||||
const SHARED_SOURCE_DIRS = ["core", "test-servers/src"];
|
||||
|
||||
// Individual root-owned TypeScript files that are shared the same way but sit
|
||||
// outside those trees. `vitest.shared.mts` is imported by every client's vitest
|
||||
// config, and `verify:typecheck-coverage` already treats it as shared
|
||||
// non-client source. It imports only Node built-ins today — which is precisely
|
||||
// why omitting it would go unnoticed until a third-party import appeared there,
|
||||
// resolved from the root, and skewed (Copilot, #1962).
|
||||
const SHARED_SOURCE_FILES = ["vitest.shared.mts"];
|
||||
|
||||
// Packages whose cross-install skew is verified benign, each with the reason.
|
||||
// This is an allowlist of *names*, not of version pairs, so an ordinary patch
|
||||
// float within one of these does not churn the file — while any package NOT
|
||||
// listed here failing the check is a genuine, unreviewed new skew.
|
||||
//
|
||||
// Being listed is NOT a blanket exemption: it tolerates skew only *within a
|
||||
// major version*. Each rationale below establishes that a patch/minor
|
||||
// difference is harmless, which is not evidence that a React 18-vs-19 or Hono
|
||||
// 4-vs-5 split across installs would be — that is a different type surface, and
|
||||
// it fails like anything else (Copilot, #1962).
|
||||
//
|
||||
// The admission test is the one the zod incident established: does the
|
||||
// package's public type surface consist of deeply recursive generics that
|
||||
// first-party code relates across the boundary? If yes it must stay in
|
||||
// lockstep; if no, a patch-level difference costs nothing.
|
||||
const TOLERATED_SKEW = new Map([
|
||||
[
|
||||
"react",
|
||||
"Types are shallow interfaces (`ReactNode`, `FC`), not recursive generics; the runtime copies never meet — each client bundles its own.",
|
||||
],
|
||||
[
|
||||
"hono",
|
||||
"Only used behind first-party wrappers in `core/mcp/remote/node`; its generic router types are not related across the boundary.",
|
||||
],
|
||||
[
|
||||
"jose",
|
||||
"Consumed as flat function calls in `core/auth`; no generic type flows between installs.",
|
||||
],
|
||||
[
|
||||
"@modelcontextprotocol/ext-apps",
|
||||
"Plain interface/constant surface for the MCP Apps UI protocol; no generic instantiation to blow up.",
|
||||
],
|
||||
]);
|
||||
|
||||
/** Package names that are Node built-ins (with or without the `node:` prefix). */
|
||||
const BUILTINS = new Set([
|
||||
...builtinModules,
|
||||
...builtinModules.map((m) => `node:${m}`),
|
||||
]);
|
||||
|
||||
// A bare package specifier: optional `@scope/`, then a name, then any subpath.
|
||||
// Anchored so prose that happens to sit after the word `from` in a comment
|
||||
// ("from cwd omitted") cannot be mistaken for an import.
|
||||
const PACKAGE_SPECIFIER = /^(?:(@[^/\s]+)\/)?([^@/\s][^/\s]*)(?:\/.*)?$/;
|
||||
|
||||
/**
|
||||
* The bare package name a module specifier resolves to — `@scope/name` or
|
||||
* `name`, with any subpath dropped (`zod/v4` → `zod`). Returns null for
|
||||
* relative paths, built-ins, URLs, and anything not shaped like a specifier.
|
||||
*/
|
||||
export function packageNameOf(specifier) {
|
||||
if (typeof specifier !== "string" || specifier === "") return null;
|
||||
if (specifier.startsWith(".") || specifier.startsWith("/")) return null;
|
||||
if (BUILTINS.has(specifier)) return null;
|
||||
const m = PACKAGE_SPECIFIER.exec(specifier);
|
||||
if (!m) return null;
|
||||
const name = m[1] ? `${m[1]}/${m[2]}` : m[2];
|
||||
// A protocol-ish specifier (`node:test`, `file:`, `data:`) is not a package.
|
||||
if (name.includes(":")) return null;
|
||||
return name;
|
||||
}
|
||||
|
||||
// Specifiers are extracted with TypeScript's own `preProcessFile` rather than
|
||||
// by regex (Copilot, #1962 — raised across three review rounds, and correctly).
|
||||
// A regex scan gets both directions wrong: it *misses* valid syntax (an
|
||||
// `import x = require(…)` in a `.cts`, an import-attributes argument, a comment
|
||||
// between tokens — each a silent miss, so the package never enters the
|
||||
// candidate set and its skew passes), and it *invents* names from prose, since
|
||||
// `// adapted from "react"` is indistinguishable from an import to a pattern
|
||||
// that can't tell code from a comment. Every widening of the regex traded one
|
||||
// of those failures for the other.
|
||||
//
|
||||
// `preProcessFile` is TypeScript's lightweight pre-parse scanner — not a full
|
||||
// parse and no type checking — and it is exactly built for this: it returns
|
||||
// every module specifier, handling all import forms, trivia, strings, and
|
||||
// regex literals correctly, and it never sees a comment as code.
|
||||
//
|
||||
// typescript is resolved from `clients/web`, which already carries it; the root
|
||||
// has no TS dependency of its own. The `createRequire` base is load-bearing —
|
||||
// a bare `import("typescript")` would resolve relative to `scripts/`, not the
|
||||
// cwd (the same reason `smoke-web-browser.mjs` resolves playwright this way).
|
||||
let tsCache;
|
||||
function typescript() {
|
||||
if (!tsCache) {
|
||||
const require_ = createRequire(
|
||||
path.join(repoRoot, "clients", "web", "package.json"),
|
||||
);
|
||||
try {
|
||||
tsCache = require_("typescript");
|
||||
} catch (cause) {
|
||||
// Fail with the cause, not a bare MODULE_NOT_FOUND: the realistic way to
|
||||
// get here is a root install run with INSPECTOR_SKIP_CLIENT_INSTALL=1,
|
||||
// which leaves `clients/web/node_modules` empty. Silently skipping the
|
||||
// check instead would be worse — an unrun guard guards nothing.
|
||||
throw new Error(
|
||||
"verify:dep-lockstep — could not resolve `typescript` from clients/web. " +
|
||||
"Run `npm install` at the repo root (the postinstall cascade installs each client); " +
|
||||
"if you set INSPECTOR_SKIP_CLIENT_INSTALL=1, this guard cannot run.",
|
||||
{ cause },
|
||||
);
|
||||
}
|
||||
}
|
||||
return tsCache;
|
||||
}
|
||||
|
||||
/**
|
||||
* The package(s) a `/// <reference types="x" />` directive can resolve to
|
||||
* (Copilot, #1962). Such a directive pulls in declarations exactly like an
|
||||
* import does, but TypeScript reports it separately from `importedFiles`, so
|
||||
* reading only the latter would let a referenced package skew unseen.
|
||||
*
|
||||
* Both candidates are returned because the directive name is the *type* name,
|
||||
* not the package: `node` resolves to `@types/node`, while a package shipping
|
||||
* its own declarations resolves to itself. Returning both over-approximates,
|
||||
* which is the safe direction — whichever isn't installed drops out. A scoped
|
||||
* name mangles as `@scope/pkg` → `@types/scope__pkg`, TypeScript's convention.
|
||||
*/
|
||||
export function typeReferencePackageNames(directive) {
|
||||
const name = packageNameOf(directive);
|
||||
if (!name) return [];
|
||||
const scoped = /^@([^/]+)\/(.+)$/.exec(name);
|
||||
const typesName = scoped
|
||||
? `@types/${scoped[1]}__${scoped[2]}`
|
||||
: `@types/${name}`;
|
||||
return [name, typesName];
|
||||
}
|
||||
|
||||
/**
|
||||
* Every third-party package name whose declarations a blob of TypeScript source
|
||||
* pulls in — via an import of any form, or a triple-slash type reference.
|
||||
* Over-approximating is safe (a name absent from every lockfile contributes
|
||||
* nothing downstream — `@inspector/core` is a build-time alias, not a package,
|
||||
* and drops out that way); under-approximating is not, since a missed package
|
||||
* never enters the candidate set and its skew would pass silently.
|
||||
*/
|
||||
export function importedPackageNames(source) {
|
||||
const names = new Set();
|
||||
// (source, readImportFiles, detectJavaScriptImports) — the latter two make it
|
||||
// report `require(…)` and dynamic imports as well as static ones.
|
||||
const { importedFiles, typeReferenceDirectives } =
|
||||
typescript().preProcessFile(source, true, true);
|
||||
for (const { fileName } of importedFiles) {
|
||||
const name = packageNameOf(fileName);
|
||||
if (name) names.add(name);
|
||||
}
|
||||
for (const { fileName } of typeReferenceDirectives ?? [])
|
||||
for (const name of typeReferencePackageNames(fileName)) names.add(name);
|
||||
return names;
|
||||
}
|
||||
|
||||
/**
|
||||
* Top-level installed versions of every package in a parsed lockfile, keyed by
|
||||
* package name. Only `node_modules/<pkg>` 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.
|
||||
*/
|
||||
export function topLevelLockVersions(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);
|
||||
}
|
||||
return versions;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a parsed lockfile has the shape this guard can read: a
|
||||
* `lockfileVersion` 2+ `packages` table, keyed by install path with `""` for
|
||||
* 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
|
||||
* 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
|
||||
* (`dependencies` only, no `packages`) lands here too, correctly: this guard
|
||||
* cannot read it, so it must say so rather than skip the install.
|
||||
*/
|
||||
export function hasReadableLockShape(lock) {
|
||||
// The declared version is checked, not just inferred from the presence of a
|
||||
// `packages` key: this function and the diagnostic it drives both promise
|
||||
// "lockfileVersion 2+", so a file claiming v1 while carrying a `packages`
|
||||
// table must be rejected rather than half-trusted (Copilot, #1962).
|
||||
const version = lock?.lockfileVersion;
|
||||
if (typeof version !== "number" || !Number.isFinite(version) || version < 2)
|
||||
return false;
|
||||
const packages = lock.packages;
|
||||
if (typeof packages !== "object" || packages === null) return false;
|
||||
return Object.prototype.hasOwnProperty.call(packages, "");
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
export function findSkew(candidates, installs) {
|
||||
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 });
|
||||
}
|
||||
return skewed;
|
||||
}
|
||||
|
||||
/**
|
||||
* The major-version component of a lockfile version string. Prerelease and
|
||||
* build metadata are irrelevant here (`2.0.0-beta.5` → `2`). Returns null for
|
||||
* anything not starting with an integer, which is treated as "cannot prove same
|
||||
* major" and therefore fails rather than passes.
|
||||
*/
|
||||
export function majorOf(version) {
|
||||
const m = /^(\d+)\./.exec(String(version ?? ""));
|
||||
return m ? m[1] : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Split skewed packages into the tolerated ones and the failures.
|
||||
*
|
||||
* Being on the allowlist tolerates skew only *within a major version*: each
|
||||
* entry's rationale establishes that a patch/minor difference is benign, which
|
||||
* says nothing about a major split, where the type surface itself changes. So a
|
||||
* 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(s.holders.map((h) => majorOf(h.version)));
|
||||
return majors.size === 1 && !majors.has(null);
|
||||
};
|
||||
return {
|
||||
failures: skewed.filter((s) => !isTolerated(s)),
|
||||
ignored: skewed.filter(isTolerated),
|
||||
};
|
||||
}
|
||||
|
||||
// The TypeScript extensions the shared trees can hold. Deliberately the same
|
||||
// four `verify:format-coverage` and `verify:typecheck-coverage` gate on: a
|
||||
// `.mts`/`.cts` under `core/` or `test-servers/src` is typechecked like any
|
||||
// other source, so its imports must reach the candidate set too. None exist
|
||||
// under those trees today, which is exactly why omitting them would go
|
||||
// unnoticed until a new shared dependency arrived through one and skewed.
|
||||
const SOURCE_EXTENSIONS = [".ts", ".tsx", ".mts", ".cts"];
|
||||
|
||||
/**
|
||||
* Whether a repo-relative path is shared first-party TypeScript — under one of
|
||||
* the shared trees, or one of the individually-named shared files.
|
||||
*/
|
||||
export function isSharedSourceFile(file) {
|
||||
if (SHARED_SOURCE_FILES.includes(file)) return true;
|
||||
if (!SOURCE_EXTENSIONS.some((ext) => file.endsWith(ext))) return false;
|
||||
// Anchored on a path boundary so a sibling whose name merely starts with a
|
||||
// shared dir (`core-internal/`, `test-servers/src-legacy/`) isn't swept in.
|
||||
return SHARED_SOURCE_DIRS.some((dir) => file.startsWith(`${dir}/`));
|
||||
}
|
||||
|
||||
/**
|
||||
* Which configured shared sources contributed no file to `files`. Each dir and
|
||||
* each individually-named file must be represented; an aggregate count can't
|
||||
* see one of them going missing, because the others keep the total nonzero.
|
||||
*/
|
||||
export function sourcesWithNoFiles(
|
||||
files,
|
||||
dirs = SHARED_SOURCE_DIRS,
|
||||
named = SHARED_SOURCE_FILES,
|
||||
) {
|
||||
const missingDirs = dirs.filter(
|
||||
(dir) => !files.some((f) => f.startsWith(`${dir}/`)),
|
||||
);
|
||||
const missingNamed = named.filter((name) => !files.includes(name));
|
||||
return [...missingDirs, ...missingNamed];
|
||||
}
|
||||
|
||||
/** Tracked TypeScript files under the shared first-party source trees. */
|
||||
function sharedSourceFiles() {
|
||||
const out = execFileSync(
|
||||
"git",
|
||||
[
|
||||
"ls-files",
|
||||
"--",
|
||||
...SHARED_SOURCE_DIRS.map((d) => `${d}/**`),
|
||||
...SHARED_SOURCE_FILES,
|
||||
],
|
||||
{ cwd: repoRoot, encoding: "utf8" },
|
||||
);
|
||||
return out.split("\n").filter(isSharedSourceFile);
|
||||
}
|
||||
|
||||
/**
|
||||
* The installs to compare: the repo root plus every `clients/*` that carries a
|
||||
* lockfile. Discovered from disk rather than listed, so a new client is covered
|
||||
* without editing this guard (the same enrollment style as
|
||||
* `verify:typecheck-coverage`).
|
||||
*/
|
||||
function installDirs() {
|
||||
const clientsDir = path.join(repoRoot, "clients");
|
||||
const clients = existsSync(clientsDir)
|
||||
? readdirSync(clientsDir, { withFileTypes: true })
|
||||
.filter((e) => e.isDirectory())
|
||||
.map((e) => `clients/${e.name}`)
|
||||
.sort()
|
||||
: [];
|
||||
// Enrolment is by `package.json` — an install we are meant to compare —
|
||||
// NOT by the presence of a lockfile (Copilot, #1962). Filtering on the
|
||||
// lockfile made a missing one silently drop that install from the
|
||||
// comparison; for the root, the install every shared source resolves from,
|
||||
// that meant the guard could report success from client locks alone. A
|
||||
// missing lockfile is now a loud failure in `main`, not an absent row. The
|
||||
// root is always enrolled: it is this repo, so its manifest is a given.
|
||||
return ["."].concat(
|
||||
clients.filter((dir) =>
|
||||
existsSync(path.join(repoRoot, dir, "package.json")),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Run the guard. Prints its verdict and `process.exit(1)`s on any failure.
|
||||
* Called only when this file is executed directly — importing it (for tests)
|
||||
* gives access to the pure helpers above without running any of this.
|
||||
*/
|
||||
export function main() {
|
||||
const rootScripts = JSON.parse(
|
||||
readFileSync(path.join(repoRoot, "package.json"), "utf8"),
|
||||
).scripts;
|
||||
|
||||
// Vouch for a sibling guard: a guard cannot detect being unrun itself, but the
|
||||
// three can vouch for one another, so dropping any single one from `validate`
|
||||
// is caught by another. `verify:format-coverage` vouches for this one in turn.
|
||||
if (!rootReachesScript(rootScripts, "verify:format-coverage")) {
|
||||
console.error(
|
||||
"verify:dep-lockstep — the root `validate` no longer runs `verify:format-coverage` (a sibling guard). Restore it.",
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const files = sharedSourceFiles();
|
||||
// Per-source, not an aggregate count (Copilot, #1962): `vitest.shared.mts`
|
||||
// alone keeps the total nonzero, so a moved or renamed `core/` would leave
|
||||
// the guard checking a near-empty candidate set and passing. Every configured
|
||||
// source must contribute, or the enumeration is broken.
|
||||
const empty = sourcesWithNoFiles(files);
|
||||
if (empty.length > 0) {
|
||||
console.error(
|
||||
`verify:dep-lockstep — ${empty.length} configured shared source(s) matched no tracked file:\n`,
|
||||
);
|
||||
for (const source of empty) console.error(` ${source}`);
|
||||
console.error(
|
||||
"\nThe guard would derive its candidates from an incomplete set and pass on skew it should catch." +
|
||||
"\nA path was moved or renamed — fix SHARED_SOURCE_DIRS / SHARED_SOURCE_FILES in this file.",
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const candidates = new Set();
|
||||
for (const file of files) {
|
||||
const source = readFileSync(path.join(repoRoot, file), "utf8");
|
||||
for (const name of importedPackageNames(source)) candidates.add(name);
|
||||
}
|
||||
|
||||
const dirs = installDirs();
|
||||
|
||||
// A missing lockfile is a failure, not a skipped install: dropping one would
|
||||
// remove its versions from the comparison and could report a real skew as
|
||||
// aligned.
|
||||
const missing = dirs.filter(
|
||||
(dir) => !existsSync(path.join(repoRoot, dir, "package-lock.json")),
|
||||
);
|
||||
if (missing.length > 0) {
|
||||
console.error(
|
||||
`verify:dep-lockstep — ${missing.length} install(s) have a package.json but no lockfile:\n`,
|
||||
);
|
||||
for (const dir of missing) console.error(` ${dir}/package-lock.json`);
|
||||
console.error(
|
||||
"\nEvery install must be compared; skipping one could report a real skew as aligned." +
|
||||
"\nRun `npm install` there, or remove the install if it is no longer part of the repo.",
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const locks = dirs.map((dir) => {
|
||||
const file = path.join(repoRoot, dir, "package-lock.json");
|
||||
let lock;
|
||||
try {
|
||||
lock = JSON.parse(readFileSync(file, "utf8"));
|
||||
} catch (cause) {
|
||||
// Unparseable is the same failure as unreadable — say which file, rather
|
||||
// than dying on a raw SyntaxError with no path in it.
|
||||
throw new Error(
|
||||
`verify:dep-lockstep — could not parse ${dir}/package-lock.json.`,
|
||||
{ cause },
|
||||
);
|
||||
}
|
||||
return { dir, lock };
|
||||
});
|
||||
|
||||
// Refuse to compare against a lockfile whose shape we can't read, instead of
|
||||
// treating it as an install that holds nothing — see `hasReadableLockShape`.
|
||||
const unreadable = locks.filter(({ lock }) => !hasReadableLockShape(lock));
|
||||
if (unreadable.length > 0) {
|
||||
console.error(
|
||||
`verify:dep-lockstep — ${unreadable.length} lockfile(s) are not in a readable format:\n`,
|
||||
);
|
||||
for (const { dir } of unreadable)
|
||||
console.error(` ${dir}/package-lock.json`);
|
||||
console.error(
|
||||
"\nThis guard reads the `packages` table of a lockfileVersion 2+ lockfile. Without it the install" +
|
||||
"\ncontributes no versions, so a real skew among the others would be reported as aligned — the gate" +
|
||||
"\nfailing open. Regenerate the lockfile with a current npm (`npm install`).",
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const installs = locks.map(({ dir, lock }) => ({
|
||||
dir,
|
||||
versions: topLevelLockVersions(lock),
|
||||
}));
|
||||
|
||||
const { failures, ignored } = partitionSkew(findSkew(candidates, installs));
|
||||
|
||||
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) {
|
||||
// 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);
|
||||
anyListed ||= listed;
|
||||
console.error(
|
||||
` ${name}${listed ? " (allowlisted — but this is a MAJOR skew)" : ""}`,
|
||||
);
|
||||
for (const { dir, version } of holders)
|
||||
console.error(` ${version} (${dir})`);
|
||||
}
|
||||
const shared = [...SHARED_SOURCE_DIRS, ...SHARED_SOURCE_FILES].join(", ");
|
||||
console.error(
|
||||
"\nThese packages' types are compiled into a single `tsc` program from two installs" +
|
||||
`\n(${shared} resolve from the root, a client's own sources from the client),` +
|
||||
"\nso a version skew makes TypeScript relate two structurally-distinct copies of the same" +
|
||||
"\ntype. For a recursive-generic surface like zod that is what exhausted the tsc heap in #1896.",
|
||||
);
|
||||
console.error(
|
||||
"\nAlign them — `npm install <pkg>@<version>` in each install that declares the package, so all" +
|
||||
"\nlockfiles agree. (Don't add it to an install that doesn't declare it: a package absent from an" +
|
||||
"\ninstall can't skew.) If instead its types genuinely cannot blow up, add it to TOLERATED_SKEW in" +
|
||||
"\nscripts/verify-dep-lockstep.mjs with the reason. See AGENTS.md.",
|
||||
);
|
||||
if (anyListed)
|
||||
console.error(
|
||||
"\nNote: an allowlisted package is tolerated only WITHIN a major version — the rationale for one" +
|
||||
"\nestablishes that a patch/minor difference is benign, not that a major split is. Align the major.",
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
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}.`,
|
||||
);
|
||||
}
|
||||
|
||||
// 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)
|
||||
main();
|
||||
491
scripts/verify-dep-lockstep.test.mjs
Normal file
491
scripts/verify-dep-lockstep.test.mjs
Normal file
|
|
@ -0,0 +1,491 @@
|
|||
// Table-driven tests for the pure helpers of the dep-lockstep guard (#1896).
|
||||
// One case per rule the guard encodes — the comment names the rule, so a future
|
||||
// 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).
|
||||
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
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.
|
||||
assert.deepEqual(
|
||||
sourcesWithNoFiles(
|
||||
["test-servers/src/b.ts", "vitest.shared.mts"],
|
||||
dirs,
|
||||
named,
|
||||
),
|
||||
["core"],
|
||||
);
|
||||
assert.deepEqual(
|
||||
sourcesWithNoFiles(["core/mcp/a.ts", "test-servers/src/b.ts"], dirs, named),
|
||||
["vitest.shared.mts"],
|
||||
);
|
||||
assert.deepEqual(sourcesWithNoFiles([], dirs, named), [
|
||||
"core",
|
||||
"test-servers/src",
|
||||
"vitest.shared.mts",
|
||||
]);
|
||||
});
|
||||
|
||||
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("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).
|
||||
const lock = {
|
||||
packages: {
|
||||
"": { name: "root" },
|
||||
"node_modules/zod": { version: "4.4.3" },
|
||||
"node_modules/yaml": { version: "2.9.0" },
|
||||
"node_modules/cosmiconfig/node_modules/yaml": { version: "1.10.3" },
|
||||
"node_modules/@modelcontextprotocol/client": { version: "2.0.0-beta.5" },
|
||||
"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"],
|
||||
]);
|
||||
});
|
||||
|
||||
test("topLevelLockVersions: 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);
|
||||
});
|
||||
|
||||
test("hasReadableLockShape: only a v2+ packages table with a root entry (Copilot, #1962)", () => {
|
||||
assert.equal(
|
||||
hasReadableLockShape({ lockfileVersion: 3, packages: { "": {} } }),
|
||||
true,
|
||||
);
|
||||
assert.equal(
|
||||
hasReadableLockShape({
|
||||
lockfileVersion: 2,
|
||||
packages: { "": {}, "node_modules/zod": { version: "4.4.3" } },
|
||||
}),
|
||||
true,
|
||||
);
|
||||
const rejected = [
|
||||
undefined,
|
||||
null,
|
||||
{},
|
||||
{ lockfileVersion: 3, packages: null },
|
||||
{ lockfileVersion: 3, packages: [] }, // an array has no `""` key
|
||||
{ lockfileVersion: 3, packages: {} }, // no root entry
|
||||
{
|
||||
lockfileVersion: 3,
|
||||
packages: { "node_modules/zod": { version: "4.4.3" } },
|
||||
},
|
||||
{ lockfileVersion: 1, dependencies: { zod: { version: "4.4.3" } } },
|
||||
// A declared v1 carrying a `packages` table: the version is checked, not
|
||||
// inferred from the key's presence, so this is rejected rather than
|
||||
// half-trusted into an empty (fail-open) version map.
|
||||
{ lockfileVersion: 1, packages: { "": {} } },
|
||||
{ packages: { "": {} } }, // no declared version at all
|
||||
{ lockfileVersion: "3", packages: { "": {} } }, // not a number
|
||||
];
|
||||
for (const lock of rejected)
|
||||
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" },
|
||||
],
|
||||
},
|
||||
{
|
||||
name: "zod",
|
||||
holders: [
|
||||
{ dir: ".", version: "4.3.6" },
|
||||
{ dir: "clients/web", version: "4.4.3" },
|
||||
],
|
||||
},
|
||||
];
|
||||
const tolerated = new Map([["react", "shallow interfaces"]]);
|
||||
const { failures, ignored } = partitionSkew(skewed, tolerated);
|
||||
assert.deepEqual(
|
||||
failures.map((s) => s.name),
|
||||
["zod"],
|
||||
);
|
||||
assert.deepEqual(
|
||||
ignored.map((s) => s.name),
|
||||
["react"],
|
||||
);
|
||||
});
|
||||
|
||||
test("partitionSkew: deny by default — nothing tolerated fails everything", () => {
|
||||
const skewed = [{ name: "zod", holders: [{ dir: ".", version: "1.0.0" }] }];
|
||||
assert.equal(partitionSkew(skewed, new Map()).failures.length, 1);
|
||||
});
|
||||
|
||||
test("partitionSkew: the allowlist tolerates skew only within a major (Copilot, #1962)", () => {
|
||||
// Each rationale establishes that a patch/minor difference is benign; that is
|
||||
// not evidence a React 18-vs-19 split is, so a listed package still fails
|
||||
// across a major boundary.
|
||||
const tolerated = new Map([["react", "shallow interfaces"]]);
|
||||
const withinMajor = [
|
||||
{
|
||||
name: "react",
|
||||
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" },
|
||||
],
|
||||
},
|
||||
];
|
||||
assert.equal(partitionSkew(withinMajor, tolerated).failures.length, 0);
|
||||
assert.equal(partitionSkew(withinMajor, tolerated).ignored.length, 1);
|
||||
assert.equal(partitionSkew(acrossMajor, tolerated).failures.length, 1);
|
||||
assert.equal(partitionSkew(acrossMajor, tolerated).ignored.length, 0);
|
||||
});
|
||||
|
||||
test("partitionSkew: an unparseable version can't be proven same-major, so it fails", () => {
|
||||
const tolerated = new Map([["react", "shallow interfaces"]]);
|
||||
const skewed = [
|
||||
{
|
||||
name: "react",
|
||||
holders: [
|
||||
{ dir: ".", version: "19.2.7" },
|
||||
{ dir: "clients/web", version: "next" },
|
||||
],
|
||||
},
|
||||
];
|
||||
assert.equal(partitionSkew(skewed, tolerated).failures.length, 1);
|
||||
});
|
||||
|
||||
test("majorOf: prerelease and build metadata are irrelevant", () => {
|
||||
const cases = [
|
||||
["4.4.3", "4"],
|
||||
["2.0.0-beta.5", "2"],
|
||||
["19.2.8", "19"],
|
||||
["1.10.3+build.7", "1"],
|
||||
];
|
||||
for (const [input, expected] of cases)
|
||||
assert.equal(majorOf(input), expected, input);
|
||||
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);
|
||||
});
|
||||
97
scripts/verify-format-coverage.main.test.mjs
Normal file
97
scripts/verify-format-coverage.main.test.mjs
Normal file
|
|
@ -0,0 +1,97 @@
|
|||
// Regression tests for `verify-format-coverage`'s sibling-guard vouch (Copilot,
|
||||
// #1962). The three root guards form a cycle so that dropping any one from
|
||||
// `validate` is caught by another — but the vouch branch itself had no test, so
|
||||
// a typo in a sibling's name would leave `test:scripts` green while that guard
|
||||
// silently stopped being enforced. That is the same "a gate that stops gating"
|
||||
// failure the cycle exists to prevent, one level up.
|
||||
//
|
||||
// The vouch runs before any file enumeration, so the fixture needs only a
|
||||
// `package.json` and the script. Run via `npm run test:scripts`.
|
||||
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { execFileSync, spawnSync } from "node:child_process";
|
||||
import {
|
||||
cpSync,
|
||||
mkdirSync,
|
||||
mkdtempSync,
|
||||
realpathSync,
|
||||
rmSync,
|
||||
writeFileSync,
|
||||
} from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
const scriptsDir = path.dirname(fileURLToPath(import.meta.url));
|
||||
|
||||
/** A `validate` chain running each named guard, plus the always-present self. */
|
||||
function scriptsRunning(guards) {
|
||||
const scripts = {
|
||||
validate: ["verify:format-coverage", ...guards]
|
||||
.map((g) => `npm run ${g}`)
|
||||
.join(" && "),
|
||||
"verify:format-coverage": "node scripts/verify-format-coverage.mjs",
|
||||
};
|
||||
for (const g of guards) scripts[g] = `node scripts/${g.slice(7)}.mjs`;
|
||||
return scripts;
|
||||
}
|
||||
|
||||
/**
|
||||
* Run `verify-format-coverage` in a throwaway repo whose root `validate` runs
|
||||
* exactly `guards`. realpath'd because the script only executes when
|
||||
* `import.meta.url` matches `process.argv[1]`, and macOS `tmpdir()` is a
|
||||
* symlink — see the note in `verify-dep-lockstep.main.test.mjs`.
|
||||
*/
|
||||
function runWithGuards(guards) {
|
||||
const dir = realpathSync(mkdtempSync(path.join(tmpdir(), "format-cov-")));
|
||||
try {
|
||||
writeFileSync(
|
||||
path.join(dir, "package.json"),
|
||||
JSON.stringify(
|
||||
{ name: "fixture", scripts: scriptsRunning(guards) },
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
mkdirSync(path.join(dir, "scripts", "lib"), { recursive: true });
|
||||
for (const rel of [
|
||||
"verify-format-coverage.mjs",
|
||||
path.join("lib", "npm-scripts.mjs"),
|
||||
])
|
||||
cpSync(path.join(scriptsDir, rel), path.join(dir, "scripts", rel));
|
||||
execFileSync("git", ["init", "-q"], { cwd: dir });
|
||||
execFileSync("git", ["add", "-A"], { cwd: dir });
|
||||
|
||||
const r = spawnSync(
|
||||
process.execPath,
|
||||
[path.join(dir, "scripts", "verify-format-coverage.mjs")],
|
||||
{ cwd: dir, encoding: "utf8" },
|
||||
);
|
||||
return { status: r.status, out: `${r.stdout}${r.stderr}` };
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
const BOTH_SIBLINGS = ["verify:typecheck-coverage", "verify:dep-lockstep"];
|
||||
|
||||
test("vouch: fails when `verify:dep-lockstep` is dropped from validate", () => {
|
||||
const { status, out } = runWithGuards(["verify:typecheck-coverage"]);
|
||||
assert.equal(status, 1, out);
|
||||
assert.match(out, /no longer runs `verify:dep-lockstep`/);
|
||||
});
|
||||
|
||||
test("vouch: fails when `verify:typecheck-coverage` is dropped from validate", () => {
|
||||
const { status, out } = runWithGuards(["verify:dep-lockstep"]);
|
||||
assert.equal(status, 1, out);
|
||||
assert.match(out, /no longer runs `verify:typecheck-coverage`/);
|
||||
});
|
||||
|
||||
test("vouch: passes when both siblings are still wired", () => {
|
||||
// The run still fails afterwards — the fixture has no client manifests to
|
||||
// harvest globs from — so assert on the *reason*, not the exit status: no
|
||||
// sibling may be reported missing.
|
||||
const { out } = runWithGuards(BOTH_SIBLINGS);
|
||||
assert.doesNotMatch(out, /no longer runs/);
|
||||
});
|
||||
|
|
@ -166,15 +166,18 @@ function trackedSourceFiles() {
|
|||
return out.split("\n").filter(Boolean);
|
||||
}
|
||||
|
||||
// Vouch for the sibling guard: a guard can't detect being unrun itself, but the
|
||||
// two coverage guards can each assert the other is still wired into `validate`,
|
||||
// so dropping either is caught here. Only deleting both slips through.
|
||||
// Vouch for the sibling guards: a guard can't detect being unrun itself, so the
|
||||
// three form a cycle instead. This one checks BOTH others; each of them checks
|
||||
// only this one. So dropping `verify:typecheck-coverage` or
|
||||
// `verify:dep-lockstep` is caught here, and dropping *this* guard is caught by
|
||||
// either of them. Only removing all three at once slips through.
|
||||
const rootScripts = JSON.parse(
|
||||
readFileSync(path.join(repoRoot, "package.json"), "utf8"),
|
||||
).scripts;
|
||||
if (!rootReachesScript(rootScripts, "verify:typecheck-coverage")) {
|
||||
for (const sibling of ["verify:typecheck-coverage", "verify:dep-lockstep"]) {
|
||||
if (rootReachesScript(rootScripts, sibling)) continue;
|
||||
console.error(
|
||||
"verify:format-coverage — the root `validate` no longer runs `verify:typecheck-coverage` (its sibling guard). Restore it.",
|
||||
`verify:format-coverage — the root \`validate\` no longer runs \`${sibling}\` (its sibling guard). Restore it.`,
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue