Wire App.tsx to v2 core/ hook layer; remove demo stub from InspectorView

[cliffhall]

Summary

• App.tsx now mounts InspectorView with live data from the v2 hook layer (useInspectorClient, useManaged{Tools,Prompts,Resources,ResourceTemplates,RequestorTasks}, useMessageLog). Lazy-instantiates InspectorClient on connect via a new createWebEnvironment helper; state managers are destroyed on server switch.
• InspectorView becomes pure prop-driven: all STUB_* / handshakeTimer / handle{Toggle,Disconnect} machinery removed. Connection state (activeServer, connectionStatus, initializeResult, latencyMs, errorMessage) and panel states (ToolCallState / GetPromptState / ReadResourceState) flow in as props.
• Action handlers (onCallTool, onGetPrompt, onReadResource, onSetLogLevel, onSubscribeResource, onUnsubscribeResource, onCancelTask, onRefresh*) route to InspectorClient methods.
• Seed server list with one hardcoded npx @modelcontextprotocol/server-filesystem /tmp entry per the agreed scope; useServers-style persistence is a separate effort.
• vite.config.ts pins the dev server to CLIENT_PORT (default 6274) with strictPort: true so it matches the dev backend's allowedOrigins. Top-level resolve.alias now spreads the same bare-module aliases the vitest projects use (the wired App pulls pino/browser.js, zustand/middleware etc. into the browser dep graph for the first time).

Manual end-to-end verification

Driven via Playwright against npx @modelcontextprotocol/server-filesystem /tmp:

1. ✅ npm run dev → browser opens at http://localhost:6274/, InspectorView renders with the seed server card.
2. ✅ Click the connection toggle → secure-filesystem-server identified, ViewHeader shows Connected (~1.3s).
3. ✅ Switch to Tools tab → 14 filesystem tools loaded (read_file, list_directory, list_allowed_directories, …).
4. ✅ Select list_allowed_directories and Execute Tool → result panel shows the real output: Allowed directories: /private/tmp (no stub).

What's intentionally noop in this PR

Server CRUD (onServerAdd / onServerEdit / etc.), history pinning, app sandbox round-trip, log export — these are wired as todoNoop callbacks. Each requires additional UI/state that's separate from "wire the hook layer." The prop interface is in place for them.

Test plan

