Uh oh!
There was an error while loading. Please reload this page.
resource refreshing - #9
Merged
Merged
Conversation
This was referenced Oct 8, 2024
Merged
Merged
Merged
Merged
Merged
jspahrsummers
approved these changes
Oct 9, 2024
cliffhall added a commit
that referenced
this pull request
May 19, 2026
…fecycle, dead code) - **#1 — Misleading `Record<string, never>` cast**: Imported `JsonValue` from `@inspector/core/mcp/index.js` and changed `args as Record<string, never>` to `args as Record<string, JsonValue>` in `onCallTool`. The new cast honestly narrows from the screen-level `Record<string, unknown>` to what `InspectorClient.callTool` actually requires. Added a comment explaining the boundary. - **#2 + #3 — Previous InspectorClient not disconnected on server switch or unmount**: Added a `useEffect` keyed on `inspectorClient` whose cleanup calls `inspectorClient.disconnect()`. One effect covers both the swap-on-switch leak (prior session's transport stayed open until GC) and the unmount-during-connected leak (HMR / tests). `setupClientForServer`'s existing state-manager `destroy()` calls handle the listener side; the new effect handles the transport side. - **#4 — Dead `void` block at bottom of file**: Deleted the trailing `void FetchRequestLogState; void StderrLogState;` block. The state managers are already constructed via `new FetchRequestLogState(client)` / `new StderrLogState(client)`, so the imports are referenced by the linter's reckoning. The `void` pair was dead code with a comment that no longer matched reality. - **#5 / #7 / #8 — Follow-up TODOs**: Added TODO refs to the new follow-up issues: - TODO(#1323): `error` event in InspectorClientEventMap so mid-session transport failures surface (connect-only catch is incomplete). - TODO(#1324): negotiated `protocolVersion` through `useInspectorClient` (currently hard-coded `"2025-06-18"`). - TODO(#1325): `useResourceSubscriptions` hook so subscribe/unsubscribe buttons reflect their server-side effect. Items #6 (useCallback dep churn), #9 (helper unit tests), and #10 (shared no-op bridge factory fixture) are acknowledged in the PR reply as deferred — no functional impact and out of scope for a "wire the hook layer" PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
cliffhall added a commit
that referenced
this pull request
May 19, 2026
…iew (#1322) * feat(web): wire App.tsx to v2 core/ hook layer; remove demo stub from InspectorView App.tsx mounts InspectorView with live data from the v2 hook layer instead of the title-page placeholder. InspectorView becomes pure prop-driven; the demo handshake stub and connection state machinery are removed. Connection lifecycle: - App.tsx lazily instantiates `InspectorClient` on the connect edge, rebuilding it (and its state managers) when the user picks a different server. State managers (Managed{Tools,Prompts,Resources,ResourceTemplates, RequestorTasks}State + MessageLog/FetchRequestLog/StderrLog) are destroyed on switch. - `useInspectorClient` + the per-primitive `useManaged*` hooks drive the view's data. `latencyMs` is captured at the connecting → connected edge via a ref so the intermediate rerenders don't lose the start timestamp. - `errorMessage` is captured from `client.connect()` rejection; status- machine-driven errors that don't surface through the promise are a known follow-up (no `error` event in the v2 InspectorClientEventMap yet). Action handlers: - `onCallTool` / `onGetPrompt` / `onReadResource` await the corresponding InspectorClient method and reflect pending → ok/error in `ToolCallState` / `GetPromptState` / `ReadResourceState` panel props. Manually verified end-to-end against `npx @modelcontextprotocol/server-filesystem` — the Tools result panel renders the real "Allowed directories: /private/tmp" output rather than a stub. - `onSetLogLevel` calls `client.setLoggingLevel(level)` and optimistically bumps `currentLogLevel` locally (the request has no echo notification). - `onSubscribeResource` / `onUnsubscribeResource` / `onCancelTask` route to their respective client methods. Server CRUD (`onServerAdd`/`onServerEdit` /…) and history pinning are intentionally `todoNoop` for now; the seed server list is hardcoded per the agreed scope (a `useServers` v2-only hook is a separate effort). - `notifications/message` notifications flowing through `MessageLogState` are filtered into `LogEntryData[]` and passed to the Logs screen, so `logging/setLevel` and the resulting log stream round-trip. InspectorView (`InspectorView.tsx`): - Removed: `STUB_*` constants, `handshakeTimer` ref, `handleToggleConnection` / `disconnect` functions, stub `bridgeFactory`/`sandboxPath`, internal connection state, `logLevel` state. - Added 38 props covering connection state, panel states, log level, and action callbacks. The view is pure presentational. - `availableTabs` is derived from `connectionStatus`; `activeTab` is clamped to whatever's currently available (avoiding the `set-state-in-effect` lint and keeping the previously-selected tab on reconnect). Stories + tests: - `InspectorView.stories.tsx` adds spy callbacks for every action prop and new `Connected` / `ConnectionError` stories for the connected/error narratives. - `InspectorView.test.tsx` is rewritten around the prop-driven contract (10 tests covering the empty list, server cards, toggle dispatch, connected header, error banner, tab snap-back on disconnect, app filtering, log-level dispatch, autoScroll local toggle). Dev backend port wiring (`vite.config.ts`): - Vite's dev server is now pinned to `CLIENT_PORT` (default 6274) with `strictPort: true`, matching the dev backend's `allowedOrigins`. Without this, Vite would fall back to 5173 while the Hono origin check still required 6274 — breaking every browser `/api/*` fetch out of the box. - Top-level `resolve.alias` now spreads the same bare-module aliases the vitest projects use. Rolldown can't reach `pino/browser.js` / `zustand/middleware` from `core/`'s parent (no node_modules there); the wired App pulls those modules into the browser dep graph for the first time, so the aliases need to be visible at build time too. createWebEnvironment helper (`src/lib/environmentFactory.ts`): - Mirrors v1.5's `lib/adapters/environmentFactory.ts`: builds `InspectorClientEnvironment` from `createRemoteTransport` + `createRemoteFetch` + `createRemoteLogger` + `BrowserOAuthStorage` + `BrowserNavigation`. Returns the assembled environment plus the logger for direct app-level use. Closes#1244. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(web): wire dev-backend auth token into createWebEnvironment + show full stdio command in ServerCard Two issues surfaced after the #1244 wiring landed locally: 1. **401 on `/api/mcp/connect`** when running `npm run dev` with the default auth (no `DANGEROUSLY_OMIT_AUTH`). The dev backend generates a random token and prints `http://localhost:6274?MCP_INSPECTOR_API_TOKEN=…` in the banner, but App.tsx was calling `createWebEnvironment(undefined, …)` so the browser never sent `x-mcp-remote-auth`. The Hono middleware correctly rejected with "Authentication required. Use the x-mcp-remote-auth header with Bearer token." Adds a small `getAuthToken()` helper in App.tsx that reads `MCP_INSPECTOR_API_TOKEN` off `window.location.search`, falls back to `sessionStorage`, and persists fresh values into sessionStorage so client-side navigation / OAuth round-trips don't drop the token. The resolved value is passed to `createWebEnvironment` whenever a new InspectorClient is built. 2. **Misleading ServerCard display** — the seed server's `config` is `{ command: "npx", args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"] }` but the card only rendered `"npx"`, so it looked like the command was incomplete. `getCommandOrUrl` now joins `command + args` for stdio configs, so the card shows the same string that gets spawned. SSE / streamable-http paths unchanged. Manual end-to-end re-verified against `npm run dev` (no DANGEROUSLY_OMIT_AUTH): - Banner prints the auth-token URL. - Browser navigates to that URL → `getAuthToken()` picks the token off the query string, persists to sessionStorage, threads it through to every `/api/*` request. - Server card shows `npx -y @modelcontextprotocol/server-filesystem /tmp`. - Toggle connect → connection succeeds; ViewHeader shows `Connected (~1.3s)`; Tools tab populates with the 14 filesystem tools. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(web): use dark-9 instead of blue-9 for dark-mode page background `main.tsx` overrode `--mantine-color-body` to `--mantine-color-blue-9` in dark mode, while `.storybook/preview.tsx` used `--mantine-color-dark-9`. The dev app then rendered with a blue background while every story (and the design intent visible in storybook) used dark grey. Switch main.tsx to `--mantine-color-dark-9` so the dev app matches the storybook reference. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(web): seed the Servers screen with the "everything" reference server Adds `@modelcontextprotocol/server-everything` to the hardcoded seed list alongside the filesystem server. The everything server exposes tools, prompts, resources, sampling, and completion — covering most of the surface a developer might want to exercise against the wired-up InspectorView without configuring anything. Comment updated to explain the two-seed shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(web): address #1244 PR review feedback (JsonValue cast, client lifecycle, dead code) - **#1 — Misleading `Record<string, never>` cast**: Imported `JsonValue` from `@inspector/core/mcp/index.js` and changed `args as Record<string, never>` to `args as Record<string, JsonValue>` in `onCallTool`. The new cast honestly narrows from the screen-level `Record<string, unknown>` to what `InspectorClient.callTool` actually requires. Added a comment explaining the boundary. - **#2 + #3 — Previous InspectorClient not disconnected on server switch or unmount**: Added a `useEffect` keyed on `inspectorClient` whose cleanup calls `inspectorClient.disconnect()`. One effect covers both the swap-on-switch leak (prior session's transport stayed open until GC) and the unmount-during-connected leak (HMR / tests). `setupClientForServer`'s existing state-manager `destroy()` calls handle the listener side; the new effect handles the transport side. - **#4 — Dead `void` block at bottom of file**: Deleted the trailing `void FetchRequestLogState; void StderrLogState;` block. The state managers are already constructed via `new FetchRequestLogState(client)` / `new StderrLogState(client)`, so the imports are referenced by the linter's reckoning. The `void` pair was dead code with a comment that no longer matched reality. - **#5 / #7 / #8 — Follow-up TODOs**: Added TODO refs to the new follow-up issues: - TODO(#1323): `error` event in InspectorClientEventMap so mid-session transport failures surface (connect-only catch is incomplete). - TODO(#1324): negotiated `protocolVersion` through `useInspectorClient` (currently hard-coded `"2025-06-18"`). - TODO(#1325): `useResourceSubscriptions` hook so subscribe/unsubscribe buttons reflect their server-side effect. Items #6 (useCallback dep churn), #9 (helper unit tests), and #10 (shared no-op bridge factory fixture) are acknowledged in the PR reply as deferred — no functional impact and out of scope for a "wire the hook layer" PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #8
Review in this order:
See #12 for demo video.