feat(connection-info): wire Connection Info modal to the connection handshake

[cliffhall]

Summary

• Add ConnectionInfoModal wrapping the existing presentational ConnectionInfoContent, mirroring ServerSettingsModal's layout (Modal + Mantine Title + CloseButton). Title: Connection Info.
• Gate the Connection Info button on ServerCard so it only renders when connection.status === "connected"; Settings continues to render in all states.
• Expose getClientCapabilities() on InspectorClient (snapshotted from the initialize-time capability build), thread it through InspectorClientProtocol, FakeInspectorClient, and useInspectorClient, then render it in the modal alongside the live initializeResult, transport, and OAuth details.
• In App.tsx, derive the active server's transport from ServerEntry.config.type and OAuth details synchronously from inspectorClient.getOAuthState() (auth URL, access token) and persisted settings.oauthScopes. Drop the modal-open flag on the InspectorClient disconnect event so a future reconnect doesn't auto-reopen the modal.
• Render server instructions through ContentViewer (copyable + wrapping Code variant), wrapped in ScrollArea.Autosize mah={280} so a long instructions payload scrolls in place instead of pushing OAuth + modal chrome off-screen.
• Add a Server Implementation heading above the name / version / protocol / transport grid to balance the existing capability section headings.

Rename: Server Info → Connection Info

The popup covers server identity AND client capabilities AND OAuth state — it's a snapshot of the live connection, not just the server. To match what it actually shows:

• ServerInfoContent → ConnectionInfoContent (directory, file, exports)
• ServerInfoModal → ConnectionInfoModal (directory, file, exports)
• ServerCard button label "Server Info" → "Connection Info"; prop onServerInfo → onConnectionInfo
• Modal title "Server Info" → "Connection Info"
• Wiring updated through App.tsx, ServerListScreen, InspectorView

Connection errors → toast (not InlineError on the ServerCard)

While testing OAuth-backed connections, the inline error on the ServerCard either flashed for a moment then vanished (the active card flips through error → disconnected → error during a failing handshake) or required a 5s visibility contract that fought InspectorView's status pipeline. Replaced it with a Mantine notifications.show({ title: \Failed to connect to "${name}"`, message, color: "red" })` call in App.tsx's connect-catch path:

• App.tsx: toast on the connect-catch path; drop errorMessage state + the in-flight ref that was only there to keep the alert visible past the transport's close event.
• InspectorView: drop the errorMessage prop and the connectionStatus === "error" && errorMessage mapping branch.
• ServerCard: drop the InlineError rendering + visibility state machine; the card is back to a pure status indicator.
• InlineError deleted (no other consumers).
• main.tsx: anchor <Notifications> at bottom-right so toasts don't overlap the ViewHeader/tabs row.

