Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions .claude/skills/local-dev/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ to tell you have hit one.
Work the rules in order: which manifest declares it, then `dependencies` vs
`devDependencies`, then whether the bundlers must also externalise it.

### Why the SDK packages are root-only
### Why `core/`'s runtime dependencies are root-only

A per-client declaration installs a **second copy** that drifts from the root's.
Not theoretical: it put two versions of `ext-apps` (1.7.4 / 1.7.5) and of the
Expand All @@ -99,7 +99,30 @@ transitive v1 `@modelcontextprotocol/sdk` (1.29.0 / 1.30.0) in the tree at once

The same reasoning extends to anything reached only through root-owned code that
has no manifest of its own (`test-servers/src`, `core/`) — hence the repo-root
alias for those in `vitest.shared.mts`. `express` and `yaml` are the two today.
alias for those in `vitest.shared.mts`. That used to be `express` and `yaml`
alone; #2195 made it the general case, moving `ajv`, `atomically`, `chokidar`,
`commander`, `hono`, `@hono/node-server`, `@napi-rs/keyring`, `open`, `pino`,
`react`, `undici` and `zod` to the root as well.

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**.
Aligned duplicate declarations still drift the next time someone bumps one of
them; no declaration at all cannot. `npm run verify:dep-lockstep` is the detector
for the skew, and consolidation is what removes the opportunity.

Two consequences that read as bugs and are not:

- **`clients/cli` and `clients/launcher` declare no runtime dependencies.** Their
manifests carry `devDependencies` only. Everything they import at runtime is
root-declared and resolves by walking up from the client directory.
- **A client's `node_modules` still contains some of these names.** They arrive
transitively (`chokidar` under `vite`, `react` as a peer of `react-dom` and
`ink`). That is why the `vitest.shared.mts` pins matter: an unpinned specifier
would resolve the nearest copy, which is the transitive one, not the root's.

When a package moves to the root, its `vitest.shared.mts` pin has to move with
it — a `path.resolve(dirname, …)` pin left behind points at a directory that no
longer exists.
Comment thread
cliffhall marked this conversation as resolved.
Outdated

### Why runtime consumption decides `dependencies`

Expand Down
5 changes: 3 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,12 @@ worktree trap — is the `local-dev` skill.
The reasoning behind each of these, and what breaks when it is ignored, is the
`local-dev` skill. The rules themselves:

- **The MCP SDK packages** — `@modelcontextprotocol/client`, `core`, `server`, `server-legacy`, `ext-apps` — **are declared in the repo-root `package.json` and nowhere else.** So is anything reached only through root-owned code with no manifest of its own (`test-servers/src`, `core/`), aliased to the repo root in `vitest.shared.mts`. The v1 SDK (`@modelcontextprotocol/sdk`) is **not** a dependency of this repo and must not become one.
- **Every runtime dependency `core/` imports is declared in the repo-root `package.json` and nowhere else.** That is the MCP SDK packages (`@modelcontextprotocol/client`, `core`, `server`, `server-legacy`, `ext-apps`) and, since #2195, the rest of them: `ajv`, `atomically`, `chokidar`, `commander`, `hono`, `@hono/node-server`, `@napi-rs/keyring`, `open`, `pino`, `proper-lockfile`, `react`, `undici`, `zod`. So is anything reached only through root-owned code with no manifest of its own (`test-servers/src`, `core/`). All of it is aliased to the repo root in `vitest.shared.mts` — a pin left pointing at `<client>/node_modules` after a package moves resolves to a directory that no longer exists, or to whatever transitive copy an unrelated dependency dragged in. The v1 SDK (`@modelcontextprotocol/sdk`) is **not** a dependency of this repo and must not become one.
Comment thread
cliffhall marked this conversation as resolved.
Outdated
- **A client declares only what that client alone consumes** — its own UI stack, its bundler-inlined packages, its dev tooling. `clients/cli` and `clients/launcher` therefore declare **no** runtime dependencies at all, and that is the expected steady state, not an omission: everything they run on is root-declared and resolves by walk-up from the client directory. Re-adding a root-declared package to a client manifest re-creates the second copy this rule exists to make impossible (#1896), so a missing module at runtime is a signal to check the **root** manifest and the client's `external` list, never to add it back.
- **`dependencies` vs `devDependencies` follows from who consumes it at runtime**, not from where it is declared. Anything `core/` imports at runtime must be a root **`dependency`** — the client builds externalize npm packages and a published install resolves them from the root manifest, where devDependencies are absent.
- **A root-declared package that `core/` imports at runtime must also be named in all three bundler `external` lists** (`clients/{cli,tui}/tsup.config.ts`, `clients/web/tsup.runner.config.ts`), since which client reaches it is a function of what `core/` imports rather than of what the client's own code names. `npm run verify:bundle-externals` enforces this against the **built output**.
- **A dependency that renders React components must be bundled** into the client that uses it (`noExternal`) and declared only there — an externalized one resolves its own `react` and splits the tree. `ink` is the single exemption, on cost, and it is only safe while the root `react` range stays open to the whole major (`^19.0.0`).
- **One version per install-crossing dependency.** When bumping a dependency the shared sources pull in, bump it in every install that declares it. Never raise the tsc heap to work around a skew. `npm run verify:dep-lockstep` enforces this.
- **One version per install-crossing dependency.** When bumping a dependency the shared sources pull in, bump it in every install that declares it. Consolidating to the root is what makes most of these unbumpable in two places at once, but it does not retire the rule — a client's `devDependencies`, and any package that arrives transitively into a client install, can still skew against the root. Never raise the tsc heap to work around one. `npm run verify:dep-lockstep` enforces this.
- **Pin a transitive dependency with an `overrides` entry**, not with `npm audit fix` — which "resolves" an advisory with no upward escape by silently downgrading.

## Contributing
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ npm run web # prod web launcher against clients/web/dist
npm run web:dev # web launcher in --dev mode (Vite)
```

v2 is **not** an npm workspace — each client under `clients/*` keeps its own `package.json` and `node_modules`, and shared code lives in `core/`, consumed via a `@inspector/core` build-time alias. What that means for adding a dependency (root vs. client, `dependencies` vs. `devDependencies`, and the bundler `external` lists) is in the [`local-dev` skill](./.claude/skills/local-dev/SKILL.md).
v2 is **not** an npm workspace — each client under `clients/*` keeps its own `package.json` and `node_modules`, and shared code lives in `core/`, consumed via a `@inspector/core` build-time alias. **Every runtime dependency `core/` imports is declared once, in the repo-root `package.json`**, and each client declares only what that client alone consumes — its UI stack, its bundler-inlined packages, its dev tooling — which leaves `clients/cli` and `clients/launcher` with no runtime dependencies of their own. What that means for adding a dependency (root vs. client, `dependencies` vs. `devDependencies`, and the bundler `external` lists) is in the [`local-dev` skill](./.claude/skills/local-dev/SKILL.md).

## Project layout

Expand Down
Loading