| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
cliffhall
|
b25ece8ded |
feat: per-server custom OAuth authorization-request parameters (#2018)
Real deployments gate IdP selection, prompt behavior, or audience on a query
parameter the OAuth/OIDC core specs never standardized (Keycloak's
`kc_idp_hint`, OIDC's `login_hint` / `prompt` / `acr_values`, Auth0's
`audience`), and none of them could be driven from the Inspector.
The Inspector never builds the authorize URL — the SDK's `auth()` does, and it
exposes no hook for extra parameters — so the merge happens at the one seam
that sees the finished URL: `BaseOAuthClientProvider.redirectToAuthorization`.
It lives in `core/`, so web, CLI, and TUI all inherit it.
- `core/auth/authorizationParams.ts`: the reserved-key list (`client_id`,
`code_challenge`, `code_challenge_method`, `redirect_uri`, `resource`,
`response_type`, `scope`, `state`), the key validator the form renders
inline, and the merge itself. Reserved keys are dropped with a warning
rather than silently overriding PKCE / the state binding / RFC 8707;
blank-key rows are skipped; the caller's URL is never mutated.
- Applied to the authorization request only — nothing wires it into the token
request.
- Persisted as `oauth.authorizationParams` (a string record) on the per-server
config beside `scopes`, edited as key/value rows in Server Settings ->
Authorization ("Additional authorization parameters"), with an inline error
per offending row plus a section-level warning.
- The enterprise-managed (EMA) leg redirects to a *different* authorization
server (the enterprise IdP), so it goes through the explicit
`redirectToExternalAuthorization` entry point, which skips the merge — a
`kc_idp_hint` or `audience` meant for the MCP server's AS must never be
appended to the IdP's authorize URL.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RDXCSrnSBV7vZfxycG5Z6E
Signed-off-by: cliffhall <cliff@futurescale.com>
|
||
|
Cliff Hall
|
da37692485 |
refactor(web): sweep AGENTS.md TypeScript + Mantine rule violations (#1771)
Codebase-wide sweep bringing the web client into strict compliance with the AGENTS.md TypeScript and Mantine/React conventions: ~170 inline 2+-static-prop elements extracted to named .withProps() constants across ~77 files, the AGENTS.md rule wording tightened to match (static-prop boundary + Accordion/Transition/data-*/Box exemptions), a real ScreenStage Box->Flex layout regression fixed (direction: column) with a Storybook regression guard, and an App.tsx double-cast tightened. Closes #1762. |
||
|
Cliff Hall
|
402d7b204d |
Server Settings: disambiguate EMA OAuth credential fields (#1692) (#1749)
* Server Settings: disambiguate EMA OAuth credential fields (#1692) Under enterprise-managed authorization the OAuth Settings "Client ID" / "Client Secret" fields hold the *resource authorization server* credentials (EMA leg 3 — the Test Client from the xaa.dev resource registration), not the app/IdP pair configured in Client Settings. The unqualified labels sat under a toggle naming the enterprise IdP, pulling readers toward the wrong pair. - When enterpriseManaged is on, relabel to "Resource AS Client ID" / "Resource AS Client Secret" and add a description naming the resource authorization server and pointing the app pair back to Client Settings. - Document the Scopes delimiter: space-separated (RFC 6749), with a warning that commas produce one invalid token, plus a placeholder example. Adds tests for both label states and the scopes hint, plus an EnterpriseManagedOAuth story. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * pr-screenshots: OAuth EMA field disambiguation proof shots (#1692) Add before/after screenshots of the Server Settings → OAuth Settings fields (EMA off = plain Client ID/Secret; EMA on = Resource AS Client ID/Secret) plus the new space-separated Scopes hint, with a README section documenting them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Address review: genericize EMA copy, reuse enterpriseManaged const (#1692) - Drop the hardcoded xaa.dev staging host from the resource-AS field description (it's a general EMA concept, not env-specific); phrase it as "the resource authorization server's registered client credential (EMA leg 3)". Also trims the copy's overlap with the checkbox description. - Reuse the existing `enterpriseManaged` const for the EMA Checkbox's `checked` prop instead of recomputing `settings.enterpriseManaged ?? false`. - Update the test assertion and pr-screenshots README/EMA-on screenshot to match the new copy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Client Settings: mirror EMA disambiguation (IdP-qualified labels) (#1692) The Server Settings OAuth fields now label the leg-3 resource-AS pair explicitly; make Client Settings parallel so each modal names its pair and points at where the other lives. - Relabel the EMA credential fields to "IdP Client ID" / "IdP Client Secret" (parallel to Server Settings' "Resource AS Client ID/Secret"), so the label alone disambiguates without relying on the description. - Add the reciprocal cross-reference to each description: these are the enterprise IdP pair (EMA legs 1–2), not the per-server resource authorization server credentials, which go in Server Settings → OAuth Settings. - Update ClientSettingsForm/ClientSettingsModal tests for the new labels; add a pr-screenshots section + Client Settings screenshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
c51d6aabd2 |
Extensions UI: own Advertised Extensions section + 'Client Advertised Extensions' heading (#1748)
* Settings: move Advertised Extensions to its own accordion section (#1747) The Advertised Extensions control (added in #1739) lived inside the Options accordion panel, wedged between "Fetch Lists One Page at a Time" and "Network Log Size". Promote it to its own top-level "Advertised Extensions" accordion section (after Options), so the distinct concept — which extensions the Inspector advertises to this server in its client capabilities — reads as its own collapsible section. - ServerSettingsForm: add "extensions" to ServerSettingsSection; move the checkbox group into a new Accordion.Item value="extensions". Its description moves up into the panel; no behavior change (still folds into settings.advertisedExtensions via onAdvertisedExtensionChange). - ServerSettingsModal: add "extensions" to allSectionsFor so expand/collapse-all covers it. - Tests: the toggle now lives under the new (collapsed-by-default) section, so the form tests render it expanded and the modal tests open it first. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Connection Info: rename "Advertised Extensions" heading to "Client Advertised Extensions" Disambiguates the client-side column from the adjacent "Server Extensions" column and from the (separate) Server Settings "Advertised Extensions" section. Label-only change; the value still renders clientCapabilities.extensions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * docs: refresh Connection Info screenshot for the renamed heading Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * docs: refresh #1747 proof screenshots (fresh capture) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Settings: add AdvertisedExtensions Storybook story (review) Address the #1748 review's optional note: the new Advertised Extensions accordion section had no story expanding it, so it wasn't documented in the visual catalog (the default story expands only ['options']). Add an AdvertisedExtensions story with expandedSections: ['extensions'] and a per-server override, showing the default-on and toggled-off checkbox states. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
fe295396af |
Extensions: advertised-extensions toggle in Server Settings (Phase 2 of #1633) (#1743)
* Extensions: advertised-extensions toggle in Server Settings (Phase 2 of #1633) Adds the debugging knob from #1633: users choose which extensions the Inspector advertises to a server, and a server may register different tools in response. Builds on Phase 1's registry + advertisedExtensions option (#1738). Settings + persistence (core): - InspectorServerSettings.advertisedExtensions + StoredMCPServer field, wired through serverList's read/write converters and INSPECTOR_FIELD_KEY_MAP (omit-when-empty, byte-stable round-trip). - validateSettings (remote PUT route): isBooleanRecord guard + carry a non-empty map; unknown/malformed rejected. - App.tsx threads savedSettings.advertisedExtensions into the client options. UI: - ServerSettingsForm renders an "Advertised Extensions" group of checkboxes driven by ADVERTISABLE_EXTENSIONS; a per-server override wins over the registry default. Modal folds a toggle into settings.advertisedExtensions. Test server + proof: - composable-test-server extensionGatedTools: a tool registered disabled and enabled on `initialized` iff the client declared the mapped extension (getClientCapabilities). Legacy stateful leg. Threaded through load-config/resolve-config; advertised-extensions-http.json showcase + README. - Integration test connects legacy with/without the tasks extension advertised and asserts the gated tool appears/disappears in tools/list — the acceptance criterion, proven end to end, not asserted. Tests: serverList round-trip (non-empty/absent/empty), ServerSettingsForm toggle rows, ServerSettingsModal fold, validateSettings accept/reject, and the integration gating test. npm run ci green (per-file ≥90 gate held). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Extensions: add #1739 proof screenshots (advertised-extensions toggle) End-to-end smoke test in the web client against the legacy advertised-extensions-http.json test server (port 3220): connecting with the Tasks extension advertised (default) lists echo + get_weather, unchecking the Advertised Extensions → Tasks toggle and reconnecting lists only echo — the server's tool registration changing on the client-declared extension. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Extensions: reconverge advertisedExtensions to no-override on default (review) Address Phase 2 review observation #1: toggling an extension back to its registry default now deletes the override key (and drops the whole map to undefined when it empties), so the on-disk `advertisedExtensions` reconverges to the byte-stable "no override" state instead of persisting a redundant `{ key: <default> }`. Matches the omit-when-default policy the other settings use. Only a value that differs from the default is persisted. Adds a modal test asserting the reconvergence (re-checking the sole disabling override drops advertisedExtensions to undefined). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
ec185f1053 |
Logging: per-request log level on modern servers (era fork) (#1629) (#1724)
Some checks failed
Run install, format, lint, build, and test on every push / build (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish-github-container-registry (push) Has been cancelled
* Logging: per-request log level on modern servers (era fork) (#1629) On modern (2026-07-28) servers `logging/setLevel` and the session-scoped level are gone: the client opts into logs per request via the `io.modelcontextprotocol/logLevel` `_meta` key, and the SDK does not attach it automatically, so server logs are silently absent until stamped. Core (`InspectorClient`): - `setModernLogLevel(level | undefined)` / `getModernLogLevel()` store the opt-in level; `mergeMeta` stamps it on every outgoing request, gated on the negotiated modern era so legacy connections are untouched. - Cleared on disconnect so it can't leak into the next connection. Web UI: - `LogControls` forks on `protocolEra`: legacy keeps the `logging/setLevel` selector + Set button; modern shows a per-request opt-in Select (Off + levels, applied immediately) with copy explaining logs arrive on the originating request's stream. - Threaded `protocolEra` / `modernLogLevel` / `onSetModernLogLevel` through LoggingScreen and InspectorView from App. Tests: unit coverage for the modern LogControls/LoggingScreen fork; integration coverage proving the `logLevel` `_meta` is stamped on a real modern server once opted in (and not on legacy). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test-servers: logging era-fork showcase configs + proof screenshots (#1629) Add two manual-testing configs and README docs for the logging era fork: - logging-legacy-http.json: plain streamable-HTTP server with `logging: true` and a `send_notification` tool. The Logs tab shows the legacy session-scoped "Set Active Level" selector + Set button; calling the tool streams logs. - logging-modern-http.json: same, with `transport.modern: true`. Connect with Protocol Era = Modern and the Logs tab shows the per-request opt-in control; the client stamps `_meta["io.modelcontextprotocol/logLevel"]` on every request (visible in the Network tab request body). pr-screenshots/ documents the verified behavior end-to-end against both real servers: the legacy control with logs streaming, the modern per-request control, and the modern tools/call request body carrying the stamped logLevel _meta. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test-servers: send_notification emits via per-request notifier so modern logs stream (#1629) The `send_notification` tool sent its `notifications/message` through the global `server.server.notification()`. On the modern (2026-07-28) stateless leg that isn't tied to the in-flight request, so the SDK answered the tool call with plain JSON and dropped the log — nothing reached the Inspector's Logs panel. Prefer the per-request `extra.sendNotification` (rebuilt from `ctx.mcpReq.notify`): scoped to the current request, it makes the SDK upgrade the response to SSE and stream the log back on the originating request's stream. Falls back to the global notifier (legacy path / no per-request context). Verified end-to-end: opting in via the Logs-tab "Log Level per Request" control and calling send_notification now streams the entry on the modern server; legacy behavior unchanged (server-notification integration tests green). README documents the modern logging showcase and the caveat that this test server streams whenever a tool emits — the SDK beta strips the per-request envelope from the handler context, so the tool can't gate on the client's logLevel opt-in (the client-side `_meta` stamp #1629 owns is still fully visible in the Network tab). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test-servers: send_notification uses SDK request-scoped log() → spec-faithful modern gating (#1629) The previous fix routed `send_notification` through `extra.sendNotification` (`ctx.mcpReq.notify`), which streams on the modern leg but emits unconditionally — the log showed up even when the client hadn't opted in via the per-request `logLevel`. The SDK already provides the correct primitive: `ctx.mcpReq.log(level, data, logger)` (added in `McpServer.buildContext`). It applies the era-correct threshold gating itself — on the modern (2026-07-28) leg it reads the per-request `logLevel` opt-in from the request envelope and drops the message when the client didn't opt in or the level is below the requested severity; on legacy it honors the session level from `logging/setLevel` — and emits through `notify`, so the modern response still upgrades to SSE and the log rides the originating request's stream. Surface it as `extra.log` (`toHandlerExtra` maps `ctx.mcpReq.log`) and have `send_notification` prefer it, falling back to the global notifier for callers without per-request context. Verified end to end: opting in via the Logs-tab "Log Level per Request" control stamps `logLevel` in the request `_meta` and streams the log; setting it back to Off omits the key and the server gates the log (plain JSON response, no notification). Legacy server-notification integration tests still green. (No SDK change needed — the earlier apparent gap was our test server using the wrong low-level API.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * web+core: add "Log Level per Request" as a per-server setting, defaulting to on (#1629) Adds the modern per-request log level as a persisted Server Settings option (Options → "Log Level per Request", below Protocol Era), so a server can opt into logs by default instead of the Logs-tab control starting at Off. - types.ts: `ModernLogLevel` (`"off"` | the 8 levels), `DEFAULT_MODERN_LOG_LEVEL` = "debug" (default-on so a modern connection surfaces server logs out of the box), `MODERN_LOG_LEVELS`, and an `isModernLogLevel` guard. Field added to `InspectorServerSettings` and `StoredMCPServer`. - serverList.ts: round-trip the field (lift on read, omit-on-default on write, drop unknown literals) mirroring `protocolEra`; add to the persisted field allowlist. - remote/node/server.ts: validateSettings accepts/validates it and the normalize path drops a malformed literal. - inspectorClient.ts: the constructor seeds `modernLogLevel` from the setting (`"off"` → not opted in), so a modern connection stamps the level from the first request without UI interaction. mergeMeta still gates on the modern era. - ServerSettingsForm/Modal: a Select (Off + 8 levels) with a modern-era description. App seeds the live Logs-tab control from the setting on connect so the two stay in sync. Tests: form Select (default/onChange/off/reflect), modal handler, serverList round-trips, route validation (accept/reject/drop), and constructor seeding (seeded level stamped from the first request; "off" opts out). The prior modern-era "stamps once set" test is updated for the new default-on behavior. Coverage gate green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Fix CI + review: format ServerSettingsForm, drop stray network export, broaden ignore - Prettier-format ServerSettingsForm.tsx (the format:check failure that broke CI — my pre-push `npm run format` from the repo root was a silent no-op since there is no root format script; formatting is per-client). - Remove `inspector-network-logging-modern-2026-07-19T15-11-27.013Z.json`, a Network-tab export debugging artifact accidentally committed in ca2aa526 (flagged by review). - Broaden the .gitignore debugging-artifacts section with `/inspector-network-*.json` so future network exports are ignored (the existing `/inspector-history-*.json` didn't match the network export naming). Verified with the full `npm run ci` (validate → coverage → smoke → storybook), green across all clients. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * test: add modern-era log delivery + gating e2e tests (#1629) The modern-era suite covered the client-side `_meta` logLevel stamp but not that a log actually flows back — the legacy suite already has that (`should receive server-initiated notifications via SSE/streamable-http`), so the modern side was the gap. Add two live e2e tests against a modern (`createMcpHandler`) server serving the `send_notification` tool with `logging: true`: - opted-in (server setting = a level): calling the tool delivers a `notifications/message` over the request's SSE stream, with the right level/logger/data. - not opted in (setting "off"): the tool still returns a result, but the server gates the log — no `notifications/message` is received. This exercises the full path end to end: the client seeding + `_meta` opt-in, and the server's `send_notification` → `ctx.mcpReq.log` era-correct gating. Verified with the full `npm run ci` (green across all clients). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * web: hide the modern "Log Level per Request" setting on legacy connections (#1629) The per-request log level is meaningless on a legacy server (which uses logging/setLevel), so hide it in Server Settings when the era is legacy: - protocolEra pinned to "legacy" → hidden (no connection needed), or - protocolEra "auto" that negotiated legacy at connect time → hidden. It stays visible for a pinned "modern" server, and for an "auto" server that is either not yet connected (era unknown) or resolved to modern. Threads the negotiated era down: App passes `negotiatedEra` to ServerSettingsModal only when the settings target is the live-connected server (`connectionStatus === "connected" && settingsModalTargetId === activeServerId`); the modal forwards it to ServerSettingsForm, which gates the control on `configuredEra === "modern" || (configuredEra === "auto" && negotiatedEra !== "legacy")`. Tests: visibility matrix on the form (legacy hidden; auto+legacy hidden; auto+unconnected and auto+modern and modern shown) and a modal hide case; the existing modern-log tests now base on a modern-era settings object. Verified in the browser (legacy hides, switching to modern reveals it reactively) and with the full `npm run ci` (green across all clients). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
b3b04c6939 |
Thread cacheMode through managed list refresh via SDK cache-aware verbs (#1721) (#1723)
Some checks failed
Run install, format, lint, build, and test on every push / build (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish-github-container-registry (push) Has been cancelled
* Thread cacheMode through managed list refresh via SDK cache-aware verbs (#1721) Manual/auto refresh of tools/resources/prompts now forces a cache-bypassing round trip on modern servers, delivering the raw-request cacheMode control deferred from #1628. The InspectorClient list/read path uses the SDK's single-page `client.request` verbs (for pagination debugging), which never consult the SDK response cache — so `cacheMode` was inert there. This adds a separate aggregate path that routes through the SDK's cache-aware high-level verbs, where `cacheMode` is honored: - InspectorClient: new `listAllTools/Prompts/Resources/ResourceTemplates( { cacheMode, metadata })` that call `client.listTools()` etc. (all-page aggregate + SEP-2243 x-mcp-header exclusion + cache consult/write). A shared `getCacheableRequestOptions` / `aggregateListParams` back all four. - InspectorClientProtocol: the four `listAll*` methods added. - ManagedListState: delegates page walking to the SDK verb (config `fetchPage` loop → `fetchAll`); `refresh(metadata?, cacheMode?)` threads cacheMode. A `list_changed` auto-refresh bypasses the cache (`cacheMode: "refresh"`). - Managed hooks: user-initiated `refresh()` passes `cacheMode: "refresh"`, so the existing per-tab Refresh buttons become genuinely cache-bypassing — no App wiring change. Connect-time load stays default (`'use'`). The single-page `listTools(cursor)` primitive is unchanged; the paged hooks still surface per-page `nextCursor`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Single-page list pagination: sidebar toggle + server setting (#1721) Delivers the deferred single-page fetch UI: Tools/Resources/Prompts can be loaded one page at a time instead of auto-aggregating every page — a defensive option for servers with very large lists. - New `singlePageLists` server setting (persisted, server-wide), plumbed through ServerConfig/InspectorServerSettings, serverList convert/default, the node + remote settings paths, and the Server Settings form ("Fetch Lists One Page at a Time"). - New `ListPaginationControls` element (a "Single page" switch + "Load next page" button + "N pages loaded" status) rendered in each list sidebar; the toggle edits the setting live (optimistic) and persists it. - The paged state stores (PagedTools/Prompts/ResourcesState) now own loading: they auto-load page 1 on connect in single-page mode and track nextCursor + page count as observable state; the managed stores skip their all-page walk in single-page mode (so nothing is over-fetched) while still lighting the list-changed indicator. `usePaginatedList` selects the display source and pagination controls per mode — pure, no data-loading in React effects. - Refresh is mode-aware: reload page 1 in single-page mode, refetch the whole aggregate otherwise. Sidebars widened 340→360 so the controls don't crowd list entries. - Test server: `test-servers/configs/pagination-http.json` (12 tools/resources/ prompts, page size 4 → 3 pages each) exercises the flow end to end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Add pr-screenshots proving single-page pagination (#1721) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Address @claude review: template load, double-click guard, auth recovery (#1721) - Fix: resource templates never loaded on connect in single-page mode. The connect-time all-page skip now applies only to lists with a paged counterpart (new `deferInSinglePage` flag on ManagedListConfig; false for resource templates), so templates still aggregate on connect regardless of the setting. - Guard `PagedTools/Prompts/ResourcesState.loadPage` against a concurrent load (fast double-click on "Load next page") so the same page can't be appended twice; the in-flight load owns the cursor and the second call is a no-op. - Thread ambient auth recovery through the single-page refresh, Load-next-page, and mode-switch loads (they previously bypassed it) — usePaginatedList's onRefresh/onLoadMore now return their promises so App can wrap them. - Drop the misleading "aggregated list" wording from the paged-store docs. Tests added for each; npm run coverage green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Review round 2: revert optimistic toggle when persisting the setting fails (#1721) Non-blocking review follow-up. If updateServerSettings rejects, the optimistic `singlePageListsOverride` is now cleared and the live client setting rolled back (the effect only clears the override when the persisted value changes, which doesn't happen on failure), so the UI and client reflect what's on disk rather than the failed edit. Added an App test for the revert path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Fix single-page Refresh not clearing list-changed indicator + UI polish (#1721) Round-3 review fix (blocking): in single-page mode a `list_changed` lights the list-changed indicator, but the single-page Refresh reloads page 1 of the paged store and bypassed `useManaged*.refresh()`, so the indicator was never cleared. Expose `clearListChanged` from useManagedTools/Prompts/Resources and call it in the single-page branch of onRefreshTools/Prompts/Resources. Tests added. UI polish (user feedback on the sidebar control): - Rename the switch label "Single page" → "Paginated". - Put "Load next page" on the same row as the switch, space-between (switch fully left, button fully right). - Move the "N pages loaded" status to its own line below the row, centered. - Hide the whole control once the list is known to be a single page (single-page mode, page 1 loaded, no nextCursor) — nothing to paginate. pr-screenshots refreshed to the new layout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Rename the singlePageLists setting to paginatedLists for clarity (#1721) `singlePageLists` read backwards from the "Paginated" switch label (it sounds like paginated=false). Rename the server setting to `paginatedLists` with the same value meaning — true = paginated/one-page-at-a-time, false = aggregate-all — and follow through on the related identifiers for consistency: - Setting field `paginatedLists` across ServerConfig / InspectorServerSettings, serverList convert/default, node + remote settings paths, and the form/modal callbacks (onPaginatedListsChange). - `usePaginatedList` / `ListPaginationControls` prop `singlePage` → `paginated`, `onSinglePageChange` → `onPaginatedChange`. - `ManagedListConfig.deferInSinglePage` → `deferWhenPaginated`. - App state (paginatedListsOverride, onTogglePaginatedLists, …) and all tests/fixtures + mode-referring comments. "single-page" wording that denotes the SDK's one-page `client.request` verb (inspectorClient) is intentionally left as-is. Purely a rename — no behavior change; unit + integration + cli + tui + coverage all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Format App.test.tsx (prettier) after the paginatedLists rename (#1721) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
e026051b47 |
Era-aware connection model & capability gating (#1626) (#1697)
Some checks failed
Run install, format, lint, build, and test on every push / build (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish-github-container-registry (push) Has been cancelled
* Era-aware connection model & capability gating (#1626) Make protocol era (SEP §7.8) a first-class, per-server connection dimension, orthogonal to the transport type, and drive the SDK Client's versionNegotiation from it. Config - Add `ServerProtocolEra` ("legacy" | "auto" | "modern"), `DEFAULT_PROTOCOL_ERA` (legacy, per the SDK's don't-auto-probe guidance), `MODERN_PROTOCOL_VERSION`, and `eraToVersionNegotiation()` in core/mcp/types.ts. - Persist `protocolEra` on `StoredMCPServer` / `InspectorServerSettings`, round-tripping through serverList.ts (omitting the default on disk) and the `/api/servers` read + write validators. InspectorClient / state - Thread `versionNegotiation` through `InspectorClientOptions` (default legacy), replacing the hardcoded pin. After connect, read `getProtocolEra()` / `getDiscoverResult()` / `getNegotiatedProtocolVersion()`, expose them via getters, dispatch `protocolEraChange` / `discoverResultChange`, and clear on disconnect. `useInspectorClient` surfaces `protocolEra` / `discoverResult`. Web UI - ServerSettingsForm: a Protocol Era selector bound to the per-server setting. - ConnectionInfoContent: era-aware — Era badge, Session ("Sessionless" on modern), and a Discovery section (supported versions + extensions) from the `server/discover` result. CLI/TUI also map the mcp.json `protocolEra` into versionNegotiation so the field applies uniformly. Version negotiation stays fully client-side; modern connect works over the existing remote tunnel and disconnect is sessionless (the SDK skips DELETE when there is no session id). Mcp-* header fidelity is left to the History/Network card. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Address @claude review: era docstrings, era validation, stdio session label, AGENTS.md rule - Fix stale docstrings/comments in inspectorClient.ts, useInspectorClient.ts, inspectorClientEventTarget.ts, and ConnectionInfoContent.tsx: getProtocolEra() returns "legacy" (not undefined) on a plain legacy connect; it is undefined only when not connected. - Validate protocolEra at the storedFieldsToInspectorSettings read boundary (new shared isProtocolEra guard) so a hand-edited mcp.json read directly by the CLI/TUI drops an unknown era instead of passing it to eraToVersionNegotiation. Keeps the exhaustive switch intact. - ConnectionInfoContent: the Session row is HTTP-only — show "N/A (stdio)" for stdio instead of the misleading "Session-based". - AGENTS.md: add a rule against unjustified `as unknown as` double casts, and drop the one such cast introduced in the serverList test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Address re-review: dedupe isProtocolEra guard in remote server Replace the local isProtocolEra in remote/node/server.ts with the shared guard now exported from serverList.ts (already imported here), so the era literal set lives in one place and can't drift. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
7fecebed5e |
OAuth: re-key store to (server, issuer) + SEP-837/2350/2352 auth UX (#1694)
Some checks failed
Run install, format, lint, build, and test on every push / build (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish-github-container-registry (push) Has been cancelled
* OAuth: re-key store to (server, issuer) + SEP-837/2350/2352 auth UX (#1625) Re-key the persisted OAuth store to (server, issuer) and add the SEP-837/2350/2352 authorization-hardening UX. The SDK v2 auth() drives the SEP-2352 flow off the provider's ctx.issuer; the Inspector's job is to honor it and round-trip the issuer stamp. SEP-2352 — issuer-bound credentials - ServerOAuthState nests issuer-bound credentials under byIssuer[issuer] = { clientInformation, clientRegistrationKind, tokens }, with an activeIssuer pointer answering the transport's ctx-less per-request bearer read. - Fix a latent bug: OAuthStorageBase re-parsed tokens/client-info through OAuthTokensSchema/OAuthClientInformationSchema, which strip the non-RFC `issuer` field and silently defeat the SDK's discardIfIssuerMismatch. The store re-attaches the stamp from the byIssuer key. - BaseOAuthClientProvider threads ctx.issuer and implements discoveryState()/saveDiscoveryState() (activates the SDK's callback-leg AuthorizationServerMismatchError binding check) and invalidateCredentials(). - Lazy, migration-free upgrade: a legacy pre-1625 blob's top-level clientInformation/tokens are returned as an unkeyed fallback until the first issuer-stamped save promotes them into byIssuer. SEP-837 — application_type - DCR declares application_type: "native" (localhost Inspector). - ClientSettingsForm renders RegistrationRejectedError detail and reflects DCR's deprecated-in-favor-of-CIMD (SEP-991) status. SEP-2350 — step-up scope union - Per-server onInsufficientScope (reauthorize | throw) setting wired ServerSettingsForm -> StoredMCPServer.oauth -> StreamableHTTPClientTransport. - StepUpAuthModal visualizes the prior-union-challenged scope set, tagging each scope `new` vs `already granted`. mcpAuth simplification - Delete the hand-rolled authorizeWithoutRefresh (mishandled issuer stamping); forward forceReauthorization/skipIssuerMetadataValidation to native SDK auth() so both the initial and step-up paths get SEP-2352 stamping. Vocabulary + docs - OAuthStep gains cimd_fetch / issuer_comparison / scope_step_up (recorded on a step-up redirect); the Network tab labels auth requests by OAuth phase. - v2_auth_hardening.md gains an as-built status section. Deferred to the era-aware card: the SdkError(EraNegotiationFailed) -> UnauthorizedError unwrap and finishAuth(URLSearchParams) form (only fire under non-legacy negotiation). Closes #1625 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address @claude review: issuer-stamp source gating + clear/activeIssuer + phase over-match - SEP-2352: stamp the issuer only when the credential actually came from the byIssuer slot (`fromSlot`), not merely when a slot exists. A partially-populated slot alongside a surviving legacy field would otherwise stamp a legacy (possibly cross-AS) credential with the current issuer, defeating discardIfIssuerMismatch. - Issuer-scoped clears no longer promote the cleared issuer to `activeIssuer` (updateIssuerSlot gains a `setActive` flag; save paths keep the promotion). - oauthNetworkPhase matches register/token/authorize as the final path segment (trailing slash tolerated) so `/token/refresh` or `/api/register/foo` no longer over-match. - Tests for all three. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
cliffhall
|
19a6df7ad3 |
SDK v2 migration (behavior-neutral, versionNegotiation: 'legacy') (#1624)
Migrate all three clients + core + test-servers from
@modelcontextprotocol/sdk@1.29 to SDK v2 (2.0.0-beta.4), keeping behavior
identical against legacy (2025-11-25) servers. Foundation for the SDK V2 +
New Spec stack.
Packages
- Split @modelcontextprotocol/sdk -> @modelcontextprotocol/client (Client,
transports, OAuth, error taxonomy, protocol types) + core (Zod schemas);
stdio -> client/stdio, Ajv -> client/validators/ajv. Test-servers ->
@modelcontextprotocol/server (+ server-legacy/sse). Deterministic import
rewrite (schemas -> core, everything else -> client); the codemod misroutes
client code to /server so it was not used for path routing.
- new Client(..., { versionNegotiation: { mode: 'legacy' } }) — nothing
2026-era goes on the wire.
Runtime (core/mcp/inspectorClient.ts)
- Error taxonomy: McpError -> ProtocolError, ErrorCode -> ProtocolErrorCode;
RequestTimeout -> SdkError/SdkErrorCode.
- Handlers take method strings; task methods (tasks/*, notifications/tasks/*)
use the 3-arg custom form (excluded from v2's spec-method set).
- Legacy tasks kept working (per project requirement): requestor tasks re-
implemented over raw client.request + explicit deprecated schemas;
experimental.tasks.callToolStream replaced by a local pollTaskToolCall
async generator (with _progressHandlers revival so task-execution progress
still reaches the caller). Receiver (server-initiated) tasks: the v2 Client
validates a sampling/elicit handler result and rejects the legacy { task }
response with -32602 — installReceiverTaskResponseBypass swaps the wrapped
entry in the private _requestHandlers map for the task-augmented branch so
{ task } reaches the wire (fixes server-initiated sampling-via-task).
- Pagination: list verbs use raw client.request({method}, List*ResultSchema)
for single-page behavior (v2's client.listX() auto-aggregate all pages).
Web / auth / apps
- Rename local JsonSchemaType -> InspectorFormSchema + add toFormSchema()
structural narrowing at the SDK-schema -> form boundary.
- ext-apps still peers on SDK v1: one compat boundary with documented casts
(remove when ext-apps#702 lands a v2 peer).
- OAuth provider saveClientInformation reconciled with v2's
OAuthClientInformationContext; InvalidClientMetadataError re-homed as a
local class (server-legacy/auth is not browser-safe).
- vitest.shared.mts dedupes @modelcontextprotocol/client+core to one copy so
vi.mock intercepts the SUT's import.
Accepted v2 behavior changes (per #1624 / spec §9.2): unknown-tool call
rejects with -32602; cancellation is stream-abort (no guaranteed
notifications/cancelled frame); output-schema strictness restored via the
Inspector's own Ajv check on the default path.
npm run ci green: validate (web 3087 / cli 212 / tui 274 / launcher 5),
coverage (>=90 per-file, all clients), smoke (launcher/cli/web), storybook
(410).
Closes #1624
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
Bob Dickinson
|
13dafe457b |
Guided-auth removal, OAuth smoke testing, and TUI/CLI support (#1531)
* 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> |
||
|
Cliff Hall
|
c96db0402e |
test: raise per-file coverage gate to 90 across all clients (closes #1271) (#1532)
* 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> |
||
|
Bob Dickinson
|
5de4e911e3 |
feat(web,core): enterprise-managed authorization (EMA) (#1509) (#1512)
Adds enterprise-managed authorization (EMA): browser-side IdP login + RFC 8693 token-exchange and JWT-bearer resource-token mint, per-server EMA opt-in in Server Settings, IdP config in Client Settings, and IdP clientSecret keychain storage. Includes the v2/main merge and review follow-ups (400 on malformed client config; normalizeIdpIssuer import consolidation). |
||
|
Cliff Hall
|
5ee347571f |
feat(web,core): persist & edit STDIO cwd/env in Server Settings (#1515) (#1518)
* feat(web,core): edit STDIO cwd/env in Server Settings (#1515) Persist and edit a STDIO server's working directory and environment variables through the Server Settings modal. - Add `env` (pair-array, mirrors headers) and `cwd?` to `InspectorServerSettings`. - `storedFieldsToInspectorSettings` lifts the stdio config's `env`/`cwd` into settings so the modal can show them; they stay on `config` for the transport. Add `envRecordToPairs` / `envPairsToRecord` helpers. - `env`/`cwd` remain config-owned on disk (the standard mcp.json location): the `/api/servers` PUT route writes an edited settings.env/cwd back onto the stored stdio config on a settings-only patch (supporting clear), and a config-providing PUT keeps config authoritative. `validateSettings` accepts `env`/`cwd`. - Server Settings modal: Working Directory field in Options and a dedicated collapsible Environment Variables section — both STDIO-only. - Registry server.json import sets no `cwd` (the schema has no working-directory concept); documented in serverJson.ts. Tests: serverList converter round-trip, route write-through (set/clear, config-precedence, non-stdio no-leak, validation), import preservation (client-config cwd, registry cwd skip), and form/modal component coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): preserve config env/cwd when a settings PUT omits them (#1515 review) Address Claude's review of #1518. - `/api/servers` PUT now distinguishes an *absent* `env`/`cwd` (preserve the stored value) from an explicit empty list / blank string (clear). Previously `validateSettings` coerced a missing `env` to `[]`, so an env-unaware caller patching only a timeout would silently wipe the server's env/cwd. The integrated web client always resends the full GET-rehydrated env, so it was unaffected — this protects the raw HTTP contract. - `validateSettings` reports `envProvided`/`cwdProvided`; the write-through only touches `config.env`/`cwd` for fields the caller actually sent. - On the omit path, rehydrate the untouched env's keychain values onto the rebuilt entry so the secret reconcile re-persists them rather than sweeping the blanked-on-disk keys out of the keychain. - Minor: guard the stdio cast in `mcpConfigToServerEntries` on the transport type instead of casting unconditionally. - Test: settings-only PUT omitting env/cwd preserves config keys + cwd + the keychain secret. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
bb32d1db5f |
fix(web): section-header hover outline + clear buttons out of tab order (#1487) (#1488)
* fix(web): section-header hover outline + clear buttons out of tab order (#1487) Two button-behavior adjustments: 1. History section headers (Pinned / History) now use the filter-toggle style — a thin outline on hover instead of a background fill — by pointing the `sectionHeader` Group variant at the shared `.filter-toggle` class. The open section keeps its filled background (passed per-instance via `bg`). 2. Every input clear button added in #1448 is removed from the keyboard tab order via `tabIndex={-1}` (CloseButton clear buttons app-wide, plus the Autocomplete clear button through `theme/Autocomplete.ts` clearButtonProps), so tabbing through a form moves to the next field, not onto the clear button. The buttons stay mouse-clickable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): apply filter-toggle hover outline to Resources accordion headers (#1487) The Resources screen `disclosure` accordion controls now share the same outline-on-hover treatment as the filter toggle buttons and History section headers: a thin border on hover instead of a background fill, plus a filled background when the section is open (keyed off `aria-expanded`). The `.filter- toggle` class is added to the disclosure control slot; the default-variant accordion (Server Settings) keeps its background-fill hover via a `:not(.filter- toggle)` guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(web): extract ClearButton element, replace 34 inline clear buttons Addresses the code-review suggestion on PR #1488: the same five-line `CloseButton` clear-affordance block (aria-label="Clear" + tabIndex={-1} + onClick) was hand-copied across 17 files. Extract a single `elements/ClearButton` (Mantine `CloseButton.withProps` with the aria-label and tabIndex baked in) and replace all 34 call sites, so the clear-button contract lives in one place. A global CloseButton theme default would be wrong here — it would also catch tabbable modal-close X buttons and the Select clear buttons — so a dedicated element is the right altitude. Net -151 lines. Adds element unit tests + a Storybook play test covering the aria-label / tabIndex / clickable contract for all sites at once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
53dc4549be |
fix(core): trace dropped fetch body update when entry rotates out (#1390) (#1480)
* fix(core): trace dropped fetch body update when entry rotates out (#1390) FetchRequestLogState.onFetchRequestBodyUpdate silently dropped a deferred response-body update when the matching entry had already been evicted by log rotation (>= maxFetchRequests newer requests appended between the append and the fire-and-forget body read). The user saw an empty Response Body with no way to distinguish "body never came back" from "body dropped after eviction". Plumb an optional pino logger into FetchRequestLogState (defaulting to the silent logger) and emit a logger.debug at the early-return site so the drop is traceable. The web client wires the existing remote logger from createWebEnvironment into the state manager. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(core): emit dropped-body trace at warn level so it is observable (#1390) Review feedback: the web remote logger (createRemoteLogger) defaults to level 'info' with no override at the environmentFactory call site, so a logger.debug at the drop site is filtered out by both the pino level gate and transmit.level — the diagnostic never reaches /api/log and the drop stays invisible in the shipped web app, defeating the issue's goal. Promote the trace to logger.warn (>= info, so it transmits by default). A response body lost to log rotation is unexpected data loss, not routine diagnostic noise, so warn is the appropriate severity. Update the comment to explain the level choice and switch the test stub from debug to warn, extracting a small typed makeWarnLogger helper per review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): configurable Network Log Size per server + body-dropped toast (#1390) Turn the otherwise-invisible "response body dropped after rotation" case into an actionable signal: 1. Per-server `maxFetchRequests` setting (default 1000, 0 = unlimited), editable as "Network Log Size" in the first (Options) accordion section of the Server Settings form. Plumbed through the full settings pipeline: InspectorServerSettings + StoredMCPServer types, the serverList disk converters (read defaults to 1000, write omits the default to keep diffs minimal — 0 is a meaningful non-default), and the node server's GET-sanitize + write-side validateSettings (non-negative number). Applied live to the active connection on settings-modal close via a new FetchRequestLogState.setMaxFetchRequests (shrinking trims immediately), and at connect time for the next session. 2. FetchRequestLogState now emits a `fetchRequestBodyDropped` event, but only for genuine rotation drops (log at capacity with a finite cap) — not for benign post-clear stragglers or unlimited logs, which previously also hit the warn path. App shows a deduped, persistent toast (stable per-server id, no auto-close) explaining what happened and the likely causes, with a link that opens this server's settings on the Options section (modal remounts via key) so the user can raise the limit if it's just a high-traffic server. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): don't treat a cleared Network Log Size field as unlimited (#1390) Review feedback: clearing the NumberInput emits "", and `parseInt("") || 0` coerced that to 0 (unlimited). Since the value applies live on modal close, a user who cleared the field to retype and then closed the modal would silently switch the log to unlimited. Fall back to the current value on an empty/NaN parse and reserve 0 for an explicit numeric entry. Adds a cleared-input test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
0890c7509a |
feat(web): add Mantine clear buttons to text inputs app-wide (#1333) (#1448)
* feat(web): add Mantine clear buttons to text inputs app-wide (#1333) Add a clear (×) affordance to every user-editable text input across the web client so a populated field can be reset in one click. - Autocomplete: enable `clearable` by default via the theme (`theme/Autocomplete.ts`) — covers the prompt-argument and resource-template variable autocomplete branches. - TextInput / Textarea: Mantine v8 does not expose a public `clearable` prop on these primitives (only the combobox-based components do), so add the clear button manually via `rightSection` + `CloseButton` (aria-label="Clear") shown only when the field has a value, wired to reset the field's controlled value to "". Applied to sidebar search inputs (prompts, tools, apps, tasks, history, resources, network, logs), prompt-argument and resource-template variable inputs, the sampling response/model fields, server config & settings forms, roots table, schema form, experimental features panel, and the server.json import panel. Read-only/display Textareas (OutputValidationModal, UrlElicitationErrorModal) are intentionally left untouched. Tests cover the clear behavior on representative inputs from each category (sidebar search, prompt argument, resource-template variable, sampling response, plus the config/settings forms). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): disable ServerConfigModal clear buttons while submitting Address review #2 on #1448: the clear CloseButton sat in `rightSection` with `rightSectionPointerEvents="auto"`, so it stayed clickable even while the field was `disabled={submitting}`, letting a user wipe a field mid-submit. Forward `disabled={submitting}` to each clear button so it locks together with its input. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(web): remove redundant Auto-respond from SamplingRequestPanel The sampling modal pre-loads its draft with the default stub result, and "Send Response" sends that draft — so on an untouched form "Auto-respond" did exactly the same thing as "Send Response", just with a fresh copy of the same defaults. Drop the redundant button and its `onAutoRespond` prop; "Send Response" now covers both the unedited (stub) and edited cases. The InlineSamplingRequest variant keeps its Auto-respond button — there it is the quick path versus "Edit & Send" (which opens this editable modal), not a duplicate of an adjacent send action. Updated PendingClientRequestModal wiring, stories, and tests accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
b0a024269e |
feat(web): list-changed handling — indicator, pull-on-demand default, and per-server auto-refresh opt-in (#1402) (#1443)
* feat(web): wire the listChanged indicator across all screens (#1402) The per-screen list-changed indicator was hardcoded false in InspectorView for Tools, Apps, Prompts, and Resources, so it could never light up. Source it from the managed-state layer instead. Each ManagedXState now tracks a listChanged flag: set true when its */list_changed notification arrives (the existing auto-refresh leaves it set), exposed via getListChanged()/a listChangedChange event, and cleared by clearListChanged() or on disconnect. The useManagedX hooks surface the flag and clear it on a user-initiated refresh (the indicator's Refresh button = acknowledge). App threads the three flags to InspectorView, which maps them to the screens — Apps shares the tools flag since apps are a filtered view of tools. Tests cover set/clear/disconnect at the state-manager layer, the hook subscription + refresh-clears behavior, and the InspectorView routing (including Apps sharing the tools flag and no cross-wiring between screens). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(core): close clear-after-await race in managed-list refresh (#1402 review) The useManagedX refresh() callbacks cleared the listChanged flag AFTER awaiting the fetch. If a */list_changed arrived mid-fetch, the state re-set the flag (and auto-refreshed), then the trailing clear wiped that genuinely-new signal. Clear BEFORE the await instead: the user's refresh acknowledges only the change in hand, and a mid-fetch change re-sets the flag and survives. Adds a race regression test on the tools hook. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): drop duplicate Refresh on the Apps screen (#1402) AppControls uniquely had a standalone toolbar Refresh button AND the ListChangedIndicator on a separate row, so once the indicator could actually appear (this PR), the Apps screen showed two Refresh buttons. Match the other three controls (Tools/Prompts/Resources): the indicator is the single header-right refresh affordance. Updates AppControls and AppsScreen tests to assert exactly one Refresh (and none when the list hasn't changed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(core): stop auto-invoking */list on list_changed; user pulls via Refresh (#1402) With auto-refresh on, the list was already current by the time the list-changed indicator appeared, making its Refresh button moot. Now the managed Tools/Prompts/Resources state managers only flag the change on */list_changed (they no longer auto-refresh); the list stays as-is until the user clicks Refresh, which fetches the new list and clears the indicator. The connect-time initial load is unchanged. Resource templates (no own indicator, part of the Resources screen) also stop auto-refreshing; onRefreshResources now refreshes both resources and templates, since a single resources/list_changed covers both. Tasks are left auto-refreshing — they have no list-changed indicator and need live status updates. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(core): correct ManagedResourceTemplatesState header comments (#1402 review) The class no longer subscribes to resourceTemplatesListChanged; update the stale file/class docstrings that still claimed it syncs on that notification to reflect connect + statusChange only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): add per-server 'Auto Refresh on List Changed Notifications' option (#1402) New Options section in ServerSettingsForm with a single checkbox. When checked, the managed Tools/Prompts/Resources (and resource templates) state auto-refreshes on */list_changed notifications; when unchecked (default) the notification only lights the list-changed indicator and the user pulls via Refresh. Plumbing: new optional autoRefreshOnListChanged on InspectorServerSettings and StoredMCPServer, round-tripped through serverList (lifted with a false default, persisted only when true, added to the exhaustive field-key map); surfaced from InspectorClient via a new getServerSettings() on the protocol; the state managers read it live at notification time to decide auto-refresh vs flag-only. Resource templates re-subscribe to their list_changed but only auto-refresh when the option is on (they have no indicator of their own). Tests cover the round-trip (true persists, false omitted), the form section (render/checked/toggle), and each state manager's on/off branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(server): persist autoRefreshOnListChanged through validateSettings (#1402) The PUT /api/servers write path runs settings through validateSettings, which rebuilds the value from explicitly-named fields and silently drops unknown keys — so autoRefreshOnListChanged never reached disk (the serverList round-trip test passed because it calls inspectorSettingsToStoredFields directly, bypassing this path). Validate it as an optional boolean and include it in the built value; false is omitted on disk by inspectorSettingsToStoredFields as before. Adds integration tests over the real PUT path: true persists, false is omitted, and a non-boolean is rejected (400). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): default the Server Settings modal to the Options section (#1402) Open 'Options' instead of 'Custom Headers' when the Server Settings modal first opens, so the new Auto Refresh toggle is the first thing surfaced. Updates the modal default-expanded section, the modal test's initial-state assertions, the three header tests (now expand 'Custom Headers' first since it's no longer open by default), and the story defaults. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: drop accidentally-committed Inspector history dump; gitignore the class 83076441 swept an exported Inspector message-history JSON (manual-testing artifact at the repo root) into the tree via git add -A. Untrack it and extend the existing repo-root debug-artifact .gitignore block (alongside /*.png) to cover /inspector-history-*.json so these can't be re-added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
49d6be6987 |
feat(web): add per-server Roots config + client roots capability (#1425) (#1431)
* feat(web): add per-server Roots config + client roots capability (#1425) Add MCP roots support to the v2 web Inspector as a per-server config section in the server settings form (no separate Roots tab/screen). - Persistence: add `roots: Root[]` to `InspectorServerSettings` and `StoredMCPServer`; thread through serverList converters (lift on read, drop empty-uri rows / blank names on write, omit when empty) and the remote server's read-side drop guard + `validateSettings`. - Capability: `setupClientForServer` always advertises the roots capability (`roots: cleanRoots(savedSettings?.roots ?? [])`) so the server can issue `roots/list` and receive `roots/list_changed`. - Live apply: edits to a connected server's roots are pushed via `inspectorClient.setRoots(...)` once, on settings-dialog close — diffed against the client's current roots — rather than per keystroke, to avoid flooding the server with `roots/list_changed` / `roots/list`. - Form: new Roots accordion section with a required URI and an optional name per row (unlike v1, which was URI-only), + Add Root, and remove. Tests cover the persistence round-trip (serverList + servers-route), the form/modal section, and reuse the existing InspectorClient roots/`setRoots` integration coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(roots): address review — dedup guards/cleanRoots, preserve extra fields, test live-apply Responding to the PR #1431 code review: - Dedup `isRootArray`: drop the copy inside `validateSettings` and reuse the module-scope guard the read path uses, so the two can't drift (core/mcp/remote/node/server.ts). - Dedup the clean-roots normalizer: hoist a single exported `cleanRoots` into core/mcp/serverList.ts and reuse it from both `inspectorSettingsToStoredFields` and App.tsx (drops the local copy). - Preserve non-form root fields (e.g. `_meta`): `cleanRoots` now spreads the carried-through fields and only normalizes uri/name, and the modal's `handleRootChange` spreads the existing root on edit — so a hand-edited root's `_meta` survives load + persist + edit instead of being silently dropped. - Test the App live-apply-on-close path: new App.test.tsx cases assert setRoots is called once with cleaned roots when changed on the active server, and not called when the cleaned roots already match what the client advertises. Adds a cleanRoots unit test (blank drop + _meta preserve). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
ed6eda7492 |
feat(web): wire up MCP task support — Tasks screen, Run-as-task, status toasts (#1422) (#1424)
Wires the ported Tasks screen + core task subsystem through App.tsx: sticky Clear Completed, cancel/refresh error toasts, core-side per-task progress correlation, Run-as-task on the Tools screen (gated on server support), per-server Task TTL, and live task-status toasts. |
||
|
Cliff Hall
|
85a4b4c057 |
feat(servers): persist per-server settings to mcp.json (#1352) (#1353)
* 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> |
||
|
Cliff Hall
|
75f74d64c1 |
test(coverage): bootstrap vitest unit infra and add tests at 90%+ per file (#1245) (#1269)
* test(coverage): bootstrap vitest unit infra and add tests at 90%+ per file (#1245) Establishes the unit-test runtime AGENTS.md mandates: a vitest "unit" project alongside the existing storybook project, RTL + jest-dom + happy-dom, a Mantine-aware render helper, and a 90%-line per-file coverage gate enforced by CI. Adds 66 test files covering all 62 components under elements/groups/screens/views, plus core/mcp and clients/web/src/utils. Total: 577 tests, 98.7% line coverage globally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(coverage): address PR #1269 review feedback - AGENTS.md: correct coverage gate description (lines ≥ 90, statements ≥ 85, functions ≥ 80, branches ≥ 50) with rationale for the relaxed branch threshold - InspectorView.test.tsx: document what the Math.random() mock values (0.1 / 0.99) control vs. STUB_SUCCESS_RATE in InspectorView.tsx - vite.config.ts / tsconfig.test.json: drop unused `globals: true` and the `vitest/globals` type now that every test file imports describe/it/expect/vi explicitly; manual `afterEach(cleanup)` in setup.ts remains required because testing-library auto-cleanup relies on globalThis.afterEach 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
|
1149288ea6 |
Refactor ServerCard stack to use id-based callbacks
Distinguishes server `id` (the unique map key in MCPConfig.mcpServers)
from `name` (display label) so containers can dispatch by id without
relying on display strings. ServerCard, ConnectionToggle, ServerListScreen,
and supporting stories all updated.
- ServerCard: add `id` prop; all action callbacks now `(id: string) => void`
- ConnectionToggle: collapse `onConnect`/`onDisconnect` into single `onToggle`
- ServerListScreen: lift `activeServer` to a prop; combine `onSetActive` +
`onToggleConnection` into a single `onToggleConnection(id)` callback
- Stories: use UUIDs for server ids so callbacks visibly carry ids, not names
Removes the "Direct"/"Via Proxy" connection mode concept, which is no
longer planned. ServerSettingsForm drops the section, ServerCard drops
the badge, InspectorServerSettings drops the field.
Removes the "Test Client Features" dropdown from ServerCard along with
its four handlers; that surface will live elsewhere.
TransportBadge now collapses `sse` and `streamable-http` into a single
"HTTP" label; ServerCard adds a grey transport descriptor next to the
badge ("Standard I/O", "SSE (Server Sent Events) [deprecated]",
"Streamable HTTP"). The version badge moves from the header row to sit
beside the transport badge.
ServerCard header no longer wraps; long server names truncate with
ellipsis.
Specification docs (`v2_scope.md`, `v2_screenshots.md`,
`v2_ux_interfaces.md`, `v2_ux_storybook.md`) updated to match.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
cliffhall
|
d27018b4f2 |
Change onOAuthChange to pass all OAuth fields together
Replace onOAuthChange(field, value) with onOAuthChange(oauth) where
oauth is { clientId, clientSecret, scopes }. Each keystroke now
reports the full OAuth state, not just the single field that changed.
This lets the consumer see the complete picture on every change.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
||
|
cliffhall
|
e04befa4dd |
Fix ServerSettingsForm stories to use useArgs for action logging
Use Storybook's useArgs hook from storybook/preview-api to update args from within the render function. This wires fn() callbacks to the Actions panel AND updates the UI state through Storybook's args system. All form interactions (add/remove/edit headers/metadata, accordion open/close, connection mode, timeouts, OAuth) now both update the UI and log to the Actions panel. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
cliffhall
|
a3be456f56 |
Remove unused fn() from ServerSettingsForm stories
The Storybook config has no addon-actions/addon-essentials, so fn() calls never produce visible output. The stories are fully interactive via useState — add/remove/edit headers, metadata, timeouts, OAuth, and connection mode all work visually in the UI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
cliffhall
|
66dcd82a42 |
Log actions alongside state updates in ServerSettingsForm stories
Each handler now both updates local state (so the UI reacts) and calls a Storybook fn() spy (so the Actions panel logs every callback with its arguments). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
cliffhall
|
39ed8b0fec |
Make ServerSettingsForm stories fully interactive
Replace fn() stubs with useState handlers so add/remove/edit for headers, metadata, connection mode, timeouts, and OAuth all work interactively in Storybook. The InteractiveForm wrapper manages settings state the same way ServerSettingsModal does. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
cliffhall
|
a6fc900271 |
Add ServerSettingsModal group, revert form to controlled component
Revert ServerSettingsForm to a controlled component with expandedSections/onExpandedSectionsChange props. The form is a dumb component — accordion state belongs to its container. Create ServerSettingsModal group that manages: - Accordion expand/collapse state with ListToggle expand-all button - Add/remove/edit header pairs - Add/remove/edit metadata pairs - Connection mode, timeout, and OAuth changes - All changes flow through a single onSettingsChange callback Update ServerSettingsForm stories with useState render functions so accordion sections are interactive. Update UnconnectedView WithSettingsModal story to use the new ServerSettingsModal component. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
cliffhall
|
adb909daf6 |
Internalize accordion state in ServerSettingsForm
Move expandedSections/onExpandedSectionsChange from props to internal useState. The accordion open/close is local display logic, not parent state. Add optional defaultExpandedSections prop for initial state. Simplify UnconnectedView WithSettingsModal story — no longer needs external state management for accordion sections. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
cliffhall
|
cc9be5e930 |
Address PR #1221 code review feedback
- Fix getCapabilityEntries to iterate fixed key sets so unsupported capabilities render red X (was only showing present/supported ones) - Merge duplicate @mantine/core import in ViewHeader - Move ConnectionState from ServerCard to core/mcp/types.ts - Narrow onConnectionModeChange to "proxy" | "direct" instead of string Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
cliffhall
|
771a67ee20 |
Phase 2: Refactor Server/Settings/ViewShell cluster to MCP SDK types
ServerCard: Replace flat scalar props (name, version, transport, status, error) with grouped objects (config: MCPServerConfig, info?: Implementation, connection: ConnectionState). Remove auto-connect useEffect — dumb components must not self-dispatch side effects. Derive transport and command/url from config. ServerInfoContent: Replace flat props with initializeResult: InitializeResult and clientCapabilities: ClientCapabilities from MCP SDK. Derive capability lists by iterating the ServerCapabilities record. ServerSettingsForm: Collapse flat setting props (connectionMode, headers, metadata, timeouts, oauth*) into settings: InspectorServerSettings. Add InspectorServerSettings type to core/mcp/types.ts. ImportServerJsonPanel: Replace scattered scalar props (jsonContent, selectedPackageIndex, serverName) with draft: InspectorServerJsonDraft. ViewHeader: Replace serverName: string with serverInfo: Implementation. Use ConnectionStatus from core types. ConnectedView: Update to pass serverInfo: Implementation instead of serverName. Use ConnectionStatus type. Update all stories for ServerCard, ServerInfoContent, ServerSettingsForm, ImportServerJsonPanel, ViewHeader, ServerListScreen, ConnectedView, and UnconnectedView. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
cliffhall
|
8483b98c37 |
Collapse ServerSettingsForm into accordion and add subtle ListToggle variant
Wraps every section of ServerSettingsForm (Connection Mode, Custom Headers, Request Metadata, Timeouts, OAuth Settings) in a controlled Mantine Accordion. The accordion is driven by new required `expandedSections` / `onExpandedSectionsChange` props so the parent owns expand/collapse state — consistent with the dumb-component principle. Adds a `variant?: "default" | "subtle"` prop to ListToggle. In subtle mode it renders as an ActionIcon with `variant="subtle"`, `color="gray"`, and a 16px icon so it visually matches Mantine's CloseButton in modal headers (transparent until hover). Adds a WithSettingsModal story on UnconnectedView that presents the form inside a Mantine Modal with a custom three-column header: subtle ListToggle on the left (expands/collapses all sections at once), "Server Settings" Title centered, CloseButton on the right. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
cliffhall
|
dd46acba87 |
Refactor renamed the component groups to something less obtuse:
Elements -> Groups -> Screens -> Views |
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
b25ece8ded | ||
|
|
da37692485 |
||
|
|
402d7b204d |
||
|
|
c51d6aabd2 |
||
|
|
fe295396af |
||
|
|
ec185f1053 |
||
|
|
b3b04c6939 |
||
|
|
e026051b47 |
||
|
|
7fecebed5e |
||
|
|
19a6df7ad3 | ||
|
|
13dafe457b |
||
|
|
c96db0402e |
||
|
|
5de4e911e3 |
||
|
|
5ee347571f |
||
|
|
bb32d1db5f |
||
|
|
53dc4549be |
||
|
|
0890c7509a |
||
|
|
b0a024269e |
||
|
|
49d6be6987 |
||
|
|
ed6eda7492 |
||
|
|
85a4b4c057 |
||
|
|
75f74d64c1 |
||
|
|
1149288ea6 | ||
|
|
d27018b4f2 | ||
|
|
e04befa4dd | ||
|
|
a3be456f56 | ||
|
|
66dcd82a42 | ||
|
|
39ed8b0fec | ||
|
|
a6fc900271 | ||
|
|
adb909daf6 | ||
|
|
cc9be5e930 | ||
|
|
771a67ee20 | ||
|
|
8483b98c37 | ||
|
|
dd46acba87 |