Commit graph inspector/core/mcp/state/fetchRequestLogState.ts
Author SHA1 Message Date
cliffhall
d57801d7b3 feat(core): report connection state on a request timeout (#2318)
A request timeout used to surface as a bare `Request timed out`. Every
fact needed to explain it was already in-process: what the client was
still waiting on, when it last heard back, and the state of the
standalone GET notification stream. The SDK's timeout now rejects with
all three, and the same snapshot is shown live in Connection Info and
on the Network entry for the stream.

- core/mcp/connectionDiagnostics.ts: the snapshot type, its one-line
  description, and `annotateRequestTimeout`, which rebuilds the SDK's
  per-request timeout with the state at that moment.
- InspectorClient decorates the SDK Client's `request` (the one funnel
  every verb goes through) so the annotation covers the whole surface;
  tracks outstanding requests with their send time, the last response,
  and the most recent long-lived transport stream; exposes
  `getConnectionDiagnostics()` and `connectionDiagnosticsChange`.
- fetchTracking watches a long-lived stream through a tee'd clone,
  counting SSE events and reporting the close, via a new
  `updateStream` callback wired through the node transport, the remote
  session (`fetch_stream_update`), and the browser transport.
- FetchRequestLogState patches `entry.stream`; the Network entry's
  placeholder reports the event count and open/closed state.
- Connection Info gains a Connection Activity section, ticking while
  open (`useTickingClock`).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APKSYHGxXBUpnR7ErFiLJn
