* refactor(web): update OAuth E2E tests to use new authentication method
- Replaced `runGuidedAuth` with `authenticate` in the InspectorClient OAuth end-to-end tests to support quick authentication with CIMD pre-registration.
- Updated comments to reflect the change in authentication method and its support for HTTP test metadata URLs.
- Ensured that the tests continue to validate the expected authorization URL after the change.
Additionally, imported `ensureCimdClientRegistration` in the OAuthManager to streamline client registration during the authentication process.
* Guided-auth removal, OAuth smoke testing, and TUI/CLI support
* Added CLI OAuth runner flags tests
* Added clarifying comment on port 0 binding
* README notes on TUI oauth callback port, prettier fixes for CLI tests
* CIMD validation uses inline like EMA IdP
* Fixed TUI test failures from merge
* test: bring 5 OAuth files over the 90 per-file coverage gate
The guided-auth-removal work added/touched several files that fell below
the repo's >=90 per-file coverage gate (npm run coverage). CI doesn't run
the gate (it's local-only), so validate + test:integration passed while
these gaps slipped through. Bring them all back over the line:
- core/auth/node/runner-oauth-callback.ts: test bad-URL/non-http-scheme
throws, no-port->80 default, and IPv6 hostname bracketing. Annotate the
provably-dead guards (empty host, out-of-range port, String(err)
fallback, pathname "/" fallback) -- all rejected by new URL() upstream --
with justified v8 ignore comments instead of lowering the gate.
- core/auth/cimd.ts: test the AS-doesn't-support-CIMD early return.
- core/react/useClientSettingsDraft.ts: test the functional-updater path,
the stale-onChange-after-close prev===null guard, and the
timer-fires-after-close value===null guard.
- ClientSettingsForm.tsx: test the CIMD checkbox toggle and CIMD URL clear.
- clients/tui/src/utils/oauthDisplay.ts: test static/dcr registration
kinds, the expired IdP session, and the configured-scope/empty-scope
branches of formatScopes.
npm run coverage now passes (exit 0) across web, cli, tui, launcher.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: cliffhall <cliff@futurescale.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(web): audit branch-coverage outliers, raise per-file gate to 70 (#1271)
The web per-file coverage gate had drifted red: the recently-landed EMA
auth feature (core/auth/ema/*) and ClientSettings components shipped
undertested, and the prior 50% branch floor let business-logic branches
go untested. CI does not run the coverage gate, so this accumulated.
Add tests to bring every failing file above the gate and push branch
coverage well past 70 across the board, then raise the per-file branches
threshold 50 -> 70 in clients/web/vite.config.ts.
- EMA core (emaFlow, resourceContext, jwt, wire): 50-88% -> 86-100% branch
- EMA OIDC/transport (idpOidc, tokenEndpoint, transportProvider): -> 97-100%
- oauthManager / discovery: 73-83% -> 88-92%
- ClientSettingsModal/Form, ConnectionInfoContent: 0-88% -> 96-100%
- useClientSettingsDraft / useEmaIdpLoginState hooks: 20% -> 100%
- TaskStatusBadge / SortableServerCard outliers: 50% -> 100%
Coverable branches reached by mocking useComputedColorScheme (dark theme),
dnd-kit useSortable (isDragging), and fetch/OIDC paths. Remaining sub-100
branches are genuinely-unreachable defensive guards or happy-dom-inherent
(portal / useMediaQuery / typeof window) and documented in test comments.
No source files changed. Global web branch coverage 88.73% -> 90.63%.
AGENTS.md updated; cli/tui/launcher stay at 50 (out of scope).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: raise per-file coverage gate to 90/90/90/90 across all clients
Follows the #1271 web branch-coverage audit (50 -> 70) by raising the
whole per-file gate to >=90 on all four dimensions — lines, statements,
functions, branches — for web, cli, tui, and launcher (core/ is measured
under the web suite). A codebase-wide audit added real tests for every
outlier and brought global coverage up across the board.
Config: all four vitest/vite coverage configs set to 90/90/90/90.
Two pure-type modules (core/mcp/import/types.ts, clients/web/server/
types.ts) added to the web coverage exclude list, matching the existing
type-only exclusions.
Tests: ~50 test files extended/created across core (auth, ema, mcp,
remote, state, storage, react), web components/screens/server, and the
cli/tui clients. Highlights: full EMA OAuth flow + transport coverage,
inspectorClient/remote server backfill, every Mantine control's handler
branches, and the cli unsupported-method / argv-fallback paths.
Genuinely-unreachable branches are annotated at the source with justified
`/* v8 ignore -- <reason> */` comments rather than lowering the gate:
happy-dom-inherent paths (Mantine portal, useMediaQuery, typeof window
SSR guards), React StrictMode effect replay, and provably-dead defensive
guards (?? fallbacks the types make impossible, Select.onChange values
outside the allowed list). No runtime logic changed; the only structural
edits are two behavior-preserving refactors (ResourceTemplatePanel hoists
the field value to a local so the ignore applies in JSX; ToolsScreen
swaps a ternary for an if) and prettier line-wrapping.
Also fixed a flaky real-timer debounce assertion in PromptArgumentsForm
(asserted an exact sibling-typing interleaving that raced the 300ms
debounce; now asserts the load-bearing fact — a non-empty fire-time
sibling capture).
AGENTS.md updated to describe the uniform 90 gate and the v8-ignore
policy. All four client coverage gates, root validate, storybook (370),
and smoke pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(web): make connect-crash SSE reader stop on the terminal event
`readSseEvents` looped until the server closed the stream (`done`). The
server closes the events stream immediately after writing the terminal
`transport_error`, but under the heavy parallel scheduler pressure of a
full `coverage` run (v8 instrumentation across all workers) that close
can lag far enough behind the event to trip the 30s test timeout — a
hang, not a wrong result. This made the test intermittently fail under
`npm run coverage` (it passed in isolation and under CI's lighter
`test:integration`).
Stop reading as soon as the terminal `transport_error` event arrives
(cancelling the reader) instead of waiting only for `done`. That event is
always the last one the session queue emits, so nothing is lost, and the
test no longer depends on stream-close timing. The `done` break is kept
for any stream that closes first.
Verified green across two consecutive full `npm run test:coverage` runs
(3094 tests) where it previously hung, plus isolation (~0.5s).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(web): build dist on demand for prod --web + end-to-end CI smoke (#1486)
Prod `mcp-inspector --web` serves static assets from clients/web/dist,
which only exists after a build. In a fresh checkout it was absent and the
prod path served a broken page (ENOENT on index.html); the launcher smoke
test only covered --help, so this path was unverified in CI.
- ensure-web-build.ts: build dist on demand (vite build via build:client)
when missing, or exit with an actionable error if the build can't run.
- run-web.ts: invoke ensureWebBuild before startHonoServer in the prod path
(--dev runs Vite directly and skips it).
- scripts/smoke-web.mjs + smoke:web: start --web against built dist and
assert GET / returns 200 with the injected __INSPECTOR_API_TOKEN__.
Wired into validate:launcher and a new CI step.
- Docs: launcher README + AGENTS.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(web): surface spawn error in build-on-demand; clarify smoke comment
Address PR #1497 review:
- runViteBuild now logs result.error.message (e.g. ENOENT when npm isn't on
PATH) before returning a null status, so the spawn-failure cause surfaces
instead of only the generic actionable error. The build seam gains a `log`
param; added a test covering the ENOENT path.
- Reword scripts/smoke-web.mjs header so it no longer implies the readiness
wait covers a cold build-on-demand vite build (it's sized for a pre-built
dist, which validate/CI guarantee).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(web): catalog vs read-only session launch model + apply --header (#1481, #1483)
Per specification/v2_catalog_launch_config.md, `mcp-inspector --web` now selects
the backend's server-list source and a session-wide writability, instead of
seeding an inert `initialMcpConfig` the UI never read:
- `--catalog <path>` (and `MCP_CATALOG_PATH`) → active WRITABLE catalog
(seed/CRUD/watch), like the default catalog.
- `--config <path>` → READ-ONLY session file: served in the UI but never
written, seeded, or plaintext-secret-migrated — safe to point at a foreign
config (e.g. claude_desktop_config.json).
- ad-hoc `--server-url`/command (+ `--header`) → one server held IN MEMORY
(no file written); `--header` is lifted onto the entry so the seeded
connection actually uses it (closes the #1483 warn-only no-op).
- nothing → default catalog (writable), unchanged.
Backend (`createRemoteApp`): new `writable` + `initialServers` options. GET
/api/servers branches three ways (in-memory / read-only file with no
seed+no migrate / writable catalog); the four mutating routes return 403 when
read-only; `/api/config` carries `writable`; the file watcher is skipped for
in-memory sessions. `--catalog`/`--config` are mutually exclusive and neither
combines with an ad-hoc target or `--header`; `--header` requires an ad-hoc
HTTP/SSE server (clear errors otherwise — no silent no-op).
UI: `useServerListWritable` reads `writable` from /api/config; the Servers
screen hides add/edit/clone/remove/reorder/settings and shows a read-only
banner when false. All new props default to writable=true (back-compatible).
Docs: spec gaps G2/G3/G4 updated; launcher README documents the flags.
Verified end-to-end against the built launcher: --config (writable:false,
file byte-stable, mutations 403), ad-hoc --header (in-memory, header lifted),
and --catalog (writable:true, POST persists).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(web): address review — Mantine <Code> in read-only banner, clarify watcher comment
- Replace raw <code> with Mantine <Code> in the read-only-session Alert
(repo convention: prefer Mantine primitives over raw HTML).
- Add a comment near `watchable` clarifying that a read-only `--config` file
is still watched on purpose (read-only blocks UI writes, not external
observation; `writable` does not gate watching).
Deferred (per reviewer): consolidating useSandboxUrl + useServerListWritable
into one useConfig hook to avoid the duplicate /api/config fetch — optional
follow-up. The redundant ad-hoc header type-guard is kept as defensive.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `UNRESOLVED_IMPORT` warnings that `quiet-config-warnings.mjs` suppressed
no longer occur, so the filter (and the build-time `onwarn` suppressions for
the same three deps) are removed.
Root cause: Vite's config bundler (Rolldown) statically walks the node-only
dev backend (`core/mcp/remote/node/server.ts`) when it loads `vite.config.ts`,
reaching `chokidar`, `atomically`, and `@napi-rs/keyring`. `core/` has no
node_modules of its own, so at config-bundle time those deps resolve from
`core/`'s parent chain — the repo-root `node_modules`. Before #1452 the deps
lived only under `clients/web/node_modules` (a sibling, off the resolution
chain), so Rolldown emitted `UNRESOLVED_IMPORT`. #1452 declared them in the
repo-root `package.json` (for the CLI/TUI/launcher, which consume core/'s node
code), installing them at the repo root and making them resolvable. Verified
by hiding the root copy: the warning returns; restore it and the dev/build
startup is clean.
A dynamic import of the backend does not help — Rolldown follows dynamic
imports during config bundling too. The fix is the deps being resolvable, so
the suppression is just removed and the invariant is documented in
`vite.config.ts`.
- Remove `clients/web/server/quiet-config-warnings.mjs` + `.d.mts` and its test
- Restore the `dev` script to plain `vite` (drop the `--import` hook)
- Remove the now-dead `build.rollupOptions.onwarn` UNRESOLVED suppressions
(never fire: the Hono plugin is `apply: 'serve'`, out of the build graph)
- Document why config-bundle resolution succeeds in `vite.config.ts`
- Update AGENTS.md project tree
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ports the CLI, TUI, and launcher from v1.5 into v2 behind a single
`mcp-inspector` entrypoint (launcher → web/cli/tui). Includes the
`mcp-inspector --web --dev` launcher fix (Vite alias resolution + prod-server
double-start), the root `postinstall` install cascade with simplified CI, and
spec/AGENTS docs (no-workspaces rationale, Development setup).
Closes#1246.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Hono dev-proxy middleware pumped streaming /api responses to the Node
ServerResponse without handling client disconnect. When the browser dropped a
stream mid-flight (SSE close, navigation, HMR reload) `res` was destroyed and
the next write failed with ERR_STREAM_DESTROYED, which the pump logged as an
error. It also had no `error` listener (a failed write could surface as an
uncaught exception) and no `close` listener (it kept reading upstream after the
client left).
Make the pump disconnect-aware:
- Listen for res `close`/`error`; either cancels the upstream reader (resolving
the pending read so the loop exits) and flips a `clientGone` flag.
- Treat ERR_STREAM_DESTROYED / EPIPE / ECONNRESET as a normal end — not logged.
- Only log genuinely unexpected stream errors.
- Await each write so a slow client applies backpressure instead of the proxy
buffering the whole stream.
Dev-only plugin (excluded from the coverage gate — it needs a real Vite server
to exercise). Verified via npm run validate.
Closes#1418
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(web): silence benign config-load dep warnings in `npm run dev`
`vite dev` printed three UNRESOLVED_IMPORT warnings at startup (chokidar,
atomically, @napi-rs/keyring). They originate from Vite's config bundler
(Rolldown): the Hono plugin statically imports the node-only dev backend
(core/mcp/remote/node/server.ts), so loading vite.config.ts bundles it and
Rolldown walks its node-only deps. Those deps are correctly externalized — the
browser never loads them and Node resolves them at runtime — so the warnings are
pure noise.
They can't be filtered the usual ways: the static import is required (the
monorepo has no node_modules under core/, so the aliases must resolve the deps
at bundle time — ssrLoadModule / `--configLoader runner` both fail to resolve
them), and the warnings are written directly to the process streams during
config load, bypassing customLogger, logLevel, and rollupOptions.onwarn.
Fix: a tiny `--import` hook (server/quiet-config-warnings.mjs) that drops only
those specific benign warning writes, wired into the `dev` script. Scoped to
`npm run dev` — build, tests, and prod are untouched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(web): harden dev warning filter per review
- Tighten BENIGN_PAIRS to repo-relative path fragments (not basenames) so an
unrelated future UNRESOLVED_IMPORT warning mentioning the same dep + a
same-named file isn't silently dropped. Verified Rolldown prints the full
repo-relative path. (review #1)
- Add a note that the matcher tracks Rolldown's message format and degrades to a
safe no-op (warnings return) if that format changes. (review #2)
- Export isBenignWarning/BENIGN_PAIRS, guard the stream patching behind
!VITEST, add a .d.mts so the pure helper is importable from TS, and add a unit
test locking the signature pairs and the over-match guards. (review #3)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(auth): inject MCP_INSPECTOR_API_TOKEN into served index.html (#1378)
Reloading the web client at the bare URL (no `?MCP_INSPECTOR_API_TOKEN=…`
query string) with empty sessionStorage made every `/api/*` request 401 —
the browser had no way to recover the backend's auth token.
Embed the token into `index.html` on every page load so the browser no
longer depends on the query string surviving navigation:
- New shared helper `clients/web/server/inject-auth-token.ts` embeds
`<script>window.__INSPECTOR_API_TOKEN__ = "…"</script>` (escaped against
`</script>` injection; no-op when auth is dangerously omitted).
- Dev: the Vite plugin injects via `transformIndexHtml`.
- Prod: the Hono server injects on the `/` route.
- `App.tsx` `getAuthToken()` now reads the injected global first, then the
query string, then sessionStorage (both fallbacks preserved).
- Shared global name lives in `INSPECTOR_API_TOKEN_GLOBAL`
(`core/mcp/remote/constants.ts`).
Tests: helper unit coverage + an integration test exercising the real
prod server's `/` → `/api/*` flow (injected token authenticates; missing
token 401s). AGENTS.md documents the token-recovery order.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(auth): inject token into prod SPA fallback + prime sessionStorage + no-store (#1378)
Addresses code-review feedback on the token-injection PR:
- Prod `server.ts` SPA deep-link fallback (e.g. `/oauth/callback`) previously
served the raw index.html off disk via serveStatic, bypassing injection — so
a bookmark/reload at a non-`/` route with empty sessionStorage would 401.
Route the SPA fallback through the same `serveIndexHtml` (inject) helper;
real static assets (paths with a dot) still serve verbatim. Dev already
injected on every HTML serve via Vite `transformIndexHtml`.
- `getAuthToken()` now persists the injected `window.__INSPECTOR_API_TOKEN__`
to sessionStorage (not just the URL-param branch), priming the backstop for
any later navigation that loses the global.
- Injected HTML responses now send `Cache-Control: no-store`, so a page
carrying a token isn't cached and served stale after a restart regenerates
the token.
Integration tests added: SPA fallback (`/oauth/callback`) carries the token,
`Cache-Control: no-store` on injected HTML, real assets served verbatim, and
unknown `/api` routes 404 rather than falling through to the HTML shell.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(servers): move oauthClientSecret and stdio env values to OS keychain (#1356)
mcp.json is designed to be tool-shareable (symlinked from Claude Desktop's
config, pasted into bug reports, synced via dotfiles), so storing OAuth
client secrets and stdio env values in plaintext there meant any of those
flows could leak them. Lift both into the OS keychain via @napi-rs/keyring
(active replacement for the archived keytar). The wire shape is unchanged:
the GET /api/servers handler rehydrates from the keychain so browser code
sees the same JSON; the on-disk file no longer contains the secret
material. Includes an idempotent migration that lifts plaintext from older
mcp.json files (or hand-edited ones) into the keychain on first read.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(servers): tolerate keychain unavailability on non-set ops (#1356)
CI on Linux failed because `@napi-rs/keyring` hard-errors without
libsecret, and the route handlers were touching the keychain even for
no-secret flows (defensive `deleteAllForServer` on POST/DELETE, sweep
on PUT). The user's intent for the Linux fallback was "hard fail when a
secret is actually involved" — not on every keychain touch.
Make `KeyringSecretStore.get / delete / deleteAllForServer` silently
degrade when the keychain is unavailable (return null / no-op); `set`
remains the one operation that hard-fails with
`KeychainUnavailableError`. The migration path catches the error and
preserves the disk plaintext rather than losing the secret.
Also inject `InMemorySecretStore` in `useServers.test.tsx` and
`servers-events.test.ts` so tests never touch a developer's real
keychain even when libsecret is present.
Adds `KeyringSecretStore` tests (vi.mock'd native bindings) and an
integration suite that simulates Linux-without-libsecret to verify the
tolerance contract end-to-end.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(servers): close GET migration race; reverse keychain/disk order (#1356)
Addresses code review:
1. **Concurrency**: GET /api/servers was reading + migrating outside the
write lock, then writing inside it — a concurrent POST/PUT/DELETE
between the read and the lock acquisition could be clobbered by the
migration write. Also affected the first-launch seed write. Fix:
take a fast unlocked peek; if the file is absent (seed branch) or
carries plaintext to migrate (migration branch), re-read inside the
write lock and decide there. Added `hasPlaintextSecrets()` as a
pure predicate so the common no-migration path stays lock-free.
2. **Write ordering**: POST and PUT wrote disk first, then keychain.
A keychain `set` failure mid-write left a half-configured entry on
disk and trapped retries at 409. Reversed to keychain-first so a
503 leaves both stores in their pre-write state. PUT in-place now
sets new fields, writes disk, then deletes obsolete fields (a
failed disk write leaves orphan keychain entries — recoverable —
rather than premature deletions of fields the user still expects
to see).
3. **Tolerance simplification**: dropped the no-entry regex in
`KeyringSecretStore.delete` — both "no entry" and "keychain
unavailable" collapse to the same desired outcome (the entry isn't
there anymore), so a uniform swallow is clearer.
4. **Parallelism**: `readKeychainEntriesFor` now `Promise.all`s the
per-field `get` calls. macOS Keychain round-trips are 10-50ms;
serializing 5+ env vars per server × N servers is a meaningful
wall-clock cost on GET.
5. **Spec doc**: reflects the tolerance contract (only `set`
hard-fails) and the keychain-before-disk write ordering rationale.
New regression test in servers-route.test.ts covers the retry-after-503
contract directly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(servers): parallel keychain writes + migration partial-state comment (#1356)
Two nice-to-have observations from the re-review:
- `writeKeychainEntriesFor` now `Promise.all`s its per-field sets,
matching the read side. No ordering requirement among distinct
(id, field) keys; on unavailability every set rejects and Promise.all
surfaces the first rejection (still 503 from the route handler).
- `migratePlaintextSecrets` catch block now spells out the
partial-migration semantics: we return the original config so the
disk plaintext stays intact, and the next successful GET retries
with the keychain-wins branch absorbing already-set entries.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(servers): parallel keychain deletes for symmetry (#1356)
Picks up the symmetry observation from the third review: `deleteKeychainFields`
now uses `Promise.all` like its read and write siblings. Distinct (id, field)
deletes have no ordering requirement, and `secretStore.delete` is already a
silent no-op on unavailability so Promise.all has no failure-mode surprise.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(servers): hot-reload server list on external mcp.json edits (#1345)
Watch ~/.mcp-inspector/mcp.json from the backend and push a change event
on a new /api/servers/events SSE channel. The web client subscribes via
fetch + ReadableStream (so the existing bearer-auth contract is
preserved — EventSource can't send custom headers) and re-fetches on
each event. The watcher is lazy: it starts on the first SSE subscriber
and stops when the last one disconnects, so tests that never open the
channel don't spin up a real fs watcher. Self-fires from the backend's
own POST/PUT/DELETE are suppressed by capturing the post-write mtime and
comparing it on each watcher event.
Chokidar is used for cross-platform editor-save semantics (temp-file +
rename produces unlink+add sequences that bare fs.watch handles poorly
on macOS/Linux); awaitWriteFinish coalesces those into a single event.
createRemoteApp now returns a close() to release the watcher, which the
standalone server and the vite dev plugin chain into their existing
close paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(servers): address PR review on hot-reload (#1357)
- writeMcpAndTrackMtime now stat()s the file before writing; if the
mtime differs from the last value we recorded, an external editor
slipped in between our writes and chokidar would coalesce both into
one event whose mtime matches our own write — broadcast directly so
peer subscribers learn about the external edit (review finding 1, 5).
- Split useServers.refresh into a public refresh() (loading toggle) and
an internal refreshInternal(background) the SSE handler calls; the
list now stays visible during background refreshes instead of
flashing skeleton state on every external mcp.json edit (finding 2).
- Coalesce multiple \n\n frames landing in one decode chunk to a single
refresh call so back-to-back broadcasts don't race two concurrent
GETs whose setState order is unspecified (finding 3).
- Clarify createRemoteApp's close() docblock about in-flight SSE stream
callbacks (finding 7).
- Add integration coverage for PUT/DELETE self-suppression and for the
external-edit-between-backend-writes broadcast (finding 6).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(servers): address pass-2 review on hot-reload (#1357)
- Clarify writeMcpAndTrackMtime comment so a future reader can't
mistake the broadcast for a content-preservation guarantee: the
external edit's content may already have been read-around and
overwritten depending on the interleave with readMcpConfig.
- Drop the no-op GET in the PUT/DELETE self-suppression tests. The
pre-stat branch is short-circuited by `lastWrittenMtimeMs === null`
with or without it; matches the POST test's structure.
- Add explicit no-flicker regression test: tracks every `loading`
value across renders during an SSE-driven refresh and asserts none
flipped back to true. Catches a future regression that reaches for
setLoading(true) in the background path even under React batching.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(servers): persist per-server settings to mcp.json (#1352)
- Add optional `settings` node alongside the transport config in each
`~/.mcp-inspector/mcp.json` entry. Round-trips through
`mcpConfigToServerEntries` / `serverEntriesToMcpConfig` and the
`/api/servers` POST/PUT routes.
- Drop `MCPServerConfig.headers` from `SseServerConfig` /
`StreamableHttpServerConfig` and remove the headers textarea from
`ServerConfigModal`. Custom headers are now entered only in
`ServerSettingsForm` and reach the wire via `settings.headers`.
- Wire `ServerSettingsModal` into `App.tsx`; saves dispatch through a new
`useServers.updateServerSettings` mutator. `updateServer` carries the
existing settings across config-only saves so they aren't dropped.
- Apply settings on the *first* outbound request after a server loads —
no need to open the settings form first:
- `settings.headers` → SSE / streamable-http wire headers.
- `settings.metadata` → `InspectorClientOptions.defaultMetadata`,
merged into every outgoing `_meta` (call-time keys win).
- `settings.requestTimeout` → `InspectorClientOptions.timeout`.
- `settings.connectionTimeout` → Promise.race wrapper around connect().
- `settings.oauth*` → pre-seeded OAuth client credentials.
- `RemoteConnectRequest` carries `settings` so the remote backend can
source transport headers from the same source as the local path.
- `oauthClientSecret` is persisted in `mcp.json` alongside stdio `env`
values, both protected by the file's `0o600` permission.
- Update `specification/v2_servers_file.md` with the new shape and the
removal of the legacy headers field.
Tests: round-trip in `serverList.test.ts`; persistence on POST/PUT in
`servers-route.test.ts`; first-connect header injection in
`transport.test.ts`; `defaultMetadata` merging in
`inspectorClient.test.ts`; `useServers.updateServerSettings` +
`updateServer` settings-preservation in `useServers.test.tsx`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(servers): rebuild InspectorClient on every connect so settings refresh
The previous onToggleConnection branch reused the existing InspectorClient
on a same-server reconnect, which froze settings at the moment the client
was first constructed. Editing headers, metadata, timeouts, or OAuth
credentials in the settings form was therefore invisible until the user
either switched servers or reloaded the page — surprising right after a
visible save.
Always call setupClientForServer on (re)connect so the latest
target.settings flow through to the transport's headers / requestInit, to
defaultMetadata, and to the OAuth manager.
Also adds remoteClientTransport tests that pin the wire-level contract:
settings round-trip through the /api/mcp/connect body when supplied, and
the field is omitted when no settings are configured.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(history): stop wiping pendingRequestEntries inside the connect event
State managers (ManagedToolsState, ManagedPromptsState, ManagedResourcesState,
ManagedResourceTemplatesState, ManagedRequestorTasksState) handle the
client's "connect" event by calling `void this.refresh()`. The synchronous
portion of refresh runs through SDK Protocol.request → transport.send →
MessageTrackingTransport.send → trackRequest, all before yielding to the
network — so each list request gets tracked into MessageLogState's
pendingRequestEntries inside the same connect event dispatch.
MessageLogState's own "connect" listener was registered after those state
managers (it's the last one constructed in App.tsx) and cleared
pendingRequestEntries on every connect. Listener order meant the clear ran
*after* the requests were tracked but before their responses arrived,
leaving the history with unmatched "response" entries marked PENDING.
The clear-on-disconnect (via statusChange → disconnected) already handles
session-boundary cleanup, so clear-on-connect was purely defensive and now
actively harmful. Drop the connect listener; keep the disconnect path.
Tests: updated the unit test to assert the new behavior (connect does not
clear); added an integration regression that wires a sibling connect
listener firing listTools and asserts the response folds into the request
entry instead of appearing as an orphan.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: drop accidentally tracked debugging screenshot
pending-requests.png was a local debugging artifact attached to a
conversation, not source. Remove it from the tree (the working copy is
kept so the user still has it locally) and rely on the existing untracked
behavior going forward.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(servers): add e2e check that settings.headers reach upstream MCP
Stitches the full v2 pipeline together in one assertion:
browser-style RemoteClientTransport
→ POST /api/mcp/connect (settings in body)
→ backend createTransportNode applies settings.headers to requestInit
→ SDK StreamableHTTPClientTransport
→ upstream MCP test server
Captures the upstream HTTP request via FetchRequestLogState and asserts
both `X-Tenant` and `X-Trace` are present on the outbound POST. Catches
any regression that breaks the wire between the form and the upstream
server in a single integration test, instead of the previous coverage
which exercised each hop in isolation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(ServerSettingsModal): invert ListToggle compact prop so icon matches state
ListToggle's `compact` prop represents the list's *current* compact state:
true means "currently compact" and renders the expand icon to advertise
the action (open up). The settings modal was passing
`compact={allExpanded}` — exactly inverted. When all sections were open
it showed the expand icon (suggesting "open more") and when collapsed it
showed the collapse icon, opposite of every other ListToggle caller in
the app.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* style(HistoryListPanel): match ServerListScreen Export button — rename + default variant
Renames "Export JSON" to "Export" and switches from the subtle ToolbarButton
to the same `Button variant="default"` used by ServerListControls so the
two Export affordances look identical across screens. Also moves the
button to the right of the ListToggle (mirroring the order on the server
list screen) and disables it when there are no entries to export.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* style(HistoryListPanel): move Clear button to top toolbar, match Export style
Moves the Clear affordance out of the "History (N)" section header and
up to the panel toolbar alongside Export, where it now uses the same
`Button variant="default"` styling. Order in the toolbar is now:
ListToggle · Clear · Export. Clear is disabled when there are no
unpinned entries to remove (pinned entries are intentionally preserved).
ToolbarButton helper is unused after this change and dropped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: ignore local debugging screenshots at repo root
Stops `git add -A` from re-staging the `*.png` files Claude Code drops
at the repo root from attached conversation screenshots. The pattern
only matches the top-level (`/*.png`) so legitimate image assets in
subdirectories (e.g. `clients/web/public/`) still get tracked when
explicitly added.
Also untracks the two screenshots that slipped through previously
(initialize-request.png, pending-requests.png).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* style(LogStreamPanel): match HistoryListPanel button styling
Clear / Export / Copy All on the Logging screen were rendered with the
subtle ToolbarButton helper while History's equivalents use
`Button variant="default"`. Switch all three to the default variant so
the toolbars look identical across screens and mirror the disabling
behavior — each button is now disabled when `entries.length === 0`.
ToolbarButton helper is unused after this change and dropped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(servers): address PR review — debounce save, preserve-on-omit PUT, validate shape, lift connect timeout
Round-up of fixes from https://github.com/modelcontextprotocol/inspector/pull/1353#issuecomment-4526861959:
1. ServerSettingsModal saves were firing on every keystroke (one PUT +
full mcp.json refresh per character). App.tsx now debounces by 300ms
and flushes on modal close so a burst of edits coalesces into one PUT
and the form value can't flicker back mid-typing.
2. `PUT /api/servers/:id` with an omitted `settings` field used to
silently wipe the persisted settings node. The route now treats the
three intents distinctly:
- field omitted → preserve existing on-disk settings
- explicit null → clear the settings node
- settings object → validate + apply
`useServers.updateServer` is simplified to drop the client-side
carry-through hack, and the test that documented the broken behavior
is flipped + augmented with explicit-clear and bad-shape cases.
3. `connectionTimeout` Promise.race is moved from App.tsx into
`InspectorClient.connect()`, reading `this.serverSettings?.connectionTimeout`.
TUI/CLI consumers now get the same behavior; on timeout the client
internally disconnects so the next connect starts clean.
4. Custom Headers hint in `ServerSettingsForm` notes that `Authorization`
is owned by the OAuth flow when OAuth is configured — avoids the
sharp edge where the SDK's authProvider silently overwrites a
user-set Authorization header.
5. New `validateSettings` on the server route rejects malformed shapes
(`settings: []`, `settings.headers: "oops"`, non-numeric timeouts,
etc.) with 400 instead of letting them persist and crash the UI.
7 (nit). Renamed `settings` → `savedSettings` inside `setupClientForServer`
so the variable holding the persisted settings doesn't shadow the
later locally-derived values.
Follow-ups opened separately:
#1356 — Move oauthClientSecret out of mcp.json into the OS keychain (review #6).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(servers): address PR review pass 2 — connect-timeout test, unmount cleanup, tagged intent, lenient settings read
Pass-2 review at https://github.com/modelcontextprotocol/inspector/pull/1353#issuecomment-4527089705:
(a) New integration test pins the new client-layer connect-timeout
contract: a hanging stub transport with `connectionTimeout: 50`
rejects with the descriptive error and lands the client in "error"
status. Documents what TUI/CLI consumers should expect from the
lifted Promise.race in InspectorClient.connect().
(b) useEffect cleanup clears `pendingSettingsTimerRef` on unmount so a
route change or HMR mid-debounce can't fire one stale PUT against
an unmounted component.
(c) Refactor the PUT-route `settingsIntent` discriminator from
`"preserve" | "clear" | { value }` to a tagged union with `kind`,
and consume it with a `switch` for exhaustiveness — reads cleaner
and makes adding a fourth intent painless.
(d) `normalizeMcpServers` now runs `validateSettings` on the on-disk
settings node and silently strips it if invalid, mirroring the
existing lenient-on-read pattern from `normalizeServerType`. Closes
the read/write asymmetry where a hand-edited malformed `settings`
could survive on disk and get carried through preserve-on-PUT.
New route test pins the strip-on-GET + no-propagate-on-preserve-PUT
behavior.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(servers): pass-3 review — patch-style PUT, toast on close-flush fail
Pass-3 review at https://github.com/modelcontextprotocol/inspector/pull/1353#issuecomment-4529276897.
(b) `PUT /api/servers/:id` now treats both `config` and `settings` as
optional patches:
- field omitted → preserve the on-disk value
- explicit body value → apply
- settings: null → clear
`updateServerSettings` stops reading `existing.config` from the
in-memory `servers` snapshot, which closes the closure-capture
hazard where a refresh between debounce-schedule and flush could
silently revert a separate edit. The route is the single source of
truth for transport config now. New tests cover settings-only PUT,
rejected non-object config, and the empty-body no-op patch case.
The previous "rejects a missing config" test is rewritten to
document the new no-op-preserve semantics.
(c) Inline comment near the PUT rebuild loop documents that writing
the full mcpServers map back is what gives the route its desirable
self-healing property for malformed settings on *other* entries —
deliberate side-effect, not an accident of normalization.
(d) `flushPendingSettings` surfaces failures via @mantine/notifications
instead of swallowing them. The modal already closed by the time
the flush PUT resolves, so a silent fail would leave the user
thinking their last edits saved (especially painful for the OAuth
client secret). The notifications wiring was already mounted in
main.tsx.
Pass-3 (a) — settings form being effectively uncontrolled mid-edit — is
acknowledged in the reply: working as intended today; the proper fix is
internal modal state when file-watching (#1345) forces the issue.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(servers): pass-4 review — strip smuggled settings out of config
Pass-4 review at https://github.com/modelcontextprotocol/inspector/pull/1353#issuecomment-4529861349.
Main finding: `normalizeServerType` spreads unknown keys, so a body
that nested `settings` inside `config` would smuggle a settings field
onto the stored entry without ever passing through `validateSettings`.
Strip the settings key off the incoming config in `buildStoredEntry`
so `validateSettings` remains the single write path for the settings
node — the "one source of truth" invariant pass 2 set up holds again.
Two new tests pin the strip on both routes:
- POST with `config.settings = { bogus }` lands an entry with no
settings node on disk.
- PUT with `config.settings = { bogus }` + `settings: null` clears
the real settings *and* doesn't re-attach the bogus payload via
the spread.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(servers): pass-5 review — absorb late connect rejection, tighten validateSettings, log smuggle warning
Pass-5 review at https://github.com/modelcontextprotocol/inspector/pull/1353#issuecomment-4529954490.
Main finding: when the connect-time `Promise.race` lost to the timeout,
the original `this.client.connect(this.transport)` promise was left
pending. After `disconnect()` tore the transport down, real transports
(SSE / streamable-http) would reject that promise later — with no
handler attached, producing a Node `unhandledRejection` (and a vitest
suite warning). Attach a no-op `.catch` right after creating the
connect promise so Node sees the late rejection as handled.
Nit (a): `validateSettings` was casting the raw object through to
`InspectorServerSettings`, which let unknown keys ride along onto disk.
Replaced the cast with an explicit pick-and-build over the known
fields. Unknown stowaways now silently drop on the way through. New
route test pins the behavior.
Nit (b): `buildStoredEntry` now logs a `warn` via the route's pino
logger when an incoming `config` carries a `settings` key. The strip
is still defensive (it isn't a documented contract) but the warning
lets a misconfigured client find out about the wasted field instead
of debugging a silent drop.
Nit (c) — the defensive `?? {}` fallback in `buildStoredEntry` —
acknowledged in the review reply; left in place as belt-and-braces.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(servers): pass-6 polish — scope absorber, thread id, drop empty OAuth, guard `in`
Pass-6 review at https://github.com/modelcontextprotocol/inspector/pull/1353#issuecomment-4529979135. Reviewer signed off as ready; landing the four marginal cleanups since they're all small.
(a) `connectPromise.catch(() => {})` moves inside the
`if (connectTimeoutMs > 0)` branch. The await-only path propagates
rejection cleanly through the outer try/catch — the absorber is
only needed when the race leaves the original promise unattended.
Code now reads as "absorber is tied to the race," which is the
real invariant.
(b) Thread `id` (POST id, PUT newId) through `buildStoredEntry` and
include it in the smuggle warning's pino bindings. Two clients
smuggling simultaneously are distinguishable in the log instead
of indistinguishable.
(c) `validateSettings` coerces empty-string OAuth fields to absent.
The form emits `""` when the user clears an input, and an empty
`oauthClientId` on disk would later be misread as "OAuth configured."
New route test pins it.
(d) `buildStoredEntry` adds a null/object guard before `"settings" in
configObj`. The route layer pre-checks this on POST and PUT, but
the helper's `unknown` parameter is a weak contract; the guard
makes it safe to call from anywhere.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(spec): reflect settings persistence + patch-PUT contract in v2_servers_file
The doc was authored when `mcp.json` only stored the server list. Bring it
forward to match what shipped in #1352 / PR #1353:
- Summary calls out that the file also stores per-server settings and
links into the Per-server settings section.
- On-disk format note explains that `settings` is an optional Inspector-
specific extension that other MCP tools ignore.
- New top paragraph on the Per-server settings section explains the
basic-config / settings UI split rationale (simpler form for the
common case, advanced settings tucked behind a second dialog).
- UI bullet is rewritten to match the patch-style PUT semantics that
emerged across the review cycle: settings-only and config-only
PUTs preserve their counterpart inside the write lock, so neither
modal can silently clobber the other half. The earlier "must
re-send settings on PUT" hack is gone.
- New bullets capture the save cadence (300ms debounce + flush-on-close
+ failure toast), the patch-PUT contract (omit-preserve / null-clear /
rename), and the write-path / read-path validator gates.
- Secret storage bullet links to #1356 (OS-keychain follow-up).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(spec): show a third entry with the optional settings node
The On-disk format example demonstrated only basic stdio entries. Add a
streamable-http entry with a populated `settings` node so readers can
see the shape inline rather than having to scroll to the Per-server
settings section. Inline comment in the block cross-links there for the
full contract and notes that other MCP tools ignore the extension key.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(web): port v1.5 dev backend (vite-hono-plugin + sandbox + remote-server bootstrap)
Ports the v1.5 in-process dev backend into v2 so `npm run dev` serves the
Hono remote-server (`/api/*`) alongside the Vite-served browser app. Without
this, the browser-side `createRemoteTransport` had nothing to talk to during
local development.
Adds under `clients/web/server/`:
- vite-hono-plugin.ts (Hono middleware on the Vite dev server; gated to
`apply: 'serve'` and skipped when `process.env.VITEST` is set so it stays
inert in vitest/storybook runs)
- sandbox-controller.ts (standalone HTTP server for the MCP Apps sandbox)
- web-server-config.ts (env parsing + initial-config payload + banner;
`autoOpen` defaults off under VITEST so test runs don't shell out to `open`)
- server.ts (standalone Hono prod server for the future v2 launcher, #1246)
- start-vite-dev-server.ts (in-process Vite starter for the future launcher)
- vite-base-config.ts (shared optimizeDeps exclusions)
Wires `honoMiddlewarePlugin(buildWebServerConfigFromEnv())` into
`vite.config.ts` with `apply: 'serve'`. Adds the `open` dependency and a
DOM lib + `@inspector/core/*` path alias to `tsconfig.node.json` so the
dev-backend files type-check alongside vite.config.ts.
Coverage: web-server-config.ts (100% lines), sandbox-controller.ts (91% lines),
and vite-base-config.ts (100%) all clear the per-file gate. The Vite plugin,
prod server, and dev-server starter are excluded from coverage as runtime
glue exercised by `npm run dev` end-to-end (callers covered separately;
their logic lives in core/mcp/remote/node, which integration tests already
exercise).
Closes#1320.
Unblocks #1244 (Wire App.tsx) — its test plan can now run end-to-end.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(web/server): address PR review feedback on the dev-backend port
- sandbox-controller: resolve start() with empty values on listen error
(e.g. EADDRINUSE on a fixed MCP_SANDBOX_PORT) instead of leaving the
promise pending forever. Without this, the Vite plugin's
`await sandboxController.start()` could hang the entire dev backend on a
port collision. Banner now silently omits the sandbox line; getUrl()
keeps returning null. Adds a regression test exercising the EADDRINUSE
path against a real socket.
- vite-hono-plugin: listen for `error` and `close` on the incoming request
stream (in addition to `end`) so an aborted POST/PUT (browser navigates
mid-upload) doesn't leak the middleware indefinitely.
- vite-hono-plugin: add a `.catch()` to the initial `pump()` kickoff so a
sync throw before the first `await reader.read()` can't surface as an
unhandled rejection.
- sandbox_proxy.html: drop the stale CSP comment — v1.5's sandbox HTTP
server never set CSP either; this PR doesn't regress anything. Replace
with the actual isolation model (iframe sandbox attribute + origin
allowlist) so future readers don't assume a CSP header is enforced.
- Hoist `WebServerHandle` to a shared `server/types.ts` so the future
launcher port (#1246) doesn't need to pick one of two duplicate
declarations.
- Extract `resolveAutoOpen()` helper out of the triple-nested ternary in
`buildWebServerConfigFromEnv` for legibility, with the three-way logic
documented in one place.
- Add an inline comment explaining why
`webServerConfigToInitialPayload` falls back to streamable-http for
unknown discriminators (forward-compat with future SDK transports)
rather than throwing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bob Dickinson
Cliff Hall