diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index aa1d6dd1..ccf73747 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -72,6 +72,12 @@ Both exist and do different jobs. Theme files (`src/theme/.ts`) custo - `src/types/` is only for ambient `.d.ts` module stubs — not a home for new domain types. - ⚠️ The coverage `include` is a **whitelist** naming `components` / `hooks` / `theme` / `lib` / `utils` / `server` (plus the `core/*` runtime). A module placed outside those directories silently falls out of the ≥90% gate. Flag new top-level files or new grab-bag directories. +## Dependency placement + +- **The MCP SDK packages (`@modelcontextprotocol/client`, `core`, `server`, `server-legacy`, `ext-apps`) belong in the root `package.json` only.** Adding one to a `clients/*/package.json` is a review finding: Node resolution walks up, so the root install already serves every client, and a per-client entry installs a second copy that drifts (it produced two versions of `ext-apps`, and of the transitive v1 SDK, at once — #1970) and reintroduces the duplicate-copy failure `vitest.shared.mts` has a `dedupe` workaround for. +- **The v1 `@modelcontextprotocol/sdk` is not a dependency of this repo** and must not become one. It is a peer of `ext-apps`, present in lock files only. +- Dependencies used only by **root-owned code with no manifest** (`test-servers/src`, `core/`) go in the root `devDependencies` and are aliased to the **repo root** in `vitest.shared.mts` — as `express` and `yaml` are — not to `/node_modules` like the other pins there. + ## Tests and the coverage gate - **All new or modified code needs tests.** The per-file gate is **≥ 90% on all four dimensions** — lines, statements, functions, **and branches** — enforced in CI for `clients/{web,cli,tui,launcher}` and the gated `core/` runtime. diff --git a/AGENTS.md b/AGENTS.md index a4185baa..164baff8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -113,6 +113,14 @@ v2 is **not** an npm workspace — each client under `clients/*` keeps its own ` After installing, `npm run build` builds all clients. The launcher scripts (`npm run web` / `web:dev`) run the built launcher, so build first; for day-to-day web iteration use `cd clients/web && npm run dev`. +## Dependency placement + +**The MCP SDK packages — `@modelcontextprotocol/client`, `core`, `server`, `server-legacy`, `ext-apps` — are declared in the repo-root `package.json` and nowhere else.** Do not add them to a `clients/*/package.json`: Node resolution walks up, so the root install already serves every client, and a per-client declaration installs a *second copy* that drifts from the root's. That drift is not theoretical — it put two versions of `ext-apps` (1.7.4 / 1.7.5), and of the transitive v1 `@modelcontextprotocol/sdk` (1.29.0 / 1.30.0), in the tree at once (#1970), and a second copy of `client`/`core` is exactly the failure the `dedupe` + `server.deps.inline` workaround in `vitest.shared.mts` exists for. + +The same rule covers anything reached only through **root-owned code that has no manifest of its own**: `express` (imported by `test-servers/src`) and `yaml` are root devDependencies, and `vitest.shared.mts` aliases both to the **repo root** rather than to `/node_modules` like its other pins. If you add a dependency to `test-servers/src` or `core/`, declare it at the root and alias it there too. + +The v1 SDK (`@modelcontextprotocol/sdk`) is **not** a dependency of this repo and must not become one — v2 uses the packages above. It appears in the lock files only as a `"peer": true` entry pulled in by `ext-apps`. + ## Contributing External contributions are accepted as **issues, not pull requests** — maintainers handle design and implementation through a prompt-driven workflow. diff --git a/README.md b/README.md index 13792764..8ae07537 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,8 @@ npm install # root install; postinstall cascades into every client The cascade (`scripts/install-clients.mjs`) is dev-only — it exits early when the package is installed as a dependency, and the published tarball ships only each client's `build/`, so end users are unaffected. Set `INSPECTOR_SKIP_CLIENT_INSTALL=1` to skip it. +**Where a dependency is declared.** The MCP SDK packages (`@modelcontextprotocol/client`, `core`, `server`, `server-legacy`, `ext-apps`) live in the **root** `package.json` only — never in a client's. Node resolution walks up, so the root install is on every client's chain, and the root manifest is already what the published tarball resolves against. Declaring them per client installs a second copy that can drift from the root's, which is how two versions of `ext-apps` (and of the transitive v1 `@modelcontextprotocol/sdk`) ended up in the tree before [#1970](https://github.com/modelcontextprotocol/inspector/issues/1970) — and a second copy of `client`/`core` is the failure `vitest.shared.mts` carries a `dedupe` workaround for. The same holds for anything used only by root-owned code with no manifest of its own: `express` (imported by `test-servers/src`) and `yaml` are root devDependencies, resolved from the root by `vitest.shared.mts`. + ## Running during development For day-to-day web iteration, run Vite directly from the web client (fast HMR, no launcher build needed):