diff --git a/.claude/skills/local-dev/SKILL.md b/.claude/skills/local-dev/SKILL.md index c22bf6a9..168a1b2c 100644 --- a/.claude/skills/local-dev/SKILL.md +++ b/.claude/skills/local-dev/SKILL.md @@ -111,10 +111,16 @@ Keep two distinctions straight, because AGENTS.md's rules split on them: `external` lists. - **Root-declared is not the same as aliased.** The `vitest.shared.mts` pins and the `clients/web/tsconfig.*.json` `paths` cover the packages whose resolution - is genuinely ambiguous — a transitive copy sits in the client install, or the - importer is `core/`, which has no manifest to resolve from. A package that - resolves unambiguously by walk-up (`ajv`, `commander`, `open`, `undici`, - `zod`) is deliberately absent from both. + is genuinely ambiguous, which is two different situations: the importer is + `core/`, which has no manifest to resolve from, **or** a copy still sits in + the client install and would win. The second case is the one that is easy to + get wrong — `chokidar` (under `vite`), `zod` (under + `eslint-plugin-react-hooks`), `open` (under Storybook) and `react` (a peer of + `react-dom` and `ink`) are all present in a client's `node_modules` without + being declared there, so "we deleted the declaration" is not the same as "it + resolves from the root now". **Check the install, don't reason from the + manifest.** `ajv`, `commander` and `undici` carry no pin because no client + install has a copy of them at all. The point of deleting the client-side copies rather than merely keeping them in step is that **a package installs only into an install root that declares it**. diff --git a/clients/web/server/start-vite-dev-server.ts b/clients/web/server/start-vite-dev-server.ts index 91a7fab7..18299b19 100644 --- a/clients/web/server/start-vite-dev-server.ts +++ b/clients/web/server/start-vite-dev-server.ts @@ -36,8 +36,9 @@ export async function startViteDevServer( // `server.fs.allow` here. Without them, App.tsx's `@inspector/core/*` // imports fail to resolve and the page 500s (#1452 smoke test). The aliases // and dedupe are factored into `vitest.shared.mts` so both paths stay in - // sync — pass the client dir (`root`) so bare-module pins resolve against - // `clients/web/node_modules`. + // sync — pass the client dir (`root`), which is what the `react` / + // `react-dom` pins resolve against. The root-declared packages resolve from + // the repo root instead, which that file derives from the same argument. const { repoRoot, sharedAliases, sharedDedupe, nodeModulesAliases } = vitestSharedPaths(root); const inlineConfig: InlineConfig = { diff --git a/clients/web/tsconfig.app.json b/clients/web/tsconfig.app.json index 57d211f0..4eaa1d8e 100644 --- a/clients/web/tsconfig.app.json +++ b/clients/web/tsconfig.app.json @@ -57,6 +57,9 @@ "atomically": ["../../node_modules/atomically"], "chokidar": ["../../node_modules/chokidar"], "@napi-rs/keyring": ["../../node_modules/@napi-rs/keyring"], + "zod": ["../../node_modules/zod"], + "zod/v4": ["../../node_modules/zod/v4"], + "open": ["../../node_modules/open"], "react-syntax-highlighter/dist/esm/prism-light": [ "./src/types/react-syntax-highlighter-prism-light" ], diff --git a/clients/web/tsconfig.node.json b/clients/web/tsconfig.node.json index 30702aee..f7f26f50 100644 --- a/clients/web/tsconfig.node.json +++ b/clients/web/tsconfig.node.json @@ -55,7 +55,10 @@ "@hono/node-server": ["../../node_modules/@hono/node-server"], "atomically": ["../../node_modules/atomically"], "chokidar": ["../../node_modules/chokidar"], - "@napi-rs/keyring": ["../../node_modules/@napi-rs/keyring"] + "@napi-rs/keyring": ["../../node_modules/@napi-rs/keyring"], + "zod": ["../../node_modules/zod"], + "zod/v4": ["../../node_modules/zod/v4"], + "open": ["../../node_modules/open"] } }, "include": ["vite.config.ts", "tsup.runner.config.ts", "server/**/*.ts"] diff --git a/clients/web/tsconfig.test.json b/clients/web/tsconfig.test.json index 6d8fd810..e12c27bd 100644 --- a/clients/web/tsconfig.test.json +++ b/clients/web/tsconfig.test.json @@ -21,6 +21,9 @@ "express": ["./node_modules/@types/express"], "yaml": ["./node_modules/yaml"], "@napi-rs/keyring": ["../../node_modules/@napi-rs/keyring"], + "zod": ["../../node_modules/zod"], + "zod/v4": ["../../node_modules/zod/v4"], + "open": ["../../node_modules/open"], "react-syntax-highlighter/dist/esm/prism-light": [ "./src/types/react-syntax-highlighter-prism-light" ], diff --git a/clients/web/vite.config.ts b/clients/web/vite.config.ts index 95ef925e..65311ba6 100644 --- a/clients/web/vite.config.ts +++ b/clients/web/vite.config.ts @@ -143,8 +143,12 @@ export default defineConfig(({ command }) => { // (#1244), the browser dep graph reached bare-module subpaths in core/ // that Rolldown couldn't resolve against `core/`'s parent (it has no // node_modules of its own). Promote the same bare-module aliases the - // vitest projects use so `vite dev` / `vite build` can resolve them - // from `clients/web/node_modules`. + // vitest projects use so `vite dev` / `vite build` can resolve them. + // + // Which install each alias points at is `vitest.shared.mts`'s decision, + // not this file's, and since #2195 it is no longer one answer: `react` + // and `react-dom` resolve from `clients/web/node_modules`, everything + // root-declared resolves from the repo root. alias: [ ...Object.entries(sharedAliases).map(([find, replacement]) => ({ find, @@ -153,9 +157,9 @@ export default defineConfig(({ command }) => { ...nodeModulesAliases, ], // Source files in core/ import bare modules (react, @testing-library/react, - // etc.) that only exist in clients/web/node_modules. Dedupe ensures Vite - // resolves them from this package rather than walking up from core/'s - // location (which has no node_modules of its own yet). + // etc.) that core/ cannot resolve for itself, having no node_modules of + // its own. Dedupe collapses each of these to a single copy per install, + // whichever install the alias above selected. dedupe: sharedDedupe, }, // Pin the Vite dev server to the same port and host the Hono plugin @@ -274,10 +278,11 @@ export default defineConfig(({ command }) => { { extends: true, // Vitest projects don't inherit `resolve` from the parent. The unit - // project runs from repoRoot (so vitest's coverage transformer can - // reach core/), but repoRoot has no node_modules of its own — the - // shared regex aliases redirect bare `react`/`pino`/etc. imports - // from core/ back into clients/web/node_modules. + // project runs from repoRoot so vitest's coverage transformer can + // reach core/, which leaves core/'s own bare imports with nothing to + // resolve against — the shared regex aliases are what answer them, + // sending `react` to clients/web/node_modules and the root-declared + // packages (`pino`, `hono`, …) to the repo root (#2195). resolve: projectResolve, test: { name: "unit", @@ -331,8 +336,9 @@ export default defineConfig(({ command }) => { extends: true, // See note on the unit project: integration tests also run from // repoRoot and import core/ modules, so they need the same alias - // setup. The shared bare-module aliases keep `pino`, `hono`, etc. - // resolving against clients/web/node_modules. + // setup, with the same split — `react` from this client's install, + // `pino` / `hono` and the rest of the root-declared set from the + // repo root. resolve: projectResolve, test: { name: "integration", diff --git a/vitest.shared.mts b/vitest.shared.mts index 6f854312..8c7883f0 100644 --- a/vitest.shared.mts +++ b/vitest.shared.mts @@ -24,6 +24,10 @@ export function vitestSharedPaths(clientDir: string) { "react-dom", "@modelcontextprotocol/client", "@modelcontextprotocol/core", + // Every SDK schema is a zod type, so a second copy breaks `instanceof` + // across the whole surface. The alias below picks the install; this keeps + // it to one copy within it. + "zod", ]; const nodeModulesAliases = [ @@ -131,6 +135,32 @@ export function vitestSharedPaths(clientDir: string) { find: /^@napi-rs\/keyring$/, replacement: path.resolve(repoRoot, "node_modules/@napi-rs/keyring"), }, + // `zod` and `open` are pinned for a different reason from everything above: + // they resolve *somewhere* without help, and the somewhere is wrong. Both + // still sit at the top level of `clients/web/node_modules` as transitive + // copies — zod under `eslint-plugin-react-hooks`, open under Storybook — + // so an unpinned bare import from a web test resolves the client copy while + // `core/` and the SDK packages resolve the root's (Copilot). + // + // For `open` that is merely wasteful. For `zod` it is the hazard this file + // exists for: two copies in one process means a schema built by one and an + // `instanceof` check made by the other, across the entire + // `@modelcontextprotocol/*` surface. The versions are identical today and + // `verify:dep-lockstep` is what keeps them that way, but identical is not + // the same as single, and only a pin makes it single. + // + // `zod/v4` needs its own entry — first-party code imports both specifiers, + // and pinning only the bare one would split the package across two installs + // rather than collapse it. + { find: /^zod$/, replacement: path.resolve(repoRoot, "node_modules/zod") }, + { + find: /^zod\/v4$/, + replacement: path.resolve(repoRoot, "node_modules/zod/v4/index.js"), + }, + { + find: /^open$/, + replacement: path.resolve(repoRoot, "node_modules/open"), + }, ]; const projectResolve = {