diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 0bc70bfca..321222058 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -100,7 +100,7 @@ Both exist and do different jobs. Theme files (`src/theme/.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. +- **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 — it derives its candidates from what each `tsc` program actually resolves (`tsc --listFilesOnly`, keeping packages that reach one program from two installs), so a package reached only through another package's `.d.ts` counts too (#1965) — so a PR bumping a package the shared sources pull in 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. - **A test or smoke must not touch real user state.** The web smokes run against a throwaway catalog via the shared `scripts/lib/prod-web-server.mjs` helper, never the developer's `~/.mcp-inspector/mcp.json` (#1977); the cli/tui smokes drive a temp `--catalog`. A new smoke spawning its own server, or teardown that removes a work dir without first awaiting `stopChild` (the #1801 race — `child-cleanup.mjs` exports both halves and both are required), should be flagged. - **Every PR references an issue**, first body line `Closes #`. - **Every PR carries exactly one version label**, `v1` or `v2`, matching its base branch. diff --git a/AGENTS.md b/AGENTS.md index de3945ebd..d07e84f4d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -118,8 +118,11 @@ v2/main/ ├── scripts/ # Root build/verify tooling: install-clients.mjs (the │ # postinstall cascade), the smoke-*.mjs runners, │ # verify-build-gate / verify-format-coverage / -│ # verify-typecheck-coverage, pack-and-verify.mjs, -│ # and lib/ shared helpers. Prettier-gated via +│ # verify-typecheck-coverage / verify-dep-lockstep, +│ # pack-and-verify.mjs, and lib/ shared helpers +│ # (tsc-program.mjs is the `tsc --listFilesOnly` +│ # measurement both coverage guards read a program +│ # through — #1965). Prettier-gated via │ # `format:check:scripts`; its own pure parsers are │ # unit-tested by `npm run test:scripts` (node --test). ├── specification/ # Build specification @@ -686,12 +689,12 @@ The ⚠️ option-deletion hazard, the snapshot rule, and the recovery recipe ab - ALWAYS do `npm run format` before committing — the **root** `format` auto-fixes `core/` (`format:core`), the root `scripts/` tooling (`format:scripts`), the root "shared" surface (`format:shared` — `test-servers/src/**`, `vitest.shared.mts`, the root `eslint.config.js`), and every client's scope in one shot. Every **client** format glob uses the uniform extension set `*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` (#1792) so a new-extension file can't slip the gate; `core/` stays `{ts,tsx}` and the shared surface `{ts,tsx,mts,cts}` (their surfaces can't hold the other extensions), and `npm run verify:format-coverage` (the first step of `validate`, #1792) is the backstop — it fails if any tracked source file is left uncovered by a `format:check` glob regardless of which glob was expected to catch it. `validate` runs `format:check` (the non-fixing variant, including `format:check:core`, `format:check:scripts`, and `format:check:shared`) and will fail in CI on any unformatted file, so always run the auto-fixer first rather than letting `format:check` catch it. - **`npm run ci` is the mandatory pre-push command** — it mirrors `.github/workflows/main.yml` (minus `npm install`): `validate` → `coverage` → `verify:build-gate` (the #1769 browser-externalized-builtin build gate) → `smoke` → Storybook play-function tests (installs Playwright chromium if needed). It now runs **`npm run coverage`**, the per-file ≥90 gate (lines/statements/functions/branches) that CI enforces — so `npm run ci` is a true superset of GitHub CI, and passing it locally means CI's gates will pass. Expect several minutes. **`npm run validate`** remains the fast inner-loop check during development (unit tests only — no coverage gate, no smoke, no Storybook), but it is **NOT** an acceptable substitute for `npm run ci` before pushing: `validate` runs `test`, not `test:coverage`, so it does **zero** coverage gating. Skipping the gate is how a push passes every fast local check and still fails CI (this exact gap broke PR #1601 on a function-coverage regression). -- ALWAYS do `npm run format` before committing, then **`npm run ci`** before pushing. From the repo root, `validate` runs **`verify:format-coverage` first** (the #1792 guard — asserts every tracked source file is covered by a `format:check` glob), then **`verify:typecheck-coverage`** (the #1791 guard — asserts every tracked `.ts`/`.tsx`/`.mts`/`.cts` in each gated Node client, plus the non-client first-party TS like `core/` and `test-servers/src`, lands in a tsconfig project), then **`verify:dep-lockstep`** (the #1896 guard — asserts no dependency *directly imported* by the shared sources resolves to two different versions across installs; transitive-only declarations are a known boundary, #1965), then **`test:scripts`** (the guards' own parser unit tests, `node --test`), then the **`core/` gate** (`validate:core`), then chains the four per-client validations (`validate:web` → `validate:cli` → `validate:tui` → `validate:launcher`); each client delegates to its own `npm run validate` in its own folder (no coverage — fast). Every client is self-validating and the top level just chains them, building each client's bundle along the way (no cross-client build dependencies). +- ALWAYS do `npm run format` before committing, then **`npm run ci`** before pushing. From the repo root, `validate` runs **`verify:format-coverage` first** (the #1792 guard — asserts every tracked source file is covered by a `format:check` glob), then **`verify:typecheck-coverage`** (the #1791 guard — asserts every tracked `.ts`/`.tsx`/`.mts`/`.cts` in each gated Node client, plus the non-client first-party TS like `core/` and `test-servers/src`, lands in a tsconfig project), then **`verify:dep-lockstep`** (the #1896 guard — asserts no dependency that reaches a single `tsc` program from two installs resolves to two different versions across them), then **`test:scripts`** (the guards' own parser unit tests, `node --test`), then the **`core/` gate** (`validate:core`), then chains the four per-client validations (`validate:web` → `validate:cli` → `validate:tui` → `validate:launcher`); each client delegates to its own `npm run validate` in its own folder (no coverage — fast). Every client is self-validating and the top level just chains them, building each client's bundle along the way (no cross-client build dependencies). - **`validate:core` is the root-owned format + lint gate (#1689, widened in #1778 and #1767).** Each client's `prettier`/`eslint` is scoped to its own dir, so nothing reached `core/`, the root `scripts/`, or the root "shared" surface before — `validate:core` closes that: it runs `format:check:core` (`prettier --check "core/**/*.{ts,tsx}"`) + `format:check:scripts` (`prettier --check "scripts/**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}"`, the root build/verify tooling — #1778) + `format:check:shared` + `lint:core` (`eslint "core/**/*.{ts,tsx}"` via the **root** `eslint.config.js`) + `lint:shared`. Use `npm run format:core` / `npm run format:scripts` / `npm run format:shared` to auto-fix (all folded into the root `format`). The **shared surface** (#1767) is `test-servers/src/**/*.{ts,tsx,mts,cts}`, the root `vitest.shared.mts`, and the root `eslint.config.js` — first-party code no client's `eslint .` / `prettier` reaches; it is both prettier-gated (`format:check:shared`) and eslint-gated (`lint:shared`, via a second `files` block in the root `eslint.config.js` scoped to Node globals). The `scripts/` gate is prettier-only — the root has no eslint config for `.mjs`. The root carries prettier/eslint as devDependencies for this; `core/` is isomorphic (browser + Node globals, no JSX today — the `{ts,tsx}` glob future-proofs against a `core/**/*.tsx`). The root `eslint.config.js` honors an `_`-prefix as the intentionally-unused marker (`argsIgnorePattern`/`varsIgnorePattern`/`caughtErrorsIgnorePattern: '^_'`). **prettier is pinned to an exact version** (not a caret) in all five `package.json`s (#1790) so the gate's verdict can't shift with an in-range patch bump. - **cli and tui now typecheck their `src` (#1689).** Their `build`/`test` run through esbuild (no type check), so each has a `typecheck` script folded into `validate`. Their `tsconfig.json` matches `clients/web/tsconfig.app.json`'s module/lib _resolution_ options — DOM lib, `moduleResolution: bundler`, and **no** `noUncheckedIndexedAccess` (web's app config does not extend `tsconfig.base`, so re-enabling it would surface `core/` issues web never gates) — so the imported `core/` sources are validated the same way web validates them. It does **not** mirror web's extra strictness flags (`noUnusedLocals`, `verbatimModuleSyntax`, ES2023 target, …), so cli/tui's own `src` is checked slightly more loosely than web's. `core/` itself still typechecks through web's `tsc -b`. - **The `__tests__` dirs are typechecked too (#1791).** The src-only `tsconfig.json` excludes `**/*.test.*`, so each of cli, tui, and launcher carries a **`tsconfig.test.json`** — extending the build config, `noEmit`, including `__tests__/**/*` (only the tests root the project; tsc pulls in the `src` they import, and the src-only config already validates all of `src` without the test-only aliases) and adding the test-only path aliases that resolve what vitest resolves via `vitest.shared.mts`. The alias set differs per client: **cli's is the widest** (`@modelcontextprotocol/inspector-test-server` → `test-servers/src`, the `@inspector/core/*` deep paths, express/vitest — cli is the only one importing the test-server package); **tui's** carries only the `@inspector/core/*` + react/vitest redirects; **launcher's** has **no** `paths` at all — it's a plain `rootDir: "."` sibling of the build config (whose `rootDir: ./src` is what rejects the tests). Each client's `typecheck` script runs **both** projects (`tsc -p tsconfig.json && tsc -p tsconfig.test.json`) so running it standalone means the same thing everywhere (launcher's `build` also `tsc`s `src`, but `typecheck` doesn't rely on that). cli additionally carries `@types/express` (devDep) so the transitively-aliased test-server source typechecks, mirroring `clients/web` (cli's `tsconfig.test.json` also names `test-servers/src/server-composable.ts` explicitly — a bin entry the barrel doesn't import, so nothing else gives it a tsc pass). The client **config files** are typechecked too: cli's/tui's (`vitest.config.ts`, `tsup.config.ts`, tui `dev.ts`) are folded into each src `tsconfig.json`'s `include`; launcher's `vitest.config.ts` goes in its `tsconfig.test.json` instead (again the `rootDir: ./src` reason). Note the gate checks mock **implementations and return types** (typing a `vi.fn()` against a real signature keeps its `mockResolvedValue`/impl in sync) but **not** `toHaveBeenCalledWith(...)` arguments — vitest types those to accept anything regardless of the mock's type parameter. **`npm run verify:typecheck-coverage`** (`scripts/verify-typecheck-coverage.mjs`, run as the second step of `validate` right after `verify:format-coverage`) is the durable guard for this invariant: it runs each client's `typecheck` projects with `tsc --listFilesOnly`, unions them, and fails on any tracked `.ts`/`.tsx`/`.mts`/`.cts` that lands in no project — for every gated Node client, which it discovers from disk (each `clients/*` is enrolled through its `typecheck` script's projects, or — for a `tsc -b` client like `clients/web` with no `typecheck` script — through its `tsconfig.json` `references`), so a new client is covered without editing the guard — the typecheck analog of `verify:format-coverage`, since a project only reaches the files its `include` names plus their transitive imports, so a new top-level file (launcher especially, whose build `rootDir: ./src` rejects package-root files) can otherwise fall out silently. Like its sibling it also asserts the gate is _wired_ (each client's typecheck pass is reachable from its `validate` — its `typecheck` script for cli/tui/launcher, or a real `tsc -b` for web — and the root chain runs each client's `validate`), so it can't stay green while measuring a pass nothing invokes. It asserts the same of **`test:scripts`** — its own parser tests — on three axes: reachable from the root `validate`, a **non-empty** tracked `scripts/**/*.{test,spec}.*` set, and **every one of those files matched by a glob harvested across the scripts reachable from `test:scripts`** (so a delegating `test:scripts` still measures correctly). The third axis exists because `node --test` silently _skips_ a file its glob misses and still exits 0 — a rename to `*.spec.mjs` would shrink the suite with a green run. Beyond the clients it also covers, **deny-by-default**, the first-party TS no client owns — everything tracked outside `clients/*` (`test-servers/src/**`, the root `vitest.shared.mts`, **all of `core/`**, and any new top-level TS location) must land in the _global_ union of client projects (cli aliases the test-server source; web's enrolled projects include `core/`). So a `core` `*.tsx` web's `include` doesn't reach, or an unimported `test-servers/src` bin entry, can't ship uncompiled-but-unchecked. The one "listed but unchecked" tier the guard structurally can't see — a per-file `// @ts-nocheck` — is owned by a different gate: `@typescript-eslint/ban-ts-comment` rejects it across every surface (`lint:core`, `lint:shared`, and each client's `eslint .`). The guard's own pure parsers (`scripts/lib/npm-scripts.mjs` + the exported helpers of `verify-typecheck-coverage.mjs`, whose execution is behind a `main()` so importing it for tests doesn't run it) are **unit-tested** — `npm run test:scripts` (node's built-in `node --test`, in `validate`; the root has no vitest harness by design) runs table-driven cases, one per rule the guard's parsers encode, and the guard itself enforces that this stays wired (above). - The one CLI nuance: `clients/cli`'s out-of-process `e2e.test.ts` spawns the built binary, so its `test` **builds first** via `pretest` (`test-servers:build && build`). To avoid building it twice, `clients/cli`'s `validate` folds that in — it is `format:check && lint && typecheck && test` with **no** separate `build` step (the other clients, whose tests don't spawn their bundle, keep an explicit `build`). `validate:web`/`validate:tui`/`validate:launcher` are the uniform `format:check && lint && (typecheck &&) build && test`. (#1778, #1789, #1792) `clients/web`'s `format`/`format:check` covers `src`, `server`, `.storybook`, and its top-level configs (the uniform `*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` glob — `vite.config.ts`, `tsup.runner.config.ts`, `eslint.config.js`, …), not just `src`, so the Node backend, Storybook config, and Vite/build config are prettier-gated too; `clients/launcher`'s covers `src`, `__tests__`, `scripts`, and its top-level configs (the `*.` top-level glob is non-recursive, so each nested dir — `.storybook`, `scripts` — is named explicitly). The `verify:format-coverage` guard (#1792) enforces that this coverage stays complete. - - **One version per install-crossing dependency (#1896).** Because v2 is not a workspace, the root and each `clients/*` carry their own `node_modules` — and a client's `tsconfig.test.json` compiles first-party sources that live *outside* the client (`test-servers/src`, `core/`), which resolve their dependencies from the **root** install while the client's own sources resolve from the client install. So the same package can appear **twice in one `tsc` program**. At the same version that duplication is harmless; on a skew, TypeScript must relate two structurally-distinct declarations of the same type. For a deeply recursive-generic surface that is exponential: zod `4.3.6` (root) against zod `4.4.3` (`clients/web`) made `clients/web`'s `tsc -b` exhaust the 4GB default heap outright via `TS2589 Type instantiation is excessively deep`, because every `@modelcontextprotocol/*` schema is built out of zod generics. **Raising the heap with `--max-old-space-size` hides this class rather than fixing it — align the versions instead.** `npm run verify:dep-lockstep` (`scripts/verify-dep-lockstep.mjs`, in `validate`) is the durable guard: it **derives** the candidate set from the packages the shared first-party TypeScript imports — `core/`, `test-servers/src`, and the individually-named `vitest.shared.mts` (root-owned and imported by every client's vitest config) — so a new shared dependency is covered without editing the guard. It reads the committed lockfiles' **top-level** `node_modules/` entries — a *nested* transitive duplicate inside one install is routine and deliberately ignored — and fails, **deny-by-default**, on any candidate held at two versions across installs. The escape hatch is `TOLERATED_SKEW` in that file, an allowlist of *names* (not version pairs, so an ordinary patch float doesn't churn it), each entry carrying why that package's types can't blow up; `react`, `hono`, `jose`, and `@modelcontextprotocol/ext-apps` are listed today. **Being listed is not a blanket exemption** — it tolerates skew only *within a major version*, since a rationale about patch-level differences says nothing about a React 18-vs-19 split, where the type surface itself changes; a cross-major skew fails even for a listed package. **When bumping a dependency that the shared sources import, bump it in every install that declares it** — that's the root plus whichever clients list it, not all four unconditionally (launcher declares no zod, for instance, and a package absent from an install can't skew, so the guard ignores it there). Don't add a dependency to a client just to satisfy this. Its pure helpers are unit-tested via `test:scripts`, and it vouches — with `verify:format-coverage` and `verify:typecheck-coverage` — that its siblings are still wired into `validate`. + - **One version per install-crossing dependency (#1896).** Because v2 is not a workspace, the root and each `clients/*` carry their own `node_modules` — and a client's `tsconfig.test.json` compiles first-party sources that live *outside* the client (`test-servers/src`, `core/`), which resolve their dependencies from the **root** install while the client's own sources resolve from the client install. So the same package can appear **twice in one `tsc` program**. At the same version that duplication is harmless; on a skew, TypeScript must relate two structurally-distinct declarations of the same type. For a deeply recursive-generic surface that is exponential: zod `4.3.6` (root) against zod `4.4.3` (`clients/web`) made `clients/web`'s `tsc -b` exhaust the 4GB default heap outright via `TS2589 Type instantiation is excessively deep`, because every `@modelcontextprotocol/*` schema is built out of zod generics. **Raising the heap with `--max-old-space-size` hides this class rather than fixing it — align the versions instead.** `npm run verify:dep-lockstep` (`scripts/verify-dep-lockstep.mjs`, in `validate`) is the durable guard: it **derives** the candidate set from **what actually enters each `tsc` program** (#1965) — every client tsconfig project is listed with `tsc --listFilesOnly` through the shared `scripts/lib/tsc-program.mjs` helper (the same machinery `verify:typecheck-coverage` reads a program through, so the two guards can't disagree about what one contains), each resolved `node_modules` file is mapped to its owning install and package, and a package reaching **one** program from **two** installs is a candidate. That is precisely the set that can put two structurally-distinct copies of a type in front of one checker — and it needs no editing when a new dependency arrives. It replaced a derivation that read the packages the shared sources named *directly*, which could not see one whose declarations arrive only through another package's `.d.ts`: `@modelcontextprotocol/sdk` is never written in first-party code (the shared sources import the split `@modelcontextprotocol/client|core|…`) yet 16 of its `.d.ts` files land in `clients/web`'s test program, so a second copy under `clients/web/node_modules` skewed unseen. Two properties are worth knowing: a package present in two installs but reached from only one in a given program is correctly **not** a candidate, and TypeScript's package-identity redirect collapses two copies at the *same* name@version (so an aligned package's own transitive dependencies load once) — the redirect stops applying the moment they skew, which is exactly when the guard needs to see both. Versions still come from the committed lockfiles, but from the entry for the **exact install path the program resolved** (`node_modules/zod`, `node_modules/a/node_modules/zod`) rather than from the install's top-level entry: a *nested* duplicate inside one install is still not a candidate on its own — folding it onto its outermost install is what keeps the set small — but once a program has loaded one, pricing it from a top-level entry that may be absent or differently versioned would let a real pair pass (Copilot). Only the installs that actually **met in one program** are compared, so a third install's copy that no program loads beside another is not evidence of anything. Any co-occurrence whose copies disagree fails, **deny-by-default**; a resolved copy with no lockfile entry fails too, since the tree and the lockfile then disagree about what was loaded. The escape hatch is `TOLERATED_SKEW` in that file, an allowlist of *names* (not version pairs, so an ordinary patch float doesn't churn it), each entry carrying why that package's types can't blow up; it is **empty today** — the four names it used to carry (`react`, `hono`, `jose`, `@modelcontextprotocol/ext-apps`) were admitted under the old derivation and none is a candidate under this one, so each would be a rationale for a skew that cannot occur. **Being listed is not a blanket exemption** — it tolerates skew only *within a major version*, since a rationale about patch-level differences says nothing about a React 18-vs-19 split, where the type surface itself changes; a cross-major skew fails even for a listed package. **When bumping a dependency that the shared sources pull in, bump it in every install that declares it** — that's the root plus whichever clients list it, not all four unconditionally (launcher declares no zod, for instance, and a package absent from an install can't skew, so the guard ignores it there). Don't add a dependency to a client just to satisfy this. Its pure helpers are unit-tested via `test:scripts` (its own, plus `scripts/lib/tsc-program.test.mjs` for the shared derivation), and it vouches — with `verify:format-coverage` and `verify:typecheck-coverage` — that its siblings are still wired into `validate`. It runs the same `tsc --listFilesOnly` pass its sibling does, in its own process, costing ~14s: the listing is deliberately **not** cached to disk between the two, because a fingerprint that missed an input would make a guard measure a program that no longer exists and pass on a real miss. - **`npm run coverage`** is the per-file ≥90 gate and is now part of `npm run ci` — never treat it as optional before a push. It supersedes the old standalone `test:integration` step: web's `test:coverage` runs the `unit` **and** `integration` projects under v8 instrumentation, so `coverage` both enforces the ≥90 gate and exercises the same web integration paths CI covers. - **`smoke` is NOT part of `validate`** — it is included in `npm run ci`. It runs `smoke:launcher` (`--help` dispatch) plus the prod `smoke:cli` / `smoke:tui` / `smoke:web` / `smoke:web:browser` / `smoke:web:app`, and contains **no build commands** — it assumes the cli/tui/launcher bundles already exist (a full `validate` builds them; `smoke:web` builds `clients/web/dist` on demand). CI runs `validate`, then the `coverage` gate (which also covers the web integration project), then `verify:build-gate` (the #1769 build gate — see below), then `smoke` (with Playwright chromium installed just before it, since `smoke:web:browser` needs it). GitHub CI runs this same chain as separate workflow steps, with the Storybook play-function tests last (see below). - `smoke:launcher` (`scripts/smoke-launcher.mjs`) runs the built launcher with `--help`, `--cli --help`, and `--tui --help`, asserting each exits 0 and prints that mode's usage banner (which also proves the launcher resolved and loaded the right client build). It's the cheap dispatch check before the heavier prod smokes below. diff --git a/README.md b/README.md index 467a4fc0a..cb57dd359 100644 --- a/README.md +++ b/README.md @@ -294,14 +294,14 @@ Each client self-validates from its own folder; the root scripts chain them. The | Script | What it does | | ----------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `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 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 reaching one `tsc` program from two installs skews across them) — 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` 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 test:scripts` | Table-driven unit tests (`node --test`) for the guard's own pure parsers (`scripts/lib/npm-scripts.mjs`, `scripts/lib/tsc-program.mjs` + the exported helpers of `verify-typecheck-coverage.mjs` and `verify-dep-lockstep.mjs`), one case per rule they encode. Runs in `validate` — and `verify:typecheck-coverage` guards *this* gate in turn (reachable from `validate`, non-empty test set, every test file matched by the `test:scripts` glob), since `node --test` silently skips a file its glob misses and still exits 0. | | `npm run verify:typecheck-coverage` | The typecheck-coverage analog of the above (#1791): for each Node client (auto-discovered from disk — enrolled via its `typecheck` script's projects, or for a `tsc -b` client like `clients/web` via its `tsconfig.json` `references`) it runs those projects with `tsc --listFilesOnly`, unions them, and **fails** listing any tracked `.ts`/`.tsx`/`.mts`/`.cts` under the client that lands in no project (so a new top-level config/helper can't silently go untypechecked). It also requires, deny-by-default, the first-party TS no client owns (`test-servers/src`, the root `vitest.shared.mts`, all of `core/`, and any new top-level location) to land in some client project's tsc pass — so a `core` `*.tsx` web's projects don't reach is caught too. Also asserts the gate is wired (each client's typecheck pass — its `typecheck` script, or web's `tsc -b` — is reachable from its `validate`, and the root chain runs each client's `validate`). Runs in `validate`. | -| `npm run verify:dep-lockstep` | Guards the "one version per install-crossing dependency" invariant (#1896). v2 is not a workspace, so a client's test project compiles the shared first-party TypeScript — `core/`, `test-servers/src`, and the root-owned `vitest.shared.mts`, all of which resolve their dependencies from the **root** install — alongside the client's own sources, putting the same package in one `tsc` program twice. At the same version that's harmless; skewed, TypeScript must relate two structurally-distinct copies of every type, which for a recursive-generic surface is exponential (zod `4.3.6` vs `4.4.3` exhausted the 4GB tsc heap in `clients/web`). Derives its candidate set from what 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 verify:dep-lockstep` | Guards the "one version per install-crossing dependency" invariant (#1896). v2 is not a workspace, so a client's test project compiles the shared first-party TypeScript — `core/`, `test-servers/src`, and the root-owned `vitest.shared.mts`, all of which resolve their dependencies from the **root** install — alongside the client's own sources, putting the same package in one `tsc` program twice. At the same version that's harmless; skewed, TypeScript must relate two structurally-distinct copies of every type, which for a recursive-generic surface is exponential (zod `4.3.6` vs `4.4.3` exhausted the 4GB tsc heap in `clients/web`). Derives its candidate set from **what actually enters each program** (#1965) — every client tsconfig project listed with `tsc --listFilesOnly` via the shared `scripts/lib/tsc-program.mjs`, each resolved `node_modules` file mapped to its owning install, keeping the packages that reach one program from two installs (a package whose declarations arrive only through another package's `.d.ts`, as `@modelcontextprotocol/sdk`'s do, is invisible to a scan of first-party imports). Prices each copy from the lockfile entry for the exact install path the program resolved, compares only the installs that met in one program, and **fails deny-by-default** on any disagreement not in the annotated `TOLERATED_SKEW` allowlist — empty today — with an allowlisted package tolerated only *within a major version*. Runs in `validate`. | `npm run ci` | **Mandatory pre-push command.** `validate` → `coverage` → `verify:build-gate` → `smoke` → Storybook. A true superset of GitHub CI. | | `npm run pack:verify` | Publish smoke — see [Publishing](#publishing). | diff --git a/scripts/lib/tsc-program.mjs b/scripts/lib/tsc-program.mjs new file mode 100644 index 000000000..98a678e66 --- /dev/null +++ b/scripts/lib/tsc-program.mjs @@ -0,0 +1,396 @@ +// What a `tsc` project actually pulls into its program — the shared measurement +// behind two guards, so the two can never disagree about what a program holds. +// +// • `verify:typecheck-coverage` (#1791) asks which FIRST-PARTY files land in a +// program, to prove every tracked source file gets a `tsc` pass. +// • `verify:dep-lockstep` (#1965) asks which INSTALLED PACKAGES land in one, to +// find the packages that can appear twice — from two different installs — in +// a single program, which is the whole failure mode #1896 was about. +// +// Both answers come from the same `tsc --listFilesOnly` run: it reports every +// file the program resolves, including the ones reached only through another +// file's imports and the `.d.ts` files under `node_modules`. That is the accurate +// measure — a tsconfig's `include` names only the roots. +// +// The listing is memoized per (client, project) for the life of the process. It +// is deliberately NOT cached to disk across the two guard processes: a stale +// cache would make either guard measure a program that no longer exists and pass +// on a real miss — failing open, the one way these gates must never fail. The +// second pass costs ~14s (measured, #1965); that is cheaper than the class of +// bug a fingerprint that misses one input would introduce. + +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { reachableScripts, tokenize } from "./npm-scripts.mjs"; + +export const repoRoot = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "..", + "..", +); + +// Match `tsc` by token basename so a path-invoked binary (`node_modules/.bin/ +// tsc`, `./node_modules/.bin/tsc.cmd`) counts, not just the bare `tsc` token. +export const isTsc = (t) => /(?:^|[\\/])tsc(?:\.(?:cmd|exe|ps1))?$/.test(t); + +// A flag that makes a `tsc` pass list files without type-checking them (so it +// gates nothing). Case-insensitive — tsc's own option parsing is. +export const isDisablingFlag = (t) => /^--(noCheck|listFilesOnly)$/i.test(t); + +/** + * The tsconfig projects a client's `typecheck` names, harvested from **every** + * script reachable from `typecheck` (not just the one string) so a delegating + * `typecheck` (`npm run typecheck:src && …`) still counts — matching how + * `verify-format-coverage.mjs` harvests globs across reachable scripts. Splits + * each script on `&&`/`||`/`;` so a flag on one command doesn't leak onto + * another. Each `tsc` command's project comes from `-p`/`--project` (or a + * `-b`/`--build` path); a `tsc` command with **no** project flag resolves the + * implicit `./tsconfig.json` (tsc's own default), so that idiomatic form counts + * too. Returns `{ projects, neutered }`: `neutered` names any project whose own + * command carries `--noCheck`/`--nocheck` or `--listFilesOnly` (matched + * case-insensitively — tsc's option parsing is) — a pass that lists files + * without type-checking them, which would otherwise satisfy the typecheck guard + * while checking nothing. + * + * A harvested `tsc -b` **solution config** (`"files": []` + `references`) lists + * nothing itself; {@link resolveLeafProjects} expands it to its references. + * + * Minor limitations, all unreachable with the plain `-p --noEmit` passes here: + * the implicit-`./tsconfig.json` fallback assumes **no file operands** (`tsc + * ` ignores the config and checks only that file, but would be credited + * the whole config's file list); the `--noCheck`/`--listFilesOnly` detection + * ignores a following boolean, so the contrived explicit `--noCheck false` + * (checking *on*) is still treated as disabling; and the `&&`/`||`/`;` split + * runs before tokenizing, so a quoted operator inside an arg would split + * mid-token (project paths carry none of those). + */ +export function typecheckProjects(scripts) { + const projects = []; + const neutered = []; + const isFlag = (t) => t.startsWith("-"); + const isProjectFlag = (t) => ["-p", "--project", "-b", "--build"].includes(t); + for (const name of reachableScripts(scripts, "typecheck")) { + const cmd = scripts?.[name]; + if (typeof cmd !== "string") continue; + for (const segment of cmd.split(/&&|\|\||;/)) { + const tokens = tokenize(segment); + if (!tokens.some(isTsc)) continue; // only tsc commands name projects + const disabling = tokens.find(isDisablingFlag); + // A project path follows `-p`/`--project`/`-b`/`--build`; a tsc command + // with none uses the implicit `./tsconfig.json` (tsc's own default). + const named = []; + for (let i = 0; i < tokens.length; i++) + if (isProjectFlag(tokens[i]) && tokens[i + 1] && !isFlag(tokens[i + 1])) + named.push(tokens[i + 1]); + if (named.length === 0) named.push("tsconfig.json"); + for (const project of named) { + if (disabling) neutered.push({ project, flag: disabling }); + else projects.push(project); + } + } + } + return { projects, neutered }; +} + +/** + * The `references` paths declared in a tsconfig's raw text (a `tsc -b` solution + * config), or `[]` if it has none / isn't parseable. Paths are as written + * (relative to that tsconfig's own directory). + */ +export function parseTsconfigReferences(raw) { + try { + // Tolerate JSONC — block AND line comments + trailing commas (tsconfig + // allows all; block comments are in fact the style of every other tsconfig + // here). Block comments are stripped first so a `//` inside one doesn't + // survive; a `//` inside a string value (e.g. an `https://` URL) is a + // theoretical false strip this guard's tsconfigs never hit. + const cfg = JSON.parse( + raw + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/.*$/gm, "") + .replace(/,(\s*[}\]])/g, "$1"), + ); + return Array.isArray(cfg.references) + ? cfg.references.map((r) => r?.path).filter((p) => typeof p === "string") + : []; + } catch { + return []; + } +} + +export function tsconfigReferences(tsconfigRel) { + try { + return parseTsconfigReferences( + readFileSync(path.join(repoRoot, tsconfigRel), "utf8"), + ); + } catch { + return []; // unreadable file (e.g. a directory / missing path) + } +} + +/** + * The `references` in a client's root `tsconfig.json`. Non-empty for a `tsc -b` + * client (like `clients/web`, which has no `typecheck` script) — the guards + * enroll such a client through these instead of exempting the whole tree. + */ +export function clientTsconfigReferences(clientDir) { + return tsconfigReferences(path.posix.join(clientDir, "tsconfig.json")); +} + +/** + * The repo-relative tsconfig FILE a `clientDir`-relative `project` entry names. + * A directory-form entry (`{ "path": "./packages/a" }`, or `tsc -p src`) means + * `/tsconfig.json` — tsc's own rule. + */ +export function projectConfigFile(clientDir, project) { + const projectRel = path.posix.join(clientDir, project); + return projectRel.endsWith(".json") + ? projectRel + : path.posix.join(projectRel, "tsconfig.json"); +} + +/** + * A `references` entry `ref` (written relative to `fromConfigFile`'s own + * directory) as a `clientDir`-relative project path, the form the rest of the + * graph walk uses. + */ +export function refToProject(clientDir, fromConfigFile, ref) { + return path.posix.relative( + clientDir, + path.posix.join(path.posix.dirname(fromConfigFile), ref), + ); +} + +/** + * The leaf tsconfig projects `project` resolves to (paths relative to + * `clientDir`): itself if it lists files (or has no `references`), else its + * `references` expanded recursively. A `tsc -b` **solution config** (`{"files": + * [], "references": […]}`) lists nothing under `--listFilesOnly`, so this is how + * it's reduced to the real projects — and doing it here (not inside a single + * caller) is what lets every consumer follow the same graph. + */ +export function resolveLeafProjects(clientDir, project, seen = new Set()) { + if (seen.has(project)) return []; + seen.add(project); + // Lists first-party files → a real leaf. (An empty set is a solution config, + // or a config that errored — either way the reference expansion below is the + // right next step: a broken config yields no references either.) + if (projectSourceFiles(clientDir, project).size > 0) return [project]; + const configFile = projectConfigFile(clientDir, project); + const refs = tsconfigReferences(configFile); + if (refs.length === 0) return [project]; // no files, no refs — itself + return refs.flatMap((ref) => + resolveLeafProjects( + clientDir, + refToProject(clientDir, configFile, ref), + seen, + ), + ); +} + +/** + * Every file ONE project's program resolves, as absolute POSIX paths and with no + * filtering at all — first-party sources, `node_modules` declarations, and the + * out-of-repo `lib.*.d.ts` — plus the config diagnostic if `tsc` exited + * non-zero. The two consumers want different slices of the files, so the slicing + * happens in {@link projectSourceFiles} / {@link projectPackageFiles} rather than + * here; they differ on the `error` too, which is why it is reported rather than + * only warned about. Memoized per (client, project): the same project is listed + * by `resolveLeafProjects` and again by whichever slice the caller asks for. + */ +const listingCache = new Map(); +export function rawProjectListing(clientDir, project) { + const key = `${clientDir}|${project}`; + const cached = listingCache.get(key); + if (cached) return cached; + let stdout; + let error = null; + try { + stdout = execFileSync( + "npx", + ["--no-install", "tsc", "-p", project, "--listFilesOnly"], + { + cwd: path.join(repoRoot, clientDir), + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + maxBuffer: 1 << 28, + }, + ); + } catch (err) { + // `--listFilesOnly` doesn't type-check, but a config error (an unreadable or + // malformed tsconfig) still exits non-zero while printing the resolved file + // list; keep stdout so a broken config doesn't mask what the callers measure. + // Echo the diagnostic — the guards run before any client's own `typecheck`, + // so this is the first place a bad `-p` config surfaces, and without the + // reason the resulting report is misleading. tsc prints config errors + // (`error TS…`) to stdout, so scan both streams for them. + stdout = typeof err.stdout === "string" ? err.stdout : ""; + const streams = + stdout + "\n" + (typeof err.stderr === "string" ? err.stderr : ""); + error = streams + .split("\n") + .filter((l) => /error TS\d+/.test(l)) + .join("\n") + .trim(); + console.warn( + `tsc -p ${project} (in ${clientDir}) exited non-zero:\n${error || "(no diagnostic captured)"}\n`, + ); + // An empty diagnostic still has to read as failure downstream, so the + // caller sees a string either way — never `""`. + error ||= "(no diagnostic captured)"; + } + const files = stdout + .split("\n") + .map((l) => l.trim()) + .filter(Boolean) + .map((abs) => abs.split(path.sep).join("/")); + const listing = { files, error }; + listingCache.set(key, listing); + return listing; +} + +/** + * The `tsc` diagnostic for a project whose listing exited non-zero, or null. A + * failed config still prints a file list — a partial, wrong one — so a consumer + * that must not measure a program it couldn't resolve has to ask. + */ +export function projectListingError(clientDir, project) { + return rawProjectListing(clientDir, project).error; +} + +/** + * Whether a repo-relative path is an installed file rather than a first-party + * one. Matched by path SEGMENT, not substring, so a first-party directory whose + * name merely embeds the word (`core/node_modules_fixtures/`) is not mistaken + * for an install — the same segment rule {@link classifyModulePath} applies. + */ +const isInstalledPath = (rel) => rel.split("/").includes("node_modules"); + +/** + * Repo-relative POSIX paths of the first-party files ONE project (no reference + * expansion) typechecks. Absolute paths outside the repo root (`lib.d.ts`) and + * anything under `node_modules` are dropped; the aliased `core/` + + * `test-servers/` sources stay in the set but are harmless — the set is only ever + * queried with in-repo paths. + */ +export function projectSourceFiles(clientDir, project) { + const covered = new Set(); + for (const abs of rawProjectListing(clientDir, project).files) { + const rel = path.relative(repoRoot, abs).split(path.sep).join("/"); + if (rel.startsWith("..") || isInstalledPath(rel)) continue; + covered.add(rel); + } + return covered; +} + +/** + * Repo-relative POSIX paths of the INSTALLED files ONE project's program + * resolves — everything under some `node_modules` inside the repo. Out-of-repo + * paths are dropped: an absolute `lib.d.ts` under the toolchain isn't an install + * this repo can align. + */ +export function projectPackageFiles(clientDir, project) { + const covered = new Set(); + for (const abs of rawProjectListing(clientDir, project).files) { + const rel = path.relative(repoRoot, abs).split(path.sep).join("/"); + if (rel.startsWith("..") || !isInstalledPath(rel)) continue; + covered.add(rel); + } + return covered; +} + +/** + * Where a repo-relative `node_modules` path came from — `{ installRoot, name, + * entryPath }`, or null for a path that names no package. + * + * - `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("/"); + const first = parts.indexOf("node_modules"); + if (first === -1) return null; + const last = parts.lastIndexOf("node_modules"); + const head = parts[last + 1]; + // `node_modules/.bin/…`, `node_modules/.package-lock.json`: npm's own + // bookkeeping, not a package. + if (!head || head.startsWith(".")) return null; + 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("/"), + }; +} + +/** + * The packages that enter a SINGLE program from two different installs — the + * only ones whose version skew can put two structurally-distinct copies of one + * 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 + * `Map>>>` — the + * co-occurrences themselves, not a flattened name list. + * + * Keeping the structure is what lets the caller compare only the copies that met + * (Copilot, #1965 r1). Flattening to names hands the comparison a package that + * is aligned wherever it co-occurs, and a third install holding a different + * version of it then fails the guard even though no program ever loads that + * copy — with a diagnostic naming an install that was never involved. + * + * The two-install test is applied **within one program**, not across the run: a + * package reached from the root install in web's program and from the client + * install in cli's program is two copies in two *separate* type checks, which + * nothing has to relate. + * + * Note what TypeScript's own package-identity dedup does to this, because it + * makes the measure self-correcting rather than leaky. Two copies of a package + * at the **same** name@version are collapsed into a redirect, and a redirected + * `.d.ts` does not resolve its own imports — so an aligned package's transitive + * dependencies are loaded once, from one install, and never become candidates. + * That is the right answer, not a miss: one copy in the program is one copy for + * the checker to instantiate, whatever the tree looks like on disk. The moment + * the pair skews, the redirect stops applying, both copies enter the program, + * and the package becomes a candidate — which is exactly when it matters. + */ +export function crossInstallPackages(programs) { + const found = new Map(); + for (const { label, files } of programs) { + // 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 (!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, byRoot] of seen) { + if (byRoot.size < 2) continue; // one install — nothing to relate + if (!found.has(name)) found.set(name, new Map()); + found.get(name).set(label, byRoot); + } + } + return found; +} diff --git a/scripts/lib/tsc-program.test.mjs b/scripts/lib/tsc-program.test.mjs new file mode 100644 index 000000000..c76825ff4 --- /dev/null +++ b/scripts/lib/tsc-program.test.mjs @@ -0,0 +1,292 @@ +// Table-driven tests for the shared `tsc` program helpers — the machinery both +// `verify:typecheck-coverage` (#1791) and `verify:dep-lockstep` (#1965) read a +// program through. One case per rule; the comment names the rule, so relaxing +// one shows up as a deleted assertion rather than a quiet behavior shift. The +// script-parsing and tsconfig-graph cases moved here with their code when the +// two guards were unified; their `(rN)` tags are the #1799 review rounds that +// found them. Run via `npm run test:scripts`. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + classifyModulePath, + crossInstallPackages, + isDisablingFlag, + isTsc, + parseTsconfigReferences, + projectConfigFile, + refToProject, + typecheckProjects, +} from "./tsc-program.mjs"; + +test("isTsc: matches by basename incl. path-invoked (r18 regression)", () => { + for (const t of [ + "tsc", + "node_modules/.bin/tsc", + "./node_modules/.bin/tsc.cmd", + ]) + assert.ok(isTsc(t), t); + for (const t of ["vitest", "prettier", "tscx", "atsc"]) + assert.ok(!isTsc(t), t); +}); + +test("isDisablingFlag: case-insensitive (r18)", () => { + for (const t of [ + "--noCheck", + "--nocheck", + "--listFilesOnly", + "--LISTFILESONLY", + ]) + assert.ok(isDisablingFlag(t), t); + for (const t of ["--noEmit", "-p", "--project", "noCheck"]) + assert.ok(!isDisablingFlag(t), t); +}); + +test("typecheckProjects: harvests -p / --project / -b, implicit tsconfig.json (r13)", () => { + const { projects, neutered } = typecheckProjects({ + typecheck: + "tsc --noEmit -p tsconfig.json && tsc --noEmit -p tsconfig.test.json", + }); + assert.deepEqual(projects, ["tsconfig.json", "tsconfig.test.json"]); + assert.equal(neutered.length, 0); + + // A bare `tsc` (no project flag) resolves the implicit ./tsconfig.json. + assert.deepEqual(typecheckProjects({ typecheck: "tsc --noEmit" }).projects, [ + "tsconfig.json", + ]); + + // Path-invoked binary still counts (r18). + assert.deepEqual( + typecheckProjects({ typecheck: "node_modules/.bin/tsc -p tsconfig.json" }) + .projects, + ["tsconfig.json"], + ); + + // A quoted project path (r17). + assert.deepEqual( + typecheckProjects({ typecheck: `tsc -p "tsconfig.test.json"` }).projects, + ["tsconfig.test.json"], + ); + + // `--project` long form, and `-b`/`--build` project paths (r13). + const proj = (cmd) => typecheckProjects({ typecheck: cmd }).projects; + assert.deepEqual(proj("tsc --noEmit --project tsconfig.json"), [ + "tsconfig.json", + ]); + assert.deepEqual(proj("tsc -b tsconfig.json"), ["tsconfig.json"]); + assert.deepEqual(proj("tsc --build tsconfig.json"), ["tsconfig.json"]); + assert.deepEqual(proj("tsc -b"), ["tsconfig.json"]); // implicit fallback +}); + +test("typecheckProjects: neutered by --noCheck / --listFilesOnly (r10)", () => { + const { projects, neutered } = typecheckProjects({ + typecheck: + "tsc --noEmit -p tsconfig.json --noCheck && tsc --noEmit -p tsconfig.test.json", + }); + assert.deepEqual(projects, ["tsconfig.test.json"]); + assert.deepEqual(neutered, [{ project: "tsconfig.json", flag: "--noCheck" }]); +}); + +test("typecheckProjects: delegating typecheck, ignores non-tsc segments (r15)", () => { + const { projects } = typecheckProjects({ + typecheck: "npm run typecheck:src && npm run typecheck:test", + "typecheck:src": "tsc --noEmit -p tsconfig.json", + "typecheck:test": "tsc --noEmit --project tsconfig.test.json", + }); + assert.deepEqual(projects.sort(), ["tsconfig.json", "tsconfig.test.json"]); +}); + +test("parseTsconfigReferences: JSONC tolerance (r17-nit2 block comments)", () => { + const refs = (raw) => parseTsconfigReferences(raw); + assert.deepEqual(refs('{ "references": [{ "path": "./a" }] }'), ["./a"]); + assert.deepEqual( + refs('/* solution */\n{ "references": [{ "path": "./a" }] }'), + ["./a"], + ); + assert.deepEqual(refs('{ "references": [{ "path": "./a" }] } // trailing'), [ + "./a", + ]); + assert.deepEqual(refs('{ "references": [{ "path": "./a" },] }'), ["./a"]); // trailing comma + assert.deepEqual(refs('{ "files": [] }'), []); // no references + assert.deepEqual(refs("{ not json"), []); // malformed + assert.deepEqual(refs('{ "references": [{ "prepend": true }] }'), []); // no path +}); + +test("projectConfigFile: directory-form entry means /tsconfig.json (r26)", () => { + assert.equal( + projectConfigFile("clients/cli", "tsconfig.test.json"), + "clients/cli/tsconfig.test.json", + ); + assert.equal( + projectConfigFile("clients/cli", "packages/a"), + "clients/cli/packages/a/tsconfig.json", + ); + assert.equal( + projectConfigFile("clients/cli", "."), + "clients/cli/tsconfig.json", + ); +}); + +test("refToProject: refs resolve against the REFERRING config's dir (r26)", () => { + // A ref is relative to the tsconfig that declares it, not to clientDir. + assert.equal( + refToProject( + "clients/web", + "clients/web/tsconfig.json", + "./tsconfig.app.json", + ), + "tsconfig.app.json", + ); + assert.equal( + refToProject( + "clients/web", + "clients/web/sub/tsconfig.json", + "../other.json", + ), + "other.json", + ); + assert.equal( + refToProject("clients/web", "clients/web/sub/tsconfig.json", "./deep"), + "sub/deep", + ); +}); + +test("classifyModulePath: install root is the OUTERMOST node_modules, package the innermost (#1965)", () => { + 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", + 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", + 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. 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", + 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", + entryPath: "node_modules/a/node_modules/@scope/b", + }, + ); +}); + +test("classifyModulePath: paths that name no package", () => { + for (const rel of [ + "core/mcp/client.ts", // first-party source + "clients/web/src/App.tsx", + "node_modules/.bin/tsc", // npm bookkeeping, not a package + "node_modules/.package-lock.json", + "node_modules/@scope", // a scope directory names no package + "node_modules", // the directory itself + ]) + assert.equal(classifyModulePath(rel), null, rel); +}); + +test("classifyModulePath: a path segment merely CONTAINING node_modules is not one", () => { + // Segment-wise matching, not substring: a first-party dir whose name embeds + // the word would otherwise be read as an install root. + assert.equal(classifyModulePath("core/node_modules_fixtures/a.ts"), null); +}); + +test("crossInstallPackages: two installs in ONE program is the whole test (#1965)", () => { + const found = crossInstallPackages([ + { + label: "clients/web/tsconfig.test.json", + files: [ + "node_modules/zod/index.d.ts", + "clients/web/node_modules/zod/index.d.ts", + "clients/web/node_modules/react/index.d.ts", + ], + }, + ]); + assert.deepEqual([...found.keys()], ["zod"]); + 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", () => { + // Two copies the type checker never has to relate — each program sees one. + const found = crossInstallPackages([ + { label: "a", files: ["node_modules/zod/index.d.ts"] }, + { label: "b", files: ["clients/cli/node_modules/zod/index.d.ts"] }, + ]); + assert.equal(found.size, 0); +}); + +test("crossInstallPackages: a nested duplicate inside one install is not a candidate", () => { + const found = crossInstallPackages([ + { + label: "a", + files: [ + "node_modules/zod/index.d.ts", + "node_modules/other/node_modules/zod/index.d.ts", + ], + }, + ]); + assert.equal(found.size, 0); +}); + +test("crossInstallPackages: every program that saw both copies is recorded", () => { + const both = [ + "node_modules/zod/index.d.ts", + "clients/web/node_modules/zod/index.d.ts", + ]; + const found = crossInstallPackages([ + { label: "web/app", files: both }, + { label: "web/test", files: both }, + { label: "cli/src", files: ["node_modules/zod/index.d.ts"] }, + ]); + assert.deepEqual([...found.get("zod").keys()].sort(), [ + "web/app", + "web/test", + ]); +}); + +test("crossInstallPackages: a nested copy is kept as its own entry path (Copilot, #1965 r1)", () => { + // Folded onto the root install for candidacy, but recorded at the path it was + // loaded from so its real version can be read. + const found = crossInstallPackages([ + { + label: "p", + files: [ + "node_modules/a/node_modules/zod/index.d.ts", + "clients/web/node_modules/zod/index.d.ts", + ], + }, + ]); + assert.deepEqual( + [...found.get("zod").get("p").get(".")], + ["node_modules/a/node_modules/zod"], + ); +}); diff --git a/scripts/verify-dep-lockstep.main.test.mjs b/scripts/verify-dep-lockstep.main.test.mjs index c00379d75..32819af98 100644 --- a/scripts/verify-dep-lockstep.main.test.mjs +++ b/scripts/verify-dep-lockstep.main.test.mjs @@ -1,14 +1,21 @@ // 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, +// so a regression in candidate derivation, 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`. +// at one — `git add`s it (install discovery is by manifest, but the sibling +// `verify:format-coverage` vouch and git-based tooling expect a repo), and runs +// the guard as a subprocess to assert the exit status and message. +// +// Since #1965 the fixture must contain a REAL tsc program: the candidate set is +// derived from what `tsc --listFilesOnly` resolves, so each fixture ships two +// installs holding the same tiny stub packages, one client tsconfig whose +// program spans both (its own `src` resolves from the client install, the shared +// `core/` it includes resolves from the root — the actual v2 shape), and a +// symlinked real `typescript` to run it. Run via `npm run test:scripts`. import { test } from "node:test"; import assert from "node:assert/strict"; @@ -30,21 +37,25 @@ 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. */ +/** The real typescript install, symlinked into each fixture so `npx tsc` runs. */ 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. */ +/** + * A lockfileVersion 3 lockfile, including the `""` root entry npm always writes. + * A key already containing `node_modules/` is used verbatim, so a nested install + * path (`node_modules/outer/node_modules/inner`) can be expressed. + */ const lock = (deps) => ({ lockfileVersion: 3, packages: { "": { name: "fixture" }, ...Object.fromEntries( Object.entries(deps).map(([name, version]) => [ - `node_modules/${name}`, + name.includes("node_modules/") ? name : `node_modules/${name}`, { version }, ]), ), @@ -52,11 +63,26 @@ const lock = (deps) => ({ }); /** - * 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. + * Build a fixture repo whose one client program spans two installs. + * + * `outer` is named by first-party code in both trees; `inner` is named ONLY by + * `outer`'s own `.d.ts`, so it reaches the program transitively — the + * `@modelcontextprotocol/sdk` shape #1965 is about. Both stubs are installed + * under the root AND under `clients/web`, so each program holds two copies of + * each and both are candidates. `solo` is installed at the root only and + * imported only by the shared tree, so it can never be a candidate however its + * lockfile entries read. */ -function makeFixture({ rootDeps, webDeps, scripts, rawWebLock }) { +function makeFixture({ + rootDeps, + webDeps, + scripts, + webScripts, + rawWebLock, + webTsconfig, + rootNestedInner, + cliDeps, +} = {}) { // realpath matters: on macOS `tmpdir()` is `/var/...`, a symlink to // `/private/var/...`. The guard only runs `main()` when `import.meta.url` // (always the resolved path) matches `process.argv[1]`, so launching it via @@ -81,34 +107,136 @@ function makeFixture({ rootDeps, webDeps, scripts, rawWebLock }) { "verify:dep-lockstep": "node scripts/verify-dep-lockstep.mjs", }, }); - write("core/client.ts", 'import { z } from "zod";\nexport const s = z;\n'); + + // The shared tree, compiled into the client's program but resolving its own + // imports from the ROOT install — the reason a package can appear twice. + write( + "core/client.ts", + 'import type { Outer } from "outer";\nimport type { Solo } from "solo";\nexport type FromCore = Outer | Solo;\n', + ); + // The client's own source, resolving from the CLIENT install. write( - "test-servers/src/server.ts", - 'import express from "express";\nexport const app = express;\n', + "clients/web/src/main.ts", + 'import type { Outer } from "outer";\nexport type FromWeb = Outer;\n', ); + write("clients/web/package.json", { + name: "web", + scripts: webScripts ?? { typecheck: "tsc --noEmit -p tsconfig.json" }, + }); write( - "vitest.shared.mts", - 'import path from "node:path";\nexport default path;\n', + "clients/web/tsconfig.json", + webTsconfig ?? { + compilerOptions: { + noEmit: true, + module: "esnext", + target: "esnext", + moduleResolution: "bundler", + types: [], + }, + include: ["src/**/*.ts", "../../core/**/*.ts"], + }, ); - write("package-lock.json", lock(rootDeps)); - write("clients/web/package.json", { name: "web" }); - write("clients/web/package-lock.json", rawWebLock ?? lock(webDeps)); + + // Stub installs. `outer` pulls `inner` in through its own declarations. The + // installed version must match the lockfile's: TypeScript keys its + // package-identity dedup on name@version, so a stub that lied about its + // version would make the program under test differ from the one the lockfile + // describes. + const root = rootDeps ?? ALIGNED; + const web = webDeps ?? ALIGNED; + const stub = (installRoot, versions, name, body) => { + if (!versions[name]) return; + write(`${installRoot}node_modules/${name}/package.json`, { + name, + version: versions[name], + types: "index.d.ts", + }); + write(`${installRoot}node_modules/${name}/index.d.ts`, body); + }; + for (const [installRoot, versions] of [ + ["", root], + ["clients/web/", web], + ]) { + stub( + installRoot, + versions, + "outer", + 'import type { Inner } from "inner";\nexport type Outer = Inner;\n', + ); + stub(installRoot, versions, "inner", "export type Inner = number;\n"); + } + stub("", root, "solo", "export type Solo = string;\n"); + + // The root reaches `inner` only through a copy NESTED under `outer`, at a + // version its top-level entry does not carry. npm's own conflict resolution, + // and the case that must still be priced from the entry the program resolved. + const rootLockDeps = { ...root }; + if (rootNestedInner) { + stub( + "node_modules/outer/", + { inner: rootNestedInner }, + "inner", + "export type Inner = number;\n", + ); + rootLockDeps["node_modules/outer/node_modules/inner"] = rootNestedInner; + } + + write("package-lock.json", lock(rootLockDeps)); + write("clients/web/package-lock.json", rawWebLock ?? lock(web)); + + // A second client, to prove a third install's copy is not dragged into a + // comparison it never took part in. Its program spans only its own install. + if (cliDeps) { + write("clients/cli/package.json", { + name: "cli", + scripts: { typecheck: "tsc --noEmit -p tsconfig.json" }, + }); + write("clients/cli/tsconfig.json", { + compilerOptions: { + noEmit: true, + module: "esnext", + target: "esnext", + moduleResolution: "bundler", + types: [], + }, + include: ["src/**/*.ts"], + }); + write( + "clients/cli/src/main.ts", + 'import type { Outer } from "outer";\nexport type FromCli = Outer;\n', + ); + stub( + "clients/cli/", + cliDeps, + "outer", + 'import type { Inner } from "inner";\nexport type Outer = Inner;\n', + ); + stub("clients/cli/", cliDeps, "inner", "export type Inner = number;\n"); + write("clients/cli/package-lock.json", lock(cliDeps)); + } // The guard resolves its repo root from its own location, so it has to live - // inside the fixture; `lib/npm-scripts.mjs` comes along as its import. + // inside the fixture; its `lib/` imports come along. mkdirSync(path.join(dir, "scripts", "lib"), { recursive: true }); for (const rel of [ "verify-dep-lockstep.mjs", path.join("lib", "npm-scripts.mjs"), + path.join("lib", "tsc-program.mjs"), ]) cpSync(path.join(scriptsDir, rel), path.join(dir, "scripts", rel)); - mkdirSync(path.join(dir, "node_modules"), { recursive: true }); + // A real typescript, plus the `.bin` entry `npx --no-install tsc` resolves by + // walking up from the client dir. + mkdirSync(path.join(dir, "node_modules", ".bin"), { recursive: true }); symlinkSync( typescriptDir, path.join(dir, "node_modules", "typescript"), "dir", ); + symlinkSync( + path.join(typescriptDir, "bin", "tsc"), + path.join(dir, "node_modules", ".bin", "tsc"), + ); execFileSync("git", ["init", "-q"], { cwd: dir }); execFileSync("git", ["add", "-A"], { cwd: dir }); @@ -134,69 +262,149 @@ function withFixture(options, fn) { } } -const ALIGNED = { zod: "4.4.3", express: "5.2.1" }; +const ALIGNED = { outer: "1.2.3", inner: "4.5.6", solo: "7.8.9" }; test("main: exits 0 when every install agrees", () => { - withFixture({ rootDeps: ALIGNED, webDeps: ALIGNED }, (dir) => { + withFixture({}, (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/); + // `outer` alone: both installs hold it and both files land in the program. + // `solo` is installed at the root only, so it is not install-crossing — and + // `inner`, at the same version in both installs, is collapsed by + // TypeScript's package-identity redirect (see the note on `crossInstallPackages` + // in lib/tsc-program.mjs): only one copy is ever loaded, so there is nothing + // for the checker to relate. The next test is the version that matters. + assert.match(out, /1 install-crossing dependencies/); + assert.match(out, /1 tsc programs/); }); }); test("main: exits 1 and names the skewed package and every holder", () => { withFixture( - { rootDeps: { ...ALIGNED, zod: "4.3.6" }, webDeps: ALIGNED }, + { rootDeps: { ...ALIGNED, outer: "1.0.0" }, 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); + assert.match(out, /\bouter\b/); + assert.match(out, /1\.0\.0\s+\(\.\/node_modules\/outer\)/); + assert.match(out, /1\.2\.3\s+\(clients\/web\/node_modules\/outer\)/); + // The program that saw both copies is named, so the claim is checkable. + assert.match(out, /in clients\/web\/tsconfig\.json/); + // `inner` agrees, so it must not be reported. + assert.doesNotMatch(out, /^\s+inner$/m); + }, + ); +}); + +test("main: a package reached only through another package's .d.ts is caught (#1965)", () => { + // `inner` is never written in first-party code — it enters the program solely + // through `outer`'s declarations. The derivation this replaced read the + // shared sources' own imports and could not see it, which is how + // `@modelcontextprotocol/sdk` sat skewed across two installs while the guard + // stayed green. Its parent skews too, which is what puts both copies of the + // parent in the program to resolve from — the real #1965 shape, where + // `ext-apps` was split 1.7.4/1.7.5 alongside the SDK. + withFixture( + { + rootDeps: { ...ALIGNED, outer: "1.0.0", inner: "4.0.0" }, + webDeps: ALIGNED, + }, + (dir) => { + const { status, out } = runGuard(dir); + assert.equal(status, 1, out); + assert.match(out, /^\s+inner$/m); + assert.match(out, /4\.0\.0\s+\(\.\/node_modules\/inner\)/); + assert.match(out, /4\.5\.6\s+\(clients\/web\/node_modules\/inner\)/); + }, + ); +}); + +test("main: a package that reaches the program from ONE install is not a candidate", () => { + // `solo` is installed at the root only, so no program ever holds two copies of + // it — its lockfile entries can disagree without TypeScript ever relating two + // declarations. Failing here would be the old derivation's false positive + // (which is what made an allowlist of inert names necessary). + withFixture( + { rootDeps: { ...ALIGNED, solo: "7.0.0" }, webDeps: ALIGNED }, + (dir) => { + const { status, out } = runGuard(dir); + assert.equal(status, 0, out); + assert.doesNotMatch(out, /\bsolo\b/); }, ); }); -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. +test("main: a third install's copy is not dragged into a comparison it never joined (Copilot, #1965 r1)", () => { + // `clients/cli` holds `outer` at a different version, but its own program is + // the only one that loads that copy and no second install meets it there. + // Flattening the candidates to names would compare cli against web's aligned + // pair and fail — naming an install that never took part. withFixture( - { rootDeps: { ...ALIGNED, express: "5.0.0" }, webDeps: ALIGNED }, + { cliDeps: { ...ALIGNED, outer: "9.9.9", inner: "9.9.9" } }, + (dir) => { + const { status, out } = runGuard(dir); + assert.equal(status, 0, out); + assert.doesNotMatch(out, /clients\/cli/); + }, + ); +}); + +test("main: a nested copy is priced from its own lockfile entry (Copilot, #1965 r1)", () => { + // The root reaches `inner` only through `node_modules/outer/node_modules/inner` + // at 9.9.9, while its top-level entry still reads 4.5.6 — the version web + // holds. Pricing the copy from the top-level entry would report the pair as + // agreeing and let a real 9.9.9-vs-4.5.6 program through. + withFixture( + { + rootDeps: { ...ALIGNED, outer: "1.0.0" }, + webDeps: ALIGNED, + rootNestedInner: "9.9.9", + }, (dir) => { const { status, out } = runGuard(dir); assert.equal(status, 1, out); - assert.match(out, /\bexpress\b/); + assert.match(out, /^\s+inner$/m); + assert.match( + out, + /9\.9\.9\s+\(\.\/node_modules\/outer\/node_modules\/inner\)/, + ); + assert.match(out, /4\.5\.6\s+\(clients\/web\/node_modules\/inner\)/); }, ); }); -test("main: exits 1 when a 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 }); +test("main: exits 1 when a client names no tsconfig project", () => { + // No `typecheck` script and no `tsconfig.json` references: none of that + // client's programs is measured, so a skew reaching only them would pass. + withFixture({ webScripts: { build: "vite build" } }, (dir) => { + rmSync(path.join(dir, "clients", "web", "tsconfig.json")); + const { status, out } = runGuard(dir); + assert.equal(status, 1, out); + assert.match(out, /names no tsconfig project/); + }); +}); + +test("main: exits 1 when a program cannot be listed, rather than measuring nothing", () => { + // A broken tsconfig makes `tsc --listFilesOnly` resolve nothing. Reading that + // as "no candidates here" is the gate failing open. + withFixture({ webTsconfig: "{ this is not json" }, (dir) => { const { status, out } = runGuard(dir); assert.equal(status, 1, out); - assert.match(out, /matched no tracked file/); - assert.match(out, /^\s+core$/m); + assert.match(out, /exited non-zero/); }); }); 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 + // install that simply holds nothing would leave the *root's* copy unopposed // and the skew below reported as aligned — the gate failing open. withFixture( { - rootDeps: { ...ALIGNED, zod: "4.3.6" }, - webDeps: ALIGNED, + rootDeps: { ...ALIGNED, outer: "1.0.0" }, rawWebLock: { lockfileVersion: 1, - dependencies: { zod: { version: "4.4.3" } }, + dependencies: { outer: { version: "1.2.3" } }, }, }, (dir) => { @@ -213,11 +421,9 @@ test("main: exits 1 on a lockfile with a `packages` table but no root entry", () // `""` 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" } }, + packages: { "node_modules/outer": { version: "1.2.3" } }, }, }, (dir) => { @@ -232,22 +438,19 @@ 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/); - }, - ); + withFixture({ rootDeps: { ...ALIGNED, outer: "1.0.0" } }, (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) => { + withFixture({}, (dir) => { rmSync(path.join(dir, "package-lock.json")); const { status, out } = runGuard(dir); assert.equal(status, 1, out); @@ -257,8 +460,9 @@ test("main: exits 1 when the ROOT lockfile is missing", () => { }); 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) => { + // Enrolling a stray directory would demand a lockfile it should never have, + // and a tsconfig project it has no way to name. + withFixture({}, (dir) => { mkdirSync(path.join(dir, "clients", "scratch"), { recursive: true }); writeFileSync( path.join(dir, "clients", "scratch", "notes.md"), @@ -269,11 +473,21 @@ test("main: a clients/ dir with no package.json is not an install", () => { }); }); +test("main: exits 1 when there is no client program to measure at all", () => { + // A moved `clients/` dir leaves the derivation with nothing to look at, and an + // empty candidate set from a broken enumeration reads exactly like a clean + // bill of health. + withFixture({}, (dir) => { + rmSync(path.join(dir, "clients"), { recursive: true, force: true }); + const { status, out } = runGuard(dir); + assert.equal(status, 1, out); + assert.match(out, /found no client program to measure/); + }); +}); + 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: { diff --git a/scripts/verify-dep-lockstep.mjs b/scripts/verify-dep-lockstep.mjs index 27b5812c9..05d9277a4 100644 --- a/scripts/verify-dep-lockstep.mjs +++ b/scripts/verify-dep-lockstep.mjs @@ -20,217 +20,101 @@ // `@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. +// The candidate set is DERIVED from what actually enters each `tsc` program +// (#1965): every client tsconfig project is listed with `tsc --listFilesOnly` +// (the shared `lib/tsc-program.mjs` machinery `verify:typecheck-coverage` reads +// too), each resolved `node_modules` file is mapped to its owning install and +// package, and a package that reaches ONE program from TWO installs is a +// candidate. That is exactly the set that can put two structurally-distinct +// copies of a type in front of one type checker — no more, no less. // -// 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. +// It replaced a derivation that read the packages the shared sources named +// *directly*, which could not see a package whose declarations arrive only +// through another package's `.d.ts`. `@modelcontextprotocol/sdk` was the live +// example: it is never written in first-party code (the shared sources import +// the split `@modelcontextprotocol/client|core|…`), yet 16 of its `.d.ts` files +// land in `clients/web`'s test program, so a second copy under +// `clients/web/node_modules` would have skewed unseen. The other derivation +// measured for #1965 — expanding the direct imports over the lockfiles' +// `dependencies` — was unusable: 155 packages, 25 of them skewed, nearly all +// irrelevant transitive tooling (`chai`, `qs`, `iconv-lite`), and it missed the +// SDK anyway. +// +// Skew among the candidates is then denied by default, with an allowlist of +// packages verified to tolerate it (below). A dependency that starts skewing +// fails `validate` and forces a decision, rather than surfacing months later as +// an unexplained OOM. 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"; +import { + clientTsconfigReferences, + crossInstallPackages, + projectListingError, + projectPackageFiles, + resolveLeafProjects, + typecheckProjects, +} from "./lib/tsc-program.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). +// major version*. A rationale 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. +// **Empty today**, and that is a consequence of the #1965 derivation rather than +// a relaxation. The four former entries were all admitted under the old +// direct-import derivation, which asked only "is this name written in shared +// source and held at two versions somewhere" — a question two installs can +// answer yes to without any program ever seeing both copies: // -// 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; -} +// • `jose` and `@modelcontextprotocol/ext-apps` are declared only at the root +// since #1970, so neither can skew at all now. +// • `react` ships no types of its own, so what lands in a program is +// `@types/react`, and it lands from a single install; the `react` package's +// own files never enter one. `hono` likewise resolves from one install per +// program (`clients/web` in web's node project). +// +// If any of them ever does reach one program from two installs, the guard fails +// and forces the decision then — with the actual version pair in hand, which is +// a better basis for a rationale than a pre-emptive entry. +const TOLERATED_SKEW = new Map(); /** - * The package(s) a `/// ` 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. + * Installed versions in a parsed lockfile, keyed by the **install path** npm + * writes — `node_modules/zod`, `node_modules/a/node_modules/zod`. * - * 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. + * Keyed by path rather than by package name so a version is read from the exact + * copy a program resolved (Copilot, #1965 r1). Reading only top-level entries + * would mean a nested copy that entered the program got compared against + * whatever sits at the install's top level — a different version, or nothing at + * all — and a real pair could pass. Nested copies are still not a *candidate* on + * their own (`classifyModulePath` folds them onto their outermost install); this + * is about pricing a copy correctly once a program has loaded it. */ -export function 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/` 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) { +export function lockVersionsByPath(lock) { const versions = new Map(); for (const [entryPath, entry] of Object.entries(lock?.packages ?? {})) { - const m = /^node_modules\/(@[^/]+\/[^/]+|[^@/][^/]*)$/.exec(entryPath); - if (!m || typeof entry?.version !== "string") continue; - versions.set(m[1], entry.version); + if (!entryPath.startsWith("node_modules/")) continue; + if (typeof entry?.version !== "string") continue; + versions.set(entryPath, entry.version); } return versions; } @@ -241,7 +125,7 @@ export function topLevelLockVersions(lock) { * the root project. * * This is checked rather than tolerated because the gate is deny-by-default and - * `topLevelLockVersions` returns an empty map for anything else. An unreadable + * `lockVersionsByPath` returns an empty map for anything else. An unreadable * lockfile would otherwise contribute no holders, and a real skew among the * remaining installs would be reported as aligned — the gate failing *open*, * which is the one way it must never fail (Copilot, #1962). A v1 lockfile @@ -262,22 +146,48 @@ export function hasReadableLockShape(lock) { } /** - * Find candidate packages that resolve to more than one version across the - * installs. `installs` is an array of `{ dir, versions }`. Returns one entry per - * skewed package, sorted by name, each listing the version each install holds. - * Packages present in fewer than two installs cannot skew and are skipped. + * Price each co-occurrence and keep the ones whose copies disagree. + * + * `found` is {@link crossInstallPackages}' output — + * `Map>>>` — and `versions` + * maps an install dir to that install's {@link lockVersionsByPath}. A package is + * skewed when the copies **one program** loaded, from two different installs, + * carry more than one version. Only the installs that actually met in that + * program are compared: a third install's copy that no program loads is not + * evidence of anything (Copilot, #1965 r1). + * + * Returns `{ skewed, unresolved }`, sorted by name. `unresolved` names a copy + * whose lockfile entry is missing — the tree and the lockfile disagree, so the + * comparison cannot be trusted and the caller must fail rather than skip it. */ -export function findSkew(candidates, installs) { +export function findSkew(found, versions) { const skewed = []; - for (const name of [...candidates].sort()) { - const holders = installs - .filter(({ versions }) => versions.has(name)) - .map(({ dir, versions }) => ({ dir, version: versions.get(name) })); - if (holders.length < 2) continue; - const distinct = new Set(holders.map((h) => h.version)); - if (distinct.size > 1) skewed.push({ name, holders }); + const unresolved = []; + for (const name of [...found.keys()].sort()) { + const occurrences = []; + for (const [program, byRoot] of found.get(name)) { + const holders = []; + for (const [dir, entryPaths] of byRoot) + for (const entryPath of entryPaths) { + const version = versions.get(dir)?.get(entryPath); + if (version === undefined) unresolved.push({ name, dir, entryPath }); + else holders.push({ dir, entryPath, version }); + } + if (new Set(holders.map((h) => h.version)).size > 1) + occurrences.push({ program, holders }); + } + if (occurrences.length > 0) skewed.push({ name, occurrences }); } - return skewed; + return { skewed, unresolved }; +} + +/** Every holder across a skewed package's occurrences, deduped by install + path. */ +export function skewHolders(entry) { + const byKey = new Map(); + for (const { holders } of entry.occurrences) + for (const holder of holders) + byKey.set(`${holder.dir}|${holder.entryPath}`, holder); + return [...byKey.values()]; } /** @@ -302,7 +212,7 @@ export function majorOf(version) { export function partitionSkew(skewed, tolerated = TOLERATED_SKEW) { const isTolerated = (s) => { if (!tolerated.has(s.name)) return false; - const majors = new Set(s.holders.map((h) => majorOf(h.version))); + const majors = new Set(skewHolders(s).map((h) => majorOf(h.version))); return majors.size === 1 && !majors.has(null); }; return { @@ -311,84 +221,124 @@ export function partitionSkew(skewed, tolerated = TOLERATED_SKEW) { }; } -// 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. + * The tsconfig projects whose programs a client contributes, from its own + * `package.json` scripts and its root `tsconfig.json` `references` — the same + * two enrollment paths `verify:typecheck-coverage` uses, so the two guards + * measure the same programs. + * + * A `neutered` project (its `typecheck` command carries `--noCheck` / + * `--listFilesOnly`) counts here even though its sibling guard rejects it: + * whether that pass type-checks is that guard's question, while this one only + * asks what the program *resolves*, and a program still resolves its imports + * either way. Dropping them would shrink what this guard measures on the + * strength of a defect the other guard is already failing on. */ -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}/`)); +export function clientProjects(scripts, references) { + if (typeof scripts?.typecheck === "string") { + const { projects, neutered } = typecheckProjects(scripts); + return [...projects, ...neutered.map((n) => n.project)]; + } + return references; } /** - * 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. + * The `clients/*` directories that are real installs — one with a + * `package.json`. Enrolment is by manifest, NOT by the presence of a lockfile + * (Copilot, #1962): filtering on the lockfile made a missing one silently drop + * that install from the comparison. */ -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]; +function clientDirs() { + const dir = path.join(repoRoot, "clients"); + if (!existsSync(dir)) return []; + return readdirSync(dir, { withFileTypes: true }) + .filter((e) => e.isDirectory()) + .map((e) => `clients/${e.name}`) + .filter((rel) => existsSync(path.join(repoRoot, rel, "package.json"))) + .sort(); } -/** 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/*` install. + * Discovered from disk rather than listed, so a new client is covered without + * editing this guard (the same enrollment style as `verify:typecheck-coverage`). + * The root is always enrolled: it is this repo, so its manifest is a given. + */ +function installDirs() { + return ["."].concat(clientDirs()); } /** - * 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`). + * List every client program and reduce it to the packages that reach one program + * from two installs. Returns `{ found, programs, problems }` — `found` is the + * {@link crossInstallPackages} map, `programs` the labels listed (for the + * success line), `problems` any reason the measurement itself can't be trusted. + * + * A program that resolves NO installed file at all is a problem, not an empty + * result: every real program pulls in at least the toolchain's own `.d.ts` from + * some `node_modules`, so an empty listing means `tsc` never ran (a missing + * install) or the config is broken. Treating it as "no candidates here" is the + * gate failing open. */ -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")), - ), - ); +function deriveCandidates() { + const problems = []; + const programs = []; + for (const clientDir of clientDirs()) { + let scripts; + try { + scripts = JSON.parse( + readFileSync(path.join(repoRoot, clientDir, "package.json"), "utf8"), + ).scripts; + } catch (cause) { + throw new Error( + `verify:dep-lockstep — could not parse ${clientDir}/package.json.`, + { cause }, + ); + } + const projects = clientProjects( + scripts, + clientTsconfigReferences(clientDir), + ); + if (projects.length === 0) { + problems.push( + `${clientDir}: names no tsconfig project (no \`typecheck\` script, no \`tsconfig.json\` references) — none of its programs is measured.`, + ); + continue; + } + const leaves = new Set(); + for (const project of projects) + for (const leaf of resolveLeafProjects(clientDir, project)) + leaves.add(leaf); + for (const leaf of leaves) { + // A failed config is not a program: tsc still prints a file list, but it + // is whatever its fallback resolved rather than what the project declares, + // so measuring it would narrow the candidate set for a reason that has + // nothing to do with the dependency tree. + const error = projectListingError(clientDir, leaf); + if (error) { + problems.push( + `${clientDir}: \`tsc -p ${leaf} --listFilesOnly\` exited non-zero — ${error.split("\n")[0]}`, + ); + continue; + } + const files = projectPackageFiles(clientDir, leaf); + if (files.size === 0) { + problems.push( + `${clientDir}: \`tsc -p ${leaf} --listFilesOnly\` resolved no installed file — the program could not be listed.`, + ); + continue; + } + programs.push({ label: `${clientDir}/${leaf}`, files }); + } + } + // No program at all means the enumeration itself broke (a moved `clients/` + // dir), not that there is nothing to check — and an empty candidate set from a + // broken enumeration is indistinguishable from a clean bill of health. + if (programs.length === 0 && problems.length === 0) + problems.push( + "found no client program to measure — every `clients/*` install must contribute at least one tsconfig project.", + ); + return { found: crossInstallPackages(programs), programs, problems }; } /** @@ -411,30 +361,19 @@ export function main() { 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) { + const { found, programs, problems } = deriveCandidates(); + if (problems.length > 0) { console.error( - `verify:dep-lockstep — ${empty.length} configured shared source(s) matched no tracked file:\n`, + `verify:dep-lockstep — ${problems.length} program(s) could not be measured:\n`, ); - for (const source of empty) console.error(` ${source}`); + for (const problem of problems) console.error(` ${problem}`); 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.", + "\nThe candidate set is derived from what each `tsc` program resolves, so an unmeasured program" + + "\nmeans real skew could pass unseen. Run `npm install` at the repo root (the postinstall cascade" + + "\ninstalls each client), and see `verify:typecheck-coverage` for the tsconfig-project enrollment rules.", ); 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 @@ -488,34 +427,58 @@ export function main() { process.exit(1); } - const installs = locks.map(({ dir, lock }) => ({ - dir, - versions: topLevelLockVersions(lock), - })); + const versions = new Map( + locks.map(({ dir, lock }) => [dir, lockVersionsByPath(lock)]), + ); + + const { skewed, unresolved } = findSkew(found, versions); + + // A copy the program loaded but the lockfile doesn't list means the installed + // tree and the lockfile disagree — the comparison would be reading a version + // that isn't the one on disk, so refuse it rather than skip the copy. + if (unresolved.length > 0) { + console.error( + `verify:dep-lockstep — ${unresolved.length} resolved package(s) have no lockfile entry:\n`, + ); + for (const { name, dir, entryPath } of unresolved) + console.error(` ${name} ${dir}/${entryPath}`); + console.error( + "\nThe program loaded these, so their versions decide the comparison — but the install's lockfile" + + "\ndoesn't list them, which means the tree and the lockfile disagree. Run `npm install` at the repo" + + "\nroot to re-sync every install.", + ); + process.exit(1); + } - const { failures, ignored } = partitionSkew(findSkew(candidates, installs)); + const { failures, ignored } = partitionSkew(skewed); if (failures.length > 0) { console.error( `verify:dep-lockstep — ${failures.length} ${failures.length === 1 ? "dependency resolves" : "dependencies resolve"} to different versions across installs:\n`, ); let anyListed = false; - for (const { name, holders } of failures) { + for (const failure of failures) { // A package already on the allowlist reached here only by skewing across // a MAJOR boundary, so say that rather than advising an entry that exists. - const listed = TOLERATED_SKEW.has(name); + const listed = TOLERATED_SKEW.has(failure.name); anyListed ||= listed; console.error( - ` ${name}${listed ? " (allowlisted — but this is a MAJOR skew)" : ""}`, + ` ${failure.name}${listed ? " (allowlisted — but this is a MAJOR skew)" : ""}`, ); - for (const { dir, version } of holders) - console.error(` ${version} (${dir})`); + // Report per program: which copies met, and where. The program is the + // whole reason the package is a candidate, and naming only the versions + // would leave the reader unable to check the claim — or to tell a nested + // copy from the install's top-level one. + for (const { program, holders } of failure.occurrences) { + console.error(` in ${program}`); + for (const { dir, entryPath, version } of holders) + console.error(` ${version} (${dir}/${entryPath})`); + } } - 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" + + "\nEach of these reaches a single `tsc` program from two installs (a client's own sources resolve" + + "\nfrom the client install, the shared `core/` + `test-servers/src` they pull in resolve from the" + + "\nroot), so 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( @@ -534,7 +497,8 @@ export function main() { const note = ignored.length > 0 ? `, ${ignored.length} tolerated` : ""; console.log( - `verify:dep-lockstep — OK: ${candidates.size} install-crossing dependencies agree across ${installs.length} installs${note}.`, + `verify:dep-lockstep — OK: ${found.size} install-crossing dependencies agree across ${dirs.length} installs${note} ` + + `(derived from ${programs.length} tsc programs).`, ); } diff --git a/scripts/verify-dep-lockstep.test.mjs b/scripts/verify-dep-lockstep.test.mjs index 55ca3b6f6..edcbbc4f5 100644 --- a/scripts/verify-dep-lockstep.test.mjs +++ b/scripts/verify-dep-lockstep.test.mjs @@ -3,287 +3,245 @@ // 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, + lockVersionsByPath, 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"], ); - 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("clientProjects: a reference client is measured through its references", () => { + assert.deepEqual( + clientProjects({ build: "tsc -b && vite build" }, ["./a.json"]), + ["./a.json"], + ); + // Neither path available — the caller reports it rather than measuring nothing. + assert.deepEqual(clientProjects({ build: "vite build" }, []), []); }); -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("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("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("lockVersionsByPath: keyed by install path, nested entries included (Copilot, #1965 r1)", () => { + // Keyed by PATH, not by package name: a nested copy that entered a program has + // to be priced from its own entry. Reading only `node_modules/` would + // compare it against the install's top-level copy — a different version, or + // none — and a real pair could pass. + const lock = { + packages: { + "": { name: "root" }, + "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([...lockVersionsByPath(lock)].sort(), [ + ["node_modules/@modelcontextprotocol/client", "2.0.0-beta.5"], + ["node_modules/cosmiconfig/node_modules/yaml", "1.10.3"], + ["node_modules/yaml", "2.9.0"], + ["node_modules/zod", "4.4.3"], ]); }); -test("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("lockVersionsByPath: a malformed or empty lockfile yields nothing", () => { + // Safe as a pure helper *because* `hasReadableLockShape` rejects these before + // any comparison — an empty map reaching `findSkew` is the fail-open path. + for (const lock of [undefined, null, {}, { packages: {} }]) + assert.equal(lockVersionsByPath(lock).size, 0); }); -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", +/** `crossInstallPackages`-shaped input: name → program → install → entry paths. */ +const occurrence = (name, program, byRoot) => + new Map([ + [ + name, + new Map([ + [ + program, + new Map( + Object.entries(byRoot).map(([dir, paths]) => [dir, new Set(paths)]), + ), + ], + ]), + ], ]); -}); -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", - ]); -}); +const lockPaths = (byDir) => + new Map( + Object.entries(byDir).map(([dir, entries]) => [ + dir, + new Map(Object.entries(entries)), + ]), + ); -test("importedPackageNames: triple-slash type references count (Copilot, #1962)", () => { - // A `/// ` 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 = [ - '/// ', - '/// ', - '/// ', - 'import { z } from "zod";', - ].join("\n"); - assert.deepEqual([...importedPackageNames(source)].sort(), [ - "@types/express", - "@types/node", - "express", - "node", - "zod", +test("findSkew: reports the copies one program loaded, with their paths", () => { + const { skewed, unresolved } = findSkew( + occurrence("zod", "clients/web/tsconfig.test.json", { + ".": ["node_modules/zod"], + "clients/web": ["node_modules/zod"], + }), + lockPaths({ + ".": { "node_modules/zod": "4.3.6" }, + "clients/web": { "node_modules/zod": "4.4.3" }, + }), + ); + assert.deepEqual(unresolved, []); + assert.deepEqual(skewed, [ + { + name: "zod", + occurrences: [ + { + program: "clients/web/tsconfig.test.json", + holders: [ + { dir: ".", entryPath: "node_modules/zod", version: "4.3.6" }, + { + dir: "clients/web", + entryPath: "node_modules/zod", + version: "4.4.3", + }, + ], + }, + ], + }, ]); }); -test("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("findSkew: only the installs that MET in a program are compared (Copilot, #1965 r1)", () => { + // `clients/cli` holds a different zod, but no program loads it beside another + // copy — nothing has to relate the two, so it is not a finding, and naming cli + // in a diagnostic about web's program would be wrong as well as noisy. + const { skewed } = findSkew( + occurrence("zod", "clients/web/tsconfig.test.json", { + ".": ["node_modules/zod"], + "clients/web": ["node_modules/zod"], + }), + lockPaths({ + ".": { "node_modules/zod": "4.4.3" }, + "clients/web": { "node_modules/zod": "4.4.3" }, + "clients/cli": { "node_modules/zod": "4.3.6" }, + }), + ); + assert.deepEqual(skewed, []); }); -test("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("findSkew: a nested copy is priced from its own entry (Copilot, #1965 r1)", () => { + // The root loaded zod through `a`'s nested copy. Pricing it from the root's + // TOP-LEVEL entry (4.4.3, aligned with web) would report the pair as agreeing. + const { skewed } = findSkew( + occurrence("zod", "clients/web/tsconfig.test.json", { + ".": ["node_modules/a/node_modules/zod"], + "clients/web": ["node_modules/zod"], + }), + lockPaths({ + ".": { + "node_modules/zod": "4.4.3", + "node_modules/a/node_modules/zod": "3.1.0", + }, + "clients/web": { "node_modules/zod": "4.4.3" }, + }), + ); + assert.deepEqual( + skewed[0].occurrences[0].holders.map((h) => `${h.dir}:${h.version}`), + [".:3.1.0", "clients/web:4.4.3"], + ); }); -test("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("findSkew: agreement is not skew", () => { + const { skewed, unresolved } = findSkew( + occurrence("zod", "p", { + ".": ["node_modules/zod"], + "clients/web": ["node_modules/zod"], + }), + lockPaths({ + ".": { "node_modules/zod": "4.4.3" }, + "clients/web": { "node_modules/zod": "4.4.3" }, + }), + ); + assert.deepEqual(skewed, []); + assert.deepEqual(unresolved, []); }); -test("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("findSkew: a copy with no lockfile entry is reported, not skipped", () => { + // Skipping it would drop a holder from the comparison and could report a real + // skew as agreement — the gate failing open. + const { skewed, unresolved } = findSkew( + occurrence("zod", "p", { + ".": ["node_modules/zod"], + "clients/web": ["node_modules/zod"], + }), + lockPaths({ ".": { "node_modules/zod": "4.4.3" }, "clients/web": {} }), + ); + assert.deepEqual(unresolved, [ + { name: "zod", dir: "clients/web", entryPath: "node_modules/zod" }, ]); + assert.deepEqual(skewed, []); }); -test("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("findSkew: results are sorted by package name", () => { + const found = new Map([ + ...occurrence("zod", "p", { + ".": ["node_modules/zod"], + "clients/web": ["node_modules/zod"], + }), + ...occurrence("hono", "p", { + ".": ["node_modules/hono"], + "clients/web": ["node_modules/hono"], + }), + ]); + const { skewed } = findSkew( + found, + lockPaths({ + ".": { "node_modules/zod": "1.0.0", "node_modules/hono": "1.0.0" }, + "clients/web": { + "node_modules/zod": "2.0.0", + "node_modules/hono": "2.0.0", + }, + }), + ); + assert.deepEqual( + skewed.map((s) => s.name), + ["hono", "zod"], + ); }); test("hasReadableLockShape: only a v2+ packages table with a root entry (Copilot, #1962)", () => { @@ -321,89 +279,32 @@ test("hasReadableLockShape: only a v2+ packages table with a root entry (Copilot assert.equal(hasReadableLockShape(lock), false, JSON.stringify(lock)); }); -test("findSkew: reports a package held at two versions", () => { - const installs = [ - { dir: ".", versions: new Map([["zod", "4.3.6"]]) }, - { dir: "clients/web", versions: new Map([["zod", "4.4.3"]]) }, - { dir: "clients/cli", versions: new Map([["zod", "4.4.3"]]) }, - ]; - assert.deepEqual(findSkew(new Set(["zod"]), installs), [ - { - name: "zod", - holders: [ - { dir: ".", version: "4.3.6" }, - { dir: "clients/web", version: "4.4.3" }, - { dir: "clients/cli", version: "4.4.3" }, - ], - }, - ]); -}); - -test("findSkew: agreement and single-install packages are not skew", () => { - const installs = [ - { - dir: ".", - versions: new Map([ - ["zod", "4.4.3"], - ["express", "5.2.1"], - ]), - }, - { dir: "clients/web", versions: new Map([["zod", "4.4.3"]]) }, - ]; - // `express` lives in one install only, so it cannot skew — a package absent - // from a client is not a finding. - assert.deepEqual(findSkew(new Set(["zod", "express"]), installs), []); -}); - -test("findSkew: a candidate in no lockfile is inert", () => { - // `@inspector/core` is a build-time alias, not a package; the scan picks it - // up and it must drop out here rather than error. - const installs = [ - { dir: ".", versions: new Map([["zod", "4.4.3"]]) }, - { dir: "clients/web", versions: new Map([["zod", "4.4.3"]]) }, - ]; - assert.deepEqual(findSkew(new Set(["@inspector/core"]), installs), []); -}); - -test("findSkew: results are sorted by package name", () => { - const installs = [ - { - dir: ".", - versions: new Map([ - ["zod", "1.0.0"], - ["hono", "1.0.0"], - ]), - }, - { - dir: "clients/web", - versions: new Map([ - ["zod", "2.0.0"], - ["hono", "2.0.0"], - ]), - }, - ]; - assert.deepEqual( - findSkew(new Set(["zod", "hono"]), installs).map((s) => s.name), - ["hono", "zod"], - ); -}); - test("partitionSkew: the allowlist is by name, not by version pair", () => { // So an ordinary patch float within a tolerated package does not churn the // allowlist, while any *unlisted* package that starts skewing still fails. const skewed = [ { name: "react", - holders: [ - { dir: ".", version: "19.2.7" }, - { dir: "clients/web", version: "19.2.8" }, + occurrences: [ + { + program: "p", + holders: [ + { dir: ".", version: "19.2.7" }, + { dir: "clients/web", version: "19.2.8" }, + ], + }, ], }, { name: "zod", - holders: [ - { dir: ".", version: "4.3.6" }, - { dir: "clients/web", version: "4.4.3" }, + occurrences: [ + { + program: "p", + holders: [ + { dir: ".", version: "4.3.6" }, + { dir: "clients/web", version: "4.4.3" }, + ], + }, ], }, ]; @@ -420,7 +321,14 @@ test("partitionSkew: the allowlist is by name, not by version pair", () => { }); test("partitionSkew: deny by default — nothing tolerated fails everything", () => { - const skewed = [{ name: "zod", holders: [{ dir: ".", version: "1.0.0" }] }]; + const skewed = [ + { + name: "zod", + occurrences: [ + { program: "p", holders: [{ dir: ".", version: "1.0.0" }] }, + ], + }, + ]; assert.equal(partitionSkew(skewed, new Map()).failures.length, 1); }); @@ -432,18 +340,28 @@ test("partitionSkew: the allowlist tolerates skew only within a major (Copilot, const withinMajor = [ { name: "react", - holders: [ - { dir: ".", version: "19.2.7" }, - { dir: "clients/web", version: "19.2.8" }, + occurrences: [ + { + program: "p", + holders: [ + { dir: ".", version: "19.2.7" }, + { dir: "clients/web", version: "19.2.8" }, + ], + }, ], }, ]; const acrossMajor = [ { name: "react", - holders: [ - { dir: ".", version: "18.3.1" }, - { dir: "clients/web", version: "19.2.8" }, + occurrences: [ + { + program: "p", + holders: [ + { dir: ".", version: "18.3.1" }, + { dir: "clients/web", version: "19.2.8" }, + ], + }, ], }, ]; @@ -458,9 +376,14 @@ test("partitionSkew: an unparseable version can't be proven same-major, so it fa const skewed = [ { name: "react", - holders: [ - { dir: ".", version: "19.2.7" }, - { dir: "clients/web", version: "next" }, + occurrences: [ + { + program: "p", + holders: [ + { dir: ".", version: "19.2.7" }, + { dir: "clients/web", version: "next" }, + ], + }, ], }, ]; @@ -479,13 +402,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); -}); diff --git a/scripts/verify-typecheck-coverage.mjs b/scripts/verify-typecheck-coverage.mjs index 20d6ab2cc..5293db6ec 100644 --- a/scripts/verify-typecheck-coverage.mjs +++ b/scripts/verify-typecheck-coverage.mjs @@ -47,6 +47,18 @@ import { rootRunsClientValidate, tokenize, } from "./lib/npm-scripts.mjs"; +// The `tsc --listFilesOnly` machinery lives in `lib/` because +// `verify-dep-lockstep` measures the same programs for a different question +// (#1965) — one implementation, so the two guards can't disagree about what a +// program contains. +import { + clientTsconfigReferences, + isDisablingFlag, + isTsc, + projectSourceFiles, + resolveLeafProjects, + typecheckProjects, +} from "./lib/tsc-program.mjs"; const repoRoot = path.resolve( path.dirname(fileURLToPath(import.meta.url)), @@ -70,15 +82,6 @@ const EXEMPT = new Map(); export const isRequiredSource = (rel) => /\.(ts|tsx|mts|cts)$/.test(rel) && !/\.d\.(ts|mts|cts)$/.test(rel); -// Match `tsc` by token basename so a path-invoked binary (`node_modules/.bin/ -// tsc`, `./node_modules/.bin/tsc.cmd`) counts, not just the bare `tsc` token. -export const isTsc = (t) => /(?:^|[\\/])tsc(?:\.(?:cmd|exe|ps1))?$/.test(t); - -// A flag that makes a `tsc` pass list files without type-checking them (so it -// gates nothing). Case-insensitive — tsc's own option parsing is. Shared by the -// `typecheck`-script path and the reference (`tsc -b`) path. -export const isDisablingFlag = (t) => /^--(noCheck|listFilesOnly)$/i.test(t); - /** * Whether a tracked test file is matched by one of the `test:scripts` command's * globs. Delegates to node's own `path.matchesGlob` (22.5.0+; the repo floor is @@ -256,51 +259,6 @@ export function testScriptProblems(scripts, testFiles) { return problems; } -/** - * The `references` paths declared in the tsconfig at repo-relative `tsconfigRel` - * (a `tsc -b` solution config), or `[]` if it has none / isn't readable. Paths - * are as written (relative to that tsconfig's own directory). - */ -export function parseTsconfigReferences(raw) { - try { - // Tolerate JSONC — block AND line comments + trailing commas (tsconfig - // allows all; block comments are in fact the style of every other tsconfig - // here). Block comments are stripped first so a `//` inside one doesn't - // survive; a `//` inside a string value (e.g. an `https://` URL) is a - // theoretical false strip this guard's tsconfigs never hit. - const cfg = JSON.parse( - raw - .replace(/\/\*[\s\S]*?\*\//g, "") - .replace(/\/\/.*$/gm, "") - .replace(/,(\s*[}\]])/g, "$1"), - ); - return Array.isArray(cfg.references) - ? cfg.references.map((r) => r?.path).filter((p) => typeof p === "string") - : []; - } catch { - return []; - } -} - -export function tsconfigReferences(tsconfigRel) { - try { - return parseTsconfigReferences( - readFileSync(path.join(repoRoot, tsconfigRel), "utf8"), - ); - } catch { - return []; // unreadable file (e.g. a directory / missing path) - } -} - -/** - * The `references` in a client's root `tsconfig.json`. Non-empty for a `tsc -b` - * client (like `clients/web`, which has no `typecheck` script) — this guard - * enrolls it through these instead of exempting the whole tree. - */ -export function clientTsconfigReferences(clientDir) { - return tsconfigReferences(path.posix.join(clientDir, "tsconfig.json")); -} - /** * How the client's `validate` runs `tsc -b` (the pass that typechecks a * reference client): `"ok"` (a real `tsc -b`), `"neutered"` (a `tsc -b` carrying @@ -388,64 +346,6 @@ function nodeClients() { return { clients, problems }; } -/** - * The tsconfig projects a client's `typecheck` names, harvested from **every** - * script reachable from `typecheck` (not just the one string) so a delegating - * `typecheck` (`npm run typecheck:src && …`) still counts — matching how - * `verify-format-coverage.mjs` harvests globs across reachable scripts. Splits - * each script on `&&`/`||`/`;` so a flag on one command doesn't leak onto - * another. Each `tsc` command's project comes from `-p`/`--project` (or a - * `-b`/`--build` path); a `tsc` command with **no** project flag resolves the - * implicit `./tsconfig.json` (tsc's own default), so that idiomatic form counts - * too. Returns `{ projects, neutered }`: `neutered` names any project whose own - * command carries `--noCheck`/`--nocheck` or `--listFilesOnly` (matched - * case-insensitively — tsc's option parsing is) — a pass that lists files - * without type-checking them, which would otherwise satisfy the guard while - * checking nothing. The config-file form (`noCheck` set in the tsconfig) is - * caught separately by {@link projectDisablesChecking}. - * - * A harvested `tsc -b` **solution config** (`"files": []` + `references`) lists - * nothing itself; {@link projectFiles} expands it to its references, so this - * form is measured whether it reaches here (a `typecheck: "tsc -b"`) or the - * dedicated reference path (`clients/web`, which declares no `typecheck`). - * - * Minor limitations, all unreachable with the plain `-p --noEmit` passes here: - * the implicit-`./tsconfig.json` fallback assumes **no file operands** (`tsc - * ` ignores the config and checks only that file, but would be credited - * the whole config's file list); the `--noCheck`/`--listFilesOnly` detection - * ignores a following boolean, so the contrived explicit `--noCheck false` - * (checking *on*) is still treated as disabling; and the `&&`/`||`/`;` split - * runs before tokenizing, so a quoted operator inside an arg would split - * mid-token (project paths carry none of those). - */ -export function typecheckProjects(scripts) { - const projects = []; - const neutered = []; - const isFlag = (t) => t.startsWith("-"); - const isProjectFlag = (t) => ["-p", "--project", "-b", "--build"].includes(t); - for (const name of reachableScripts(scripts, "typecheck")) { - const cmd = scripts?.[name]; - if (typeof cmd !== "string") continue; - for (const segment of cmd.split(/&&|\|\||;/)) { - const tokens = tokenize(segment); - if (!tokens.some(isTsc)) continue; // only tsc commands name projects - const disabling = tokens.find(isDisablingFlag); - // A project path follows `-p`/`--project`/`-b`/`--build`; a tsc command - // with none uses the implicit `./tsconfig.json` (tsc's own default). - const named = []; - for (let i = 0; i < tokens.length; i++) - if (isProjectFlag(tokens[i]) && tokens[i + 1] && !isFlag(tokens[i + 1])) - named.push(tokens[i + 1]); - if (named.length === 0) named.push("tsconfig.json"); - for (const project of named) { - if (disabling) neutered.push({ project, flag: disabling }); - else projects.push(project); - } - } - } - return { projects, neutered }; -} - /** * Whether the tsconfig `project` sets `noCheck` (which disables type-checking as * thoroughly as the CLI flag, but can't be seen in the `typecheck` script @@ -466,116 +366,11 @@ function projectDisablesChecking(clientDir, project) { } } -/** - * Repo-relative POSIX paths of the files ONE project (no reference expansion) - * typechecks. Absolute paths outside the repo root (lib.d.ts) and anything under - * `node_modules` are dropped; the aliased `core/` + `test-servers/` sources stay - * in the set but are harmless — the set is only ever queried with client-relative - * paths. Cached: `resolveLeafProjects` and `projectFiles` both list a project. - */ -const rawFilesCache = new Map(); -function rawProjectFiles(clientDir, project) { - const key = `${clientDir}|${project}`; - const cached = rawFilesCache.get(key); - if (cached) return cached; - const absClient = path.join(repoRoot, clientDir); - let stdout; - try { - stdout = execFileSync( - "npx", - ["--no-install", "tsc", "-p", project, "--listFilesOnly"], - { cwd: absClient, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }, - ); - } catch (err) { - // `--listFilesOnly` doesn't type-check, but a config error (an unreadable or - // malformed tsconfig) still exits non-zero while printing the resolved file - // list; keep stdout so a broken config doesn't mask a coverage gap. Echo the - // diagnostic — since this guard runs before any client's own `typecheck`, - // it's the first place a bad `-p` config surfaces, and without the reason - // the resulting "file in no project" report is misleading. tsc prints config - // errors (`error TS…`) to stdout, so scan both streams for them. - stdout = typeof err.stdout === "string" ? err.stdout : ""; - const streams = - stdout + "\n" + (typeof err.stderr === "string" ? err.stderr : ""); - const diagnostic = streams - .split("\n") - .filter((l) => /error TS\d+/.test(l)) - .join("\n") - .trim(); - console.warn( - `verify:typecheck-coverage — \`tsc -p ${project}\` (in ${clientDir}) exited non-zero:\n${diagnostic || "(no diagnostic captured)"}\n`, - ); - } - const covered = new Set(); - for (const line of stdout.split("\n")) { - const abs = line.trim(); - if (!abs) continue; - const rel = path.relative(repoRoot, abs); - if (rel.startsWith("..") || rel.includes("node_modules")) continue; - covered.add(rel.split(path.sep).join("/")); - } - rawFilesCache.set(key, covered); - return covered; -} - -/** - * The repo-relative tsconfig FILE a `clientDir`-relative `project` entry names. - * A directory-form entry (`{ "path": "./packages/a" }`, or `tsc -p src`) means - * `/tsconfig.json` — tsc's own rule. - */ -export function projectConfigFile(clientDir, project) { - const projectRel = path.posix.join(clientDir, project); - return projectRel.endsWith(".json") - ? projectRel - : path.posix.join(projectRel, "tsconfig.json"); -} - -/** - * A `references` entry `ref` (written relative to `fromConfigFile`'s own - * directory) as a `clientDir`-relative project path, the form the rest of the - * graph walk uses. - */ -export function refToProject(clientDir, fromConfigFile, ref) { - return path.posix.relative( - clientDir, - path.posix.join(path.posix.dirname(fromConfigFile), ref), - ); -} - -/** - * The leaf tsconfig projects `project` resolves to (paths relative to - * `clientDir`): itself if it lists files (or has no `references`), else its - * `references` expanded recursively. A `tsc -b` **solution config** (`{"files": - * [], "references": […]}`) lists nothing under `--listFilesOnly`, so this is how - * it's reduced to the real projects — and doing it here (not just inside - * `projectFiles`) is what lets BOTH coverage and the non-inertness check follow - * the same graph, so a `noCheck` in a *referenced* project is caught no matter - * which enrollment path harvested the solution. - */ -export function resolveLeafProjects(clientDir, project, seen = new Set()) { - if (seen.has(project)) return []; - seen.add(project); - // Lists files → a real leaf. (An empty set is a solution config, or a config - // that errored — either way the reference expansion below is the right next - // step: a broken config yields no references too.) - if (rawProjectFiles(clientDir, project).size > 0) return [project]; - const configFile = projectConfigFile(clientDir, project); - const refs = tsconfigReferences(configFile); - if (refs.length === 0) return [project]; // no files, no refs — itself - return refs.flatMap((ref) => - resolveLeafProjects( - clientDir, - refToProject(clientDir, configFile, ref), - seen, - ), - ); -} - /** Repo-relative files a project covers, following a solution config's references. */ function projectFiles(clientDir, project) { const covered = new Set(); for (const leaf of resolveLeafProjects(clientDir, project)) - for (const f of rawProjectFiles(clientDir, leaf)) covered.add(f); + for (const f of projectSourceFiles(clientDir, leaf)) covered.add(f); return covered; } diff --git a/scripts/verify-typecheck-coverage.test.mjs b/scripts/verify-typecheck-coverage.test.mjs index 22151c254..d2cca80a6 100644 --- a/scripts/verify-typecheck-coverage.test.mjs +++ b/scripts/verify-typecheck-coverage.test.mjs @@ -2,24 +2,22 @@ // module exposes these without running the guard (its execution is behind // `main()`, called only when the file is run directly). Each case pins a rule a // #1799 review round found. Run via `npm run test:scripts`. +// +// The `tsc`-script and tsconfig-graph parsers this guard used to own moved to +// `lib/tsc-program.mjs` when `verify:dep-lockstep` began measuring the same +// programs (#1965); their cases moved to `lib/tsc-program.test.mjs` with them. import { test } from "node:test"; import assert from "node:assert/strict"; import { matchesTestGlob, - isDisablingFlag, isRequiredSource, - isTsc, - parseTsconfigReferences, - projectConfigFile, - refToProject, integrityAdvice, isScriptsTestFile, testScriptGlobs, testScriptNarrowingFlags, testScriptProblems, tscBuildStatus, - typecheckProjects, } from "./verify-typecheck-coverage.mjs"; test("isRequiredSource: TS extensions, ambient .d.ts excluded (r7)", () => { @@ -29,83 +27,6 @@ test("isRequiredSource: TS extensions, ambient .d.ts excluded (r7)", () => { assert.ok(!isRequiredSource(f), f); }); -test("isTsc: matches by basename incl. path-invoked (r18 regression)", () => { - for (const t of [ - "tsc", - "node_modules/.bin/tsc", - "./node_modules/.bin/tsc.cmd", - ]) - assert.ok(isTsc(t), t); - for (const t of ["vitest", "prettier", "tscx", "atsc"]) - assert.ok(!isTsc(t), t); -}); - -test("isDisablingFlag: case-insensitive (r18)", () => { - for (const t of [ - "--noCheck", - "--nocheck", - "--listFilesOnly", - "--LISTFILESONLY", - ]) - assert.ok(isDisablingFlag(t), t); - for (const t of ["--noEmit", "-p", "--project", "noCheck"]) - assert.ok(!isDisablingFlag(t), t); -}); - -test("typecheckProjects: harvests -p / --project / -b, implicit tsconfig.json (r13)", () => { - const { projects, neutered } = typecheckProjects({ - typecheck: - "tsc --noEmit -p tsconfig.json && tsc --noEmit -p tsconfig.test.json", - }); - assert.deepEqual(projects, ["tsconfig.json", "tsconfig.test.json"]); - assert.equal(neutered.length, 0); - - // A bare `tsc` (no project flag) resolves the implicit ./tsconfig.json. - assert.deepEqual(typecheckProjects({ typecheck: "tsc --noEmit" }).projects, [ - "tsconfig.json", - ]); - - // Path-invoked binary still counts (r18). - assert.deepEqual( - typecheckProjects({ typecheck: "node_modules/.bin/tsc -p tsconfig.json" }) - .projects, - ["tsconfig.json"], - ); - - // A quoted project path (r17). - assert.deepEqual( - typecheckProjects({ typecheck: `tsc -p "tsconfig.test.json"` }).projects, - ["tsconfig.test.json"], - ); - - // `--project` long form, and `-b`/`--build` project paths (r13). - const proj = (cmd) => typecheckProjects({ typecheck: cmd }).projects; - assert.deepEqual(proj("tsc --noEmit --project tsconfig.json"), [ - "tsconfig.json", - ]); - assert.deepEqual(proj("tsc -b tsconfig.json"), ["tsconfig.json"]); - assert.deepEqual(proj("tsc --build tsconfig.json"), ["tsconfig.json"]); - assert.deepEqual(proj("tsc -b"), ["tsconfig.json"]); // implicit fallback -}); - -test("typecheckProjects: neutered by --noCheck / --listFilesOnly (r10)", () => { - const { projects, neutered } = typecheckProjects({ - typecheck: - "tsc --noEmit -p tsconfig.json --noCheck && tsc --noEmit -p tsconfig.test.json", - }); - assert.deepEqual(projects, ["tsconfig.test.json"]); - assert.deepEqual(neutered, [{ project: "tsconfig.json", flag: "--noCheck" }]); -}); - -test("typecheckProjects: delegating typecheck, ignores non-tsc segments (r15)", () => { - const { projects } = typecheckProjects({ - typecheck: "npm run typecheck:src && npm run typecheck:test", - "typecheck:src": "tsc --noEmit -p tsconfig.json", - "typecheck:test": "tsc --noEmit --project tsconfig.test.json", - }); - assert.deepEqual(projects.sort(), ["tsconfig.json", "tsconfig.test.json"]); -}); - test("tscBuildStatus: ok / neutered / none (r25)", () => { const status = (build) => tscBuildStatus({ validate: "npm run build", build }); @@ -116,22 +37,6 @@ test("tscBuildStatus: ok / neutered / none (r25)", () => { assert.equal(status("tsc --noEmit -p tsconfig.json"), "none"); // -b required }); -test("parseTsconfigReferences: JSONC tolerance (r17-nit2 block comments)", () => { - const refs = (raw) => parseTsconfigReferences(raw); - assert.deepEqual(refs('{ "references": [{ "path": "./a" }] }'), ["./a"]); - assert.deepEqual( - refs('/* solution */\n{ "references": [{ "path": "./a" }] }'), - ["./a"], - ); - assert.deepEqual(refs('{ "references": [{ "path": "./a" }] } // trailing'), [ - "./a", - ]); - assert.deepEqual(refs('{ "references": [{ "path": "./a" },] }'), ["./a"]); // trailing comma - assert.deepEqual(refs('{ "files": [] }'), []); // no references - assert.deepEqual(refs("{ not json"), []); // malformed - assert.deepEqual(refs('{ "references": [{ "prepend": true }] }'), []); // no path -}); - test("matchesTestGlob: the guard's contract, not node's glob engine", () => { // Only the two properties the guard actually relies on — the rest of node's // glob semantics are node's to test, which is the point of delegating to it. @@ -386,42 +291,3 @@ test("integrityAdvice: typecheck footer only for typecheck issues (r35 finding 2 /Restore the `typecheck` wiring/, ); }); - -test("projectConfigFile: directory-form entry means /tsconfig.json (r26)", () => { - assert.equal( - projectConfigFile("clients/cli", "tsconfig.test.json"), - "clients/cli/tsconfig.test.json", - ); - assert.equal( - projectConfigFile("clients/cli", "packages/a"), - "clients/cli/packages/a/tsconfig.json", - ); - assert.equal( - projectConfigFile("clients/cli", "."), - "clients/cli/tsconfig.json", - ); -}); - -test("refToProject: refs resolve against the REFERRING config's dir (r26)", () => { - // A ref is relative to the tsconfig that declares it, not to clientDir. - assert.equal( - refToProject( - "clients/web", - "clients/web/tsconfig.json", - "./tsconfig.app.json", - ), - "tsconfig.app.json", - ); - assert.equal( - refToProject( - "clients/web", - "clients/web/sub/tsconfig.json", - "../other.json", - ), - "other.json", - ); - assert.equal( - refToProject("clients/web", "clients/web/sub/tsconfig.json", "./deep"), - "sub/deep", - ); -});