Merge branch 'v2/main' into dependabot/github_actions/v2/main/github-actions-d1cfab875f
This commit is contained in:
Cliff Hall
GitHub
commit
372403d9cc
63 changed files with 5417 additions and 2731 deletions
1
.gitattributes
vendored
1
.gitattributes
vendored
|
|
@ -1 +1,2 @@
|
|||
* text=auto eol=lf
|
||||
package-lock.json linguist-generated=true
|
||||
|
|
|
|||
13
.github/copilot-instructions.md
vendored
13
.github/copilot-instructions.md
vendored
|
|
@ -72,6 +72,14 @@ Both exist and do different jobs. Theme files (`src/theme/<Component>.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 reached only through **root-owned code with no manifest** (`test-servers/src`, `core/`) are declared at the root and aliased to the **repo root** in `vitest.shared.mts` — as `express` and `yaml` are — not to `<client>/node_modules` like the other pins there.
|
||||
- **A dependency that renders React components must be bundled into the client that uses it, and is then not a root dependency.** An externalized package resolves its own `react` from wherever npm placed it in the *consumer's* tree, beside a React satisfying *its* peer range — looser than ours, which is all it takes to split React. `ink-form`/`ink-scroll-view` declare `">=18"`, so a consumer's React 18 satisfies them, the TUI ends up with two React instances, and it crashes on the first hook (#1952). Both are inlined via `noExternal` in `clients/tui/tsup.config.ts`. **`ink` is the one exemption, justified by cost (~1.4MB) — never by a peer range**: flag any claim that `">=19"` keeps npm from misplacing it, which is false and was in this repo once. What keeps it safe is the **root `react` range staying open to the whole major (`^19.0.0`)** so npm can dedupe with a consumer's pinned React 19; treat narrowing that range as reopening the bug. `clients/tui/__tests__/tsupConfig.test.ts` enforces the split, the root-declaration of exempt packages, and that range.
|
||||
- **Which section is a separate question from which manifest.** A package `core/` imports at runtime must be in root **`dependencies`**: client builds externalize npm packages, so a published install resolves them from the root manifest and devDependencies are absent there. Only test/build-only packages (`express`) belong in `devDependencies`. Flag a runtime `core/` import added to `devDependencies` — it passes every local check and breaks the published package.
|
||||
|
||||
## 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.
|
||||
|
|
@ -85,15 +93,18 @@ Both exist and do different jobs. Theme files (`src/theme/<Component>.ts`) custo
|
|||
|
||||
### Rendering components in tests
|
||||
|
||||
- **Always render through `renderWithMantine`** from `src/test/renderWithMantine.tsx`. Never hand-roll a bare `MantineProvider` — that reintroduces a real failure class where a `Transition`/`Modal` timer fires after happy-dom tears down `window` and fails the entire run.
|
||||
- **Always render through `renderWithMantine`** from `src/test/renderWithMantine.tsx`. A hand-rolled bare `MantineProvider` skips the project theme and the helper's options, and drifts from every other test. (It does *not* reintroduce the timer-leak class — an older version of this rule said so; the leaked-timer net in `setup.ts` is global and covers every unit test regardless of how it renders.)
|
||||
- For a forced color scheme, pass the option — `renderWithMantine(ui, { colorScheme: "dark" })` — rather than a hand-rolled `defaultColorScheme` provider.
|
||||
- Only when asserting _mid-flight_ transition state, use `renderWithMantineTransitions`, passing `settleMs` derived from the component's real animation duration. Do **not** combine it with `vi.useFakeTimers()`, and use the `unmount()` it returns if the test unmounts the tree itself.
|
||||
|
||||
## Gates and PR hygiene
|
||||
|
||||
- `npm run format` before committing; **`npm run ci` before pushing** (`validate` → `coverage` → `verify:build-gate` → `smoke` → Storybook). `npm run validate` is the fast inner-loop check and is **not** a substitute — it runs `test`, not `test:coverage`, so it does zero coverage gating.
|
||||
- **A dependency bump must land in every install that declares it.** v2 is not a workspace — the root and each `clients/*` have their own `node_modules`, and a client's test project compiles `core/` and `test-servers/src` (which resolve from the **root**) alongside the client's own sources. Bumping a shared dependency in one manifest only puts two versions of it in one `tsc` program; for a recursive-generic surface like zod that exhausts the tsc heap (#1896). `verify:dep-lockstep` fails the build on this, so a PR bumping a package that the shared sources import should update the root **and every client that already lists it** — not every client unconditionally, since a package absent from an install can't skew and adding it there would be a spurious dependency.
|
||||
- **A test or smoke must not touch real user state.** The web smokes run against a throwaway catalog via the shared `scripts/lib/prod-web-server.mjs` helper, never the developer's `~/.mcp-inspector/mcp.json` (#1977); the cli/tui smokes drive a temp `--catalog`. A new smoke spawning its own server, or teardown that removes a work dir without first awaiting `stopChild` (the #1801 race — `child-cleanup.mjs` exports both halves and both are required), should be flagged.
|
||||
- **Every PR references an issue**, first body line `Closes #<ISSUE_NUMBER>`.
|
||||
- **Every PR carries exactly one version label**, `v1` or `v2`, matching its base branch.
|
||||
- **Commits carry a `Signed-off-by:` trailer.** The DCO check is a hard merge gate and fails on a single unsigned commit; it matches the trailer against the author *or* committer, and skips only merge and bot commits. Use `git commit -s` — note `format.signOff` does *not* sign `git commit` (only `format-patch`). Repairing pushed commits means `git rebase HEAD~<n> --signoff` + `git push --force-with-lease`; remediation commits are not enabled on this repo.
|
||||
- Update the relevant `README.md` / `AGENTS.md` when a change adds, removes, renames, or repurposes a file or folder, changes the structure or tech stack, or introduces a command, dependency, or architectural pattern.
|
||||
|
||||
## What to prioritize in review
|
||||
|
|
|
|||
262
AGENTS.md
262
AGENTS.md
File diff suppressed because one or more lines are too long
|
|
@ -31,8 +31,12 @@ RUN npm install -g /tmp/inspector.tgz && rm /tmp/inspector.tgz
|
|||
ENV HOST=0.0.0.0 \
|
||||
DANGEROUSLY_BIND_ALL_INTERFACES=true \
|
||||
CLIENT_PORT=6274 \
|
||||
MCP_SANDBOX_PORT=6275 \
|
||||
MCP_AUTO_OPEN_ENABLED=false
|
||||
EXPOSE 6274
|
||||
# 6275 is the MCP Apps sandbox, a second listener the browser reaches directly.
|
||||
# It is only needed for the Apps tab, so it is EXPOSEd but publishing it is
|
||||
# optional — see the Docker section of the root README.
|
||||
EXPOSE 6274 6275
|
||||
|
||||
# Run as the non-root `node` user the base image ships. The inspector resolves
|
||||
# its runtime-state dir (default catalog, OAuth token storage) from `HOME`
|
||||
|
|
|
|||
76
README.md
76
README.md
|
|
@ -14,6 +14,8 @@ npx @modelcontextprotocol/inspector --cli # CLI
|
|||
npx @modelcontextprotocol/inspector --tui # TUI
|
||||
```
|
||||
|
||||
> **Upgrading from v1?** Read the [v1 → v2 migration guide](./docs/v1-to-v2-migration.md) — CLI flags, the new `--config` vs. `--catalog` split, the Node engine bump, and what no longer ships.
|
||||
|
||||
> **Repo status.** This is the **v2** line of the Inspector. Active development happens on **`v2/main`** (the develop branch — all v2 PRs target it), which is merged into **`main`** at milestone releases; `main` is the default branch and holds the latest released v2, published to the npm `latest` tag. The legacy **v1** line lives on **`v1/main`** — security fixes only, published straight from that branch to the npm `v1-latest` tag (`npx @modelcontextprotocol/inspector@v1-latest`). See [`AGENTS.md`](./AGENTS.md) for branch/board conventions.
|
||||
|
||||
## Project layout
|
||||
|
|
@ -37,8 +39,8 @@ inspector/
|
|||
│ ├── react/ # React hooks over the state stores
|
||||
│ └── storage/ # File I/O helpers for the OAuth persist backends
|
||||
├── test-servers/ # Composable MCP test servers + fixtures used by integration tests
|
||||
├── scripts/ # Root build/verify tooling (install cascade, smokes, verify-build-gate, verify-format-coverage, pack:verify)
|
||||
├── docs/ # Task-oriented guides (server configuration, MCP App review, launcher/config plan)
|
||||
├── scripts/ # Root build/verify tooling (install cascade, smokes, verify-build-gate, verify-format-coverage, verify-dep-lockstep, pack:verify)
|
||||
├── docs/ # Task-oriented guides (v1→v2 migration, server configuration, MCP App review, launcher/config plan)
|
||||
├── specification/ # Design/build specifications
|
||||
├── AGENTS.md # Contribution rules for agents AND humans (see below)
|
||||
└── README.md # You are here
|
||||
|
|
@ -49,6 +51,7 @@ Each client has its own README with client-specific detail:
|
|||
|
||||
Task-oriented guides live under [`docs/`](./docs):
|
||||
|
||||
- [Migrating from v1 to v2](./docs/v1-to-v2-migration.md) — the v1 → v2 map: CLI flag mapping, `--config` vs. `--catalog` semantics with before/after examples, the Node engine bump (`>=22.7.5` → `>=22.19.0`), env-var renames, and the sub-packages that no longer ship.
|
||||
- [MCP server configuration](./docs/mcp-server-configuration.md) — which server(s) the Inspector connects to: `--catalog` vs. `--config`, ad-hoc targets, the `--` separator, the file format and its Inspector-specific per-server fields. Shared by all three clients; the cli and tui READMEs delegate their server-options sections to it.
|
||||
- [Reviewing an MCP App](./docs/mcp-app-review.md) — the CLI-first → one-shot-web recipe for automated App-tool review: `--app-info` probe → deep-link navigate → rendered widget, plus OAuth handoff and proxy support.
|
||||
- [Launcher and config consolidation](./docs/launcher-config-consolidation-plan.md) — why the launcher runs a client in-process rather than spawning it, and how the shared config processor fits in.
|
||||
|
|
@ -66,6 +69,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 root-only placement holds for anything reached solely through root-owned code with no manifest of its own (`test-servers/src`, `core/`), and `vitest.shared.mts` aliases those to the repo root — `express` and `yaml`, both reached through `test-servers/src`, are the two today. **Whether such a package is a `dependency` or a `devDependency` follows from who consumes it at runtime, not from where it is declared:** anything `core/` imports at runtime must be a root **`dependency`**, because the client builds externalize npm packages and a published install resolves them from the root manifest, where devDependencies are absent. `express` is test-only and is a devDependency; `yaml` currently sits in `dependencies`.
|
||||
|
||||
## Running during development
|
||||
|
||||
For day-to-day web iteration, run Vite directly from the web client (fast HMR, no launcher build needed):
|
||||
|
|
@ -277,17 +282,18 @@ Individual clients: `build:web`, `build:cli`, `build:tui`, `build:launcher`. The
|
|||
|
||||
Each client self-validates from its own folder; the root scripts chain them. There is **no** aggregate root `test` script — use `validate` (fast) or `coverage` (the gate).
|
||||
|
||||
| Script | What it does |
|
||||
| --------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `npm run validate` | Runs `verify:format-coverage` (asserts every tracked source file is format-gated) first, then `validate:core` (the shared `core/` `format:check` + `lint` gate), then per client: `format:check` + `lint` + **`typecheck`** (cli/tui only) + `build` + fast unit tests. The quick inner-loop check. |
|
||||
| `npm run coverage` | The **per-file ≥90% gate** (lines/statements/functions/branches) under v8 instrumentation, per client. CI-enforced. For web this also runs the integration project and covers the shared `core/` runtime (including `core/json` and `core/client`). |
|
||||
| `npm run smoke` | End-to-end smokes through the built launcher (`--help` dispatch + prod cli/tui/web), plus two headless-Chromium smokes: a boot smoke that runs the prod web bundle and asserts a clean first render (no uncaught error — sync exception or unhandled rejection, how a Node built-in reaching the browser bundle manifests), and an **MCP Apps** smoke (`smoke:web:app`) that drives connect → open app → `data-app-status="ready"` against a composable App server, covering the sandbox proxy and UI-protocol bridge. |
|
||||
| `npm run verify:build-gate` | Runs a real `vite build` with a Node built-in forced into the browser graph and asserts the build **fails** via the #1769 gate (which turns Vite's browser-externalization warning into a hard error). Guards against the warning phrasing drifting in a Vite bump and silently disabling the gate. Part of `npm run ci`. |
|
||||
| `npm run verify:format-coverage` | Parses the `format:check` globs out of every `package.json` (only those reachable from `validate`), enumerates all tracked source files, and **fails** listing any not covered by a glob — the durable guard for the "every first-party source file is format-gated" invariant (#1792). Runs first in `validate`. |
|
||||
| `npm run test:scripts` | Table-driven unit tests (`node --test`) for the guard's own pure parsers (`scripts/lib/npm-scripts.mjs` + the exported helpers of `verify-typecheck-coverage.mjs`), one case per rule they encode. Runs in `validate` — and `verify:typecheck-coverage` guards *this* gate in turn (reachable from `validate`, non-empty test set, every test file matched by the `test:scripts` glob), since `node --test` silently skips a file its glob misses and still exits 0. |
|
||||
| `npm run verify:typecheck-coverage` | The typecheck-coverage analog of the above (#1791): for each Node client (auto-discovered from disk — enrolled via its `typecheck` script's projects, or for a `tsc -b` client like `clients/web` via its `tsconfig.json` `references`) it runs those projects with `tsc --listFilesOnly`, unions them, and **fails** listing any tracked `.ts`/`.tsx`/`.mts`/`.cts` under the client that lands in no project (so a new top-level config/helper can't silently go untypechecked). It also requires, deny-by-default, the first-party TS no client owns (`test-servers/src`, the root `vitest.shared.mts`, all of `core/`, and any new top-level location) to land in some client project's tsc pass — so a `core` `*.tsx` web's projects don't reach is caught too. Also asserts the gate is wired (each client's typecheck pass — its `typecheck` script, or web's `tsc -b` — is reachable from its `validate`, and the root chain runs each client's `validate`). Runs in `validate`. |
|
||||
| `npm run ci` | **Mandatory pre-push command.** `validate` → `coverage` → `verify:build-gate` → `smoke` → Storybook. A true superset of GitHub CI. |
|
||||
| `npm run pack:verify` | Publish smoke — see [Publishing](#publishing). |
|
||||
| Script | What it does |
|
||||
| ----------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `npm run validate` | Runs the three durable guards first — `verify:format-coverage` (every tracked source file is format-gated), `verify:typecheck-coverage` (every one lands in a tsconfig project), `verify:dep-lockstep` (no dependency the shared sources directly import skews across installs) — then `test:scripts` (the guards' own parser unit tests), then `validate:core` (the shared `core/` `format:check` + `lint` gate), then per client: `format:check` + `lint` + **`typecheck`** (cli/tui/launcher; web typechecks via `tsc -b` inside its `build`) + `build` + fast unit tests. The quick inner-loop check. |
|
||||
| `npm run coverage` | The **per-file ≥90% gate** (lines/statements/functions/branches) under v8 instrumentation, per client. CI-enforced. For web this also runs the integration project and covers the shared `core/` runtime (including `core/json` and `core/client`). |
|
||||
| `npm run smoke` | End-to-end smokes through the built launcher (`--help` dispatch + prod cli/tui/web), plus two headless-Chromium smokes: a boot smoke that runs the prod web bundle and asserts a clean first render (no uncaught error — sync exception or unhandled rejection, how a Node built-in reaching the browser bundle manifests), and an **MCP Apps** smoke (`smoke:web:app`) that drives connect → open app → `data-app-status="ready"` against a composable App server, covering the sandbox proxy and UI-protocol bridge. |
|
||||
| `npm run verify:build-gate` | Runs a real `vite build` with a Node built-in forced into the browser graph and asserts the build **fails** via the #1769 gate (which turns Vite's browser-externalization warning into a hard error). Guards against the warning phrasing drifting in a Vite bump and silently disabling the gate. Part of `npm run ci`. |
|
||||
| `npm run verify:format-coverage` | Parses the `format:check` globs out of every `package.json` (only those reachable from `validate`), enumerates all tracked source files, and **fails** listing any not covered by a glob — the durable guard for the "every first-party source file is format-gated" invariant (#1792). Runs first in `validate`. |
|
||||
| `npm run test:scripts` | Table-driven unit tests (`node --test`) for the guard's own pure parsers (`scripts/lib/npm-scripts.mjs` + the exported helpers of `verify-typecheck-coverage.mjs` and `verify-dep-lockstep.mjs`), one case per rule they encode. Runs in `validate` — and `verify:typecheck-coverage` guards *this* gate in turn (reachable from `validate`, non-empty test set, every test file matched by the `test:scripts` glob), since `node --test` silently skips a file its glob misses and still exits 0. |
|
||||
| `npm run verify:typecheck-coverage` | The typecheck-coverage analog of the above (#1791): for each Node client (auto-discovered from disk — enrolled via its `typecheck` script's projects, or for a `tsc -b` client like `clients/web` via its `tsconfig.json` `references`) it runs those projects with `tsc --listFilesOnly`, unions them, and **fails** listing any tracked `.ts`/`.tsx`/`.mts`/`.cts` under the client that lands in no project (so a new top-level config/helper can't silently go untypechecked). It also requires, deny-by-default, the first-party TS no client owns (`test-servers/src`, the root `vitest.shared.mts`, all of `core/`, and any new top-level location) to land in some client project's tsc pass — so a `core` `*.tsx` web's projects don't reach is caught too. Also asserts the gate is wired (each client's typecheck pass — its `typecheck` script, or web's `tsc -b` — is reachable from its `validate`, and the root chain runs each client's `validate`). Runs in `validate`. |
|
||||
| `npm run verify:dep-lockstep` | Guards the "one version per install-crossing dependency" invariant (#1896). v2 is not a workspace, so a client's test project compiles the shared first-party TypeScript — `core/`, `test-servers/src`, and the root-owned `vitest.shared.mts`, all of which resolve their dependencies from the **root** install — alongside the client's own sources, putting the same package in one `tsc` program twice. At the same version that's harmless; skewed, TypeScript must relate two structurally-distinct copies of every type, which for a recursive-generic surface is exponential (zod `4.3.6` vs `4.4.3` exhausted the 4GB tsc heap in `clients/web`). Derives its candidate set from what those shared sources import, compares the committed lockfiles' top-level entries, and **fails deny-by-default** on any skew not in the annotated `TOLERATED_SKEW` allowlist — and an allowlisted package is tolerated only *within a major version*. Runs in `validate`. |
|
||||
| `npm run ci` | **Mandatory pre-push command.** `validate` → `coverage` → `verify:build-gate` → `smoke` → Storybook. A true superset of GitHub CI. |
|
||||
| `npm run pack:verify` | Publish smoke — see [Publishing](#publishing). |
|
||||
|
||||
Per-client scripts exist too (`validate:web`, `coverage:cli`, `smoke:tui`, …), plus root `validate:core` / `format:core` for the shared `core/` package, `format:scripts` for the root `scripts/` tooling, and `format:shared` / `lint:shared` for the root "shared" surface (`test-servers/src/**`, `vitest.shared.mts`, the root `eslint.config.js`). Run `npm run format` before committing — the root `format` fixes `core/`, the root `scripts/`, the shared surface, and every client; `validate` runs the non-fixing `format:check` and fails CI on any unformatted file.
|
||||
|
||||
|
|
@ -303,7 +309,8 @@ The root `package.json` `"files"` allowlist is the source of truth for the tarba
|
|||
|
||||
- **No source maps.** The client bundlers set `sourcemap: false` (`clients/{cli,tui}/tsup.config.ts`, `clients/web/tsup.runner.config.ts`); Vite and the launcher's `tsc` already emit none. Maps are ~half the unpacked size and aren't needed at runtime — debug via `npm run dev` on the source.
|
||||
- **`clients/web/build` ships via `clients/web/.npmignore`.** `clients/web/.gitignore` lists `build/`, and npm's packlist honors that nested `.gitignore` over the root `"files"` allowlist — so the prod web-server runner was silently missing from the tarball while `clients/web/dist` slipped through (its `.gitignore` only lists `dist-ssr`). `clients/web/.npmignore` overrides the `.gitignore` for publishing so both `build/` (runner) and `dist/` (SPA) ship. The other clients don't need this — none ship a nested `.gitignore`.
|
||||
- **`clients/web/static` ships the MCP Apps sandbox proxy.** `clients/web/static/sandbox_proxy.html` is a committed source file (not a build artifact), read from disk at runtime by `clients/web/server/sandbox-controller.ts` as `<runner dir>/../static/sandbox_proxy.html`. It was missing from the root `"files"` allowlist entirely, so every published build failed the Apps tab with **"Sandbox not loaded"** ([#1859](https://github.com/modelcontextprotocol/inspector/issues/1859)) while working fine in the repo. Because the path is resolved *relative to* `clients/web/build`, the directory must ship at that exact location — `pack:verify` asserts both the tarball entry and the installed-on-disk path.
|
||||
- **`clients/web/static` ships the MCP Apps sandbox proxy.** `clients/web/static/sandbox_proxy.html` is a committed source file (not a build artifact), read from disk at runtime by `clients/web/server/sandbox-controller.ts` as `<runner dir>/../static/sandbox_proxy.html`. It was missing from the root `"files"` allowlist entirely, so every published build failed the Apps tab with **"Sandbox not loaded"** ([#1859](https://github.com/modelcontextprotocol/inspector/issues/1859)) while working fine in the repo. Because the path is resolved _relative to_ `clients/web/build`, the directory must ship at that exact location — `pack:verify` asserts both the tarball entry and the installed-on-disk path.
|
||||
- **A dependency that renders React is bundled, not externalized.** An externalized package resolves its own `react` from wherever npm placed **it** in the consumer's tree, which is not necessarily where the bundle resolves ours — npm places a package beside a React satisfying *its* peer range, and those ranges are looser than ours. `ink-form` and `ink-scroll-view` declare `">=18"`, so a project holding React 18 satisfies them and gets them hoisted while the Inspector's React 19 nests underneath: two React copies, and the TUI dies with `TypeError: Cannot read properties of null (reading 'useState')` the moment a tool test form or a scroll view mounts ([#1952](https://github.com/modelcontextprotocol/inspector/issues/1952)). Both are therefore inlined by `clients/tui/tsup.config.ts` and are **not** root dependencies: the tarball ships their code inside `clients/tui/build/index.js` rather than having consumers install them. Bundling also pins their transitive deps to what this repo's install resolved (notably `ink-select-input@6` via `overrides`, which npm ignores for a package installed as a dependency). **`ink` is the one exception, on cost:** bundling it works but adds ~1.4 MB (`react-reconciler` and `yoga-layout` come along, plus a `createRequire` banner for the inlined CJS), so it stays external — *not* because its `">=19"` peer makes it safe, which it does not. What keeps that tolerable is the root `react` range: `"^19.0.0"` is deliberately open to the whole major so npm can dedupe our React with whatever React 19 a consumer pins, leaving an external `ink` on the same copy the bundle uses. **Narrowing that range reopens the bug for the renderer itself** — `clients/tui/__tests__/tsupConfig.test.ts` pins it to `ink`'s peer floor, and guards the rest of the split; see the [TUI README](./clients/tui/README.md#bundling-react-rendering-dependencies-must-be-inlined-1952).
|
||||
- **A single version number, read from the root `package.json`.** The Inspector ships as one package with one version, so only the **root** `package.json` carries a `version` — the four `clients/*/package.json`s deliberately have none. Every Node client (CLI, TUI, and the web backend) resolves the version through the shared `readInspectorVersion()` reader in `core/node/version.ts`, which walks up to the root manifest (always present in the tarball). No client `package.json` is read at runtime, so none needs to ship. The web **browser** can't read the filesystem; it gets its version from the backend via `GET /api/config` (see [#1639](https://github.com/modelcontextprotocol/inspector/issues/1639)).
|
||||
|
||||
### `npm run pack:verify` — publish smoke against the real tarball
|
||||
|
|
@ -319,16 +326,42 @@ Publishing is automated by two release-gated jobs in [`.github/workflows/main.ym
|
|||
|
||||
A v2 release is cut from **`main`**, after the milestone's work has been merged there from `v2/main` — not from `v2/main` itself. (The v1 line releases independently from `v1/main` to the `v1-latest` tag and never touches `main`; see [Repo status](#mcp-inspector).)
|
||||
|
||||
Because there is **one version number** (only the root `package.json` has one — the clients carry none, so there is nothing to keep in sync and no `check-version` step), the release flow is just:
|
||||
Because there is **one version number** (only the root `package.json` has one — the clients carry none, so there is nothing to keep in sync and no `check-version` step), the release flow is three steps.
|
||||
|
||||
**1. Bump on `v2/main`, before the milestone merge.** The bump is part of the milestone's work, so it belongs on the develop branch and flows into `main` with everything else:
|
||||
|
||||
Substitute the real issue number and release below — the commands are written to be copy-pasteable as-is (a `2.2.0` → `2.3.0` minor bump):
|
||||
|
||||
```bash
|
||||
npm version <major|minor|patch> # bumps the root package.json + tags
|
||||
git push --follow-tags
|
||||
git checkout -b v2/chore/2010-bump-2-3-0 v2/main
|
||||
npm version minor --no-git-tag-version # or major / patch; bump only, no tag
|
||||
# PR → v2/main
|
||||
```
|
||||
|
||||
⚠️ **`--no-git-tag-version` is load-bearing.** A bare `npm version` also tags, and the tag would land on a `v2/main` commit — but the release must be cut from `main`, so the tag has to point at the merge commit there (step 3). Tagging here creates a tag on a commit that is never released.
|
||||
|
||||
**2. Merge `v2/main` → `main`** through the usual milestone-merge branch. It now carries the bump, so the release lands on `main` with the version already correct.
|
||||
|
||||
Between steps 1 and 2 the two branches **do** differ, and that is expected, not drift: `v2/main` reads the version being built while `main` still reads the one currently released. What this ordering removes is *post-release* drift — once the milestone merge lands they agree again, and `v2/main` is never left **behind** `main`. If you see `v2/main` ahead of `main`, a release is in flight; if you see it behind, something went wrong.
|
||||
|
||||
**3. Tag the `main` commit and draft the Release:**
|
||||
|
||||
```bash
|
||||
git fetch origin main
|
||||
git tag 2.3.0 origin/main && git push origin 2.3.0
|
||||
# then draft & publish a GitHub Release for that tag → triggers `publish`
|
||||
```
|
||||
|
||||
⚠️ **Tag `origin/main`, not your local `HEAD`.** `git checkout main && git pull` resolves through whatever merge-or-rebase strategy you have configured, so a divergent local `main` can quietly produce or replay local commits. Tagging `HEAD` there tags a commit that is not on `origin/main`, and `git push origin <tag>` pushes only the tag — leaving a release whose commit was never published. Naming `origin/main` explicitly makes the tagged commit exactly what the remote branch points at, regardless of local state.
|
||||
|
||||
⚠️ **No `v` prefix.** This repo's release tags are bare `x.y.z` — `2.2.0`, `2.1.0`, `2.0.0` — so tag `2.3.0`, not `v2.3.0`. Note npm's own `tag-version-prefix` defaults to `v` and the repo sets no `.npmrc`, so a bare `npm version` would have produced a `v`-prefixed tag that does not match the convention. Tagging by hand (step 3) is what keeps it right. The workflow's assert step strips a leading `v` before comparing, so a `v`-prefixed tag would still publish — it would just be inconsistent with every previous release.
|
||||
|
||||
The release's target commit selects which workflow runs, so this only publishes when a release is cut from a commit carrying this (v2) workflow.
|
||||
|
||||
**Why the bump goes on `v2/main` first ([#2010](https://github.com/modelcontextprotocol/inspector/issues/2010)).** It used to happen on the milestone-merge branch, which is cut from `main` — so the bump existed only *downstream* of `v2/main` and nothing carried it back. `v2/main` sat at `2.0.0` through both the 2.1.0 and 2.2.0 releases. That is not cosmetic: a branch cut from a milestone-merge branch silently carries the bump into an unrelated PR (this happened on [#2009](https://github.com/modelcontextprotocol/inspector/issues/2009), where a container bugfix arrived with a `2.0.0 → 2.2.0` diff), and anything reading the version in development — `readInspectorVersion()`, `--version`, `GET /api/config` — reported a version two releases old.
|
||||
|
||||
Do **not** "fix" a future drift by merging `main` back into `v2/main`. `main` carries the entire pre-v2 v1 history (retained through `ec5d8e13 chore: replace main's tree with v2` — ~230 commits `v2/main` does not have), so a back-merge grafts all of it into the develop branch's log permanently in order to deliver a two-file change. Bumping first means there is nothing to back-merge.
|
||||
|
||||
### Docker
|
||||
|
||||
A container image is published to GHCR (`ghcr.io/modelcontextprotocol/inspector`, `linux/amd64` + `linux/arm64`) by the release workflow. The [`Dockerfile`](Dockerfile) is a two-stage build: the first stage installs and `npm pack`s the publishable tarball; the second stage `npm install -g`s that tarball, so the image ships the exact same artifact as npm, with a clean `mcp-inspector` bin.
|
||||
|
|
@ -342,6 +375,15 @@ docker build -t mcp-inspector .
|
|||
docker run --rm -p 127.0.0.1:6274:6274 mcp-inspector
|
||||
```
|
||||
|
||||
**Using the Apps tab? Publish `6275` too.** The MCP Apps sandbox is a second listener the browser reaches directly, on `MCP_SANDBOX_PORT` (default `6275`). Nothing else needs it, so the single-port commands above are fine for ordinary inspection — but the Apps tab renders a blank widget without it:
|
||||
|
||||
```bash
|
||||
docker run --rm -p 127.0.0.1:6274:6274 -p 127.0.0.1:6275:6275 \
|
||||
ghcr.io/modelcontextprotocol/inspector
|
||||
```
|
||||
|
||||
Publish it on the **same port number** inside and out. The sandbox URL is handed to the browser via `/api/config` as `http://localhost:<container port>/sandbox`, so remapping it (`-p 9000:6275`) advertises a port the browser can't reach; use `-e MCP_SANDBOX_PORT=9000 -p 127.0.0.1:9000:9000` instead.
|
||||
|
||||
**Keep the `127.0.0.1:` prefix on the published port.** A bare `-p 6274:6274` publishes on **every host interface**, putting the Inspector on your local network. The container's `HOST=0.0.0.0` is a separate concern — it governs the _container's_ interfaces, not the host's — so the `DANGEROUSLY_BIND_ALL_INTERFACES` opt-in that guards a wildcard bind outside a container does not cover this. It matters more here than for an ordinary web app: the backend spawns processes on request, `GET /` embeds the API token into the served HTML, and a request arriving with **no** `Origin` header skips the origin allow-list entirely — so for any non-browser client the API token is the only guard. Publishing wider needs a real access-control boundary in front of the Inspector — a reverse proxy that authenticates, an SSH tunnel, a private network. Setting your own `MCP_INSPECTOR_API_TOKEN` does **not** substitute: `GET /` discloses whatever token is in use, so a custom one is harvested exactly as easily as a generated one.
|
||||
|
||||
**Keeping the servers you add.** The Inspector saves your server list to `$HOME/.mcp-inspector/mcp.json`, which in the image is `/home/node/.mcp-inspector/mcp.json` — inside the container's writable layer, so `--rm` discards it and every run starts with an empty list. Mount a volume there to keep it:
|
||||
|
|
|
|||
|
|
@ -12,6 +12,8 @@ npx @modelcontextprotocol/inspector --cli node build/index.js
|
|||
|
||||
Supports tools, resources, and prompts (plus `--method servers/list` / `servers/show` for catalog entries without connecting).
|
||||
|
||||
> Coming from the v1 CLI? See the [v1 → v2 migration guide](../../docs/v1-to-v2-migration.md) — every v1 flag still exists, but exit codes, argument ordering, and the `--` separator changed.
|
||||
|
||||
### Examples
|
||||
|
||||
**Basic usage**
|
||||
|
|
|
|||
1024
clients/cli/package-lock.json
generated
1024
clients/cli/package-lock.json
generated
File diff suppressed because it is too large
Load diff
|
|
@ -32,10 +32,6 @@
|
|||
"format:check": "prettier --check src __tests__ \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\""
|
||||
},
|
||||
"dependencies": {
|
||||
"@modelcontextprotocol/client": "2.0.0-beta.5",
|
||||
"@modelcontextprotocol/core": "2.0.0-beta.5",
|
||||
"@modelcontextprotocol/server": "2.0.0-beta.5",
|
||||
"@modelcontextprotocol/server-legacy": "2.0.0-beta.5",
|
||||
"@napi-rs/keyring": "^1.3.0",
|
||||
"ajv": "^8.17.1",
|
||||
"atomically": "^2.1.1",
|
||||
|
|
@ -43,7 +39,7 @@
|
|||
"open": "^10.2.0",
|
||||
"pino": "^9.14.0",
|
||||
"undici": "^8.5.0",
|
||||
"zod": "^4.3.6"
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@eslint/js": "^10.0.1",
|
||||
|
|
|
|||
|
|
@ -102,7 +102,93 @@ The repo-root `validate:tui` just delegates here. `eslint.config.js` registers
|
|||
stricter react-hooks@7 rules are not enforced on the interim component surface
|
||||
(#1501).
|
||||
|
||||
Tests live in `__tests__/`. The coverage gate currently covers the TUI's
|
||||
non-React logic (server resolution, logger, tab metadata, and the `utils/`
|
||||
form/URL helpers); the Ink components and `App.tsx` are an interim exclusion in
|
||||
`vitest.config.ts` pending a renderer-based follow-up.
|
||||
Tests live in `__tests__/`. The coverage gate covers **all of `src/**`**, React
|
||||
surface included — the Ink components mount through `ink-testing-library` (with
|
||||
the `ink-scroll-view` / `ink-form` passthrough doubles in `__tests__/helpers/`),
|
||||
`App.tsx` mounts against a mock of the `@inspector/core` surface, and keypresses
|
||||
are driven through stdin. The former interim exclusion of the components and
|
||||
`App.tsx` was lifted in #1501; the only exclusion left in `vitest.config.ts` is
|
||||
`src/tui-servers.ts`, a pure re-export of core's server resolver with no runtime
|
||||
statements of its own (its logic is measured in `core/` via the web suite, and
|
||||
`tui-servers.test.ts` still exercises it behaviorally — it is excluded only so it
|
||||
doesn't surface as a misleading 0/0 row).
|
||||
|
||||
### Bundling: React-rendering dependencies must be inlined (#1952)
|
||||
|
||||
`tsup.config.ts` splits the TUI's dependencies into bundled (`noExternal`) and
|
||||
externalized (`external`). For anything that **renders React components**, that
|
||||
choice decides which React instance it gets at runtime, and getting it wrong
|
||||
crashes the TUI in a way nothing in this repo can see.
|
||||
|
||||
An external package's `import "react"` resolves from wherever npm placed **that
|
||||
package** — and npm places it beside a React satisfying *its* peer range, which
|
||||
is looser than ours in every case here:
|
||||
|
||||
| Package | Its `react` peer | Placed beside a different React when… |
|
||||
| --- | --- | --- |
|
||||
| `ink-form`, `ink-scroll-view` | `">=18"` | the consumer has React 18 |
|
||||
| `ink` | `">=19"` | the consumer pins React 19.0 (our `^19.2.4` then nests) |
|
||||
|
||||
Either way the bundle renders through one React while the external package calls
|
||||
hooks on another, whose dispatcher is null — so opening a tool test form (or any
|
||||
scroll view, or in `ink`'s case simply starting the TUI) dies with
|
||||
`TypeError: Cannot read properties of null (reading 'useState')`.
|
||||
|
||||
`ink-form` and `ink-scroll-view` are therefore **inlined**, which removes npm
|
||||
from the decision: an inlined package's `import "react"` is emitted into
|
||||
`build/index.js`, so it resolves from the build directory exactly like the
|
||||
bundle's own. Inlining also pins transitive deps to what *this* install
|
||||
resolved, notably `ink-select-input@6` via the `overrides` entry — npm ignores a
|
||||
dependency's `overrides`, so a consumer install would otherwise pull the
|
||||
React-18-era v5 that `ink-form` asks for.
|
||||
|
||||
#### Why `ink` itself is the exception
|
||||
|
||||
`ink` is external for **cost**, not because its `">=19"` peer makes it safe — it
|
||||
does not, and an earlier revision of this section wrongly claimed it did.
|
||||
Bundling `ink` works (it was built and verified against both consumer repros)
|
||||
but adds ~1.4 MB, since `react-reconciler` and `yoga-layout` come with it, plus
|
||||
a `createRequire` banner: inlined CJS calls `require` at runtime
|
||||
(`react-reconciler` for `"react"`, `signal-exit@3` for `"assert"`) and esbuild's
|
||||
interop shim throws `Dynamic require of "x" is not supported` without a real
|
||||
`require` in scope. A 602 KB bundle beat a 2 MB one.
|
||||
|
||||
What makes the exemption tolerable is a **separate lever**: the root manifest
|
||||
declares `react: "^19.0.0"` — open to the whole major, rather than pinned at the
|
||||
version we happen to develop against. That lets npm satisfy our React and a
|
||||
consumer's pinned React 19.x with a single copy, so an external `ink` resolves
|
||||
*ours*. Verified against a consumer pinning `react@19.0.0` alongside `ink@6.8.0`
|
||||
— the case that splits under a narrower range:
|
||||
|
||||
```
|
||||
bundle react node_modules/react/index.js
|
||||
ink -> react node_modules/react/index.js # same copy
|
||||
```
|
||||
|
||||
Narrow that range and the exemption turns straight back into #1952, one level
|
||||
up — breaking TUI startup rather than just its forms. `tsupConfig.test.ts` pins
|
||||
the root range to `ink`'s own peer floor so it can't drift silently. The
|
||||
residual after all this is a React **20**-era consumer that also depends on
|
||||
`ink`; that gets revisited when the Inspector moves to React 20.
|
||||
|
||||
`__tests__/tsupConfig.test.ts` enforces the whole split: every dependency
|
||||
declaring a `react` peer must be in `noExternal` unless it is listed as external
|
||||
by design, each exempt package must also be a root dependency (external means
|
||||
consumers install it), and the root `react` range must stay open to the major.
|
||||
Add a React-rendering dependency, and that test tells you to bundle it.
|
||||
|
||||
### The `ink-form` label patch
|
||||
|
||||
Bundling `ink-form` also makes it patchable, which one label needs: the hint
|
||||
under an incomplete form reads "you have not **competed** yet". It is upstream's
|
||||
string, hardcoded in `ink-form/lib/SubmitButton.js` with no prop to override,
|
||||
and `ink-form` was last published in 2024 — so `tsup.config.ts` corrects it with
|
||||
an esbuild `onLoad` hook as the file enters the bundle. It is reported upstream
|
||||
as [lukasbach/ink-form#14](https://github.com/lukasbach/ink-form/issues/14); if
|
||||
a release ever carries the fix, drop the patch.
|
||||
|
||||
The hook **throws** when the string isn't found rather than passing the source
|
||||
through. A silent no-op would let an `ink-form` upgrade retire the patch without
|
||||
anyone noticing it had stopped applying — or leave a patch aimed at a string
|
||||
that no longer exists. If the build fails there, check whether upstream fixed
|
||||
the typo and delete the patch instead of re-targeting it.
|
||||
|
|
|
|||
262
clients/tui/__tests__/tsupConfig.test.ts
Normal file
262
clients/tui/__tests__/tsupConfig.test.ts
Normal file
|
|
@ -0,0 +1,262 @@
|
|||
/**
|
||||
* Bundling invariant for the TUI's React-rendering dependencies (#1952).
|
||||
*
|
||||
* The published TUI is a single ESM bundle whose bare `import "react"` resolves
|
||||
* from `clients/tui/build/`. Any package left *external* resolves its own
|
||||
* `react` from wherever npm placed **it** instead — and npm places a package
|
||||
* next to a version satisfying its declared peer range. `ink-form` and
|
||||
* `ink-scroll-view` accept `react: ">=18"`, so a consumer project holding React
|
||||
* 18 satisfies them, they hoist to that project's root, and the Inspector's
|
||||
* React 19 nests beneath it. Two React copies later, the first hook either of
|
||||
* them calls reads a null dispatcher and the TUI dies with
|
||||
* "Cannot read properties of null (reading 'useState')" — the moment a tool
|
||||
* test form or a scroll view mounts.
|
||||
*
|
||||
* Inlining them removes npm from the decision entirely: their `import "react"`
|
||||
* is emitted into the bundle, so it resolves exactly where the bundle's does.
|
||||
*
|
||||
* This test is the durable guard, because the failure is invisible in the repo
|
||||
* (a dev install has one React) and in every smoke (same) — it only appears
|
||||
* once the package is installed *under* another project that renders React.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import type { Options } from "tsup";
|
||||
import tsupConfig, {
|
||||
INK_FORM_INCOMPLETE_HINT,
|
||||
INK_FORM_SUBMIT_BUTTON,
|
||||
fixInkFormIncompleteHint,
|
||||
inkFormLabelPatch,
|
||||
} from "../tsup.config.js";
|
||||
|
||||
const clientDir = path.resolve(
|
||||
path.dirname(fileURLToPath(import.meta.url)),
|
||||
"..",
|
||||
);
|
||||
|
||||
/**
|
||||
* `defineConfig()` is typed as a union — a single options object, an array of
|
||||
* them, or a function returning either. The TUI config is the first form, so
|
||||
* narrow to it at runtime rather than assuming: a future config that grows a
|
||||
* second build should fail loudly here instead of silently checking nothing.
|
||||
*/
|
||||
function singleConfig(config: typeof tsupConfig): Options {
|
||||
if (typeof config !== "object" || Array.isArray(config)) {
|
||||
throw new Error(
|
||||
"clients/tui/tsup.config.ts is expected to export one options object",
|
||||
);
|
||||
}
|
||||
return config;
|
||||
}
|
||||
|
||||
/** The string entries of an `external` / `noExternal` list (ours are all strings but the type allows RegExp). */
|
||||
function stringEntries(list: (string | RegExp)[] | undefined): string[] {
|
||||
return (list ?? []).filter((entry) => typeof entry === "string");
|
||||
}
|
||||
|
||||
/**
|
||||
* React-rendering dependencies left external by an explicit trade-off.
|
||||
*
|
||||
* `ink` is here for its **cost**, not because its peer range makes it safe.
|
||||
* Bundling it works (verified against both consumer repros) but adds ~1.4MB —
|
||||
* react-reconciler and yoga-layout come with it — plus a `createRequire` banner
|
||||
* for the inlined CJS. The smaller tarball won.
|
||||
*
|
||||
* The exemption is only tolerable because the root manifest keeps `react` open
|
||||
* to the whole major (`^19.0.0`), which lets npm dedupe our React with whatever
|
||||
* React 19 a consumer pins — so an external `ink` resolves the same copy the
|
||||
* bundle does. The test below pins that relationship: narrowing the root range
|
||||
* silently reopens #1952 for the renderer itself.
|
||||
*/
|
||||
const EXTERNAL_BY_DESIGN = new Set(["ink"]);
|
||||
|
||||
function readPackageJson(name: string): {
|
||||
peerDependencies?: Record<string, string>;
|
||||
} {
|
||||
// Resolve through the client's own node_modules rather than `require.resolve`
|
||||
// on the package name: several of these packages export only their entry
|
||||
// point, so `<name>/package.json` is not a resolvable subpath.
|
||||
const manifest = path.join(clientDir, "node_modules", name, "package.json");
|
||||
return JSON.parse(readFileSync(manifest, "utf8"));
|
||||
}
|
||||
|
||||
/** Dependencies that declare a `react` peer — i.e. that render React themselves. */
|
||||
function reactRenderingDependencies(): string[] {
|
||||
const { dependencies } = JSON.parse(
|
||||
readFileSync(path.join(clientDir, "package.json"), "utf8"),
|
||||
) as { dependencies: Record<string, string> };
|
||||
|
||||
return Object.keys(dependencies).filter(
|
||||
(name) => readPackageJson(name).peerDependencies?.react !== undefined,
|
||||
);
|
||||
}
|
||||
|
||||
/** The real `ink-form` module the label patch targets. */
|
||||
const submitButtonPath = path.join(
|
||||
clientDir,
|
||||
"node_modules",
|
||||
"ink-form",
|
||||
"lib",
|
||||
"SubmitButton.js",
|
||||
);
|
||||
|
||||
const config = singleConfig(tsupConfig);
|
||||
const noExternal = stringEntries(config.noExternal);
|
||||
const external = stringEntries(config.external);
|
||||
|
||||
describe("tui tsup config", () => {
|
||||
it("finds the React-rendering dependencies to check", () => {
|
||||
// A guard on the guard: if this resolves to nothing (a rename, a moved
|
||||
// node_modules), every assertion below would pass vacuously.
|
||||
expect(reactRenderingDependencies().length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("bundles every React-rendering dependency that is not exempt", () => {
|
||||
const shouldInline = reactRenderingDependencies().filter(
|
||||
(name) => !EXTERNAL_BY_DESIGN.has(name),
|
||||
);
|
||||
|
||||
expect(shouldInline.length).toBeGreaterThan(0);
|
||||
for (const name of shouldInline) {
|
||||
expect(
|
||||
noExternal,
|
||||
`${name} renders React, so it must be bundled`,
|
||||
).toContain(name);
|
||||
expect(external, `${name} must not be external`).not.toContain(name);
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps the root react range open to the whole major, so npm can dedupe", () => {
|
||||
// This is what keeps the one exemption tolerable. `ink` is external, so it
|
||||
// resolves whatever React npm placed beside it — and npm can only place it
|
||||
// beside *ours* if our range admits the consumer's React too. Pinning the
|
||||
// root range above the major floor (say `^19.2.4`) means a consumer holding
|
||||
// React 19.0 gets a second copy: `ink` renders through theirs, the bundle
|
||||
// through ours, and the TUI dies at startup on a null dispatcher.
|
||||
//
|
||||
// So the root range must start at the same floor `ink`'s own peer does.
|
||||
const rootReact = (
|
||||
JSON.parse(
|
||||
readFileSync(path.join(clientDir, "..", "..", "package.json"), "utf8"),
|
||||
) as { dependencies: Record<string, string> }
|
||||
).dependencies.react;
|
||||
const inkPeerReact = readPackageJson("ink").peerDependencies?.react ?? "";
|
||||
|
||||
const floor = /^>=(\d+)\.0\.0$/.exec(inkPeerReact)?.[1];
|
||||
expect(floor, `unexpected ink peer range: ${inkPeerReact}`).toBeDefined();
|
||||
expect(
|
||||
rootReact,
|
||||
`root react must be ^${floor}.0.0 so npm can dedupe with any React ${floor} a consumer pins`,
|
||||
).toBe(`^${floor}.0.0`);
|
||||
});
|
||||
|
||||
it("keeps each exempt package external, and declared for consumers", () => {
|
||||
// An external package is not shipped in the bundle, so the root manifest
|
||||
// has to install it — the mirror image of the inlined ones, which must NOT
|
||||
// be root dependencies. Getting this backwards breaks the published TUI at
|
||||
// startup with an unresolved import.
|
||||
const rootDependencies = JSON.parse(
|
||||
readFileSync(path.join(clientDir, "..", "..", "package.json"), "utf8"),
|
||||
) as { dependencies: Record<string, string> };
|
||||
|
||||
for (const name of EXTERNAL_BY_DESIGN) {
|
||||
expect(external, `${name} is external by design`).toContain(name);
|
||||
expect(
|
||||
Object.keys(rootDependencies.dependencies),
|
||||
`${name} is external, so consumers must install it`,
|
||||
).toContain(name);
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps react itself external, as the single shared instance", () => {
|
||||
expect(external).toContain("react");
|
||||
expect(noExternal).not.toContain("react");
|
||||
});
|
||||
|
||||
it("corrects ink-form's misspelled incomplete-form hint", () => {
|
||||
// Read the real dependency, so an `ink-form` upgrade that fixes or rewords
|
||||
// the label fails here — the patch must then be removed, not left silently
|
||||
// matching nothing.
|
||||
const patched = fixInkFormIncompleteHint(
|
||||
readFileSync(submitButtonPath, "utf8"),
|
||||
submitButtonPath,
|
||||
);
|
||||
expect(patched).toContain(INK_FORM_INCOMPLETE_HINT.fixed);
|
||||
expect(patched).not.toContain(INK_FORM_INCOMPLETE_HINT.typo);
|
||||
});
|
||||
|
||||
it("routes the real module through the plugin, not just the helper", async () => {
|
||||
// The helper is only reached if the plugin's onLoad filter matches. Drive
|
||||
// the plugin as esbuild would — register, then invoke — so a filter that
|
||||
// stops matching the real module's path fails here instead of no-opping
|
||||
// through a green build (the exact silence this patch exists to prevent).
|
||||
type OnLoadCallback = (args: {
|
||||
path: string;
|
||||
}) => Promise<{ contents: string }>;
|
||||
const registered: { filter: RegExp; callback: OnLoadCallback }[] = [];
|
||||
|
||||
const build = {
|
||||
onLoad: (options: { filter: RegExp }, callback: OnLoadCallback) =>
|
||||
registered.push({ filter: options.filter, callback }),
|
||||
};
|
||||
// esbuild's `PluginBuild` carries far more than this patch touches, and the
|
||||
// stub above deliberately implements only the one hook it registers — so
|
||||
// the double cast is bridging a real structural gap, not hiding a mismatch.
|
||||
// A hook the patch called but the stub lacks fails as undefined here rather
|
||||
// than passing silently.
|
||||
inkFormLabelPatch.setup(
|
||||
build as unknown as Parameters<typeof inkFormLabelPatch.setup>[0],
|
||||
);
|
||||
|
||||
expect(registered).toHaveLength(1);
|
||||
const [{ filter, callback }] = registered;
|
||||
expect(filter.test(submitButtonPath)).toBe(true);
|
||||
|
||||
const { contents } = await callback({ path: submitButtonPath });
|
||||
expect(contents).toContain(INK_FORM_INCOMPLETE_HINT.fixed);
|
||||
expect(contents).not.toContain(INK_FORM_INCOMPLETE_HINT.typo);
|
||||
});
|
||||
|
||||
it("scopes the patch to ink-form's SubmitButton and nothing else", () => {
|
||||
expect(INK_FORM_SUBMIT_BUTTON.test(submitButtonPath)).toBe(true);
|
||||
// A Windows-style path must match too — the filter runs against whatever
|
||||
// esbuild resolved, and its separator is the platform's.
|
||||
expect(
|
||||
INK_FORM_SUBMIT_BUTTON.test(
|
||||
"C:\\repo\\node_modules\\ink-form\\lib\\SubmitButton.js",
|
||||
),
|
||||
).toBe(true);
|
||||
for (const other of [
|
||||
"/repo/node_modules/ink-form/lib/Form.js",
|
||||
"/repo/node_modules/ink-select-input/build/SubmitButton.js",
|
||||
"/repo/src/SubmitButton.jsx",
|
||||
]) {
|
||||
expect(INK_FORM_SUBMIT_BUTTON.test(other), other).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("fails loudly rather than silently skipping a label it cannot find", () => {
|
||||
expect(() =>
|
||||
fixInkFormIncompleteHint("no such label here", "SubmitButton.js"),
|
||||
).toThrow(/no longer contains the ink-form label/);
|
||||
});
|
||||
|
||||
it("only rewrites the misspelling", () => {
|
||||
// The two strings must differ by exactly the fix, or the patch is silently
|
||||
// changing copy nobody reviewed.
|
||||
expect(INK_FORM_INCOMPLETE_HINT.fixed).toBe(
|
||||
INK_FORM_INCOMPLETE_HINT.typo.replace("competed", "completed"),
|
||||
);
|
||||
});
|
||||
|
||||
it("verifies the deps it exempts are still declared", () => {
|
||||
// If `ink` ever leaves the dependency list, the exemption above is stale
|
||||
// and would silently excuse a future package that took its name.
|
||||
for (const name of EXTERNAL_BY_DESIGN) {
|
||||
expect(reactRenderingDependencies()).toContain(name);
|
||||
}
|
||||
});
|
||||
});
|
||||
79
clients/tui/package-lock.json
generated
79
clients/tui/package-lock.json
generated
|
|
@ -8,8 +8,6 @@
|
|||
"name": "@modelcontextprotocol/inspector-tui",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@modelcontextprotocol/client": "2.0.0-beta.5",
|
||||
"@modelcontextprotocol/core": "2.0.0-beta.5",
|
||||
"@napi-rs/keyring": "^1.3.0",
|
||||
"ajv": "^8.17.1",
|
||||
"atomically": "^2.1.1",
|
||||
|
|
@ -20,7 +18,7 @@
|
|||
"open": "^10.2.0",
|
||||
"pino": "^9.14.0",
|
||||
"react": "^19.2.4",
|
||||
"zod": "^4.3.6"
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"bin": {
|
||||
"mcp-inspector-tui": "build/index.js"
|
||||
|
|
@ -1054,36 +1052,6 @@
|
|||
"@jridgewell/sourcemap-codec": "^1.4.14"
|
||||
}
|
||||
},
|
||||
"node_modules/@modelcontextprotocol/client": {
|
||||
"version": "2.0.0-beta.5",
|
||||
"resolved": "https://registry.npmjs.org/@modelcontextprotocol/client/-/client-2.0.0-beta.5.tgz",
|
||||
"integrity": "sha512-YuuNm5f2TMoFQRje1UqVP8TJRjijCXMz4ckvoVpx1cUXuBEmykWQ2d8R536pek6UKcXT41T5nWc4qR1JFIbEmg==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@modelcontextprotocol/core": "2.0.0-beta.5",
|
||||
"cross-spawn": "^7.0.5",
|
||||
"eventsource": "^3.0.2",
|
||||
"eventsource-parser": "^3.0.0",
|
||||
"jose": "^6.1.3",
|
||||
"pkce-challenge": "^5.0.0",
|
||||
"zod": "^4.2.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=20"
|
||||
}
|
||||
},
|
||||
"node_modules/@modelcontextprotocol/core": {
|
||||
"version": "2.0.0-beta.5",
|
||||
"resolved": "https://registry.npmjs.org/@modelcontextprotocol/core/-/core-2.0.0-beta.5.tgz",
|
||||
"integrity": "sha512-HKbY9XTbsDy1Y6r2I55TGE3JEapM0vg96e1MUmBIF9LGjos5gjhcIrTz1yvBPLg2aFKHjwhUAQfRdrCEnPxNew==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"zod": "^4.2.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=20"
|
||||
}
|
||||
},
|
||||
"node_modules/@napi-rs/keyring": {
|
||||
"version": "1.3.0",
|
||||
"resolved": "https://registry.npmjs.org/@napi-rs/keyring/-/keyring-1.3.0.tgz",
|
||||
|
|
@ -2830,6 +2798,7 @@
|
|||
"version": "7.0.6",
|
||||
"resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz",
|
||||
"integrity": "sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"path-key": "^3.1.0",
|
||||
|
|
@ -3247,27 +3216,6 @@
|
|||
"node": ">=0.10.0"
|
||||
}
|
||||
},
|
||||
"node_modules/eventsource": {
|
||||
"version": "3.0.7",
|
||||
"resolved": "https://registry.npmjs.org/eventsource/-/eventsource-3.0.7.tgz",
|
||||
"integrity": "sha512-CRT1WTyuQoD771GW56XEZFQ/ZoSfWid1alKGDYMmkt2yl8UXrVR4pspqWNEcqKvVIzg6PAltWjxcSSPrboA4iA==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"eventsource-parser": "^3.0.1"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=18.0.0"
|
||||
}
|
||||
},
|
||||
"node_modules/eventsource-parser": {
|
||||
"version": "3.1.0",
|
||||
"resolved": "https://registry.npmjs.org/eventsource-parser/-/eventsource-parser-3.1.0.tgz",
|
||||
"integrity": "sha512-kJezFj9YFAMLeORyi7aCLxLbD5/qWMQnoMVlVPyHIll7lgRJCc3JVln9Vgl9nwQi0YkMnhdGTMNn7CkRRAptMg==",
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=18.0.0"
|
||||
}
|
||||
},
|
||||
"node_modules/expect-type": {
|
||||
"version": "1.4.0",
|
||||
"resolved": "https://registry.npmjs.org/expect-type/-/expect-type-1.4.0.tgz",
|
||||
|
|
@ -3793,6 +3741,7 @@
|
|||
"version": "2.0.0",
|
||||
"resolved": "https://registry.npmjs.org/isexe/-/isexe-2.0.0.tgz",
|
||||
"integrity": "sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw==",
|
||||
"dev": true,
|
||||
"license": "ISC"
|
||||
},
|
||||
"node_modules/istanbul-lib-coverage": {
|
||||
|
|
@ -3834,15 +3783,6 @@
|
|||
"node": ">=8"
|
||||
}
|
||||
},
|
||||
"node_modules/jose": {
|
||||
"version": "6.2.3",
|
||||
"resolved": "https://registry.npmjs.org/jose/-/jose-6.2.3.tgz",
|
||||
"integrity": "sha512-YYVDInQKFJfR/xa3ojUTl8c2KoTwiL1R5Wg9YCydwH0x0B9grbzlg5HC7mMjCtUJjbQ/YnGEZIhI5tCgfTb4Hw==",
|
||||
"license": "MIT",
|
||||
"funding": {
|
||||
"url": "https://github.com/sponsors/panva"
|
||||
}
|
||||
},
|
||||
"node_modules/joycon": {
|
||||
"version": "3.1.1",
|
||||
"resolved": "https://registry.npmjs.org/joycon/-/joycon-3.1.1.tgz",
|
||||
|
|
@ -4529,6 +4469,7 @@
|
|||
"version": "3.1.1",
|
||||
"resolved": "https://registry.npmjs.org/path-key/-/path-key-3.1.1.tgz",
|
||||
"integrity": "sha512-ojmeN0qd+y0jszEtoY48r0Peq5dwMEkIlCOu6Q5f41lfkswXuKtYrhgoTpLnyIcHm24Uhqx+5Tqm2InSwLhE6Q==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=8"
|
||||
|
|
@ -4608,15 +4549,6 @@
|
|||
"node": ">= 6"
|
||||
}
|
||||
},
|
||||
"node_modules/pkce-challenge": {
|
||||
"version": "5.0.1",
|
||||
"resolved": "https://registry.npmjs.org/pkce-challenge/-/pkce-challenge-5.0.1.tgz",
|
||||
"integrity": "sha512-wQ0b/W4Fr01qtpHlqSqspcj3EhBvimsdh0KlHhH8HRZnMsEa0ea2fTULOXOS9ccQr3om+GcGRk4e+isrZWV8qQ==",
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=16.20.0"
|
||||
}
|
||||
},
|
||||
"node_modules/pkg-types": {
|
||||
"version": "1.3.1",
|
||||
"resolved": "https://registry.npmjs.org/pkg-types/-/pkg-types-1.3.1.tgz",
|
||||
|
|
@ -4964,6 +4896,7 @@
|
|||
"version": "2.0.0",
|
||||
"resolved": "https://registry.npmjs.org/shebang-command/-/shebang-command-2.0.0.tgz",
|
||||
"integrity": "sha512-kHxr2zZpYtdmrN1qDjrrX/Z1rR1kG8Dx+gkpK1G4eXmvXswmcE1hTWBWYUzlraYw1/yZp6YuDY77YtvbN0dmDA==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"shebang-regex": "^3.0.0"
|
||||
|
|
@ -4976,6 +4909,7 @@
|
|||
"version": "3.0.0",
|
||||
"resolved": "https://registry.npmjs.org/shebang-regex/-/shebang-regex-3.0.0.tgz",
|
||||
"integrity": "sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=8"
|
||||
|
|
@ -6211,6 +6145,7 @@
|
|||
"version": "2.0.2",
|
||||
"resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz",
|
||||
"integrity": "sha512-BLI3Tl1TW3Pvl70l3yq3Y64i+awpwXqsGBYWkkqMtnbXgrMD+yj7rhW0kuEDxzJaYXGjEW5ogapKNMEKNMjibA==",
|
||||
"dev": true,
|
||||
"license": "ISC",
|
||||
"dependencies": {
|
||||
"isexe": "^2.0.0"
|
||||
|
|
|
|||
|
|
@ -27,8 +27,6 @@
|
|||
"format:check": "prettier --check src __tests__ \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\""
|
||||
},
|
||||
"dependencies": {
|
||||
"@modelcontextprotocol/client": "2.0.0-beta.5",
|
||||
"@modelcontextprotocol/core": "2.0.0-beta.5",
|
||||
"@napi-rs/keyring": "^1.3.0",
|
||||
"ajv": "^8.17.1",
|
||||
"atomically": "^2.1.1",
|
||||
|
|
@ -39,7 +37,7 @@
|
|||
"open": "^10.2.0",
|
||||
"pino": "^9.14.0",
|
||||
"react": "^19.2.4",
|
||||
"zod": "^4.3.6"
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"overrides": {
|
||||
"ink-select-input": "^6.2.0"
|
||||
|
|
|
|||
|
|
@ -1,10 +1,76 @@
|
|||
import { defineConfig } from "tsup";
|
||||
import { defineConfig, type Options } from "tsup";
|
||||
import { readFile } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
const dirname = path.dirname(fileURLToPath(import.meta.url));
|
||||
const repoRoot = path.resolve(dirname, "../..");
|
||||
|
||||
/**
|
||||
* `ink-form` hardcodes a misspelled hint under an incomplete form — "you have
|
||||
* not competed yet" — with no prop to override it. It is upstream's string
|
||||
* (`ink-form/lib/SubmitButton.js`), last published in 2024, but it renders in
|
||||
* the Inspector's tool/prompt/resource test forms, so we correct it on the way
|
||||
* into the bundle. Reported upstream as lukasbach/ink-form#14; drop this patch
|
||||
* if a release ever carries the fix.
|
||||
*
|
||||
* This is only possible because `ink-form` is inlined (see `noExternal` below).
|
||||
*/
|
||||
export const INK_FORM_INCOMPLETE_HINT = {
|
||||
typo: "There are still required inputs you have not competed yet.",
|
||||
fixed: "There are still required inputs you have not completed yet.",
|
||||
};
|
||||
|
||||
/**
|
||||
* Applies the correction above, throwing if the string is no longer there.
|
||||
*
|
||||
* Failing loudly is the point: a silent no-op would let an `ink-form` upgrade
|
||||
* (or a fixed upstream, or a reworded label) quietly retire this patch with
|
||||
* nobody noticing it had stopped applying — or, worse, leave a patch here for a
|
||||
* string that no longer exists. If this throws, check whether upstream fixed
|
||||
* the typo; if so, delete this patch rather than re-targeting it.
|
||||
*/
|
||||
export function fixInkFormIncompleteHint(source: string, file: string): string {
|
||||
if (!source.includes(INK_FORM_INCOMPLETE_HINT.typo)) {
|
||||
throw new Error(
|
||||
`${file} no longer contains the ink-form label this build patches ` +
|
||||
`(${JSON.stringify(INK_FORM_INCOMPLETE_HINT.typo)}). If upstream fixed ` +
|
||||
`the typo, remove fixInkFormIncompleteHint from tsup.config.ts.`,
|
||||
);
|
||||
}
|
||||
return source.replaceAll(
|
||||
INK_FORM_INCOMPLETE_HINT.typo,
|
||||
INK_FORM_INCOMPLETE_HINT.fixed,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Which module the patch above is applied to.
|
||||
*
|
||||
* Exported so the tests can check it against the *resolved* path of the real
|
||||
* `ink-form` module — a filter that stops matching is the one way this patch
|
||||
* can silently no-op, since `fixInkFormIncompleteHint` would then never run.
|
||||
*/
|
||||
export const INK_FORM_SUBMIT_BUTTON = /ink-form[\\/]lib[\\/]SubmitButton\.js$/;
|
||||
|
||||
// The plugin type is derived from tsup rather than imported from `esbuild`:
|
||||
// `esbuild` is tsup's transitive dependency, not a declared one of this client,
|
||||
// so a direct import typechecks only while npm happens to hoist it.
|
||||
type EsbuildPlugin = NonNullable<Options["esbuildPlugins"]>[number];
|
||||
|
||||
export const inkFormLabelPatch: EsbuildPlugin = {
|
||||
name: "ink-form-label-patch",
|
||||
setup(build) {
|
||||
build.onLoad(
|
||||
{ filter: INK_FORM_SUBMIT_BUTTON },
|
||||
async ({ path: file }) => ({
|
||||
contents: fixInkFormIncompleteHint(await readFile(file, "utf8"), file),
|
||||
loader: "js",
|
||||
}),
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
export default defineConfig({
|
||||
entry: ["index.ts"],
|
||||
format: ["esm"],
|
||||
|
|
@ -15,12 +81,39 @@ export default defineConfig({
|
|||
sourcemap: false,
|
||||
target: "node22",
|
||||
platform: "node",
|
||||
noExternal: [/^@inspector\/core/],
|
||||
// Every package here renders React components, so it MUST share the one React
|
||||
// instance the bundle imports. Bundling is what guarantees that: an inlined
|
||||
// package's `import "react"` is emitted into build/index.js, so it resolves
|
||||
// from *this* directory exactly like the bundle's own, and no consumer install
|
||||
// layout can point it elsewhere (#1952).
|
||||
//
|
||||
// Left external, npm is free to place a package beside a *different* React,
|
||||
// because it places one beside a version satisfying that package's own peer
|
||||
// range — looser than ours in every case here. `ink-form` and
|
||||
// `ink-scroll-view` accept ">=18", so a consumer's React 18 satisfies them
|
||||
// while the Inspector's React 19 nests underneath: two React copies, and the
|
||||
// first hook they call reads a null dispatcher ("Cannot read properties of
|
||||
// null (reading 'useState')") the moment a tool test form or a scroll view
|
||||
// mounts. That is the reported crash, and inlining them is its fix.
|
||||
//
|
||||
// `__tests__/tsupConfig.test.ts` guards this list.
|
||||
noExternal: [/^@inspector\/core/, "ink-form", "ink-scroll-view"],
|
||||
external: [
|
||||
// `react` is deliberately external — the single instance every inlined
|
||||
// package above resolves to, from this build directory.
|
||||
"react",
|
||||
// `ink` is external by a deliberate trade-off, NOT because a ">=19" peer
|
||||
// makes it safe — it does not, and that claim was wrong here once already.
|
||||
// Bundling it works (verified) but costs ~1.4MB, since react-reconciler and
|
||||
// yoga-layout come with it, plus a `createRequire` banner for the inlined
|
||||
// CJS. The smaller tarball won.
|
||||
//
|
||||
// What makes that tolerable is the root manifest's `react: ^19.0.0`: being
|
||||
// open to the whole major lets npm satisfy our React and a consumer's
|
||||
// pinned one with a single copy, so an external `ink` resolves *ours*. Narrow
|
||||
// that range and this exemption turns back into the #1952 crash, one level
|
||||
// up — `__tests__/tsupConfig.test.ts` guards it.
|
||||
"ink",
|
||||
"ink-form",
|
||||
"ink-scroll-view",
|
||||
"open",
|
||||
"commander",
|
||||
"pino",
|
||||
|
|
@ -28,6 +121,7 @@ export default defineConfig({
|
|||
"@modelcontextprotocol/core",
|
||||
"@napi-rs/keyring",
|
||||
],
|
||||
esbuildPlugins: [inkFormLabelPatch],
|
||||
esbuildOptions(options) {
|
||||
options.alias = {
|
||||
"@inspector/core": path.join(repoRoot, "core"),
|
||||
|
|
|
|||
|
|
@ -1,7 +0,0 @@
|
|||
import * as a11yAddonAnnotations from "@storybook/addon-a11y/preview";
|
||||
import { setProjectAnnotations } from "@storybook/react-vite";
|
||||
import * as projectAnnotations from "./preview";
|
||||
|
||||
// This is an important step to apply the right configuration when testing your stories.
|
||||
// More info at: https://storybook.js.org/docs/api/portable-stories/portable-stories-vitest#setprojectannotations
|
||||
setProjectAnnotations([a11yAddonAnnotations, projectAnnotations]);
|
||||
|
|
@ -162,7 +162,9 @@ The dev/prod backend guards every `/api/*` route with `x-mcp-remote-auth: Bearer
|
|||
|
||||
## Host binding & the origin allow-list
|
||||
|
||||
Both the prod backend (`server/web-server-config.ts`) and the dev Vite server (`vite.config.ts`) resolve their bind host through one shared guard, `server/resolve-bind-host.ts`. It binds `localhost` by default and **refuses an all-interfaces host** (`0.0.0.0`, `::`, empty, or any equivalent spelling — `0`, `0x0`, `0.0`, `::0`, `::ffff:0.0.0.0`, … are all folded to the wildcard and refused) — which would expose the process-spawning backend to the whole network, the exposure DNS-rebinding attacks target — unless `DANGEROUSLY_BIND_ALL_INTERFACES=true` is set. The Docker image sets that flag (a container must bind `0.0.0.0` to be reachable through `-p`); a bare `HOST=0.0.0.0` anywhere else exits with an actionable error.
|
||||
Both the prod backend (`server/web-server-config.ts`) and the dev Vite server (`vite.config.ts`) resolve their bind host through one shared guard, `server/resolve-bind-host.ts`. It binds **`127.0.0.1`** by default and **refuses an all-interfaces host** (`0.0.0.0`, `::`, empty, or any equivalent spelling — `0`, `0x0`, `0.0`, `::0`, `::ffff:0.0.0.0`, … are all folded to the wildcard and refused) — which would expose the process-spawning backend to the whole network, the exposure DNS-rebinding attacks target — unless `DANGEROUSLY_BIND_ALL_INTERFACES=true` is set. The Docker image sets that flag (a container must bind `0.0.0.0` to be reachable through `-p`); a bare `HOST=0.0.0.0` anywhere else exits with an actionable error.
|
||||
|
||||
**The default is the IPv4 loopback _address_, deliberately not the name `localhost`** (#1951). `server.listen(port, host)` resolves a name through `dns.lookup` and binds the **single** address it gets back; since Node 17 the default result order is `verbatim`, so on a glibc Linux host — whose `/etc/hosts` maps `localhost` to both `127.0.0.1` and `::1` — that first address is `::1`. `HOST=localhost` there binds IPv6 loopback **only** and refuses every IPv4 client. On a desktop this is invisible (the browser resolves `localhost` the same way and lands on `::1` too); it breaks wherever the connection is made by something that pins `127.0.0.1` — a VS Code dev container's port forwarder, `ssh -L 6274:127.0.0.1:6274`, a container healthcheck. Binding the address takes the resolver out of the decision. Browsing to `http://localhost:PORT` still works, since browsers fall back across address families and the loopback allow-list covers all three forms. An explicit `HOST=localhost` is still honored as typed — the single-family bind is then your choice.
|
||||
|
||||
The backend's `/api/*` routes also enforce an **origin allow-list** (`allowedOrigins`) as DNS-rebinding protection. When left to default on a loopback host, it expands to all three interchangeable loopback origin forms for the port — `http://localhost:PORT`, `http://127.0.0.1:PORT`, and `http://[::1]:PORT` — because `localhost` resolves to either IPv4 or IPv6 loopback and Node/Vite may bind the IPv6 form, so the browser can legitimately arrive at `http://[::1]:PORT`. Set `ALLOWED_ORIGINS` (comma-separated) to override; entries are canonicalized (`new URL(o).origin`), so a trailing slash / uppercase host / explicit `:80` still match. **Each entry must include the scheme** — `http://localhost:6274`, not `localhost:6274` (a scheme-less value is dropped with a warning). `ALLOWED_ORIGINS` **replaces** the default list (it does not merge), so **list every origin you'll browse from, including the loopback forms** you still want (`http://localhost:PORT`, `http://127.0.0.1:PORT`, `http://[::1]:PORT`) — otherwise local access stops working. A blank `ALLOWED_ORIGINS` does **not** disable the check — it falls back to the default (fail closed); there is no env knob to turn origin validation off.
|
||||
|
||||
|
|
@ -176,7 +178,7 @@ The guard blocks only the **wildcard** all-interfaces addresses. Binding a **spe
|
|||
|
||||
The bind-host guard and the `ALLOWED_ORIGINS` allow-list apply to both the prod server and `--dev`. Note that in **`--dev`** the Vite dev server _additionally_ enforces its own `server.allowedHosts` Host-header check, whose default accepts loopback and IP-literal hosts. The host you **bind** is auto-allowed (Vite adds the resolved `server.host` — which this config sets from `HOST` — to the allow-list), so `HOST=<hostname>` works out of the box under `--dev` too. What needs an explicit `server.allowedHosts` entry is reaching the dev server at a **different** name than the one bound — e.g. a wildcard bind reached by hostname, or a reverse-proxy domain. For those, prefer the prod server (`mcp-inspector --web`) or add the host to `server.allowedHosts`.
|
||||
|
||||
**MCP Apps caveats.** The MCP Apps sandbox runs on a **separate** port (`MCP_SANDBOX_PORT`, dynamic by default). For the Apps tab to work off loopback, that sandbox port must be independently reachable from the browser — pin it with `MCP_SANDBOX_PORT` and expose/forward it too (the Docker image `EXPOSE`s only `6274`). (Under a `0.0.0.0` wildcard bind the sandbox URL is advertised as `localhost`, which is reachable — a wildcard bind serves loopback — so only the port needs handling.) Also note the sandbox iframe is gated by a `frame-ancestors` CSP, and **a bracketed IPv6 literal is not a valid CSP host-source** — so MCP Apps requires browsing the app at a name or IPv4 (`localhost`, `127.0.0.1`, a hostname, a LAN IPv4), **not** a bare `http://[::1]:…` address. Finally, the sandbox URL is always `http://` — so **behind TLS** (an `https://` app page) the browser blocks the `http://…/sandbox` iframe as mixed content and MCP Apps can't render; the Apps tab needs a plain-`http` app origin today.
|
||||
**MCP Apps caveats.** The MCP Apps sandbox runs on a **separate** port — `MCP_SANDBOX_PORT`, defaulting to a fixed **`6275`** (#2008; it was OS-assigned before, which meant it changed every run and so could never be named in a `forwardPorts` / `-p` / tunnel config written ahead of time). For the Apps tab to work off loopback, that sandbox port must be independently reachable from the browser — expose/forward `6275` alongside `6274`, or set `MCP_SANDBOX_PORT` to pick another. If the port is already taken the sandbox falls back to an OS-assigned one and warns, so a second Inspector still gets a working Apps tab locally — but the forwarded port is then wrong, which is what the warning tells you. (Under a `0.0.0.0` wildcard bind the sandbox URL is advertised as `localhost`, which is reachable — a wildcard bind serves loopback — so only the port needs handling.) Also note the sandbox iframe is gated by a `frame-ancestors` CSP, and **a bracketed IPv6 literal is not a valid CSP host-source** — so MCP Apps requires browsing the app at a name or IPv4 (`localhost`, `127.0.0.1`, a hostname, a LAN IPv4), **not** a bare `http://[::1]:…` address. Finally, the sandbox URL is always `http://` — so **behind TLS** (an `https://` app page) the browser blocks the `http://…/sandbox` iframe as mixed content and MCP Apps can't render; the Apps tab needs a plain-`http` app origin today.
|
||||
|
||||
In every case, exposing the Inspector beyond loopback also means anyone who can reach it can drive its backend — keep authentication on (do **not** set `DANGEROUSLY_OMIT_AUTH`) and prefer a specific bind address over the wildcard.
|
||||
|
||||
|
|
|
|||
1077
clients/web/package-lock.json
generated
1077
clients/web/package-lock.json
generated
File diff suppressed because it is too large
Load diff
|
|
@ -41,11 +41,6 @@
|
|||
"@mantine/form": "^8.3.17",
|
||||
"@mantine/hooks": "^8.3.17",
|
||||
"@mantine/notifications": "^8.3.17",
|
||||
"@modelcontextprotocol/client": "2.0.0-beta.5",
|
||||
"@modelcontextprotocol/core": "2.0.0-beta.5",
|
||||
"@modelcontextprotocol/ext-apps": "^1.7.4",
|
||||
"@modelcontextprotocol/server": "2.0.0-beta.5",
|
||||
"@modelcontextprotocol/server-legacy": "2.0.0-beta.5",
|
||||
"@napi-rs/keyring": "^1.3.0",
|
||||
"ajv": "^8.17.1",
|
||||
"atomically": "^2.1.1",
|
||||
|
|
@ -61,7 +56,7 @@
|
|||
"react-markdown": "^10.1.0",
|
||||
"react-syntax-highlighter": "^16.1.1",
|
||||
"remark-gfm": "^4.0.1",
|
||||
"zod": "~4.3.6"
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@chromatic-com/storybook": "^5.2.1",
|
||||
|
|
@ -87,7 +82,6 @@
|
|||
"eslint-plugin-react-hooks": "^7.1.1",
|
||||
"eslint-plugin-react-refresh": "^0.5.2",
|
||||
"eslint-plugin-storybook": "^10.5.5",
|
||||
"express": "^5.2.1",
|
||||
"globals": "^17.4.0",
|
||||
"happy-dom": "^20.9.0",
|
||||
"playwright": "^1.58.2",
|
||||
|
|
|
|||
|
|
@ -26,21 +26,46 @@ function isEnabled(value: string | undefined): boolean {
|
|||
return v === "true" || v === "1";
|
||||
}
|
||||
|
||||
/**
|
||||
* The default bind host: the IPv4 loopback **address**, deliberately not the
|
||||
* name `localhost`.
|
||||
*
|
||||
* `server.listen(port, host)` resolves a name through `dns.lookup` and binds the
|
||||
* **single** address it gets back. Since Node 17 the default result order is
|
||||
* `verbatim`, so Node takes whatever the resolver returns first rather than
|
||||
* preferring IPv4 — and on a glibc Linux host with the stock `/etc/hosts`
|
||||
* (`127.0.0.1 localhost` *and* `::1 localhost`) that is `::1`. `HOST=localhost`
|
||||
* therefore binds IPv6 loopback **only**, and every IPv4 client is refused.
|
||||
*
|
||||
* That is invisible on a desktop, where the browser resolves `localhost` the
|
||||
* same way and lands on `::1` too. It breaks wherever the connection is made by
|
||||
* something that pins IPv4 `127.0.0.1` instead — a VS Code dev container's port
|
||||
* forwarder (#1951), an `ssh -L …:127.0.0.1:…` tunnel, a container healthcheck.
|
||||
* Binding the address directly removes the resolver from the decision, and the
|
||||
* loopback origin allow-list already covers `localhost`, `127.0.0.1`, and
|
||||
* `[::1]` alike, so browsing to `http://localhost:PORT` still works (browsers
|
||||
* fall back across address families; a Node `listen` does not).
|
||||
*/
|
||||
export const DEFAULT_BIND_HOST = "127.0.0.1";
|
||||
|
||||
/**
|
||||
* Resolve the bind hostname from `env` (default `process.env`), defaulting to
|
||||
* `localhost`. Refuses an all-interfaces host (`0.0.0.0` / `::` / empty / their
|
||||
* legacy spellings) unless {@link BIND_ALL_INTERFACES_ENV} is explicitly
|
||||
* enabled — the published Docker image sets it, since a container must bind
|
||||
* `0.0.0.0` to be reachable through `-p`. Throws (fail fast, loudly) rather than
|
||||
* silently binding wide open. The returned value is trimmed and de-bracketed
|
||||
* (an IPv6 literal is returned bare, e.g. `HOST=[::1]` → `::1`) so detection,
|
||||
* `listen()`, and the origin list all consume the same value; `formatHostForUrl`
|
||||
* re-adds the brackets wherever a URL is built.
|
||||
* {@link DEFAULT_BIND_HOST}. Refuses an all-interfaces host (`0.0.0.0` / `::` /
|
||||
* empty / their legacy spellings) unless {@link BIND_ALL_INTERFACES_ENV} is
|
||||
* explicitly enabled — the published Docker image sets it, since a container
|
||||
* must bind `0.0.0.0` to be reachable through `-p`. Throws (fail fast, loudly)
|
||||
* rather than silently binding wide open. The returned value is trimmed and
|
||||
* de-bracketed (an IPv6 literal is returned bare, e.g. `HOST=[::1]` → `::1`) so
|
||||
* detection, `listen()`, and the origin list all consume the same value;
|
||||
* `formatHostForUrl` re-adds the brackets wherever a URL is built.
|
||||
*
|
||||
* An explicit `HOST=localhost` is honored as typed — the single-family bind
|
||||
* described on {@link DEFAULT_BIND_HOST} is then the caller's choice, not ours.
|
||||
*/
|
||||
export function resolveBindHostname(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): string {
|
||||
const host = (env.HOST ?? "localhost").trim();
|
||||
const host = (env.HOST ?? DEFAULT_BIND_HOST).trim();
|
||||
if (isAllInterfacesHost(host) && !isEnabled(env[BIND_ALL_INTERFACES_ENV])) {
|
||||
// Show the resolved address when it differs from the typed spelling — the
|
||||
// guard now catches forms the resolver folds to the wildcard (a fullwidth
|
||||
|
|
|
|||
|
|
@ -11,13 +11,14 @@ import {
|
|||
canonicalUrlHost,
|
||||
isAllInterfacesHost,
|
||||
} from "../../../core/node/hostUrl.ts";
|
||||
import { DEFAULT_BIND_HOST } from "./resolve-bind-host.js";
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
|
||||
export interface SandboxControllerOptions {
|
||||
/** Port to bind (0 = dynamic). */
|
||||
port: number;
|
||||
/** Host to bind (default localhost). */
|
||||
/** Host to bind (default {@link DEFAULT_BIND_HOST}). */
|
||||
host?: string;
|
||||
/**
|
||||
* The backend's origin allow-list (the embedder origins). Used to build the
|
||||
|
|
@ -95,10 +96,28 @@ function parseListenPort(raw: string | undefined): number | undefined {
|
|||
}
|
||||
|
||||
/**
|
||||
* Resolve sandbox port from env: MCP_SANDBOX_PORT → SERVER_PORT → 0 (dynamic).
|
||||
* An invalid value falls through rather than crashing the boot; a set-but-invalid
|
||||
* MCP_SANDBOX_PORT (the dedicated knob) is warned so the fall-through isn't
|
||||
* silent — matching the warn-and-drop precedent for ALLOWED_ORIGINS.
|
||||
* The default sandbox listen port — **fixed**, deliberately not `0`.
|
||||
*
|
||||
* A `0` default lets the OS assign, which is fine on a desktop (the browser
|
||||
* learns the port from `/api/config`) but unusable anywhere the port has to be
|
||||
* declared *before* the process starts: a dev container's `forwardPorts`, a
|
||||
* `docker run -p`, a k8s service, an SSH tunnel. The port changed on every run,
|
||||
* so it could never be named in that config and the Apps tab was unreachable
|
||||
* (#2008). `CLIENT_PORT` has always defaulted to a fixed `6274` for the same
|
||||
* reason; this is its sandbox counterpart.
|
||||
*
|
||||
* `6275` sits next to the web port so the pair is easy to forward together. It
|
||||
* is not a v1 carry-over — v2 has no proxy server on it.
|
||||
*/
|
||||
export const DEFAULT_SANDBOX_PORT = 6275;
|
||||
|
||||
/**
|
||||
* Resolve sandbox port from env: MCP_SANDBOX_PORT → SERVER_PORT →
|
||||
* {@link DEFAULT_SANDBOX_PORT}. An invalid value falls through rather than
|
||||
* crashing the boot; a set-but-invalid MCP_SANDBOX_PORT (the dedicated knob) is
|
||||
* warned so the fall-through isn't silent — matching the warn-and-drop
|
||||
* precedent for ALLOWED_ORIGINS. An explicit `0` is still honored as "let the
|
||||
* OS assign" ({@link parseListenPort} accepts it).
|
||||
*/
|
||||
export function resolveSandboxPort(): number {
|
||||
const fromSandbox = parseListenPort(process.env.MCP_SANDBOX_PORT);
|
||||
|
|
@ -107,13 +126,21 @@ export function resolveSandboxPort(): number {
|
|||
`Ignoring invalid MCP_SANDBOX_PORT="${process.env.MCP_SANDBOX_PORT}" (need an integer 0–65535); falling back.`,
|
||||
);
|
||||
}
|
||||
return fromSandbox ?? parseListenPort(process.env.SERVER_PORT) ?? 0;
|
||||
return (
|
||||
fromSandbox ??
|
||||
parseListenPort(process.env.SERVER_PORT) ??
|
||||
DEFAULT_SANDBOX_PORT
|
||||
);
|
||||
}
|
||||
|
||||
export function createSandboxController(
|
||||
options: SandboxControllerOptions,
|
||||
): SandboxController {
|
||||
const { port, host = "localhost", allowedOrigins } = options;
|
||||
// Defaulted to the same address the web server binds, never the name
|
||||
// `localhost` — a name resolves to one address family and would reintroduce
|
||||
// the #1951 split (web on IPv4, sandbox on IPv6) for any future call site
|
||||
// that omits `host`. Both call sites pass `config.sandboxHost` today.
|
||||
const { port, host = DEFAULT_BIND_HOST, allowedOrigins } = options;
|
||||
let server: Server | null = null;
|
||||
let sandboxUrl: string | null = null;
|
||||
|
||||
|
|
@ -177,8 +204,28 @@ export function createSandboxController(
|
|||
});
|
||||
res.end(sandboxHtml);
|
||||
});
|
||||
// A taken port used to mean "no sandbox at all", which was tolerable
|
||||
// only because the default was dynamic and therefore never collided.
|
||||
// Now that it's fixed (see DEFAULT_SANDBOX_PORT), a second Inspector —
|
||||
// or anything else already on 6275 — would take the Apps tab down, so
|
||||
// retry once on an OS-assigned port. The retry is announced loudly:
|
||||
// whoever pinned MCP_SANDBOX_PORT to forward it needs to know the pin
|
||||
// didn't take, and a silently-moved port is exactly the failure this
|
||||
// default was introduced to fix.
|
||||
let retriedDynamic = false;
|
||||
server.on("error", (err: NodeJS.ErrnoException) => {
|
||||
if (err.code === "EADDRINUSE") {
|
||||
if (!retriedDynamic && port !== 0) {
|
||||
retriedDynamic = true;
|
||||
console.warn(
|
||||
`Sandbox: port ${port} in use; falling back to an OS-assigned port. ` +
|
||||
`The MCP Apps tab will work locally, but the sandbox is no longer ` +
|
||||
`on a predictable port — set MCP_SANDBOX_PORT to a free one if you ` +
|
||||
`need to forward it (container / SSH tunnel / reverse proxy).`,
|
||||
);
|
||||
server!.listen(0, host);
|
||||
return;
|
||||
}
|
||||
console.error(
|
||||
`Sandbox: port ${port || "dynamic"} in use. MCP Apps tab may not work.`,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -52,7 +52,9 @@ export interface ResourceControlsProps {
|
|||
* Modern-era `subscriptions/listen` stream state (#1630). When `active`
|
||||
* (modern era with at least one subscription) the Subscriptions section shows
|
||||
* a stream-status badge in its panel and a status dot in its header. Legacy
|
||||
* connections pass `active: false` (or omit it) and see neither.
|
||||
* connections pass `active: false` (or omit it) and see neither — and so does
|
||||
* a stream open purely for list-change notifications, which this section has
|
||||
* nothing to say about (#1920).
|
||||
*/
|
||||
subscriptionStreamState?: ResourceSubscriptionStreamState;
|
||||
/** Negotiated protocol era; gates the modern subscription stream chrome. */
|
||||
|
|
|
|||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Reference in a new issue