Follow-up issue (#1378) tracks the related "missing auth token → silent 401" dev-UX problem: the only way the browser learns MCP_INSPECTOR_API_TOKEN today is via the URL the launcher prints, so a reload at the bare URL still 401s.

Closes #1364

Screenshot

Test plan

• npm run validate (format, lint, build, unit + integration with coverage gate)
• npm run test:storybook
• Manual: connect a stdio server → Connection Info button visible, opens modal with serverInfo/capabilities/protocol/instructions/transport → close via X/Escape/click-outside
• Manual: disconnected/connecting/error states → Connection Info button hidden, Settings still present
• Manual: OAuth-backed HTTP server → OAuth section populated with auth URL / scopes / access token; failed handshake fires a bottom-right toast with the underlying error
• Manual: server with a very long instructions payload → instructions section scrolls in place; OAuth section + close button stay visible

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

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

---

Claude PR Review

• Read PR diff against origin/v2/main
• Review ConnectionInfoModal and updated ConnectionInfoContent
• Review App.tsx wiring (OAuth state, transport derivation, disconnect handling)
• Review InspectorClient / protocol / fake changes (getClientCapabilities)
• Review ServerCard gating + tests
• Check theme + favicon/index.html changes
• Post final review

Summary

Solid PR. The rename is mechanically clean and consistent across files, tests, stories, and props. The new getClientCapabilities() snapshot pattern is well-documented at every layer (InspectorClient, protocol, hook), and the disconnect handler that clears connectionInfoModalOpen is exactly the right fix for the "modal re-pops on reconnect" footgun. The ScrollArea.Autosize mah={280} around instructions, plus the new Title order={4} modal header + Title order={5} section headings, give the modal good vertical discipline. Tests cover the new gating, hook field, and modal open/close paths.

A few small things worth a look before merging:

Findings

1. clientCapabilities ?? {} returns a fresh {} on every render — core/react/useInspectorClient.ts:124

clientCapabilities: inspectorClient?.getClientCapabilities() ?? {},

The comment says "consumers can treat this as a stable object," but the fallback {} is a new literal each render when no client is attached. That can invalidate downstream useMemo/useEffect deps that key on clientCapabilities. Hoist a module-scope constant:

const EMPTY_CLIENT_CAPABILITIES: ClientCapabilities = Object.freeze({});
// …
clientCapabilities: inspectorClient?.getClientCapabilities() ?? EMPTY_CLIENT_CAPABILITIES,

Fix this →

2. connectionInfoOAuth is memoized over inspectorClient but reads mutable OAuth state — clients/web/src/App.tsx:363-381

The useMemo deps are [connectionStatus, inspectorClient, activeServer]. None of those flip during an OAuth state transition mid-session (token refresh, scope change, late oauthComplete). If the modal is opened during a refresh, the displayed access token / auth URL will be stale until the next reconnect. Three options, in order of how invasive they are:

• Tolerate it (snapshot-at-open semantics, which the modal already implies).
• Compute inline (no useMemo) — cheap enough.
• Subscribe to oauthStepChange / oauthComplete and re-derive.

If you intend snapshot semantics, the comment above this block should say so explicitly so a future reader doesn't try to "fix" the staleness.

3. Modal close animation is short-circuited by the outer {initializeResult && …} guard — clients/web/src/App.tsx:1010-1019

On disconnect, initializeResult becomes undefined and the modal is unmounted from the tree the same render as setConnectionInfoModalOpen(false), so Mantine's exit animation never plays. Mantine's <Modal opened={…}> is designed for the parent to keep it mounted and just toggle opened. Low-priority polish; if the snap-shut is intentional, ignore.

4. connectionInfoTransport falls back to "stdio" for a missing active server — clients/web/src/App.tsx:353-356

const connectionInfoTransport = useMemo(
  () => activeServer?.config.type ?? "stdio",
  [activeServer],
);

The modal is only rendered when initializeResult is truthy (which requires connectionStatus === "connected"), so activeServer should always be present here. The silent "stdio" default would mask a regression if that invariant ever breaks. Consider making transport optional on ConnectionInfoContentProps and rendering "—" when absent, or console.warn + omit the row.

5. Auth URL preference: metadata endpoint vs. constructed URL — clients/web/src/App.tsx:366-368

const authUrl =
  oauthState?.oauthMetadata?.authorization_endpoint ??
  oauthState?.authorizationUrl?.toString();

These are semantically different. authorization_endpoint is the AS's advertised endpoint; authorizationUrl is the full URL the user was redirected to (with state, code_challenge, etc.). For a "Connection Info" snapshot, the constructed URL is arguably more informative about what actually happened. Worth confirming which one matches user expectations; either is defensible.

6. Test uses raw class selector for the close button — clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.test.tsx:80-83

const closeBtn = document.querySelector(
  "button.mantine-CloseButton-root",
) as HTMLButtonElement | null;

Couples the test to Mantine's internal class naming and uses a non-null assertion (!). screen.getByRole("button", { name: /close/i }) or screen.getByLabelText(/close/i) would survive a Mantine upgrade.

7. Scope splitter is broader than OAuth 2.1's SP separator — clients/web/src/App.tsx:370-372

const scopes = activeServer?.settings?.oauthScopes?.split(/\s+/).filter(Boolean);

OAuth 2.1 §3.3 specifies scopes are space-separated (%x20). /\s+/ also matches tabs / newlines. Harmless in practice (the input is config-controlled), just noting in case you want to be strict on the input.

Nits

• ConnectionInfoContent.tsx: SERVER_CAPABILITY_KEYS and CLIENT_CAPABILITY_KEYS are identical to the pre-rename file — fine, just confirming nothing was lost in the move.
• Capability check key in capabilities && capabilities[key] != null correctly distinguishes "absent" from "present-but-empty-object," which matches MCP spec semantics where {} means "supported, no sub-features."

What looks great

• The disconnect handler clearing setConnectionInfoModalOpen(false) with the comment explaining the re-pop scenario.
• The getClientCapabilities() JSDoc note that it's a snapshot, not a subscription on the SDK's mutable registerCapabilities().
• Parameterized "omits Connection Info button when status is X" test in ServerCard.test.tsx:171-186.
• Storybook coverage of MinimalCapabilities + WithOAuth + Closed states.
  · Branch: feat/1364-server-info-modal

[cliffhall]

Thanks for the review. Addressed in 05634b6a:

1. clientCapabilities ?? {} stability — ✅ Hoisted EMPTY_CLIENT_CAPABILITIES = Object.freeze({}) to module scope in core/react/useInspectorClient.ts. Fallback now returns a stable identity.

2. connectionInfoOAuth snapshot semantics — Tolerating it. Expanded the memo comment to say so explicitly: the modal is framed as "info about the connection at this moment," and subscribing a read-only dialog to oauthStepChange/oauthComplete is extra wiring for marginal benefit. If we later surface live token refresh state, we'll switch to subscribing on those events.

3. Modal close animation — Intentional. The {initializeResult && activeServer && …} guard snap-shuts the modal on disconnect because a stale dialog rendering OAuth/serverInfo/capabilities for a no-longer-live client is more confusing than a missed exit animation. Not changing this.

4. connectionInfoTransport "stdio" fallback — Tightened. Added activeServer to the modal-render guard ({initializeResult && activeServer && <ConnectionInfoModal … />}), so the only path that needs the ?? "stdio" default is the type narrower itself (config.type is schema-optional). Updated the comment to explain why the default still has to materialize here (the modal prop is a required ServerType).

5. Auth URL preference (metadata endpoint vs. constructed URL) — Kept the metadata endpoint first. Added a comment explaining: the constructed authorizationUrl carries per-flight noise (state, code_challenge) that's interesting for debugging but reads as noise in a connection summary; the AS-advertised endpoint is the stable "which AS is in use" identifier. Either is defensible; explicitly documenting the choice.

6. Close-button selector — ✅ Replaced document.querySelector("button.mantine-CloseButton-root") with screen.getByRole("button", { name: /close/i }). Added aria-label="Close" to CloseButton in ConnectionInfoModal.tsx so the test (and assistive tech) has a name to match.

7. Scope splitter (/\s+/ → " ") — ✅ Switched to the OAuth 2.1 §3.3 literal-space split. Added a comment noting the spec.

Re: nits — confirmed SERVER_CAPABILITY_KEYS / CLIENT_CAPABILITY_KEYS are unchanged from the pre-rename file, and the key in capabilities && capabilities[key] != null capability check is intentional (distinguishes "absent" from "present-but-empty-object" per MCP spec).

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 20s —— View job

---

Re-review of follow-up changes (commit 05634b6a)

• Pull latest and diff against origin/v2/main
• Verify each of the 7 follow-up items in commit 05634b6
• Check for any regressions or new issues

Verdict

LGTM. The follow-up commit cleanly addresses every actionable finding from the previous review, and the items left as "intentional" are well-justified with clear comments in the code itself (not just in the PR comment), so a future reader won't be tempted to "fix" them.

Per-finding check

1. EMPTY_CLIENT_CAPABILITIES stability — ✅ Resolved. core/react/useInspectorClient.ts:16 hoists Object.freeze({}) to module scope and the comment at lines 12–15 explains the staleness problem the constant solves. The fallback at line 130 now returns a stable identity, and Object.freeze defends against accidental mutation by a consumer.

2. connectionInfoOAuth snapshot semantics — ✅ Resolved as "tolerate-with-docs." clients/web/src/App.tsx:367–373 now spells out that token refreshes mid-session won't update the displayed token, and points at oauthStepChange/oauthComplete as the migration path if live state is ever needed. Fine — the memo behavior matches a snapshot dialog, the future-direction note is exactly the right thing to leave behind.

3. Modal exit animation short-circuited — ✅ Acknowledged as intentional. The "snap-shut on disconnect rather than render a stale info dialog" reasoning is defensible; the commit message captures the rationale. No change requested.

4. connectionInfoTransport "stdio" fallback — ✅ Resolved. Render guard at App.tsx:1037 now requires activeServer alongside initializeResult, making the invariant explicit. The ?? "stdio" default at line 358 remains, but only as the ServerType narrower for the optional config.type, and the new comment at 351–356 explains exactly why it lives there. Good.

5. Auth URL preference (endpoint vs. constructed URL) — ✅ Documented. The comment at App.tsx:375–380 makes the choice explicit and gives a one-line reason ("stable identifier of which AS is in use" vs. "per-flight noise"). That's the right resolution — either was defensible; documenting the trade-off in the source is the better outcome than just picking one.

6. Close-button selector — ✅ Resolved. ConnectionInfoModal.test.tsx:92 now uses screen.getByRole("button", { name: /close/i }), and ConnectionInfoModal.tsx:42 adds aria-label="Close" to give the button an accessible name. Bonus: this is a real a11y improvement for screen readers, not just a test-coupling fix.

7. Scope splitter — ✅ Resolved. App.tsx:394 now splits on the literal " " per OAuth 2.1 §3.3, with the rationale captured in the comment block at 382–385.

One small thing (non-blocking)

On the memo at App.tsx:404, the deps are [connectionStatus, inspectorClient, activeServer]. The comment frames this as "snapshot at modal open," but technically the memo re-runs whenever any of those three deps change — including when activeServer re-references after a settings edit (which would re-derive scopes). The current copy is close enough to be useful, but if you want to be precise the phrasing could be "snapshot at last derivation" rather than "snapshot at open." Not worth a re-spin.

Approval

All previously raised findings either fixed or explicitly accepted with documentation. Ready to merge from a review perspective.
· Branch: feat/1364-server-info-modal

[cliffhall]

Picked up the non-blocking nit too — 260e0393 rewords the OAuth memo from "snapshot-at-open" to "snapshot-at-last-derivation" and enumerates the natural triggers (server switch, reconnect, settings edit) so a future reader doesn't conclude the memo is open-only. Thanks for the catch.