• npm run validate — 1489 / 1489 (lint + per-file coverage gate clean)
• npm run test:integration — 379 / 379
• npm run test:storybook — 300 / 300
• Manual: connect to npx @modelcontextprotocol/server-filesystem /tmp end-to-end (Playwright-driven, captured above)
• Manual: logging/setLevel round-trip — defer to a server that emits log notifications (filesystem-server only emits "Method not found" for logging/setLevel; the wiring works but there's nothing visible to verify against)
• Manual: resources/read round-trip — same caveat; filesystem-server has tools but no resources catalog

Known limitations

• Tab snap-back on disconnect now uses a clamp pattern instead of an effect; if the user disconnects while on Tools and reconnects, they land back on Tools (previously: reset to Servers). Trade-off chosen to avoid set-state-in-effect. Reasonable UX either way.
• connect() errors are captured from the Promise rejection only — the InspectorClient state machine can transition status → error without a Promise rejection (e.g. transport dies mid-session) and we'd miss the message. Follow-up needs an error event in InspectorClientEventMap.

Closes #1244.

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 32s —— View job

---

Claude PR Review

• Fetch PR diff against origin/v2/main
• Read changed files
• Verify concerns against core types (InspectorClient.callTool, MessageLogState, state-manager lifecycles)
• Post review

---

Overall this is clean, well-commented, and the scope (pure prop-driven view + parent-owned orchestration) is exactly right. The manual e2e capture is convincing and the storybook/integration suites pass. A few things worth flagging before merge — most are minor; one is a latent resource issue that'll bite as soon as server CRUD lands.

High-signal

1. Misleading Record<string, never> cast — clients/web/src/App.tsx:347-350

const invocation = await inspectorClient.callTool(
  tool,
  args as Record<string, never>,
);

InspectorClient.callTool actually takes Record<string, JsonValue> (core/mcp/inspectorClient.ts:1217). Casting through Record<string, never> compiles only because never is assignable to JsonValue, but it's a lie — it claims the object is empty. A future reader hitting this will not understand it. Two cleaner options:

• Tighten the prop type: onCallTool: (name: string, args: Record<string, JsonValue>) => void on InspectorViewProps, then drop the cast entirely, or
• Cast honestly: args as Record<string, JsonValue> (import JsonValue from @inspector/core/json/index.js).

Fix this →

2. Previous InspectorClient is not disconnected on server switch — App.tsx:238-290

setupClientForServer destroys the eight state managers (unsubscribing their listeners from the old client), but it never calls this.inspectorClient?.disconnect() on the outgoing client before replacing it. For a seed list of one, this is unreachable today — but the moment onServerAdd/edit lands and the user flips between servers while the first is still connected, the prior client's transport stays open (stdio subprocess, SSE stream, or HTTP session) until GC eventually lets go. InspectorClient has no destroy() — disconnect() is the lifecycle hook (it closes the transport, clears resource subscriptions, cancels receiver TTLs).

Recommendation: at the top of setupClientForServer, if inspectorClient && id !== activeServerId, await inspectorClient.disconnect() before creating the new client. Since the callback would then need to be async, easier is to do the disconnect inside onToggleConnection just before invoking setupClientForServer.

3. No unmount cleanup

App unmounting while connected leaves the transport open — no useEffect(() => () => { client?.disconnect(); managers.forEach(m => m?.destroy()); }, [...]). In a SPA this almost never fires in production, but it's the kind of thing that bites under HMR or during tests. One effect keyed on [inspectorClient] returning a cleanup closure would cover it (and would also subsume the switch-server teardown from #2).

Medium-signal

4. Dead state: fetchRequestLogState / stderrLogState — App.tsx:135-139, 275-276, 553-558

These two state managers are instantiated and destroyed but never read. The trailing void FetchRequestLogState; void StderrLogState; pair is confusing: the new FetchRequestLogState(client) call at 275 already keeps the import "used" from the linter's perspective, so the void expressions aren't doing what the comment claims. Pick one:

• Keep the instances (they subscribe to client events, so the side effect is real), and drop the bottom-of-file void pair — it's dead.
• Or defer the instantiation until Apps tab — AppRenderer with imperative ref #1262 actually consumes the state, and remove the three (state, setter, destroy call) along with the imports.

5. connect() error path — App.tsx:316-322

Already called out in the PR description as a known limitation. The Promise rejection path captures failures during the initial handshake, but a transport dying mid-session transitions the client's status to "error" without rejecting any pending promise, and errorMessage stays stale. The right shape is an error event on InspectorClientEventMap (which the PR body acknowledges). Worth opening a follow-up issue and linking it in a // TODO(#xxxx) next to the catch block so the gap doesn't get forgotten.

6. useCallback dep churn on setupClientForServer

The callback depends on all eight state managers, each of which changes twice per switch. That means the callback identity changes ~16 times during a single server flip, and every handler closing over it (onToggleConnection) rebuilds too. No user-visible bug because these closures are only invoked imperatively from clicks, but it's unnecessary churn. The teardown reads each manager's current value only once during the call — refs (or reading from a single managersRef.current object) would stabilize the callback identity.

Low-signal / nits

7. InitializeResult.protocolVersion hardcoded — App.tsx:221

Comment acknowledges it; fine for now. Consider adding protocolVersion to the useInspectorClient return so the real negotiated value flows through. Worth a separate issue.

8. subscriptions={[]} — App.tsx:492

The Resources screen will always render "no subscriptions" even after subscribeToResource() succeeds, because the live subscription list on InspectorClient (subscribedResources) isn't surfaced as a hook yet. Scoped out per PR body, but it makes the subscribe/unsubscribe buttons feel no-op from the user's perspective. Worth an issue — useManagedSubscriptions (or adding it to useInspectorClient) would complete the loop.

9. messagesToLogEntries could be unit-tested directly — App.tsx:82-97

Small pure helper; one or two tests covering (a) non-notification skip, (b) wrong-method skip, (c) correct extraction would keep per-file coverage safe if more log-entry sources land later. No blocker.

10. stubBridgeFactory duplicated in three places

The same as unknown as AppBridge no-op factory lives in App.tsx, InspectorView.test.tsx, and InspectorView.stories.tsx. Minor; extracting to src/test/fixtures/noopBridgeFactory.ts (or similar) would keep them in sync. Not a blocker.

Things that look good

• The seed-server pattern is appropriately scoped and documented. The "useServers is a separate effort" callout in the comment prevents scope creep.
• Splitting bare-module aliases into sharedAliases + nodeModulesAliases and reusing across top-level resolve and per-project configs is exactly right — the comment on line 123-128 explains the underlying rolldown-resolution issue clearly.
• strictPort: true + CLIENT_PORT pin on the Vite dev server is a real-world bug prevented before it happens; the comment naming allowedOrigins and buildWebServerConfigFromEnv() is what makes the change auditable.
• The activeTab clamp pattern (pure derived value vs. effect that resets state) is the right call — avoids the set-state-in-effect anti-pattern cleanly. Snap-back UX trade-off noted in the PR body is defensible.
• Storybook coverage for Default / NoServers / Connected / ConnectionError exercises the four meaningful branches of the header rendering.

Recommendation

Happy for this to merge once #1 (the Record<string, never> cast) and #2 (disconnect the previous client) are addressed — both are small fixes. Everything else is follow-up material.
· Branch: 1244-wire-app-tsx