Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 7 additions & 3 deletions .claude/skills/local-dev/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,13 @@ To run the CLI or TUI: `node clients/launcher/build/index.js --cli …` /
`package.json`** β€” it is not published on its own. Each client bundles it via a
build-time alias:

- **CLI / TUI** β€” `esbuildOptions.alias` in `tsup.config.ts` maps
- **TUI / mcpdo** β€” `esbuildOptions.alias` in `tsup.config.ts` maps
`@inspector/core` β†’ the repo `core/` directory, with
`noExternal: [/^@inspector\/core/]` inlining it.
- **CLI** β€” the same two settings under their tsdown names in
`tsdown.config.ts`: a top-level `alias`, and `deps.alwaysBundle` (#1492).
Its externals are `deps.neverBundle`, where a bare string matches the exact
specifier only β€” the config expands each name to cover its subpaths.
- **Web** β€” **two** configs, because the client is two builds:
`clients/web/vite.config.ts` for the browser app, and
`clients/web/tsup.runner.config.ts` for the Node backend runner, which
Expand Down Expand Up @@ -178,7 +182,7 @@ toolchain". `tsx`, `playwright`, `storybook`, `happy-dom`, `ink-testing-library`
`vite-node` and each client's own `@types/*` are toolchain too and stay where
they are β€” hoisting them would make every client install the union of all five.
So do the ones **more than one** client declares without all of them doing so:
`tsup` sits in web, cli and tui, and `vite` in web and tui on top of the root
`tsup` sits in web, mcpdo and tui, and `vite` in web and tui on top of the root
_runtime_ `dependency` that `--web --dev` needs. Neither is in scope here;
whether to consolidate them is a separate call with a separate rationale (`vite`
especially, since its root declaration is a `dependency`, not a
Expand Down Expand Up @@ -270,7 +274,7 @@ Two live examples worth knowing:

### Why a root dependency must also be externalised

tsup and Vite externalise what the **client's** `package.json` declares, and a
tsup, tsdown and Vite externalise what the **client's** `package.json` declares, and a
root-only dependency is in none of them β€” so it is **bundled**, silently. For a
CJS package inlined into an ESM bundle that is fatal: esbuild leaves a
`Dynamic require of "path" is not supported` shim that throws at _import_ time,
Expand Down
2 changes: 1 addition & 1 deletion .claude/skills/project-structure/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ inspector/
β”‚ β”‚ β”œβ”€β”€ src/ Browser app: components, hooks, lib/, utils/, theme/
β”‚ β”‚ β”œβ”€β”€ server/ Node-only dev/prod backend wiring (see below)
β”‚ β”‚ └── static/ sandbox_proxy.html β€” served for the MCP Apps tab
β”‚ β”œβ”€β”€ cli/ Scriptable CLI (tsup bundle, @inspector/core alias)
β”‚ β”œβ”€β”€ cli/ Scriptable CLI (tsdown bundle, @inspector/core alias)
β”‚ β”œβ”€β”€ tui/ Ink + React terminal UI (tsup bundle)
β”‚ └── launcher/ The `mcp-inspector` bin; dispatches to web/cli/tui in-process
β”œβ”€β”€ core/ Shared code, consumed via the `@inspector/core` alias (no package.json)
Expand Down
8 changes: 4 additions & 4 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ inspector/
β”‚ β”‚ β”œβ”€β”€ src/ Browser app: components, hooks, lib/, utils/, theme/
β”‚ β”‚ β”œβ”€β”€ server/ Node-only dev/prod backend wiring
β”‚ β”‚ └── static/ sandbox_proxy.html β€” served for the MCP Apps tab
β”‚ β”œβ”€β”€ cli/ Scriptable CLI (tsup bundle, @inspector/core alias)
β”‚ β”œβ”€β”€ cli/ Scriptable CLI (tsdown bundle, @inspector/core alias)
β”‚ β”œβ”€β”€ mcpdo/ Experimental connection CLI (`mcpdo` bin β€” connect once, many
β”‚ β”‚ commands; implicit Unix-socket connection daemon). Bundled
β”‚ β”‚ into the published package β€” see clients/mcpdo/README.md
Expand Down Expand Up @@ -107,10 +107,10 @@ The reasoning behind each of these, and what breaks when it is ignored, is the
- **A client declares only what that client alone consumes** β€” its own UI stack, its bundler-inlined packages, its dev tooling. `clients/cli`, `clients/mcpdo` 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.
- **A package that moves to the root moves its `vitest.shared.mts` pin with it.** Left pointing at `<client>/node_modules` a pin resolves to a directory that no longer exists β€” or, where a transitive copy happens to sit there (`chokidar` under `vite`, `react` as a peer of `react-dom` and `ink`), to the very duplicate the pin list exists to prevent. **`react` and `react-dom` are the deliberate exception** and stay pinned per client, so a client's renderer and the React it calls into come from one install; every other root-owned pin resolves from the repo root.
- **`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.
- **The shared toolchain is declared once, at the repo root, and in no client manifest.** `eslint`, `@eslint/js`, `typescript-eslint`, `globals`, `prettier`, `typescript`, `vitest`, `@vitest/coverage-v8` and `@types/node` are used by every client's own scripts, and a client that declares none of them still resolves the root copy by walk-up β€” `npm run` puts each ancestor `node_modules/.bin` on `PATH`, and Node and TypeScript walk parent `node_modules` / `node_modules/@types` the same way. `clients/launcher` declares no `devDependencies` at all and its `validate` is unchanged. A client-side declaration buys nothing and installs a second copy free to drift, as `globals` (`^17.7.0` root / `^17.4.0` clients) and `typescript-eslint` (`^8.65.0` / `^8.56.1`) had before #2196. These stay **`devDependencies`** β€” none is consumed at runtime and the tarball ships only each client's `build/`. The boundary is **used by every client**, not "used by one": anything narrower stays where it is, whether one client declares it (`tsx`, `playwright`, `storybook`, `happy-dom`, `ink-testing-library`, `vite-node`, each client's own `@types/*`) or several do β€” `tsup` is declared in web, cli and tui, and `vite` in web and tui on top of the root **runtime** `dependency` that `--web --dev` needs. Those are out of scope here; consolidating them is a different call with a different rationale.
- **The shared toolchain is declared once, at the repo root, and in no client manifest.** `eslint`, `@eslint/js`, `typescript-eslint`, `globals`, `prettier`, `typescript`, `vitest`, `@vitest/coverage-v8` and `@types/node` are used by every client's own scripts, and a client that declares none of them still resolves the root copy by walk-up β€” `npm run` puts each ancestor `node_modules/.bin` on `PATH`, and Node and TypeScript walk parent `node_modules` / `node_modules/@types` the same way. `clients/launcher` declares no `devDependencies` at all and its `validate` is unchanged. A client-side declaration buys nothing and installs a second copy free to drift, as `globals` (`^17.7.0` root / `^17.4.0` clients) and `typescript-eslint` (`^8.65.0` / `^8.56.1`) had before #2196. These stay **`devDependencies`** β€” none is consumed at runtime and the tarball ships only each client's `build/`. The boundary is **used by every client**, not "used by one": anything narrower stays where it is, whether one client declares it (`tsx`, `playwright`, `storybook`, `happy-dom`, `ink-testing-library`, `vite-node`, each client's own `@types/*`) or several do β€” `tsup` is declared in web, mcpdo and tui (`tsdown` in cli, #1492), and `vite` in web and tui on top of the root **runtime** `dependency` that `--web --dev` needs. Those are out of scope here; consolidating them is a different call with a different rationale.
- ⚠️ **Deleting the declaration does not always delete the copy, and the local copy still wins.** npm auto-installs an unmet **peer** into the install that needs it, and it has no visibility into the root's tree β€” so a client-only ESLint plugin drags a client-local `eslint` in (`eslint-plugin-react-refresh`/`-storybook` in web, `eslint-plugin-react-hooks` in tui), and web's Storybook/Vitest stack drags in a local `typescript` and `vitest`. A hoisted transitive does the same: `@types/express` puts an `@types/node` in web and cli. Those copies sit _nearer_ than the root's and take precedence. The consolidation is therefore about **one declaration and one place to bump**, not about a single copy on disk. ⚠️ **Nothing keeps the surviving copies aligned automatically β€” but since #2226 the guard rejects the drift.** A **peer** copy is at least constrained by its holder's peer range β€” tightly for `vitest` (an exact peer, hence the pin below), loosely for `eslint` (`^9 || ^10`), where the copies agree only because npm resolves the same latest in both installs. A **transitive** copy is constrained by nothing of ours at all, and cli's `@types/node` (`24.13.1` against the root's `24.13.3`) diverged on exactly that. **That is detection, not alignment: `verify:dep-lockstep` fails on this class since #2226, and you still do the bump by hand.** Its second tier compares every package any install _declares_ (`dependencies`, `devDependencies`, `optionalDependencies`; not peers) against every top-level copy across all five installs, independent of what a `tsc` program loads, so a transitive drift and a peer shadow (`eslint`, `typescript`, `vitest`) are both in scope now. Two limits remain: the tier reads lockfiles, so a tool binary you installed by hand and never committed is still invisible; and it only compares names some manifest declares, so a purely transitive package no manifest names is out of scope in both tiers unless a `tsc` program loads both copies. Aligning a stale install is `npm update <pkg>` there; a transitive copy that will not move takes an `overrides` entry in that install (`clients/cli` pins `@types/node` this way).
- ⚠️ **`vitest`, `@vitest/coverage-v8` and web's `@vitest/browser-playwright` are pinned exactly, and move together.** `@vitest/browser-playwright` declares an **exact** peer on `vitest`, so it β€” not the root range β€” decides which `vitest` web installs. Left to float, the root resolves a newer patch and web's tests then run on one `vitest` while loading a coverage provider built against another. Bumping means editing all three in one change, the same discipline the exact `prettier` pin (#1790) exists for. ⚠️ **Editing the three is necessary but not sufficient β€” `clients/web` also carries a `vitest` `overrides` entry that has to move with them.** Web does not declare `vitest`, so its copy is the peer shadow above; its lockfile pins that copy at the old patch, and the exact peer plus the lockfile form a knot `npm install` resolves by refusing outright (`Conflicting peer dependency: vitest@<new>`), while `npm update` will not move it either. Deleting web's lockfile clears the error and re-resolves every caret range in the tree at once β€” an uncontrolled dependency update wearing a security patch's clothes. The `overrides` entry is the controlled alternative, the same mechanism `clients/cli` uses for `@types/node`: it moves the shadowed copy and nothing else, keeping the churn inside the vitest constellation. So a vitest bump is **four** edits, and the override's version is an exact pin like the other three (#2301).
- **A root-declared package that `core/` imports at runtime must also be named in all four bundler `external` lists** (`clients/{cli,mcpdo,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 root-declared package that `core/` imports at runtime must also be named in all four bundler `external` lists** (`clients/cli/tsdown.config.ts`, `clients/{mcpdo,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. 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 in two tiers: packages that reach one `tsc` **program** from two installs (the #1896 heap-exhaustion class), and β€” since #2226 β€” every package any install **declares** that more than one install holds a top-level copy of, whether or not a program ever sees both.
- **Pin a transitive dependency with an `overrides` entry**, not with `npm audit fix` β€” which "resolves" an advisory with no upward escape by silently downgrading.
Expand Down Expand Up @@ -551,7 +551,7 @@ Pick by how many notifications the wait needs:

## Build output is never a gate target

**No gate β€” lint, format, or typecheck β€” may read generated output.** The gated surface is first-party source only: `clients/*/src`, `clients/*/__tests__`, `clients/web/{server,.storybook}`, each client's top-level configs, `core/`, `test-servers/src`, `scripts/`, and the root shared files. Everything a build writes is out of scope: `clients/*/build` (the tsup/tsc bundles), `clients/web/dist` (the Vite SPA), `clients/web/storybook-static`, `clients/*/coverage`, `test-servers/build`, `core/**/{build,dist}`, and any `*.tsbuildinfo`. Each scope states this itself β€” `globalIgnores([...])` in every `eslint.config.js`, the `format`/`format:check` globs in each `package.json`, and a tsconfig `include` that names source directories rather than the package root.
**No gate β€” lint, format, or typecheck β€” may read generated output.** The gated surface is first-party source only: `clients/*/src`, `clients/*/__tests__`, `clients/web/{server,.storybook}`, each client's top-level configs, `core/`, `test-servers/src`, `scripts/`, and the root shared files. Everything a build writes is out of scope: `clients/*/build` (the tsup/tsdown/tsc bundles), `clients/web/dist` (the Vite SPA), `clients/web/storybook-static`, `clients/*/coverage`, `test-servers/build`, `core/**/{build,dist}`, and any `*.tsbuildinfo`. Each scope states this itself β€” `globalIgnores([...])` in every `eslint.config.js`, the `format`/`format:check` globs in each `package.json`, and a tsconfig `include` that names source directories rather than the package root.

Why it matters, given that these paths are all gitignored and the findings are usually warnings:

Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ v2 is **not** an npm workspace β€” each client under `clients/*` keeps its own `
inspector/
β”œβ”€β”€ clients/
β”‚ β”œβ”€β”€ web/ Web client (Vite + React + Mantine). src/ = browser app; server/ = Node backend
β”‚ β”œβ”€β”€ cli/ CLI client (tsup bundle, @inspector/core alias)
β”‚ β”œβ”€β”€ cli/ CLI client (tsdown bundle, @inspector/core alias)
β”‚ β”œβ”€β”€ mcpdo/ Experimental connection CLI (`mcpdo` bin) β€” bundled into the
β”‚ β”‚ published package; see clients/mcpdo/README.md
β”‚ β”œβ”€β”€ tui/ TUI client (Ink + React, tsup bundle)
Expand Down Expand Up @@ -87,6 +87,7 @@ Each client has its own README with client-specific detail:
| [Writing a skill](./docs/skill-authoring.md) | How to write a skill description that actually fires, and eval cases that measure it β€” the case shapes that work, and the tuning loop |
| [Test servers](./docs/test-servers.md) | The composable test servers and the showcase config for every feature β€” what to run, what to click, and what the broken build did |
| [Publishing](./docs/publishing.md) | What ships in the tarball, the packaging invariants, and `pack:verify` |
| [Migrating from tsup to tsdown](./docs/tsdown-migration.md) | Why the client bundles are moving to tsdown, the option mapping, the gotchas that don't fail a build, and the order for the remaining clients |
| [Serving the web UI over HTTPS](./docs/tls.md) | `INSPECTOR_TLS_KEY_PATH` / `INSPECTOR_TLS_CERT_PATH`, a trusted local mkcert setup, custom hostnames and `ALLOWED_ORIGINS`, and `NODE_EXTRA_CA_CERTS` for outbound private-CA servers |
| [Docker](./docs/docker.md) | Running the container image β€” ports, volumes, and making secrets durable in a container |
| [Where secrets are stored](./docs/secret-storage.md) | How the secret store is chosen on every runtime β€” OS keychain, `secrets.json` or memory β€” plus file encryption, locking, and moving back to a keychain |
Expand Down
2 changes: 1 addition & 1 deletion clients/cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ Proxy routing is powered by the [`undici`](https://www.npmjs.com/package/undici)

Because undici's `Response` is a different class from `globalThis.Response`, the proxied fetch re-wraps each response as a genuine global `Response`, streaming preserved. Without that, `res instanceof Response` is `false` for callers that test it β€” including the MCP SDK, whose OAuth error formatter would degrade every message to `Raw body: [object Response]`.

`undici` is declared **only** in the root `package.json`, and every client's tsup config lists it as `external`. Both halves matter: tsup auto-externalizes what the _nearest_ manifest declares, so without the explicit entry the web and TUI bundles inlined it β€” and a CommonJS package inlined into an ESM bundle throws `Dynamic require of "assert" is not supported` the first time it is used. `npm run verify:bundle-externals` is the durable guard.
`undici` is declared **only** in the root `package.json`, and every client's bundler config lists it as `external` (this client's is `clients/cli/tsdown.config.ts`). Both halves matter: the bundler auto-externalizes what the _nearest_ manifest declares, so without the explicit entry the web and TUI bundles inlined it β€” and a CommonJS package inlined into an ESM bundle throws `Dynamic require of "assert" is not supported` the first time it is used. `npm run verify:bundle-externals` is the durable guard.

### Shell completion

Expand Down
Loading
Loading