chore: declare the MCP SDK packages only at the repo root (#1970)

[cliffhall]

Closes #1970

Declares @modelcontextprotocol/client, core, server, server-legacy — and ext-apps — in exactly one place: the repo-root package.json. Every client resolves them from there, which is how the root manifest already describes itself ("Runtime dependencies are declared on the root package.json; client builds … externalize npm packages resolved from the root install").

The drift was already real

Not a hypothetical. Before this change, a node_modules census across the repo:

	ext-apps	v1 @modelcontextprotocol/sdk
repo root	1.7.4	1.29.0
clients/web	1.7.5	1.30.0

Five manifests pinning the same packages have to be bumped in lockstep or a client builds against a different copy than the one the tarball resolves — and a second copy of client/core is precisely the failure vitest.shared.mts already carries a dedupe + server.deps.inline workaround for (a vi.mock that misses because mock and import resolved to different copies).

After a clean install there is exactly one copy of each, at the root.

On @modelcontextprotocol/sdk (the v1 SDK)

Confirmed: nothing in this repo needs it directly. It appears in no package.json of ours and no source file imports it — including the composable test servers, which build on the v2 server/server-legacy packages. It is installed solely as a peer dependency of @modelcontextprotocol/ext-apps (^1.29.0), which npm auto-installs; that is why it shows up only as "peer": true in the lock files.

So there is nothing to remove — but consolidating ext-apps is the only lever we have over it, and it collapses two copies at two versions into one. npm update then takes ext-apps to 1.7.5 (latest) and its SDK peer to 1.30.0 (latest). A bonus from that bump: 1.30.0 widens its @hono/node-server peer to ^1.19.9 || ^2.0.5, which matches our ^2 and lets npm drop a nested duplicate copy of that package.

express — a latent bug this surfaced

test-servers/src imports express, but no manifest declared it. It was arriving in clients/cli/node_modules as a peer of express-rate-limit, a dependency of @modelcontextprotocol/server-legacy. Removing server-legacy from cli's manifest took express with it, and every cli test that spawns a test server failed to resolve it (14 files, Cannot find package 'express').

Fixed at the root of the problem rather than by putting the pin back: express is now a root devDependency (test-servers is root-owned code with no manifest of its own), and vitest.shared.mts resolves it from the repo root — joining yaml, which was already the precedent for exactly this shape. The clients/web duplicate is dropped; its only reference there is a type-only import, covered by @types/express.

clients/launcher declared none of these and is untouched.

Gate

Full npm run ci after a from-scratch reinstall (all node_modules deleted): validate, verify:build-gate, smoke, and Storybook (466) pass, plus 4881 unit+integration and 304 cli tests.

The coverage step exits 1 locally on two pre-existing Connection closed unhandled rejections from inspectorClient.test.ts, reproduced identically on v2/main with this branch stashed — unrelated to this change.

No screenshots: dependency plumbing, no user-facing surface.

[Copilot]

Pull request overview

Centralizes MCP SDK dependencies at the repository root to prevent duplicate installations and version drift across clients.

Changes:

• Removes duplicate MCP dependencies from Web, CLI, and TUI manifests.
• Declares Express at the root for test-server usage.
• Updates Vitest resolution and lockfiles for root-level dependencies.

Reviewed changes

Copilot reviewed 5 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
package.json	Adds root Express development dependency.
package-lock.json	Updates consolidated dependency graph.
vitest.shared.mts	Resolves Express from root dependencies.
clients/web/package.json	Removes duplicated MCP and Express declarations.
clients/web/package-lock.json	Removes client-local dependency copies.
clients/cli/package.json	Removes duplicated MCP dependencies.
clients/cli/package-lock.json	Removes client-local dependency copies.
clients/tui/package.json	Removes duplicated MCP dependencies.
clients/tui/package-lock.json	Removes client-local dependency copies.

Files not reviewed (3)

• clients/cli/package-lock.json: Generated file
• clients/tui/package-lock.json: Generated file
• clients/web/package-lock.json: Generated file

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[cliffhall]

Round 1 review response (e132d55)

Copilot returned no comments on this one, so nothing to address. Pushing one follow-up of my own: the maintenance rules require a change to the dependency layout to land with its documentation, which the original diff was missing.

• README (Setup) — the placement rule plus the concrete drift it prevents.
• AGENTS.md — a Dependency placement section: the MCP SDK packages are root-only; the v1 @modelcontextprotocol/sdk is not ours and must not become a dependency; anything reached solely through root-owned code with no manifest (test-servers/src, core/) is a root devDependency aliased to the repo root in vitest.shared.mts, as express and yaml now are.
• .github/copilot-instructions.md — the distilled form, so a future diff re-adding one of these to a client manifest gets flagged in review. (Per AGENTS, review-relevant rules change in both files in the same PR.)

Re-requesting review since the diff grew after the first pass. npm run validate clean.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 12 changed files in this pull request and generated no new comments.

Files not reviewed (3)

• clients/cli/package-lock.json: Generated file
• clients/tui/package-lock.json: Generated file
• clients/web/package-lock.json: Generated file

Suppressed comments (4)

README.md:69

• This states that both packages are devDependencies, but package.json:100 declares yaml in runtime dependencies while only express is in devDependencies. Avoid documenting the wrong install classification, since runtime dependencies are retained in published installs while devDependencies are not.

**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`.

AGENTS.md:120

• This guidance incorrectly says all dependencies used by core/ belong in devDependencies; however, the published client bundles externalize npm packages (README.md:284), so a runtime import from core must remain in root dependencies. It also contradicts the current manifest, where yaml is a runtime dependency. Distinguish runtime from test/build-only packages while retaining the root-only placement rule.

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 `<client>/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.

.github/copilot-instructions.md:79

• This mirrored rule would direct reviewers to require every new core dependency in devDependencies, even though core's runtime npm imports are externalized and must be installed for published consumers. It also calls yaml a devDependency despite package.json:100. Mirror the corrected runtime-versus-development distinction from AGENTS.md.

- 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 `<client>/node_modules` like the other pins there.

vitest.shared.mts:88

• The ownership explanation is inaccurate: the only yaml source import is test-servers/src/load-config.ts, not core or root tooling. Correct this so the alias rationale points to the actual root-owned consumer.

    // of its own — `test-servers/src` imports express, and `core`/the root
    // tooling uses yaml — so the root is where they are declared and the only

[cliffhall]

Round 2 review response (5a3a343)

No new inline comments; four suppressed ones, and all four were right — every one a factual error in the docs I added in e132d551, not in the dependency change itself. Fixed together.

1 & 2. yaml is a runtime dependency, not a devDependency. Verified: package.json has it under dependencies (^2.9.0); only express is a devDependency. Corrected in all three docs.

3. The rule as written was actively dangerous for core/. It said anything reached through root-owned code goes in devDependencies — which for a runtime core/ import would break the published package silently: the client builds externalize npm packages, so a published install resolves them from the root manifest, and devDependencies are not installed there. Every local check would still pass. The docs now separate the two questions:

• Placement (which manifest): root-only, for anything reached through test-servers/src or core/.
• Classification (which section): follows from who consumes it at runtime. A core/ runtime import → root dependencies. Test/build-only → devDependencies.

The copilot-instructions mirror now explicitly asks reviewers to flag a runtime core/ import added to devDependencies, since that is the case that passes locally and fails in the tarball.

4. The vitest.shared.mts ownership comment was wrong. yaml's only source import is test-servers/src/load-config.ts, not core/ or the root tooling. Both aliases now point at their real consumer: express via the http/oauth test servers, yaml via load-config.ts.

One thing I deliberately did not do: yaml sits in dependencies despite its only importer being a test server, so it is arguably misclassified. Moving it changes what ships, which is not a docs change and not this PR's scope — so it is called out in AGENTS.md with a note to confirm no published path reads YAML before touching it.

npm run validate clean.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 12 changed files in this pull request and generated no new comments.

Files not reviewed (3)

• clients/cli/package-lock.json: Generated file
• clients/tui/package-lock.json: Generated file
• clients/web/package-lock.json: Generated file

[cliffhall]

Cross-reference for whoever merges: #1962 (chore(deps): align zod across installs and guard against version skew, closes #1896) touches the same files — all four package.jsons and lockfiles, plus README / AGENTS / copilot-instructions — so whichever lands second needs a rebase.

They are complementary rather than competing, and in the same direction:

• chore(deps): align zod across installs and guard against version skew #1962 diagnoses two copies of a package at mismatched versions inside one tsc program (zod 4.3.6 root vs 4.4.3 web blowing the heap via TS2589) and adds verify-dep-lockstep to keep the remaining duplicates aligned.
• This PR removes the duplicate declarations for the MCP packages outright, so there is nothing left to keep in lockstep for those.

No functional conflict: #1962's guard reads top-level lockfile entries and ignores a package absent from an install ("a package absent from an install can't skew … don't add a dependency to a client just to satisfy this"), which is exactly the state this PR produces. One tidy-up for afterwards: @modelcontextprotocol/ext-apps sits in #1962's TOLERATED_SKEW allowlist, and once this merges there is only one copy of it, so that entry becomes dead and can be dropped.