feat(web): display negotiated MCP protocol version on ServerCard (#1324)

[cliffhall]

Summary

Closes #1324.

Surfaces the MCP protocol version negotiated during initialize (e.g. 2025-06-18) end-to-end, and displays it as a dimmed MCP <version> label at the end of the transport row on the active connected ServerCard.

Previously App.tsx hard-coded protocolVersion: "2025-06-18" when building the InitializeResult for the connected header, because useInspectorClient didn't expose the real value. This PR removes that hard-code and plumbs the real negotiated value through.

How it works

The SDK Client doesn't expose protocolVersion — it stamps it onto the transport via transport.setProtocolVersion(...) after the handshake. So:

• MessageTrackingTransport now implements setProtocolVersion as a concrete method (rather than delegating the base transport's optional one). It captures the negotiated version and forwards to the base transport when present. This is key for stdio, whose base transport has no setProtocolVersion — the SDK only calls it because the wrapper always defines it.
• InspectorClient reads it off the transport in fetchServerInfo(), stores it, exposes getProtocolVersion(), dispatches a new protocolVersionChange event, and clears it on disconnect.
• useInspectorClient subscribes to the event and returns protocolVersion.
• App.tsx reads it from the hook (drops the hard-code; the initializeResult memo now gates on it, and it's dispatched alongside serverInfo so it's always present when connected).
• InspectorView splices initializeResult.protocolVersion onto the active server's connection, which ServerCard renders.

Testing

• MessageTrackingTransport: captures the version, forwards to a base that defines setProtocolVersion, and works when the base omits it (stdio path).
• useInspectorClient: initial value + protocolVersionChange subscription.
• Integration (real stdio connect): getProtocolVersion() returns a real date-shaped version after connect and clears on disconnect — proves the stdio capture path end-to-end.
• InspectorView/ServerCard: the active connected card shows MCP <version>; nothing shows while disconnected or when none was negotiated.
• npm run validate (lint/build/format/typecheck + unit & integration coverage gate) and npm run test:storybook both green.

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

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

---

Review — surface negotiated MCP protocol version (#1442)

• Read the diff against origin/v2/main
• Review core plumbing (transport, InspectorClient, hook)
• Review web UI (App, InspectorView, ServerCard)
• Review tests
• Post review feedback

Nice, well-scoped change. The transport-capture approach is the right one given the SDK doesn't expose protocolVersion on Client, and removing the hard-coded "2025-06-18" is a real correctness win. The event/getter plumbing mirrors the existing serverInfo/instructions pattern exactly, and test coverage is thorough (stdio capture path, base transport with/without setProtocolVersion, hook subscription, and both UI presence/absence cases). I have one thing worth considering and a few minor notes — nothing blocking.

Worth considering

1. initializeResult now disappears entirely if the protocol version isn't captured — App.tsx:889

The memo gained || !protocolVersion in its guard:

if (connectionStatus !== "connected" || !serverInfo || !protocolVersion)
  return undefined;

initializeResult doesn't only feed the MCP label — it also gates the Connection Info modal (App.tsx:2202) and the connected ViewHeader (App.tsx:2088 → InspectorView). So if the negotiated version is ever absent, you lose the whole connected-info surface, not just the dimmed MCP <version> label.

In practice this is safe today: the transport is always wrapped in MessageTrackingTransport (inspectorClient.ts:628), and the SDK always calls setProtocolVersion because the wrapper always defines it — so protocolVersion is reliably set on a successful connect. But it couples two unrelated concerns: a missing/edge-case version would now hide the Connection Info modal. Since protocolVersion is optional everywhere downstream (ConnectionState.protocolVersion?, the ServerCard guard), the safer shape is to keep initializeResult gated on serverInfo only and let protocolVersion flow through as the optional field it already is — the ServerCard already conditionally renders the label (ServerCard.tsx:142), so dropping the extra guard wouldn't show a bogus version. Fix this →

Minor notes (non-blocking)

• MessageTrackingTransport.setProtocolVersion is an improvement beyond capture — messageTrackingTransport.ts:155. The old getter returned this.baseTransport.setProtocolVersion unbound, so the SDK invoking it would have lost this. The new concrete method calls this.baseTransport.setProtocolVersion?.(version) as a proper method call — correctly bound and it captures. Worth noting this quietly fixes a latent binding bug for HTTP transports. 👍

• Two same-named methods with different meanings. MessageTrackingTransport.setProtocolVersion (SDK transport hook) and FakeInspectorClient.setProtocolVersion (a test setter that dispatches protocolVersionChange) read as the same thing but do different jobs. Fine since they're on different classes, but the fake's name slightly muddies the "set vs. negotiated" distinction. Optional.

• fetchServerInfo swallows protocol-version dispatch in the catch. inspectorClient.ts:2211-2214 is inside the try whose catch {} ignores everything. Consistent with how serverInfo/instructions are handled there, so no change needed — just confirming it's intentional.

• Styling is compliant — ProtocolText uses Text.withProps({ size, c: 'dimmed', ml: 'auto' }) (CLAUDE.md: prefer .withProps, no inline styles, c="dimmed" token). The ml: 'auto' cleanly right-aligns it at the end of the transport row. Good.

Overall this looks correct and ready; the only thing I'd genuinely weigh is decoupling the initializeResult guard from protocolVersion (#1 above). The rest are observations.

[cliffhall]

Thanks for the thorough review! Addressed the actionable item; notes on the rest below.

1. initializeResult over-coupled to protocolVersion — fixed (a5baeb76)

Good catch, and you're right that this matters: ConnectionInfoContent.tsx:102 actually renders {protocolVersion}, so gating initializeResult on the version would have hidden the entire Connection Info modal (and the connected ViewHeader) in the edge case, not just the dimmed label. Reverted the guard to connected && serverInfo only, and protocolVersion now flows through as the optional field it already is: protocolVersion: protocolVersion ?? "". The "" reads as "unknown" — the modal shows it blank and the ServerCard label's truthiness guard keeps it hidden. I also tightened the InspectorView injection to initializeResult?.protocolVersion so an empty version never lands on the connection object. Added an InspectorView test covering exactly this: connected with an empty version keeps the connected surface alive while the MCP <version> label stays hidden.

Minor notes:

• Latent binding bug in the old getter — agreed, and thanks for calling it out. The previous get setProtocolVersion() returned this.baseTransport.setProtocolVersion unbound; the concrete method now calls this.baseTransport.setProtocolVersion?.(version) properly bound, so HTTP transports get a correct call in addition to the capture. Quiet improvement, intentional.
• Two same-named setProtocolVersion methods — fair. They're on different classes (transport hook vs. test event-setter), so I've left them, but I take the point about the "set vs. negotiated" naming. Optional, leaving as-is.
• fetchServerInfo catch swallows the dispatch — intentional and consistent with how serverInfo/instructions are handled in that same try. No change.
• Styling — 👍 thanks.

[cliffhall]

@claude review

[claude]

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

---

Review — surface negotiated MCP protocol version (#1442)

• Read the full diff against origin/v2/main
• Re-review core plumbing (transport, InspectorClient, hook)
• Re-review web UI (App, InspectorView, ServerCard) — verify the a5baeb76 fix
• Review tests
• Post review feedback

The follow-up commit a5baeb76 addresses the one item from the prior review cleanly, and the change is correct end-to-end. LGTM — nothing blocking.

The decoupling fix is right

initializeResult is back to gating only on connected && serverInfo (App.tsx:892), with protocolVersion: protocolVersion ?? "" flowing through as the optional field. The ""-as-unknown contract now holds consistently across all three consumers:

• ServerCard — protocolVersion && truthiness guard hides the label on "" (ServerCard.tsx:145).
• InspectorView — only splices the field when initializeResult?.protocolVersion is truthy (InspectorView.tsx:495), so "" never lands on the connection object.
• Connection Info modal — renders the blank value rather than disappearing.

The new InspectorView test (connected + protocolVersion: "" keeps the switch checked while the MCP <version> label stays hidden) pins exactly the edge case that motivated the change. 👍

Verified the core plumbing

• Transport capture is sound. this.transport is always a MessageTrackingTransport (inspectorClient.ts:628), so the instanceof guard at inspectorClient.ts:2211 is reliable and doubles as the type-narrowing for .protocolVersion. The concrete setProtocolVersion method (messageTrackingTransport.ts:155) means the SDK always invokes it post-handshake — including stdio, whose base transport omits it — and forwards to the base via ?.() when present. The integration test exercising the real stdio path (getProtocolVersion() date-shaped after connect, undefined after disconnect) confirms it.
• Lifecycle is symmetric. Set + dispatched in fetchServerInfo (:2212), cleared + dispatched in the disconnect reset (:1054, :1060) right alongside serverInfo/instructions — same pattern, no orphaned state.
• Hook/event wiring mirrors the existing fields exactly — new protocolVersionChange entry in the event map, getter on the protocol interface, subscribe/initial-read/teardown in useInspectorClient. No gaps.

Notes from the prior review — all resolved as discussed

• Latent this-binding bug in the old get setProtocolVersion() getter is now fixed by the concrete method (proper binding + capture).
• The two same-named setProtocolVersion methods (transport hook vs. fake test setter) — left as-is on different classes, reasonable.
• fetchServerInfo catch swallowing the dispatch — intentional and consistent with serverInfo/instructions.

One tiny redundancy (non-blocking, no action needed): both InspectorView (:494) and ServerCard (:111) independently gate on status === "connected" before reading the version. Harmless defense-in-depth — the field can only ever be set when connected anyway.

Clean, well-tested, ready to merge.

[cliffhall]

Thanks — glad the decoupling landed cleanly. I went ahead and removed the one tiny redundancy you flagged, since it was trivially safe to do (073c057e).

Redundant connected-check in the InspectorView splice — removed. You're right that the cross-component double-guard was harmless, but it was also genuinely redundant: initializeResult is derived from connectionStatus in App's useMemo (same render), so a truthy initializeResult?.protocolVersion already implies a live connection. Simplified the splice condition to just initializeResult?.protocolVersion. I kept ServerCard's own status === "connected" guard intact — that one is legitimate dumb-component robustness (a story/caller could populate the field on a non-connected card), not redundancy. No behavior change; all tests + validate + storybook still green.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 47s —— View job

---

Review — surface negotiated MCP protocol version (#1442)

• Read the full diff against origin/v2/main
• Verify the 073c057e splice simplification
• Re-check core plumbing & tests
• Post review feedback

The third pass looks good. Commit 073c057e lands the splice simplification cleanly and I don't see anything new to flag. LGTM — ready to merge.

The 073c057e simplification is correct

InspectorView.tsx:495 now gates the splice on initializeResult?.protocolVersion alone, dropping the redundant connectionStatus === "connected" check. This is sound: initializeResult is derived from connectionStatus in App's useMemo (App.tsx:891, returns undefined unless connected), so a truthy initializeResult?.protocolVersion already implies a live connection within the same render. The inline comment captures that reasoning well. And ServerCard's own connection.status === "connected" guard (ServerCard.tsx:111) is rightly kept — that's genuine dumb-component robustness against a caller/story populating the field on a non-connected card, not redundancy.

End-to-end shape still holds

• ""-as-unknown contract is consistent across all three consumers: ServerCard truthiness guard hides the label (ServerCard.tsx:145), InspectorView never splices an empty version onto the connection, and the Connection Info modal renders blank rather than disappearing. The decoupled initializeResult guard (connected && serverInfo only) keeps the connected header + modal alive in the edge case.
• Transport capture is reliable — this.transport is always a MessageTrackingTransport, the concrete setProtocolVersion (messageTrackingTransport.ts:155) is always invoked by the SDK post-handshake (including stdio, whose base omits it) and forwards via ?.() when present.
• Lifecycle symmetric — set+dispatched in fetchServerInfo, cleared+dispatched in the disconnect reset, alongside serverInfo/instructions.
• Hook/event/interface wiring mirrors the existing fields exactly (event map, protocol getter, fake client setter, useInspectorClient subscribe/initial-read/teardown).

Tests

Coverage is thorough and pins the edge cases that motivated each revision: stdio capture (integration), base transport with/without setProtocolVersion, hook subscription, and the three InspectorView cases (connected shows label, disconnected hides, protocolVersion: "" keeps the connected surface while hiding the label). The date-shaped regex /^MCP \d{4}-\d{2}-\d{2}$/ neatly avoids false-matching the "MCP Inspector" header.

Nothing blocking — clean, well-tested, and all prior review items resolved.
• feat/protocol-version-servercard-1324