Add MCP_SANDBOX_FULL_ADDRESS and MCP_APP_ORIGIN_FULL_ADDRESS so a reverse
proxy or ingress deployment can advertise the sandbox proxy URL and the
_meta.ui.domain app origin the browser actually reaches, instead of the
bind-derived http://localhost:<port> ones. The process still listens on
MCP_SANDBOX_PORT / MCP_APP_ORIGIN_PORT.
Both values are validated in public-address.ts and refused (warned, bind-
derived address kept) when malformed or when their origin collapses onto
the Inspector UI's (or, for the app origin, the sandbox's) — the MCP Apps
spec requires host != sandbox origin, and the single-hostname proxy layout
is exactly the one that breaks it. Plain http under an https UI is warned.
The sandbox proxy page's referrer check was a hardcoded
http://localhost|127.0.0.1 pattern no https embedder could match. It now
compares the referrer's parsed origin against an exact list injected by the
controller, derived from the same filtered allow-list as its frame-ancestors
header, with the old loopback check kept only as the no-list fallback.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HzuzRPJv7M8wGe3wyw5sSg
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 3 on #2100.
- MCP_APP_ORIGIN_PORT could equal CLIENT_PORT or MCP_SANDBOX_PORT. The
app-origin listener starts BEFORE the web server binds, so that is not a
race it loses — it wins the port and the Inspector dies with EADDRINUSE.
`CLIENT_PORT=6278` was a valid config before this feature existed. It now
resolves to a dynamic port with a warning: of the three listeners this is
the only one that can move, and refusing to boot would cost the user the
whole Inspector over a feature they may not be using.
- The proxy's same-origin refusal covered only its own origin. An app served
from the INSPECTOR's origin is no safer: same-origin with the top document
means direct access to window.top, its DOM and its storage, and the
cross-origin frame in between buys nothing. Both trusted origins are now
refused before allow-same-origin is granted.
- Replaced the `as unknown as number` on `server.address()` with a real
narrowing of the string/null members of its return type.
- MCP_APP_ORIGIN_PORT added to the v1→v2 migration guide's env table, its
Docker recipe, and a troubleshooting entry for the symptom this actually
produces (the app renders, but its own backend rejects `Origin: null`).
- clients/web/README.md now states which failure is outside the fallback
guarantee — a published document the browser cannot reach — and why there
is no honest signal to fall back on. Docs and code no longer disagree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
An MCP App is rendered by handing its HTML to the sandbox proxy as
`srcdoc`, in a frame sandboxed without `allow-same-origin` — the #1565
isolation model. That gives the app document an opaque origin, so every
request it makes carries `Origin: null` and no CORS / OAuth-callback /
API-key allowlist can admit it. `_meta.ui.domain` is the spec field a
server uses to ask its host for a stable, dedicated origin, and it was
being dropped on the floor.
The field is explicitly host-dependent and the Inspector owns no domain
infrastructure, so it treats any non-empty value as a request rather than
an address, and answers with a real loopback origin on its own listener
(MCP_APP_ORIGIN_PORT, default 6276). One shared origin, path-keyed per
document: that delivers what the field is for without minting a port or
DNS name per app, and is therefore not a per-app isolation boundary.
The inner frame is granted `allow-same-origin` on this path only, which
is what makes the origin real. It is not a #1565 regression: the listener
is on its own port, so the app stays cross-origin to both the proxy and
the Inspector. The proxy refuses the grant if the URL's origin equals its
own, and the grant is never reachable from the server-supplied `sandbox`
string, which is still stripped. The per-app CSP is delivered as a real
response header there rather than only as a <meta>.
Every failure path falls back to the default srcdoc render with a console
warning naming the field, rather than blanking the app.
Also fixes a pre-existing `tsc -b` break on v2/main (useServers.test.tsx
passed `metadata: []` where `RequestMetadata` is expected), which has had
CI red since #2087 and blocks the pre-push gate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Closes#1950
Before this, a box with no reachable OS keychain — the published
container (no D-Bus session, #1848), Android/Termux (no platform binary,
#1905), a minimal Linux install without libsecret — could not persist an
OAuth client secret or a stdio `env:` value at all. `KeyringSecretStore`
degraded into a store that silently holds nothing: `get` returned null,
`delete` no-opped, and `set` hard-failed with a 503. That is the correct
failure behavior, but it left the documented, supported way to run the
Inspector with no secret persistence whatsoever.
Adds the two missing implementations and, more importantly, the policy
that picks one and the surfaces that say which was picked.
- `FileSecretStore` — one `0600` JSON document at
`~/.mcp-inspector/secrets.json`, encrypted with AES-256-GCM whenever
`MCP_INSPECTOR_SECRET_KEY` is set (scrypt, per-file random salt). The
whole map is encrypted as a unit rather than value-by-value so the
account names — `${serverId}:${field}` — are hidden too; a per-value
scheme would leave a readable index of which servers you hold a client
secret for. A file that can no longer be decrypted reads as empty
(matching the interface's read tolerance) but *refuses to be written*,
because replacing a file of still-valid secrets to satisfy an additive
request destroys data.
- `secret-store-selection.ts` — the policy.
`MCP_INSPECTOR_SECRET_STORE=keyring|file|memory` wins outright;
otherwise probe the keychain and, if unreachable, fall back loudly. The
fallback is `memory` in a container whose secrets directory is not on a
mount, and `file` otherwise — so mounting the volume the README already
recommends for the catalog flips the same run to durable storage with
no configuration, and an unmounted container gets the honest answer
rather than a file `--rm` will discard. Cached per process so the
banner, `/api/config`, and the store doing the writing cannot disagree.
- Automatic, but never silent. The downgrade prints at startup, rides
`GET /api/config`, and is named in a permanent 32px footer at the
bottom of both dialogs that accept a secret. A startup banner is seen
once by whoever started the process; a toast is seen once; a
dismissible banner is the thing a user dismisses before doing the work
it describes. The plaintext and memory cases render their caveat inline
in a warning tone — a warning you must hover to discover is not one.
- `SecretStoreUnavailableError` is now the base every store's `set`
throws, so the routes translate one condition into a 503 instead of
matching `KeychainUnavailableError` by name.
Docs: the container section of the root README gains a table of which
store you get and how to make secrets survive, plus the
`MCP_INSPECTOR_SECRET_KEY` upgrade/mismatch rules.
Tests: the existing interface contract block is extracted to
`secretStoreContract.ts` and run against all three implementations, so
none can drift on the availability contract. `FileSecretStore` is tested
against a real temp filesystem — mode, cross-instance round-trip, that
the ciphertext does not contain the plaintext, the read-modify-write
race, and every refusal path. Full `npm run ci` passes, per-file gate
included.
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot round 2 (PR #2039) surfaced no new visible comments but six
suppressed ones. Four were actionable.
`clients/tui/src/App.tsx` — a stale-completion race in the new banner:
start disconnecting server A, switch to B, then let A's disconnect
reject, and A's failure appears under B. Rapid same-server retries had
the same shape, and the stale attempt is the one most likely to reject.
Claim a monotonic attempt token before awaiting and publish only if it
is still current and the server has not changed; `handleConnect` bumps
the same counter so a connect retires an in-flight disconnect. The
rejection is still owned when superseded — just not published. The
regression test drives a deferred rejection across a server switch, and
fails without the guard.
`tsconfig.lint.json` — the root type-aware block matches
`test-servers/src/**/*.tsx` but the parser project omitted that
extension, so such a file would have failed with "not found in any of
the provided project(s)" instead of being linted. Added, and AGENTS.md
now states the invariant: the include must stay a superset of the lint
globs.
`clients/web/server/{server,vite-hono-plugin}.ts` — the `open()`
rejection branch was untested, and duplicated. Both were fixed by
removing the duplication: `openBrowser` now lives beside
`printServerBanner` in `web-server-config.ts`, with one implementation
to test. Two copies of a one-line `.catch` is two chances for one to
drift back to a bare `open(url)`. The dev plugin is not booted in tests
— it returns early under VITEST by design — but it calls the same
tested helper.
`core/mcp/remote/node/server.ts` — `closeAbortedStream`'s rejecting-close
path, the entire reason the helper exists, had no test. Exported it (as
this file already does for `requestIdForSendWait` / `mcpParamHeadersOnly`)
and covered both legs, asserting no unhandledRejection fires.
Also corrects AGENTS.md, which said each client lists "both" tsconfig
projects — true for cli/tui/launcher, but web lists four leaf projects.
Signed-off-by: cliffhall <cliff@futurescale.com>
Closes#1795.
Enforce localhost-only binding in code (the "never 0.0.0.0" rule was only a spec note): refuse an all-interfaces HOST — across every spelling the OS binds as the wildcard (inet_aton decimal/octal/hex, IPv6, IPv4-mapped, zone-scoped, IDNA/Unicode) — unless DANGEROUSLY_BIND_ALL_INTERFACES=true (Docker's sanctioned opt-in). Fixes the reported stdio-connect 403 by expanding the default loopback origin allow-list to all three interchangeable forms.
Hardening surfaced across an extended review: closes an IDNA/fullwidth wildcard bypass, a blank-ALLOWED_ORIGINS fail-open, and the CLI/TUI OAuth-callback wildcard bind (loopback-only now, RFC 8252); canonicalizes the origin allow-list the way a browser does; derives frame-ancestors from the real embedder list (IPv6-literal exclusion verified in headless Chromium); strictPort on all three bind points; and preserves the CLI exit-code map + JSON envelope through the mcp-inspector --cli launcher path. Shared host normalization lives in core/node/hostUrl.ts. Docs + tests + smokes throughout.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Address PR #1641 review feedback:
- Add readInspectorVersionSafe() (non-throwing) in core/node/version.ts and use
it in the web backend, so a failure to resolve the root manifest hides the
(cosmetic) version badge instead of crashing the backend at startup. Covered
by tests for both the resolved and unresolvable paths.
- Update MonitoringScreen's docstrings: it now wraps every supplied screen in a
ScreenStage (only the active tab mounts, cross-fading) rather than rendering
just the active tab's node.
The third fetch-consolidation suggestion (one useInitialConfig instead of three
GET /api/config hooks) is tracked as a follow-up in #1643.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
Display the single-source version (root package.json) in the web UI's
lower-right corner, in grey — matching the CLI and TUI. The browser can't read
the filesystem, so the version flows backend → browser via GET /api/config:
- Add `version?: string` to InitialConfigPayload; the web backend
(webServerConfigToInitialPayload) reads it once via the shared
readInspectorVersion() and tags it onto the payload.
- Add a `useInspectorVersion` hook mirroring useSandboxUrl/useServerListWritable.
- Add a presentational `VersionBadge` element rendered fixed lower-right, grey,
non-interactive; styling lives in a `versionBadge` Text theme variant (no
inline styles, --inspector-* tokens).
- Wire it into App.tsx.
Verified live: /api/config serves `version: "2.0.0"` and the badge renders
`v2.0.0` fixed at the lower-right in grey. Tests: useInspectorVersion (full
branch coverage), VersionBadge (unit + Storybook), web-server-config payload.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
* 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>
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>
* 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>
cliffhall
Bob Dickinson