Signed-off-by: cliffhall <cliff@futurescale.com>
2026-09-13 21:25:31 -04:00
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>
2026-07-19 01:06:31 -04:00
Bob Dickinson
2488ca5e38
feat: port CLI, TUI, and launcher from v1.5 into v2 (#1452)
Ports the CLI, TUI, and launcher from v1.5 into v2 behind a single
`mcp-inspector` entrypoint (launcher → web/cli/tui). Includes the
`mcp-inspector --web --dev` launcher fix (Vite alias resolution + prod-server
double-start), the root `postinstall` install cascade with simplified CI, and
spec/AGENTS docs (no-workspaces rationale, Development setup).

Closes #1246.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-14 20:27:49 -04:00
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>
2026-06-14 17:35:52 -04:00
Cliff Hall
5c5e5ab754
feat(network): show auth response bodies with masked secrets + reveal toggle (#1386) (#1387)
* feat(auth): inject MCP_INSPECTOR_API_TOKEN into served index.html (#1378)

Reloading the web client at the bare URL (no `?MCP_INSPECTOR_API_TOKEN=…`
query string) with empty sessionStorage made every `/api/*` request 401 —
the browser had no way to recover the backend's auth token.

Embed the token into `index.html` on every page load so the browser no
longer depends on the query string surviving navigation:

- New shared helper `clients/web/server/inject-auth-token.ts` embeds
  `<script>window.__INSPECTOR_API_TOKEN__ = "…"</script>` (escaped against
  `</script>` injection; no-op when auth is dangerously omitted).
- Dev: the Vite plugin injects via `transformIndexHtml`.
- Prod: the Hono server injects on the `/` route.
- `App.tsx` `getAuthToken()` now reads the injected global first, then the
  query string, then sessionStorage (both fallbacks preserved).
- Shared global name lives in `INSPECTOR_API_TOKEN_GLOBAL`
  (`core/mcp/remote/constants.ts`).

Tests: helper unit coverage + an integration test exercising the real
prod server's `/` → `/api/*` flow (injected token authenticates; missing
token 401s). AGENTS.md documents the token-recovery order.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(auth): wire OAuth authorization-code flow into App.tsx (#1379)

OAuth-protected MCP servers could not be connected to from the v2 web
client: the core OAuth pipeline exists, but App.tsx never invoked it, so a
connect attempt 401'd and surfaced "Remote send failed (401): … Missing
Authorization header" as a toast.

Wire the two missing entry points (all core primitives already in place):

- Auto-trigger on 401: in onToggleConnection's catch, detect an upstream
  401 (isUnauthorizedError) and call client.authenticate(), which runs
  discovery + DCR (backend-proxied) and redirects the page to the auth
  server via BrowserNavigation. The initiating server id is persisted to
  sessionStorage first, since the OAuth `state` carries only mode+authId
  and the full-page redirect wipes React state.
- /oauth/callback handler: a mount effect that, once `servers` hydrate,
  parses the callback params, recovers the pending server, rebuilds its
  InspectorClient, runs completeOAuthFlow(code) (PKCE verifier + DCR client
  info survive in BrowserOAuthStorage), replaceState("/") so a reload can't
  replay the single-use code, then connect(). An `error=` callback toasts
  instead of retrying.

connect() already attaches the OAuth provider to the transport
(inspectorClient.ts), so once tokens land in BrowserOAuthStorage the
outbound request carries the bearer token.

Extracted the pure pieces (constants + isUnauthorizedError) to
src/utils/oauthFlow.ts with unit tests. Verified end-to-end in a real
browser against the MCP SDK demo OAuth server: Connect -> redirect ->
auto-approve -> callback -> Connected, with the access token shown in the
Connection Info modal (#1377).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(auth): persist OAuth pre-redirect Network log across the redirect (#1384)

After #1379, the Network tab showed only the post-redirect auth HTTP
(discovery re-run + POST /token); the pre-redirect discovery and the DCR
POST /register that run during authenticate() were lost when the page
navigated to the auth server.

Root causes:

1. Ordering — BrowserNavigation set `window.location.href` before the
   client's `saveSession` event fired (OAuthManager calls onBeforeOAuthRedirect
   *after* auth() already navigated), so the save raced the unload and was
   dropped. Fix: BrowserNavigation now runs a synchronous `beforeNavigate`
   hook immediately before assigning location.href; App wires it through
   createWebEnvironment to flush the active fetch log to RemoteInspectorClient
   Storage (keyed by the authId parsed from the auth URL) via a keepalive POST
   that outlives the unload.

2. Illegal invocation — RemoteInspectorClientStorage defaulted to
   `this.fetchFn = globalThis.fetch` and called `this.fetchFn(...)`, which
   re-binds `this` and makes native fetch throw "Illegal invocation"
   (swallowed by the catch). This silently broke *all* session save/load.
   Fix: default to a wrapper that preserves the global receiver.

3. Restore race — hydrateFetchRequests replaced the list, so a load that
   resolved after the resuming connect appended live entries would clobber
   them. Fix: merge restored (older) entries ahead of live ones, dedupe by id.

saveSession also now uses keepalive: true.

Verified end-to-end against the MCP SDK demo OAuth server: the connected
page's Network tab shows the full handshake — pre-redirect discovery + DCR
/register plus post-redirect discovery + /token as `auth`, alongside
`transport`. Added unit tests for the beforeNavigate ordering and the
hydrate merge/dedupe.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(network): show auth response bodies with masked secrets + reveal toggle (#1386)

Auth-category Network entries showed request body/headers/status but never
the response body (rendered "(empty)"), because buildEffectiveAuthFetch
deliberately skipped capturing it to avoid surfacing access_token /
refresh_token. That hid the most useful thing to inspect when debugging
OAuth — the token exchange.

Capture auth response bodies, but mask sensitive OAuth fields by default
behind a click-to-reveal toggle so they aren't exposed at a glance during
a screen-share:

- inspectorClient: wire updateResponseBody on the auth fetcher.
- src/utils/maskSecrets.ts: maskSecretsInBody() masks access_token,
  refresh_token, id_token, client_secret (case-insensitive, nested) in JSON
  bodies; reports whether anything was masked. Non-JSON / secret-free bodies
  pass through untouched.
- NetworkEntry BodyPreview: when a body has masked fields, render it masked
  with a Reveal/Hide toggle (copy honors the shown view). Masking is a UI
  concern; the raw entry is unchanged so reveal shows the real value.

access_token / refresh_token live in the post-redirect /token response,
which is never written to the session-restore files (#1384); only
pre-redirect bodies (public discovery, DCR /register) persist, so no bearer
token hits disk.

Verified end-to-end: the /token response shows masked by default
(access_token: "••••••••"), Reveal exposes the raw value, and discovery /
the public DCR /register response (no secret) render with no toggle. Added
util + component unit tests and a story play function.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(auth): inject token into prod SPA fallback + prime sessionStorage + no-store (#1378)

Addresses code-review feedback on the token-injection PR:

- Prod `server.ts` SPA deep-link fallback (e.g. `/oauth/callback`) previously
  served the raw index.html off disk via serveStatic, bypassing injection — so
  a bookmark/reload at a non-`/` route with empty sessionStorage would 401.
  Route the SPA fallback through the same `serveIndexHtml` (inject) helper;
  real static assets (paths with a dot) still serve verbatim. Dev already
  injected on every HTML serve via Vite `transformIndexHtml`.
- `getAuthToken()` now persists the injected `window.__INSPECTOR_API_TOKEN__`
  to sessionStorage (not just the URL-param branch), priming the backstop for
  any later navigation that loses the global.
- Injected HTML responses now send `Cache-Control: no-store`, so a page
  carrying a token isn't cached and served stale after a restart regenerates
  the token.

Integration tests added: SPA fallback (`/oauth/callback`) carries the token,
`Cache-Control: no-store` on injected HTML, real assets served verbatim, and
unknown `/api` routes 404 rather than falling through to the HTML shell.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(auth): address #1383 review — split OAuth/connect toasts, tighten 401 match (#1379)

Code-review feedback on the OAuth-wiring PR:

- Callback effect: split completeOAuthFlow vs connect() into separate
  try/catch blocks. A token-exchange failure now reads "OAuth token exchange
  failed … Please try connecting again." (the single-use code is spent and the
  URL was cleared, so a reload can't retry); a post-OAuth connect failure reads
  "Failed to connect" since OAuth actually succeeded and re-clicking Connect
  reuses the persisted tokens.
- isUnauthorizedError: anchor the message fallback on the transport's
  `failed …(401)` wording instead of a bare `(401)`, so an unrelated `(401)`
  spliced into an error message can't trip the OAuth flow. Added a test.
- Documented that clearing the pending id + URL before the server lookup is
  intentional (deleted/renamed server mid-flow → require a fresh Connect).

Also merges the squash-merged #1382 base from v2/main.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(auth): address #1385 review — clarify double-save + keepalive cap + test fetch default (#1384)

Code-review feedback on the OAuth Network-log persistence PR:

- Documented the double-save: `FetchRequestLogState`'s `saveSession` listener
  is the backstop; `BrowserNavigation`'s `beforeNavigate` hook is the primary
  flush for the redirect case. Notes the listener may lose the navigation race
  and is harmless when it duplicates (last-writer-wins, identical payload).
- Reworded the keepalive comment in `RemoteInspectorClientStorage.saveSession`:
  the 64KB cap is general (the method is also reachable from the listener with
  the full session log), so a long session could exceed it and drop silently —
  acceptable since the persisted log is best-effort, not load-bearing.
- Added a regression test that constructs `RemoteInspectorClientStorage`
  without a `fetchFn`, stubs `globalThis.fetch`, and asserts the default
  wrapper calls it (locks in the "Illegal invocation" fix, which callers
  otherwise swallow).

Optional items (logger.warn on swallowed save errors; setupClientForServer dep
churn) acknowledged on the PR, not changed.

Also merges the squash-merged #1383 base from v2/main.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(network): address #1387 review — form-encoded masking, cleaner flag, a11y (#1386)

Code-review feedback on the auth-response-body masking PR:

- Extended masking to form-urlencoded bodies (finding #1): the token *request*
  is `application/x-www-form-urlencoded` and carries `code` / `code_verifier` /
  `client_secret` / `refresh_token`. `maskSecretsInBody` now masks those in
  form bodies too (preserving formatting; placeholder not percent-encoded).
  `code`/`code_verifier`/`client_assertion` are form-only sensitive keys —
  deliberately NOT masked in JSON, where `code` is usually an error/status code.
- Replaced the double-stringify `hasSecrets` heuristic with an explicit
  masked-flag propagated out of `maskNode` (finding #2) — robust if the
  transform ever grows non-identity behavior, and one fewer serialization.
- Reset reveal state on body change via `key={body}` remount instead of a
  setState-in-effect (finding #3; avoids the cascading-render lint rule).
- a11y (finding #4): `aria-label` on the Reveal/Hide button and `aria-live`
  on the hidden/revealed status text.
- Security tripwire comment near the `saveSession` listener (finding #6):
  captured auth bodies are unmasked at source (masking is UI-only), so any new
  post-token-exchange persistence path must redact first.

Tests: form-encoded masking (token + refresh requests), JSON `code` NOT masked,
non-object JSON pass-through, default-fetch wrapper. Story play + NetworkEntry
tests updated for the new aria-labels and the now-masked form request body.
Finding #5 (pretty-print asymmetry) is already handled by ContentViewer's JSON
formatting; #4-aria and #6 are the doc/a11y bits.

Also merges the squash-merged #1385 base from v2/main.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(network): address #1387 third-pass review — content-type masking, wholesale mask, dedup (#1386)

Third-pass code-review feedback on the auth-body masking PR:

- maskSecretsInBody now takes the body's content-type (finding #1): `*json*`
  → JSON masking, form-urlencoded → form masking, any other known type
  (HTML/plaintext/XML) → no masking; absent/unknown → sniff as before. Removes
  the implicit "non-JSON ⇒ form" guess for error pages etc. NetworkEntry passes
  the request/response `content-type` header through to BodyPreview.
- Mask any non-empty value under a sensitive key wholesale (finding #2):
  a non-standard object/array value under e.g. `access_token` is replaced
  rather than recursed into, so it can't leak. Empty-string still not flagged.
- Extracted `isSensitiveKey(set, key)` to dedupe the JSON/form key checks
  (finding #3).
- Reworded `MaskResult.masked` doc to cover the form (non-pretty-printed) case
  (finding #6).

Tests: non-string-value-under-sensitive-key wholesale mask; repeated form
param; empty form value not flagged; content-type honored (JSON type skips
form masking, text/plain skips masking, explicit form type masks).

Finding #5 (story re-hide) skipped per reviewer — unit test already covers
re-mask. Finding #4 (form edge cases) added.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(network): address #1387 fourth-pass nits — content-type contract, key, form test (#1386)

Fourth-pass review (LGTM) — minor, non-blocking items:

- Documented the content-type matching contract on `maskSecretsInBody`:
  substring match (`*json*` / `*x-www-form-urlencoded*`), and we trust the
  wire's own label (a mislabeled body renders raw — acceptable for the
  screen-share threat model) (finding #1).
- Key `<BodyPreview>` by content-type + body (not body alone) so a header-only
  change would also reset the reveal state (finding #3; not reachable today).
- Storybook `AuthSuccess`: assert revealed count `>=` reveal-button count to
  mirror the `hidden >= 2` check, so adding a non-masked body later can't drift
  the assertion silently (finding #4).
- Added a `NetworkEntry` unit test for form-encoded request-body masking
  (code/code_verifier) — previously exercised only via the Storybook play.

Finding #2 (silent decodeURIComponent fallback on malformed keys) acknowledged
as a known quirk outside the threat model; no change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* perf(network): address #1387 fifth-pass — memoize masking, mask DCR mgmt token (#1386)

Fifth-pass review (LGTM):

- Memoized `maskSecretsInBody` in `BodyPreview` (recommended item #1): a
  Reveal/Hide click no longer re-parses + re-walks the body; cost is once per
  mount, and the `key` remount re-runs it on body/content-type change. Guarded
  so a too-large body is never parsed (the hook runs unconditionally, with the
  size check inside the memo).
- Added `registration_access_token` (RFC 7592 DCR management credential, same
  bearer class) to the masked key set, and a confidential-client `/register`
  response fixture test (#5) asserting client_secret + registration_access_token
  masked, client_id/metadata visible.
- Tightened the `isMaskableValue` doc to state the exact contract: non-null,
  non-empty-string values are masked wholesale (#3).

Items #2 (transport parse cost — capped by the memo) and #4 (test-only
MASK_PLACEHOLDER export) left as-is per the review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(network): clarify body-update emits list event only (#1387 sixth pass) (#1386)

Sixth-pass review (LGTM, all nits). Added a comment on
`onFetchRequestBodyUpdate` noting it re-emits only `fetchRequestsChange`, not
the per-entry `fetchRequest` event — list-reading consumers pick up the body
on the next render; a future per-entry subscriber would need its own event.

Other items left as-is per the review: full-string `BodyPreview` key (#1,
micro-perf only if profiled), NaN under a sensitive key (#3, consistent with
the mask-non-null-non-empty contract), `&`-only form separator (#4, OAuth
never emits `;`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-05-31 08:52:13 -04:00
Cliff Hall
98192fd86e
fix(auth): persist OAuth pre-redirect Network log across the redirect (#1384) (#1385)
* feat(auth): inject MCP_INSPECTOR_API_TOKEN into served index.html (#1378)

Reloading the web client at the bare URL (no `?MCP_INSPECTOR_API_TOKEN=…`
query string) with empty sessionStorage made every `/api/*` request 401 —
the browser had no way to recover the backend's auth token.

Embed the token into `index.html` on every page load so the browser no
longer depends on the query string surviving navigation:

- New shared helper `clients/web/server/inject-auth-token.ts` embeds
  `<script>window.__INSPECTOR_API_TOKEN__ = "…"</script>` (escaped against
  `</script>` injection; no-op when auth is dangerously omitted).
- Dev: the Vite plugin injects via `transformIndexHtml`.
- Prod: the Hono server injects on the `/` route.
- `App.tsx` `getAuthToken()` now reads the injected global first, then the
  query string, then sessionStorage (both fallbacks preserved).
- Shared global name lives in `INSPECTOR_API_TOKEN_GLOBAL`
  (`core/mcp/remote/constants.ts`).

Tests: helper unit coverage + an integration test exercising the real
prod server's `/` → `/api/*` flow (injected token authenticates; missing
token 401s). AGENTS.md documents the token-recovery order.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(auth): wire OAuth authorization-code flow into App.tsx (#1379)

OAuth-protected MCP servers could not be connected to from the v2 web
client: the core OAuth pipeline exists, but App.tsx never invoked it, so a
connect attempt 401'd and surfaced "Remote send failed (401): … Missing
Authorization header" as a toast.

Wire the two missing entry points (all core primitives already in place):

- Auto-trigger on 401: in onToggleConnection's catch, detect an upstream
  401 (isUnauthorizedError) and call client.authenticate(), which runs
  discovery + DCR (backend-proxied) and redirects the page to the auth
  server via BrowserNavigation. The initiating server id is persisted to
  sessionStorage first, since the OAuth `state` carries only mode+authId
  and the full-page redirect wipes React state.
- /oauth/callback handler: a mount effect that, once `servers` hydrate,
  parses the callback params, recovers the pending server, rebuilds its
  InspectorClient, runs completeOAuthFlow(code) (PKCE verifier + DCR client
  info survive in BrowserOAuthStorage), replaceState("/") so a reload can't
  replay the single-use code, then connect(). An `error=` callback toasts
  instead of retrying.

connect() already attaches the OAuth provider to the transport
(inspectorClient.ts), so once tokens land in BrowserOAuthStorage the
outbound request carries the bearer token.

Extracted the pure pieces (constants + isUnauthorizedError) to
src/utils/oauthFlow.ts with unit tests. Verified end-to-end in a real
browser against the MCP SDK demo OAuth server: Connect -> redirect ->
auto-approve -> callback -> Connected, with the access token shown in the
Connection Info modal (#1377).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(auth): persist OAuth pre-redirect Network log across the redirect (#1384)

After #1379, the Network tab showed only the post-redirect auth HTTP
(discovery re-run + POST /token); the pre-redirect discovery and the DCR
POST /register that run during authenticate() were lost when the page
navigated to the auth server.

Root causes:

1. Ordering — BrowserNavigation set `window.location.href` before the
   client's `saveSession` event fired (OAuthManager calls onBeforeOAuthRedirect
   *after* auth() already navigated), so the save raced the unload and was
   dropped. Fix: BrowserNavigation now runs a synchronous `beforeNavigate`
   hook immediately before assigning location.href; App wires it through
   createWebEnvironment to flush the active fetch log to RemoteInspectorClient
   Storage (keyed by the authId parsed from the auth URL) via a keepalive POST
   that outlives the unload.

2. Illegal invocation — RemoteInspectorClientStorage defaulted to
   `this.fetchFn = globalThis.fetch` and called `this.fetchFn(...)`, which
   re-binds `this` and makes native fetch throw "Illegal invocation"
   (swallowed by the catch). This silently broke *all* session save/load.
   Fix: default to a wrapper that preserves the global receiver.

3. Restore race — hydrateFetchRequests replaced the list, so a load that
   resolved after the resuming connect appended live entries would clobber
   them. Fix: merge restored (older) entries ahead of live ones, dedupe by id.

saveSession also now uses keepalive: true.

Verified end-to-end against the MCP SDK demo OAuth server: the connected
page's Network tab shows the full handshake — pre-redirect discovery + DCR
/register plus post-redirect discovery + /token as `auth`, alongside
`transport`. Added unit tests for the beforeNavigate ordering and the
hydrate merge/dedupe.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(auth): inject token into prod SPA fallback + prime sessionStorage + no-store (#1378)

Addresses code-review feedback on the token-injection PR:

- Prod `server.ts` SPA deep-link fallback (e.g. `/oauth/callback`) previously
  served the raw index.html off disk via serveStatic, bypassing injection — so
  a bookmark/reload at a non-`/` route with empty sessionStorage would 401.
  Route the SPA fallback through the same `serveIndexHtml` (inject) helper;
  real static assets (paths with a dot) still serve verbatim. Dev already
  injected on every HTML serve via Vite `transformIndexHtml`.
- `getAuthToken()` now persists the injected `window.__INSPECTOR_API_TOKEN__`
  to sessionStorage (not just the URL-param branch), priming the backstop for
  any later navigation that loses the global.
- Injected HTML responses now send `Cache-Control: no-store`, so a page
  carrying a token isn't cached and served stale after a restart regenerates
  the token.

Integration tests added: SPA fallback (`/oauth/callback`) carries the token,
`Cache-Control: no-store` on injected HTML, real assets served verbatim, and
unknown `/api` routes 404 rather than falling through to the HTML shell.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(auth): address #1383 review — split OAuth/connect toasts, tighten 401 match (#1379)

Code-review feedback on the OAuth-wiring PR:

- Callback effect: split completeOAuthFlow vs connect() into separate
  try/catch blocks. A token-exchange failure now reads "OAuth token exchange
  failed … Please try connecting again." (the single-use code is spent and the
  URL was cleared, so a reload can't retry); a post-OAuth connect failure reads
  "Failed to connect" since OAuth actually succeeded and re-clicking Connect
  reuses the persisted tokens.
- isUnauthorizedError: anchor the message fallback on the transport's
  `failed …(401)` wording instead of a bare `(401)`, so an unrelated `(401)`
  spliced into an error message can't trip the OAuth flow. Added a test.
- Documented that clearing the pending id + URL before the server lookup is
  intentional (deleted/renamed server mid-flow → require a fresh Connect).

Also merges the squash-merged #1382 base from v2/main.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(auth): address #1385 review — clarify double-save + keepalive cap + test fetch default (#1384)

Code-review feedback on the OAuth Network-log persistence PR:

- Documented the double-save: `FetchRequestLogState`'s `saveSession` listener
  is the backstop; `BrowserNavigation`'s `beforeNavigate` hook is the primary
  flush for the redirect case. Notes the listener may lose the navigation race
  and is harmless when it duplicates (last-writer-wins, identical payload).
- Reworded the keepalive comment in `RemoteInspectorClientStorage.saveSession`:
  the 64KB cap is general (the method is also reachable from the listener with
  the full session log), so a long session could exceed it and drop silently —
  acceptable since the persisted log is best-effort, not load-bearing.
- Added a regression test that constructs `RemoteInspectorClientStorage`
  without a `fetchFn`, stubs `globalThis.fetch`, and asserts the default
  wrapper calls it (locks in the "Illegal invocation" fix, which callers
  otherwise swallow).

Optional items (logger.warn on swallowed save errors; setupClientForServer dep
churn) acknowledged on the PR, not changed.

Also merges the squash-merged #1383 base from v2/main.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-05-30 20:39:25 -04:00
Cliff Hall
4641339cc3
feat(network): add Network screen surfacing backend fetch log (#1355) (#1363)
* feat(network): add Network screen surfacing backend fetch log (#1355)

Routes `FetchRequestLogState` to a new Network tab so users can inspect
HTTP-layer request/response headers, status, duration, and category
(auth vs transport) of every fetch the backend makes for an active
session. Adds `NetworkControls` + `NetworkStreamPanel` + `NetworkEntry`
groups, a `monoBreak` Text theme variant for long header values, and
wires onClearNetwork / onExportNetwork (JSON download) from `App.tsx`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(network): include request / response bodies in search filter

Headers were already covered but bodies were not, so searches for
strings that live inside the JSON-RPC payload (e.g. "jsonrpc",
"initialize", a tool name) silently returned no matches.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(network): always render Response Body section, note streaming

Streaming responses (text/event-stream, application/x-ndjson, POST to
/mcp) are intentionally not body-captured by the fetch tracker, so the
section was silently omitted. Now whenever a response was received we
render the section, with either the body, a "(empty)" note, or a
"Streaming response — body not captured" note keyed off content-type.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(network): capture response bodies via async body-update events

Two related fixes:

1. The fetch tracker was marking every streamable-HTTP POST /mcp call as
   "not captured" because the response content-type is text/event-stream.
   In practice those streams are bounded — the server sends the
   JSON-RPC reply (sometimes preceded by progress events) and closes —
   so we can clone the body and read it. Only GET + SSE / ndjson is the
   unbounded long-lived channel that has to stay uncaptured.

2. Reading the body inline blocked the transport from processing
   progress notifications until the entire stream finished, breaking the
   `resetTimeoutOnProgress` integration test. The body is now read in
   the background and dispatched via a new `fetchRequestBodyUpdate`
   event that the FetchRequestLogState patches into the matching entry
   by id. Plumbing wires through node/transport, the remote SSE channel
   (RemoteSession, RemoteClientTransport), and the remote event types.

Also hides the Network tab when the active server is stdio (no HTTP
traffic to surface) and bumps the inline body preview limit to 100 KB
so typical tools/list responses render rather than tripping the "too
large" notice.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(network): show orange SSE badge for long-lived stream entries

Adds a small "SSE" badge to the right side of the NetworkEntry header
(left of the HTTP status badge) when the response is a long-lived
streaming channel (GET + text/event-stream or application/x-ndjson) —
the only case where the body genuinely isn't captured. Makes the
"not captured" placeholder less surprising at a glance.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(network): address PR review findings

- Stop capturing OAuth response bodies (drop updateResponseBody from the
  effectiveAuthFetch tracker). Restores pre-PR behavior for auth-category
  entries so live access_token / refresh_token values can't leak into the
  Network tab body preview during a screen-share. Headers + status are
  still tracked.
- Extract `isLongLivedStreamResponse(method, contentType)` to
  core/mcp/fetchTracking so the rule lives in one place; consumed by the
  tracker and the NetworkEntry UI placeholder.
- Document the intent of NetworkEntry's `useEffect` (the list-level
  toggle is authoritative; per-entry overrides are intentionally
  discarded on each list toggle).
- Rename MAX_INLINE_BODY_BYTES → MAX_INLINE_BODY_CHARS (`.length` is
  UTF-16 code units, not bytes) and update the user-facing notice.
- Switch NetworkStreamPanel's search from joined-string `.includes` to
  per-field `.some(.includes)` so a term can't span field boundaries.
  Regression test added.
- Add aria-label to ListToggle ("Expand all" / "Collapse all") — both an
  a11y improvement and a stable selector that lets the panel test stop
  fishing the button out by text-content negation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(network): use ListToggle aria-label in panel + modal tests

Cleanup spotted in PR review follow-up:

- HistoryListPanel + ServerSettingsModal previously fished the ListToggle
  out by empty text content because the toggle had no accessible name.
  Now that ListToggle exposes "Expand all" / "Collapse all", both tests
  can use getByRole + name and the stale comments go away.
- ListToggle's own tests now assert the aria-label contract so a future
  refactor can't silently drop it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(network): expose category toggle state via aria-pressed

The category UnstyledButtons in NetworkControls previously communicated
their toggled state only via background color. Adds aria-pressed={active}
so assistive tech can discover the on/off state without visuals, with a
test that asserts the attribute tracks the visibleCategories prop.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-27 08:35:25 -04:00
Cliff Hall
e3e70470b5
feat(core): port v1.5 hook layer (#1243) (#1300)
* feat(core): port v1.5 hook layer (useInspectorClient + useManaged*/usePaged* + log hooks)

Wholesale port of v1.5's React hook layer and the EventTarget-based state machinery the hooks subscribe to (#1243). Hooks decouple from the concrete InspectorClient via a new `InspectorClientProtocol` interface so this layer can land before the runtime class is ported.

Added under `core/`:
- `core/react/` — useInspectorClient + 5 useManaged* + 5 usePaged* + 3 log hooks
- `core/mcp/state/` — 5 managed*State + 5 paged*State + messageLogState + stderrLogState + fetchRequestLogState
- `core/mcp/inspectorClientProtocol.ts`, `inspectorClientEventTarget.ts`, `typedEventTarget.ts`, `samplingCreateMessage.ts`, `sessionStorage.ts`
- v2-only wrapper types in `core/mcp/types.ts`: InspectorUrlElicitRequest, InspectorPendingRequest, InspectorRequestHistoryItem

Tests live under `clients/web/src/test/core/` (28 new test files, 264 new tests). Test fake: `core/mcp/__tests__/fakeInspectorClient.ts`.

Build wiring:
- `vite.config.ts`: dedupe react/react-dom so core/ files resolve them via clients/web/node_modules.
- `tsconfig.app.json` / `tsconfig.test.json`: paths map react/react-dom/vitest to clients/web/node_modules so `tsc -b` resolves bare imports from core/ source files (which sit above the package's node_modules). Exclude core/__tests__ from app config; include in test config.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* review: address PR #1300 review findings

Bugs:
- fetchRequestLogState: add .catch() on sessionStorage.loadSession so a corrupt
  or unreadable session doesn't surface as an unhandled rejection (matches the
  saveSession swallow pattern).
- messageLogState: sync pendingRequestEntries when clearMessages() evicts a
  pending request entry, so a later response with the same id falls through
  to the append branch instead of mutating an unreachable map entry. Add tests
  for both predicate and no-predicate paths.

Cleanups:
- Extract mergeTaskIntoList helper to core/mcp/state/mergeTaskIntoList.ts so
  managed/paged RequestorTasks state classes share one definition.
- Add a one-line JSDoc note on each PagedTools/Prompts/Resources/Templates
  state explaining why they deliberately do not subscribe to *ListChanged
  (cursor invalidation mid-pagination).
- Document on useInspectorClient that appRendererClient is read lazily on
  every render and is not subscribed (changes once at connect time).

Carry-overs from v1.5 deliberately left for the follow-up InspectorClient
runtime port:
- Refresh/status-change race in Managed*State.refresh() (status check is only
  at entry; a disconnect during await can re-clear before refresh resumes).
- nextCursor exposure asymmetry across usePaged* (only usePagedRequestorTasks
  surfaces it via state field; others return it from loadPage() only).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-13 17:43:07 -04:00