diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 0bc70bfc..32122205 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 de3945eb..eac73b7f 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 themselves still come from the committed lockfiles' **top-level** `node_modules/` entries — a *nested* transitive duplicate inside one install is routine and deliberately ignored — and any candidate held at two versions fails, **deny-by-default**. The escape hatch is `TOLERATED_SKEW` in that file, an allowlist of *names* (not version pairs, so an ordinary patch float doesn't churn it), each entry carrying why that package's types can't blow up; it is **empty today** — the four names it used to carry (`react`, `hono`, `jose`, `@modelcontextprotocol/ext-apps`) were admitted under the old derivation and none is a candidate under this one, so each would be a rationale for a skew that cannot occur. **Being listed is not a blanket exemption** — it tolerates skew only *within a major version*, since a rationale about patch-level differences says nothing about a React 18-vs-19 split, where the type surface itself changes; a cross-major skew fails even for a listed package. **When bumping a dependency that the shared sources pull in, bump it in every install that declares it** — that's the root plus whichever clients list it, not all four unconditionally (launcher declares no zod, for instance, and a package absent from an install can't skew, so the guard ignores it there). Don't add a dependency to a client just to satisfy this. Its pure helpers are unit-tested via `test:scripts` (its own, plus `scripts/lib/tsc-program.test.mjs` for the shared derivation), and it vouches — with `verify:format-coverage` and `verify:typecheck-coverage` — that its siblings are still wired into `validate`. It runs the same `tsc --listFilesOnly` pass its sibling does, in its own process, costing ~14s: the listing is deliberately **not** cached to disk between the two, because a fingerprint that missed an input would make a guard measure a program that no longer exists and pass on a real miss. - **`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 467a4fc0..0bca3291 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). Compares those against the committed lockfiles' top-level entries and **fails deny-by-default** on any skew not in the annotated `TOLERATED_SKEW` allowlist — empty today — and an allowlisted package is tolerated only *within a major version*. Runs in `validate`. | `npm run 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 00000000..189c13df --- /dev/null +++ b/scripts/lib/tsc-program.mjs @@ -0,0 +1,382 @@ +// 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; +} + +/** + * The install a repo-relative `node_modules` path belongs to, and the package it + * holds — `{ installRoot, name }`, or null for a path that names neither. + * + * The install root is the prefix before the **first** `node_modules` segment + * (`.` for the repo root, `clients/web` for a client install), while the package + * name comes from after the **last** one. That asymmetry is deliberate: a + * *nested* `node_modules/a/node_modules/b` is npm resolving a transitive + * conflict **inside one install**, which is routine and not a cross-install + * skew — the same reason the lockfile comparison reads only top-level entries. + * Folding it onto its outermost install is what keeps the candidate set the + * handful of genuinely install-crossing packages instead of every transitive + * duplicate in the tree. + */ +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 name = head.startsWith("@") + ? 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, + }; +} + +/** + * 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 a Map keyed by + * package name, each value `{ installRoots, programs }` naming where it was seen + * — the diagnostic a failure needs. + * + * 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) { + const roots = new Map(); // name -> Set(installRoot), within THIS program + for (const rel of files) { + const hit = classifyModulePath(rel); + if (!hit) continue; + if (!roots.has(hit.name)) roots.set(hit.name, new Set()); + roots.get(hit.name).add(hit.installRoot); + } + for (const [name, installRoots] of roots) { + if (installRoots.size < 2) continue; + if (!found.has(name)) + found.set(name, { installRoots: new Set(), programs: new Set() }); + const entry = found.get(name); + for (const root of installRoots) entry.installRoots.add(root); + entry.programs.add(label); + } + } + return found; +} diff --git a/scripts/lib/tsc-program.test.mjs b/scripts/lib/tsc-program.test.mjs new file mode 100644 index 00000000..fa9bb9e1 --- /dev/null +++ b/scripts/lib/tsc-program.test.mjs @@ -0,0 +1,259 @@ +// 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", + }); + assert.deepEqual( + classifyModulePath("clients/web/node_modules/zod/index.d.ts"), + { installRoot: "clients/web", name: "zod" }, + ); + // A scoped package keeps both segments. + assert.deepEqual( + classifyModulePath("node_modules/@modelcontextprotocol/sdk/dist/x.d.ts"), + { installRoot: ".", name: "@modelcontextprotocol/sdk" }, + ); + // A NESTED copy folds onto its outermost install: npm resolving a transitive + // conflict inside one install is routine, not a cross-install skew, and the + // lockfile comparison ignores it for the same reason. + assert.deepEqual( + classifyModulePath("node_modules/a/node_modules/zod/index.d.ts"), + { installRoot: ".", name: "zod" }, + ); + assert.deepEqual( + classifyModulePath( + "clients/cli/node_modules/a/node_modules/@scope/b/index.d.ts", + ), + { installRoot: "clients/cli", name: "@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"]); + assert.deepEqual([...found.get("zod").installRoots].sort(), [ + ".", + "clients/web", + ]); + assert.deepEqual( + [...found.get("zod").programs], + ["clients/web/tsconfig.test.json"], + ); +}); + +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").programs].sort(), [ + "web/app", + "web/test", + ]); +}); diff --git a/scripts/verify-dep-lockstep.main.test.mjs b/scripts/verify-dep-lockstep.main.test.mjs index c00379d7..eb410167 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,7 +37,7 @@ 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", @@ -52,11 +59,24 @@ 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, +} = {}) { // 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 +101,91 @@ 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( - "test-servers/src/server.ts", - 'import express from "express";\nexport const app = express;\n', + "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( - "vitest.shared.mts", - 'import path from "node:path";\nexport default path;\n', + "clients/web/src/main.ts", + 'import type { Outer } from "outer";\nexport type FromWeb = Outer;\n', ); - write("package-lock.json", lock(rootDeps)); - write("clients/web/package.json", { name: "web" }); - write("clients/web/package-lock.json", rawWebLock ?? lock(webDeps)); + write("clients/web/package.json", { + name: "web", + scripts: webScripts ?? { typecheck: "tsc --noEmit -p tsconfig.json" }, + }); + write( + "clients/web/tsconfig.json", + webTsconfig ?? { + compilerOptions: { + noEmit: true, + module: "esnext", + target: "esnext", + moduleResolution: "bundler", + types: [], + }, + include: ["src/**/*.ts", "../../core/**/*.ts"], + }, + ); + + // 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"); + + write("package-lock.json", lock(root)); + write("clients/web/package-lock.json", rawWebLock ?? lock(web)); // 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 +211,110 @@ 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+\(\.\)/); + assert.match(out, /1\.2\.3\s+\(clients\/web\)/); + // The program that saw both copies is named, so the claim is checkable. + assert.match(out, /seen in clients\/web\/tsconfig\.json/); + // `inner` agrees, so it must not be reported. + assert.doesNotMatch(out, /^\s+inner$/m); }, ); }); -test("main: a package imported only by test-servers/src is still checked", () => { - // Guards the enumeration of the *second* shared tree: if only `core/` were - // scanned, this skew would pass. +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, express: "5.0.0" }, webDeps: ALIGNED }, + { + 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, /\bexpress\b/); + assert.match(out, /^\s+inner$/m); + assert.match(out, /4\.0\.0\s+\(\.\)/); + assert.match(out, /4\.5\.6\s+\(clients\/web\)/); }, ); }); -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: 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: 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, /matched no tracked file/); - assert.match(out, /^\s+core$/m); + 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, /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 +331,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 +348,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 +370,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 +383,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 27b5812c..3fe3a40e 100644 --- a/scripts/verify-dep-lockstep.mjs +++ b/scripts/verify-dep-lockstep.mjs @@ -20,210 +20,89 @@ // `@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; -} - -/** - * 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. - * - * Both candidates are returned because the directive name is the *type* name, - * not the package: `node` resolves to `@types/node`, while a package shipping - * its own declarations resolves to itself. Returning both over-approximates, - * which is the safe direction — whichever isn't installed drops out. A scoped - * name mangles as `@scope/pkg` → `@types/scope__pkg`, TypeScript's convention. - */ -export function typeReferencePackageNames(directive) { - const name = packageNameOf(directive); - if (!name) return []; - const scoped = /^@([^/]+)\/(.+)$/.exec(name); - const typesName = scoped - ? `@types/${scoped[1]}__${scoped[2]}` - : `@types/${name}`; - return [name, typesName]; -} - -/** - * Every third-party package name whose declarations a blob of TypeScript source - * pulls in — via an import of any form, or a triple-slash type reference. - * Over-approximating is safe (a name absent from every lockfile contributes - * nothing downstream — `@inspector/core` is a build-time alias, not a package, - * and drops out that way); under-approximating is not, since a missed package - * never enters the candidate set and its skew would pass silently. - */ -export function importedPackageNames(source) { - const names = new Set(); - // (source, readImportFiles, detectJavaScriptImports) — the latter two make it - // report `require(…)` and dynamic imports as well as static ones. - const { importedFiles, typeReferenceDirectives } = - typescript().preProcessFile(source, true, true); - for (const { fileName } of importedFiles) { - const name = packageNameOf(fileName); - if (name) names.add(name); - } - for (const { fileName } of typeReferenceDirectives ?? []) - for (const name of typeReferencePackageNames(fileName)) names.add(name); - return names; -} +// • `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(); /** * 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. + * one install, which is routine and not what this guard is about (the same + * asymmetry `classifyModulePath` applies to a resolved path). */ export function topLevelLockVersions(lock) { const versions = new Map(); @@ -311,84 +190,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]; -} - -/** 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); +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(); } /** - * 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`). + * 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() { - 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")), - ), - ); + return ["."].concat(clientDirs()); +} + +/** + * 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 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,29 +330,20 @@ 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 candidates = new Set(found.keys()); const dirs = installDirs(); @@ -510,12 +420,15 @@ export function main() { ); for (const { dir, version } of holders) console.error(` ${version} (${dir})`); + // Which program saw both copies is the whole reason the package is a + // candidate — without it the reader has no way to check the claim. + for (const label of [...found.get(name).programs].sort()) + console.error(` seen in ${label}`); } - 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 +447,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: ${candidates.size} install-crossing dependencies agree across ${installs.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 55ca3b6f..25a643ab 100644 --- a/scripts/verify-dep-lockstep.test.mjs +++ b/scripts/verify-dep-lockstep.test.mjs @@ -3,258 +3,61 @@ // change that relaxes one is visible as a deleted assertion rather than a quiet // behavior shift. Run via `npm run test:scripts` (node:test; the root has no // vitest harness). +// +// The candidate derivation itself lives in `lib/tsc-program.mjs` (shared with +// `verify:typecheck-coverage` since #1965) and is covered by +// `lib/tsc-program.test.mjs`; what stays here is the lockfile comparison and the +// client-enrollment rule this guard owns. import { test } from "node:test"; import assert from "node:assert/strict"; import { + clientProjects, findSkew, hasReadableLockShape, - importedPackageNames, - isSharedSourceFile, majorOf, - packageNameOf, partitionSkew, - sourcesWithNoFiles, topLevelLockVersions, - typeReferencePackageNames, } from "./verify-dep-lockstep.mjs"; -test("isSharedSourceFile: all four TS extensions, not just .ts/.tsx", () => { - // `.mts`/`.cts` are gated by `verify:format-coverage` and - // `verify:typecheck-coverage` too. None exist under the shared trees today, - // so dropping them here would go unnoticed until a new shared dependency - // arrived through one — the skew this guard exists to catch (Copilot, #1962). - for (const ext of [".ts", ".tsx", ".mts", ".cts"]) { - assert.equal(isSharedSourceFile(`core/mcp/thing${ext}`), true, ext); - assert.equal( - isSharedSourceFile(`test-servers/src/thing${ext}`), - true, - `test-servers ${ext}`, - ); - } -}); - -test("isSharedSourceFile: non-TS files and other trees are excluded", () => { - const rejected = [ - "core/README.md", // not TypeScript - "core/mcp/data.json", - "clients/web/src/App.tsx", // a client's own sources resolve from the client - "scripts/verify-dep-lockstep.mjs", - "test-servers/configs/modern-http.json", - // Path-boundary anchoring: a sibling dir whose name merely starts with a - // shared dir's name must not be swept in. - "core-internal/thing.ts", - "test-servers/src-legacy/thing.ts", - ]; - for (const file of rejected) - assert.equal(isSharedSourceFile(file), false, file); -}); - -test("sourcesWithNoFiles: each configured source must contribute (Copilot, #1962)", () => { - const dirs = ["core", "test-servers/src"]; - const named = ["vitest.shared.mts"]; - const complete = [ - "core/mcp/a.ts", - "test-servers/src/b.ts", - "vitest.shared.mts", - ]; - assert.deepEqual(sourcesWithNoFiles(complete, dirs, named), []); - - // The failure an aggregate count can't see: `core/` moved, but the other two - // sources keep `files.length` nonzero, so the guard would derive candidates - // from an incomplete set and pass on skew it should catch. +test("clientProjects: a `typecheck` script's projects win over references", () => { + // Both enrollment paths exist (cli/tui/launcher declare `typecheck`, + // `clients/web` is a `tsc -b` solution) and this guard must measure the same + // programs as `verify:typecheck-coverage`, which prefers the script. assert.deepEqual( - sourcesWithNoFiles( - ["test-servers/src/b.ts", "vitest.shared.mts"], - dirs, - named, + clientProjects( + { + typecheck: "tsc --noEmit -p tsconfig.json && tsc -p tsconfig.test.json", + }, + ["./ignored.json"], ), - ["core"], + ["tsconfig.json", "tsconfig.test.json"], ); +}); + +test("clientProjects: a reference client is measured through its references", () => { assert.deepEqual( - sourcesWithNoFiles(["core/mcp/a.ts", "test-servers/src/b.ts"], dirs, named), - ["vitest.shared.mts"], + clientProjects({ build: "tsc -b && vite build" }, ["./a.json"]), + ["./a.json"], ); - assert.deepEqual(sourcesWithNoFiles([], dirs, named), [ - "core", - "test-servers/src", - "vitest.shared.mts", - ]); + // Neither path available — the caller reports it rather than measuring nothing. + assert.deepEqual(clientProjects({ build: "vite build" }, []), []); }); -test("sourcesWithNoFiles: a prefix sibling does not vouch for a dir", () => { - // `core-internal/` starts with `core` but is not it — the boundary check has - // to be on a path separator, or a renamed dir would look present. - assert.deepEqual(sourcesWithNoFiles(["core-internal/a.ts"], ["core"], []), [ - "core", - ]); -}); - -test("packageNameOf: bare names, scopes, and subpaths", () => { - const cases = [ - ["zod", "zod"], - ["zod/v4", "zod"], // subpath dropped — one package, one version - ["@modelcontextprotocol/client", "@modelcontextprotocol/client"], - ["@modelcontextprotocol/client/core", "@modelcontextprotocol/client"], - ["react-dom/client", "react-dom"], - ]; - for (const [input, expected] of cases) - assert.equal(packageNameOf(input), expected, input); -}); - -test("packageNameOf: non-packages are rejected", () => { - // Relative/absolute paths, built-ins with and without the `node:` prefix, - // protocol specifiers, and prose that follows the word `from` in a comment. - const rejected = [ - "./foo", - "../core/mcp", - "/abs/path", - "fs", - "path", - "node:crypto", - "node:test", - "file:", - "data:text/plain,x", - "cwd omitted", - "", - ]; - for (const input of rejected) - assert.equal(packageNameOf(input), null, JSON.stringify(input)); - assert.equal(packageNameOf(undefined), null); -}); - -test("importedPackageNames: CommonJS and awkward dynamic-import forms (Copilot, #1962)", () => { - // Under-approximating is the dangerous direction: a package the scan misses - // never enters the candidate set, so its skew passes the guard silently. - // `.cts` sources in the shared trees use `import x = require(…)` as ordinary - // syntax, and a dynamic import may carry import attributes or a static - // template literal — none of which the original three patterns matched. - const source = ` - import express = require("express"); - const yaml = require("yaml"); - const a = await import("undici", { with: { type: "json" } }); - const b = await import(\`jose\`); - `; - assert.deepEqual([...importedPackageNames(source)].sort(), [ - "express", - "jose", - "undici", - "yaml", - ]); -}); - -test("importedPackageNames: comment trivia between tokens (Copilot, #1962)", () => { - // TypeScript allows a comment anywhere whitespace is legal, so all of these - // are valid imports. Missing one is the dangerous direction: the package - // never enters the candidate set and its skew passes the guard silently. - const source = ` - import { a } from /* explanation */ "express"; - const b = await import(/* webpackIgnore: true */ "undici"); - const c = require(/* lazy */ "yaml"); - import /* side effect */ "pino"; - `; - assert.deepEqual([...importedPackageNames(source)].sort(), [ - "express", - "pino", - "undici", - "yaml", - ]); -}); - -test("importedPackageNames: line-comment trivia, not just block (Copilot, #1962)", () => { - // `//` runs to end-of-line and is legal in every position a block comment is, - // so a specifier can sit on the next line and these are still valid imports. - const source = [ - "import { a } from // reason", - ' "express";', - "const b = await import(// lazy", - ' "undici");', - "const c = require(// lazy", - ' "yaml");', - ].join("\n"); - assert.deepEqual([...importedPackageNames(source)].sort(), [ - "express", - "undici", - "yaml", - ]); -}); - -test("importedPackageNames: static, side-effect, and dynamic forms; builtins and relatives dropped", () => { - const source = ` - import { z } from "zod/v4"; - export type { Foo } from '@modelcontextprotocol/core'; - import "./side-effect.css"; - import "pino"; - const mod = await import("chokidar"); - import fs from "node:fs"; - import { helper } from "../local/helper"; - `; - assert.deepEqual([...importedPackageNames(source)].sort(), [ - "@modelcontextprotocol/core", - "chokidar", - "pino", - "zod", - ]); -}); - -test("importedPackageNames: triple-slash type references count (Copilot, #1962)", () => { - // A `/// ` 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("typeReferencePackageNames: both the bare and the @types form (Copilot, #1962)", () => { - // The directive names a *type*, not a package: `node` resolves to - // `@types/node`, while a package shipping its own declarations resolves to - // itself. Returning both over-approximates, the safe direction — whichever - // isn't installed drops out downstream. - assert.deepEqual(typeReferencePackageNames("node"), ["node", "@types/node"]); - // Scoped names mangle with a double underscore, TypeScript's convention. - assert.deepEqual(typeReferencePackageNames("@scope/pkg"), [ - "@scope/pkg", - "@types/scope__pkg", - ]); - assert.deepEqual(typeReferencePackageNames("./relative"), []); - assert.deepEqual(typeReferencePackageNames(""), []); -}); - -test("importedPackageNames: prose in comments never becomes a package (Copilot, #1962)", () => { - // The regex scan this replaced could not tell code from a comment, so - // `// adapted from "react"` added `react` to the candidate set — and if that - // installed package were skewed, an unrelated comment would fail `validate`. - // These use REAL package names, which is the case the old prose test missed: - // it only passed because `cwd omitted` isn't a valid package name. - const source = ` - // adapted from "react" - /** Mirrors the behavior of "express", see require("yaml") below. */ - /** The excluded set derived from \\\`hono\\\`-style paths. */ - // const disabled = await import("undici"); - import { z } from "zod"; - `; - assert.deepEqual([...importedPackageNames(source)], ["zod"]); -}); - -test("importedPackageNames: a specifier inside a string literal is not an import", () => { - const source = ` - const msg = 'run require("chokidar") to load it'; - const re = /"jose"/; - import { z } from "zod"; - `; - assert.deepEqual([...importedPackageNames(source)], ["zod"]); +test("clientProjects: a NEUTERED project still contributes its program (#1965)", () => { + // `--noCheck` stops that pass type-checking, which is the sibling guard's + // complaint; the program still resolves its imports, and dropping it here + // would shrink what THIS guard measures on the strength of that other defect. + assert.deepEqual( + clientProjects( + { + typecheck: + "tsc -p tsconfig.json --noCheck && tsc -p tsconfig.test.json", + }, + [], + ).sort(), + ["tsconfig.json", "tsconfig.test.json"], + ); }); test("topLevelLockVersions: nested duplicates are ignored", () => { @@ -479,13 +282,3 @@ test("majorOf: prerelease and build metadata are irrelevant", () => { for (const bad of ["next", "", undefined, null, "v4.4.3"]) assert.equal(majorOf(bad), null, JSON.stringify(bad)); }); - -test("isSharedSourceFile: individually-named shared files are included (Copilot, #1962)", () => { - // `vitest.shared.mts` is root-owned, imported by every client's vitest - // config, and already treated as shared by `verify:typecheck-coverage`. It - // imports only Node built-ins today, which is why omitting it would go - // unnoticed until a third-party import appeared there and skewed. - assert.equal(isSharedSourceFile("vitest.shared.mts"), true); - // Still anchored: a same-named file nested elsewhere is not the shared one. - assert.equal(isSharedSourceFile("clients/web/vitest.shared.mts"), false); -}); diff --git a/scripts/verify-typecheck-coverage.mjs b/scripts/verify-typecheck-coverage.mjs index 20d6ab2c..5293db6e 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 22151c25..d2cca80a 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", - ); -});