fix(web): render the connected header when a modern server omits serverInfo (#1772)

[cliffhall]

Closes #1772

Problem

Connecting to a modern-era MCP server whose server/discover omits the (now-optional) serverInfo produces a connected session with no header tab bar at all — no Servers / Tools / Resources / Tasks, just the "MCP Inspector" title. The server is effectively uninspectable through the UI even though the connection succeeded.

Reproduced against test-servers/configs/tasks-modern-http.json (port 3222): green "Connected", modern server/discover + tools/list in the Protocol view, but no tabs. Confirmed via live DOM that data-status="connected" while the header renders the disconnected/title branch — i.e. initializeResult was falsy despite being connected.

Root cause

• InspectorView gates the connected header (and its whole tab bar) on connectionStatus === "connected" && initializeResult.
• App's initializeResult returned undefined whenever !serverInfo.
• serverInfo comes from the SDK's getServerVersion(), filled from serverInfoFromDiscover(), which reads discover._meta["io.modelcontextprotocol/serverInfo"]. The modern spec makes that optional (SHOULD, not MUST), so a conforming modern server may omit it → serverInfo stays undefined → initializeResult undefined → the header collapses to the title branch.

Legacy initialize always carries serverInfo, so only modern servers hit this.

Fix

Gate initializeResult on connectionStatus alone (never on serverInfo — matching the existing deliberate choice not to gate on protocolVersion), and fall back to the active server's catalog name when serverInfo is absent. Everything downstream already tolerates an empty version (and now an inferred name). The fallback only ever fires for a modern server that skipped the optional field; a reported serverInfo still wins.

Tests

Adds three App tests: initializeResult is built when connected without serverInfo, is absent while disconnected, and uses the reported serverInfo name when present. Full web unit suite green.

Note

Surfaced while manually testing #1771 (the AGENTS.md .withProps() styling sweep), but it's a pre-existing connection/modern-era bug — unrelated to that PR (which changes none of the header-gate / initializeResult / serverInfo / InspectorView / core-connection code).

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

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

---

Code review — PR #1773

• Read the PR diff
• Review the initializeResult change in context
• Trace downstream consumers (InspectorView, ViewHeader, ConnectionInfoContent)
• Check serverInfo derivation/dispatch ordering in core/mcp/inspectorClient.ts
• Review test coverage
• Post review

Verdict: the diagnosis and the fix are right, and it's the minimal change — initializeResult is the single funnel into both the header and the Connection Info modal (App.tsx:4225, App.tsx:4439), so un-gating it on serverInfo genuinely restores the tab bar for a modern server that omits the optional _meta stamp. I confirmed the disconnect path clears serverInfo alongside status (core/mcp/inspectorClient.ts:1672-1687), so nothing goes stale across sessions. A few things to consider before merge — none blocking except possibly the first.

---

1. The synthesized serverInfo is presented as server-reported data (fidelity)

ConnectionInfoContent destructures initializeResult and renders the name under "Server Implementation → Name":

// ConnectionInfoContent.tsx:182-198
const { serverInfo, protocolVersion, capabilities, instructions } = initializeResult;
...
<Text size="sm">Name</Text>
<ValueText>{serverInfo.name}</ValueText>

After this change that value can be the catalog name the user typed, indistinguishable from what the server actually sent. The Inspector is a protocol debugging tool, and one of the exact questions a user opens that modal to answer is "did my modern server stamp _meta["io.modelcontextprotocol/serverInfo"]?" — and the Discovery section only surfaces supportedVersions (ConnectionInfoContent.tsx:216-228), so nothing else reveals the omission. Right now they'd see a plausible name and conclude it did.

The header fallback is clearly the right call (a nameless header is worse than an inferred one). The modal is the part that arguably wants "not reported". Cheapest shape: pass the raw hook value through as a sibling prop (e.g. reportedServerInfo?: Implementation, or a serverInfoReported: boolean) and have ConnectionInfoContent render — (not reported by server) when it's absent. Your call whether that's in scope for the bugfix, but if not, it's worth a follow-up issue so the invented-name-looks-reported behavior is tracked. Fix this →

2. version: "" renders as a blank row, not —

The new code comment says everything downstream "already tolerates an empty version … so "" reads as 'unknown'". It doesn't quite:

// ConnectionInfoContent.tsx:199
<ValueText>{serverInfo.version ?? "—"}</ValueText>

"" ?? "—" is "", so the Version row renders empty, not an em dash. || instead of ?? matches the comment's stated intent. Pre-existing, but this PR is what makes the empty-version path reachable in a normal flow. (For contrast, the TUI's InfoTab.tsx:163-171 already treats an empty name/version as absent.) Fix this →

3. Stale comment in the test file's default mock

App.test.tsx:222-224 still says:

// Left undefined so `initializeResult` stays undefined and the
// ConnectionInfoModal (gated on it) never mounts during the test.
serverInfo: undefined,

That rationale is exactly what this PR invalidates — initializeResult is now truthy whenever status === "connected", so the modal's gate (App.tsx:4435) reduces to activeServer && open. Worth updating in this PR so the next reader doesn't trust it.

4. The first new test doesn't actually exercise the fallback

expect(screen.getByTestId("init-result")).not.toHaveTextContent("none");

In that test nothing has been connected through the UI, so activeServerId is undefined, servers.find(...) misses, and the fallback resolves name: "" → the harness prints name:(empty). The assertion passes, but the catalog-name lookup you added is only asserted in its degenerate form. A test that clicks connect (like the #1621 tests just above do) with serverInfo: undefined and asserts name:A would pin the behavior the fix actually depends on. (The ?? "" right-hand branch is probably covered incidentally by other tests in the file, so the ≥90 gate likely doesn't complain — that's the argument for asserting it deliberately rather than relying on it.) Fix this →

5. Duplicate lookup — activeServer already exists 20 lines below

// App.tsx:1463
const resolvedServerInfo = serverInfo ?? {
  name: servers.find((s) => s.id === activeServerId)?.name ?? "",
  ...

App.tsx:1485 is the identical servers.find((s) => s.id === activeServerId) memo. Hoisting activeServer above initializeResult and using activeServer?.name removes the duplicate and shrinks the dep list to [activeServer]. Minor side effect of the current form: initializeResult now gets a fresh object identity on every servers change (any catalog edit), even when serverInfo is present and the fallback is irrelevant — that propagates into InspectorView's memos keyed on it (InspectorView.tsx:866, :1117). No visible bug (ViewHeader's snapshotKey compares by value), just avoidable work. Fix this →

6. Nit — the comment's "only ever fires for a modern server" is slightly too absolute

core/mcp/inspectorClient.ts:1206-1215 dispatches statusChange → "connected" before await this.fetchServerInfo(), which is where serverInfoChange fires. So there is a window on every connect (legacy included) where connectionStatus === "connected" with serverInfo === undefined. In practice it's invisible: fetchServerInfo reaches its serverInfoChange dispatch with no intervening await, so both state updates land in the same microtask and React batches them into one render. Worth softening the wording to "…for a modern server that skipped it (plus the batched instant between the status and serverInfo dispatches)" rather than claiming it can't happen otherwise — that's the kind of assumption that quietly rots.

---

Things I checked that are fine

• InspectorView.tsx:1274's connectionStatus === "connected" && initializeResult is now redundant on the App path but still needed for type narrowing (the prop is optional) and for the stories/tests that pass undefined — correct to leave alone.
• No other consumer reads the hook's raw serverInfo in the web client — App.tsx:939 destructures it and initializeResult is the only sink, so there's no second place with the same gate bug.
• TUI (InfoTab.tsx:152) conditionally renders its server-info block, so it degrades gracefully and doesn't have the equivalent "no tabs" failure. No cross-client follow-up needed.
• The _meta["io.modelcontextprotocol/serverInfo"]-is-optional premise is consistent with the code path (client.getServerVersion() at core/mcp/inspectorClient.ts:4108).

Caveat on verification

I did not run the test suite — this checkout has no installed dependencies (clients/web/node_modules absent, and the SDK isn't present under the root either), so a root npm install + npm run ci was out of scope for a review pass. My claims about test behavior (finding 4 in particular) are reads of the code, not observed runs. The author reports the web unit suite green; the coverage gate is the thing to confirm in CI.
· branch v2/fix-1772-modern-serverinfo-header

[cliffhall]

All six items addressed in 9865afc. Thanks — #1 in particular was a real fidelity gap for a debugging tool.

1. Synthesized name looked server-reported — threaded a serverInfoReported flag App → ConnectionInfoModal → ConnectionInfoContent. When the server didn't report serverInfo (a modern server that skipped the optional _meta stamp), the Server Implementation section now renders "— (not reported by server)" for Name and Version instead of the catalog-name fallback. Required on the Modal (keeps the app path type-safe), optional-default-true on the presentational Content (existing fixtures with a real serverInfo need no change).
2. version ?? "—" → || "—" so a reported-but-empty version reads as unknown, not a blank row — with a test.
3. Stale mock comment updated (initializeResult is built when connected as of this PR).
4. Fallback test — added one that clicks connect (server "A") and asserts the catalog name "PlotRocket" surfaces, exercising the lookup rather than only the degenerate empty-name case. Kept the empty case too (documents the no-active-server edge).
5. Duplicate servers.find removed — hoisted activeServer above initializeResult and reused it; dep list is now [activeServer], so initializeResult no longer re-identifies on every catalog edit.
6. Comment softened — notes the batched instant on any connect (legacy included) between the status and serverInfo dispatches.

Verified in the worktree: web validate (3478 tests), the ≥90 coverage gate, and Storybook (456) all green.

[cliffhall]

@claude review

[claude]

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

---

Code review — PR #1773 (round 2)

• Read the full PR diff (both commits)
• Verify the six round-1 items were addressed
• Review the new serverInfoReported threading
• Check test coverage of the new branches
• Post review

Verdict: all six round-1 items are genuinely addressed, and the serverInfoReported shape is the right one — the fix keeps the header/tab bar for a serverInfo-less modern server while the modal stops presenting the synthesized name as server-sent. I re-verified the mechanics rather than taking the checklist at face value:

#	Item	Status
1	Synthesized name looked server-reported	✅ flag threaded App (App.tsx:1454) → Modal (required) → Content (optional, defaults true); NOT_REPORTED used for both Name and Version
2	version ?? "—" → || "—"	✅ ConnectionInfoContent.tsx:205, with a dedicated test
3	Stale mock comment	✅ App.test.tsx:223-227 now describes the new behavior
4	Fallback test only exercised the degenerate case	✅ new test clicks connect (onToggleConnection("A"), App.test.tsx:442) and asserts name:PlotRocket from SERVER_A (App.test.tsx:193) — the real lookup, plus the empty case kept
5	Duplicate servers.find	✅ activeServer hoisted to App.tsx:1447, deps now […, activeServer]. Hoist is safe — servers (604) and activeServerId (662) both predate it, and the only other reader (App.tsx:1508) is below
6	Over-absolute comment	✅ softened, calls out the batched instant on any connect

Branch coverage of the new code looks complete without leaning on incidental hits: activeServer?.name and the ?? "" arm are each pinned by one of the two new App tests, and serverInfoReported's true/false arms plus version || "—"'s both arms are pinned in ConnectionInfoContent.test.tsx. Header side is fine too — ViewHeader reads only serverInfo.name (ViewHeader.tsx:246), never the version, so the synthesized version: "" never surfaces there.

Three small things left, none blocking.

1. displayName still has the blank-row bug you just fixed for version

// ConnectionInfoContent.tsx:204-207
const displayName = serverInfoReported ? serverInfo.name : NOT_REPORTED;
const displayVersion = serverInfoReported ? serverInfo.version || "—" : NOT_REPORTED;

Version got the falsy fallback; name didn't. A server that reports serverInfo: { name: "", version: "1.0.0" } — legacy initialize mandates the field, not a non-empty value — renders an empty Name row, which is exactly the failure mode item 2 was about. serverInfo.name || "—" makes the two rows symmetric. Fix this →

2. No story for the new — (not reported by server) state

ConnectionInfoModal.stories.tsx only gained serverInfoReported: true at the meta level (needed for the now-required prop), and ConnectionInfoContent.stories.tsx is untouched — so the new visual state exists only in unit tests. Per the repo's Storybook-first convention (every group/element component documents its states, play functions double as interaction tests), a ServerInfoNotReported story on ConnectionInfoContent — modern era, serverInfoReported: false, a catalog-ish name in the fixture — would both document the modern-omitted case and act as a second guard. Fix this →

3. The App → Modal wiring of serverInfoReported isn't asserted anywhere

App.test.tsx never mounts the Connection Info modal (nothing calls onConnectionInfo, and the mocked InspectorView at App.test.tsx:360-450 exposes no control for it), and the new init-result harness span prints only the name. So serverInfoReported = serverInfo !== undefined is executed on every render (no coverage gap) but its value at the App boundary is unpinned — a refactor that hard-coded serverInfoReported to true would keep the whole suite green and silently reintroduce the fidelity bug this commit set out to fix. Cheapest pin: add reported:${props.serverInfoReported}-style output, or a button that calls props.onConnectionInfo(), to the mock and assert — (not reported by server) appears for the default (serverInfo: undefined) mock. Fix this →

---

Things I checked that are fine

• Making serverInfoReported required on ConnectionInfoModal and optional (default true) on ConnectionInfoContent is the right split: App is the only ConnectionInfoModal consumer (App.tsx:4442 — verified by grep), so requiring it there is a compile-time guarantee that the flag can't be forgotten, while the default keeps every existing Content fixture/story truthful.
• The — (not reported by server) string doesn't collide with the em-dash assertions: the existing getAllByText("—")).toHaveLength(3) tests use exact-match text queries, so the longer string isn't counted.
• Discovery section only renders supportedVersions (ConnectionInfoContent.tsx:240-250), so there's no second place presenting a serverInfo name — nothing else needed the flag.
• serverInfoReported is a plain const, not a memo — correct, it's a boolean and can't destabilize a dep list.
• The comment at the resolvedServerInfo site now explicitly points readers at serverInfoReported for the faithful value, which is what keeps the parallel-channel design from rotting.
• The initializeResult identity churn from item 5 is genuinely gone: deps are the five hook fields plus activeServer, so an unrelated catalog edit no longer re-identifies it.

Caveat on verification

Same as round 1: this checkout has no installed dependencies (clients/web/node_modules absent), so I did not run validate, the coverage gate, or Storybook. Every claim above is a read of the code, not an observed run. You report all three green in the worktree; CI's coverage job is the authority.
· branch v2/fix-1772-modern-serverinfo-header

[cliffhall]

All three round-2 items addressed in e29efff.

1. displayName blank-row bug — good catch, I fixed version but not name. Now serverInfo.name || "—" so a reported-but-empty name reads as unknown too (symmetric with version), plus a test for the empty reported name.
2. Story for the "not reported" state — added ServerInfoNotReported to ConnectionInfoContent.stories.tsx (modern era, serverInfoReported: false) with a play function asserting Name/Version render "— (not reported by server)" and the catalog name isn't shown. Documents the state and acts as a browser guard.
3. App → Modal wiring unpinned — added an open-connection-info control to the mocked InspectorView and two App tests that open the real ConnectionInfoModal: one asserts "not reported" for the default serverInfo: undefined mock (and that "PlotRocket" isn't shown), the other asserts the reported name surfaces for a stamped serverInfo. Now hard-coding the flag would fail a test.

Verified in the worktree: web validate (3481 tests), the ≥90 coverage gate, and Storybook (457) all green.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 4m 11s —— View job

---

Code review — PR #1773 (round 3)

• Read the full PR diff (all three commits)
• Verify the three round-2 items were addressed
• Review the new story + App wiring tests
• Check for anything the earlier rounds missed
• Post review

Verdict: all three round-2 items are done and done correctly. The fix is now complete and self-consistent: the header/tab bar survives a serverInfo-less modern server, and the modal no longer passes the client-side catalog name off as server-reported. Nothing blocking — two small consistency nits and a story-fidelity suggestion.

#	Round-2 item	Status
1	displayName blank-row asymmetry	✅ serverInfo.name || "—" (ConnectionInfoContent.tsx:206), symmetric with version, with a dedicated empty-name test
2	No story for the "not reported" state	✅ ServerInfoNotReported (ConnectionInfoContent.stories.tsx:83) with a play function asserting both rows and that my-catalog-name isn't rendered
3	App → Modal wiring of serverInfoReported unpinned	✅ open-connection-info control on the mock + two tests that mount the real modal and assert both arms (App.test.tsx:704, :718)

Item 3 is the one that mattered and it's pinned the right way — the tests go through the real ConnectionInfoModal/Content, so hard-coding the flag to true in App.tsx now fails a test rather than silently reintroducing the fidelity bug.

---

1. Nit — the Protocol row is now the only value cell without a falsy fallback

// ConnectionInfoContent.tsx:224-230
<ValueText>{displayName}</ValueText>      // name    || "—"
<ValueText>{displayVersion}</ValueText>   // version || "—"
<ValueText>{protocolVersion}</ValueText>  // ← no fallback

App supplies protocolVersion ?? "", and un-gating initializeResult on serverInfo made an empty one marginally more reachable: fetchServerInfo() dispatches capabilities → serverInfo → era → protocolVersion inside one try/catch {} (core/mcp/inspectorClient.ts:4097-4131), so a throw partway leaves a connected session with none of them — which previously collapsed the header (the bug) and now renders the modal with a blank Protocol row. {protocolVersion || "—"} finishes the sweep items 2 and 1 started. (For the normal modern-omitted case this is fine — protocolVersion comes from getNegotiatedProtocolVersion() (:4127), which is independent of the _meta serverInfo stamp, so it's populated. I verified that rather than assuming it.) Fix this →

2. Nit — NOT_REPORTED copy is hard-coded in four places; this file already has the export pattern for that

"— (not reported by server)" is a module-private const in the component but re-typed verbatim in ConnectionInfoContent.test.tsx:113, App.test.tsx:708/:730, and ConnectionInfoContent.stories.tsx:100. The same file already exports CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL for exactly this reason, and the test imports it (ConnectionInfoContent.test.tsx:8-11) — exporting NOT_REPORTED and importing it in the three consumers follows the established convention and means a copy tweak can't leave a stale assertion behind. Fix this →

3. Suggestion — the new story omits discoverResult, so it isn't quite the modern connection it depicts

ServerInfoNotReported sets protocolEra: "modern" but no discoverResult, so the Discovery section doesn't render (ConnectionInfoContent.tsx:243) — whereas a real modern connection that skipped the _meta stamp still has a discover result (supportedVersions + capabilities, just no serverInfo). Adding one — mirroring ModernEra's shape minus serverInfo — makes the story an accurate picture of the case it documents, and incidentally demonstrates that Discovery doesn't leak a name either. Fix this →

---

Things I checked that are fine

• No blank-name flash on disconnect. Un-gating on serverInfo raised the question of whether a render could land with connected + no activeServer (→ name: "") and clobber ViewHeader's exit snapshot (Only show the Apps, Prompts, Resources, and Tasks tabs when the server exposes the relevant items #1450). It can't: both teardown paths dispatch statusChange → "disconnected" before the disconnect event (core/mcp/inspectorClient.ts:713-717 for transport onclose, :1621-1623 for explicit disconnect()), so by the time App's listener clears activeServerId (App.tsx:1209) initializeResult is already undefined and the header is on its retained snapshot. The crash path settles at "error", also non-connected.
• No second web consumer of the raw serverInfo. Grepped: App.tsx:939 destructures it, and its only sinks are initializeResult and the new serverInfoReported (:1457). ViewHeader reads serverInfo.name only (ViewHeader.tsx:246); InspectorView reads initializeResult.capabilities/.protocolVersion (:837, :1111) — never the version string — so the synthesized version: "" can't surface outside the modal.
• availableTabs is unaffected by the change in kind — it gates on initializeResult?.capabilities (InspectorView.tsx:837), which is capabilities ?? {}, so a modern server that omits serverInfo but advertises capabilities gets its full tab bar. That's the actual bug being fixed, end to end.
• Cross-client: the CLI just emits serverInfo: inspectorClient.getServerInfo() into its JSON payload (clients/cli/src/cli.ts:341 — undefined drops out of the JSON, which is faithful), and the TUI guards on serverState.serverInfo and on each field being non-empty (InfoTab.tsx:152-171). Neither has the web hard-gate, so no follow-up is needed there.
• Coverage of the new branches doesn't rely on incidental hits: activeServer?.name vs. the ?? "" arm are pinned by the two new App tests; serverInfoReported true/false and name/version || arms are each pinned in ConnectionInfoContent.test.tsx plus the two modal-wiring App tests.
• Round-2's dep-list fix holds — initializeResult's deps are the hook fields plus activeServer (App.tsx:1487-1493), so an unrelated catalog edit no longer re-identifies it, and the activeServer hoist is still above its only other reader (:1508 comment site / :4441 guard).
• One cosmetic note on the new negative assertion: expect(screen.queryByText("PlotRocket")).not.toBeInTheDocument() (App.test.tsx:713) passes for the right reason, but partly because the mock's harness span prints name:PlotRocket (not an exact text match). If that span is ever changed to print the bare name, the test will fail confusingly rather than catching a real regression — worth a comment if you touch it.

Caveat on verification

Same as rounds 1 and 2: this checkout has no installed dependencies (node_modules absent at the root and under clients/web), so I did not run validate, the ≥90 coverage gate, or Storybook. Every claim above is a read of the code. You report all three green in the worktree (3481 tests / 457 stories); CI's coverage job is the authority.
· branch v2/fix-1772-modern-serverinfo-header

[cliffhall]

All three round-3 items addressed in a06a1e4 (all non-blocking, but they finish the consistency sweep):

1. Protocol row fallback — the only value cell still missing it; now {protocolVersion || "—"}, with a test for an empty protocol version.
2. Exported NOT_REPORTED — from ConnectionInfoContent (matching the existing CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL export) and imported it in the test, story, and App.test instead of re-typing the copy in four places — a copy tweak can't leave a stale assertion now.
3. Story discoverResult — added to ServerInfoNotReported (with supportedVersions + capabilities, no serverInfo), so it accurately depicts a real modern server that skipped the _meta stamp and shows the Discovery section doesn't leak a name either.

Verified: web validate (3482 tests), the ≥90 coverage gate, and Storybook (457) all green.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 3m 50s —— View job

---

Code review — PR #1773 (round 4)

• Read the full PR diff (all four commits)
• Verify the three round-3 items were addressed
• Re-check the App-side gate and modal wiring
• Look for anything earlier rounds missed
• Post review

Verdict: all three round-3 items are done correctly, and I found nothing new that needs changing. The change is coherent end to end: initializeResult is gated on connectionStatus alone so a serverInfo-less modern server keeps its header and tab bar, while the modal reports the omission truthfully instead of laundering the catalog name. LGTM.

#	Round-3 item	Status
1	Protocol row had no falsy fallback	✅ {protocolVersion || "—"} (ConnectionInfoContent.tsx:231) + a test (ConnectionInfoContent.test.tsx:119)
2	NOT_REPORTED copy re-typed in four places	✅ exported (ConnectionInfoContent.tsx:78) and imported in the test (:11), story (.stories.tsx:7), and App.test.tsx:550
3	Story omitted discoverResult	✅ ServerInfoNotReported now carries supportedVersions + capabilities, no serverInfo (.stories.tsx:99-102)

I re-derived the em-dash counts rather than trusting them: fullResult/fullClientCaps advertise no extensions on either side, so the baseline is 2 — (the two Extensions sections). Each of the four toHaveLength(3) assertions therefore pins exactly one additional em dash — missing version, empty version, empty name, empty protocol — and the serverInfoReported: false test's getAllByText(NOT_REPORTED)).toHaveLength(2) can't be satisfied by those, since exact-match text queries don't conflate — with — (not reported by server). Each new branch is pinned by its own assertion, not incidentally.

---

Three optional notes, none of which I'd hold the PR for.

1. Note — the "synthesized value + a flag saying don't trust it" shape is a parallel channel

initializeResult.serverInfo.name is now sometimes client-invented, and the only thing preventing a consumer from presenting it as server-reported is remembering to read serverInfoReported. Today that's airtight — there are exactly two consumers, and both are handled: ViewHeader (via InspectorView.tsx:1277) reads only .name and wants the inferred name, and the modal takes the flag. But the invariant lives in prose, so a third consumer added later inherits the inferred name silently.

The cleanest alternative — keep serverInfo faithful and pass a display name separately — isn't free, because InitializeResult.serverInfo is required by the SDK type, so you'd be constructing something that isn't an InitializeResult. Given that, the current shape is the pragmatic one, and the comment at App.tsx:1467-1475 explicitly points the next reader at serverInfoReported, which is what keeps it from rotting. Recording the tradeoff rather than asking for a change.

2. Nit — NOT_REPORTED is a very generic name for a cross-module export

At the import site it reads without context (import { NOT_REPORTED } from ".../ConnectionInfoContent" in App.test.tsx:550), unlike its neighbour CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL, which is self-describing. SERVER_INFO_NOT_REPORTED (or ..._LABEL, matching the existing export's suffix) would say what it labels. Purely cosmetic. Fix this →

3. Carry-over from round 3 — the PlotRocket negative assertion is still uncommented

expect(screen.queryByText("PlotRocket")).not.toBeInTheDocument() (App.test.tsx:713) passes for the right reason, but partly because the init-result harness span renders name:PlotRocket, which an exact-match query doesn't match. If that span is ever changed to print the bare name, this line fails confusingly instead of catching a regression. A one-line comment saying "exact-match query — the init-result span's name:… prefix is deliberately not matched" would make that dependency visible. (Not raised again as a defect; just noting it wasn't picked up.)

---

Things I checked that are fine

• ConnectionInfoContent has exactly one non-test consumer. Grepped: only ConnectionInfoModal.tsx:70. So making the prop required on the modal (App is its only caller, App.tsx:4446) and optional-defaulting to true on the content is a complete guard — there's no third render path that could skip the flag.
• The synthesized version: "" can't surface outside the modal. Grepped every serverInfo.version / initializeResult.serverInfo read: ViewHeader.tsx:246 takes .name only (into its exit snapshot), and ConnectionInfoContent.tsx:210 is now behind serverInfoReported. Nothing else reads the version.
• The user-visible symptom is pinned at both ends. App-side: the new tests assert initializeResult is built when connected without serverInfo. View-side: InspectorView.test.tsx:478/:501 already pin tab derivation from initializeResult.capabilities (capabilities ?? {}), and InspectorView.tsx:1274's connected && initializeResult is the gate. So "modern server omits serverInfo → tabs render" follows from two covered halves; no third test needed.
• No blank-name flash on the rename-while-connected path. activeServer going momentarily undefined would yield name: "". onConfigSubmit (App.tsx:3739-3747) awaits updateServer → refresh() (core/react/useServers.ts:238), which setServers from a new id list, and only then calls setActiveServerId(id). That continuation is a microtask, while React schedules a default-lane render through the Scheduler (MessageChannel task), so the two updates land in one render — no intermediate servers-new/activeServerId-old commit. The remove path (:3670-3699) disconnects first, so connectionStatus leaves connected before activeServerId clears.
• serverInfoReported is computed from the hook field unconditionally (App.tsx:1457), which is safe because the disconnect path clears serverInfo alongside status — verified in earlier rounds — so it can't report true against a stale name.
• The Storybook meta-level serverInfoReported: true (ConnectionInfoModal.stories.tsx:57) satisfies the now-required prop for every story, since Storybook merges meta args into story args rather than replacing them.
• The #1772 describe block resets both clientInstances and the useInspectorClient mock in beforeEach, so the four connected/disconnected/reported/not-reported permutations don't leak into each other.
• App.test.tsx's mocked-InspectorView prop interface widening (initializeResult?: { serverInfo: { name; version } }, onConnectionInfo) is a structural subset of the real props — no any, no cast, consistent with the file's existing mock style.

Caveat on verification

Same as all three previous rounds: this checkout has no installed dependencies (node_modules absent at the root and under clients/web, and the SDK isn't resolvable), so I did not run validate, the ≥90 coverage gate, or Storybook — every claim above is a read of the code, including the em-dash arithmetic. You report 3482 tests / 457 stories / the gate green in the worktree; CI's coverage job is the authority.
· branch v2/fix-1772-modern-serverinfo-header

[cliffhall]

Round-4 was LGTM — applied the two cosmetic nits in 7e01c25 (item 1 was a recorded tradeoff you explicitly said needs no change; agreed — the two-consumer invariant is pointed at serverInfoReported in the App.tsx comment, and keeping serverInfo faithful would mean constructing a non-InitializeResult).

2. Renamed the export NOT_REPORTED → SERVER_INFO_NOT_REPORTED_LABEL so it reads with context at the import site (matching CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL); updated all four sites.
3. Commented the PlotRocket negative assertion — it's an exact-match query, deliberately not matching the harness span's prefixed name:PlotRocket, only the modal's bare Name cell.

Verified: web validate (3482 tests), the ≥90 coverage gate, and Storybook (457) all green.

[cliffhall]

@claude review

[claude]

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

---

Code review — PR #1773 (round 5)

• Read the full PR diff (all five commits)
• Verify the two round-4 nits were applied
• Re-check the fix end to end (App gate, modal wiring, header, core dispatch order)
• Look for anything the earlier rounds missed
• Post review

Verdict: LGTM. Both round-4 cosmetic nits are applied and I found no new defects. The change is coherent: initializeResult is gated on connectionStatus alone so a serverInfo-less modern server keeps its header and tab bar, and the modal reports the omission truthfully instead of laundering the catalog name.

#	Round-4 nit	Status
2	NOT_REPORTED too generic for a cross-module export	✅ renamed SERVER_INFO_NOT_REPORTED_LABEL (ConnectionInfoContent.tsx:78); all four sites updated — content test (:11), story (.stories.tsx:8), App.test.tsx:552. Grepped for stragglers: no bare NOT_REPORTED identifier and no re-typed "— (not reported by server)" literal remains
3	PlotRocket negative assertion undocumented	✅ comment at App.test.tsx:717-720 spells out that it's an exact-match query and the harness span's name:PlotRocket prefix is deliberately not matched
1	"synthesized value + a flag" parallel channel	Recorded tradeoff, no change asked — the App.tsx:1459-1475 comment points the next reader at serverInfoReported, which is what keeps it from rotting

---

One thing I verified this round that hadn't been checked: no tab-bar flash

Un-gating initializeResult on serverInfo means it can now be truthy with capabilities ?? {} — which availableTabs reads (InspectorView.tsx:837). If there were any suspension point between the connected status dispatch and the capabilitiesChange dispatch, every connect would briefly commit a header whose tab bar was Servers/Protocol-only, then re-render with the real tabs. It doesn't happen: connect() does this.status = "connected"; dispatch(statusChange) and then await this.fetchServerInfo() (core/mcp/inspectorClient.ts:1207-1216), and fetchServerInfo contains no await before its capabilities → serverInfo → era → protocolVersion dispatches (:4098-4131 — getServerCapabilities()/getServerVersion()/getNegotiatedProtocolVersion() are all sync cache reads). So the whole body runs in the same synchronous block and React batches all five setStates into one commit. I also confirmed status = "connected" is assigned in exactly one place (grepped), so there's no second entry point that reaches connected without that same block. The comment's "batched instant" wording is accurate, and it covers capabilities too, not just the name.

Optional note — the App-side fallback is object-level (??), so a reported-but-empty name still blanks the header

// App.tsx:1477-1480
const resolvedServerInfo = serverInfo ?? { name: activeServer?.name ?? "", version: "" };

Rounds 2–3 made the modal handle { name: "", version: "1.0.0" } (serverInfo.name || "—"). The header doesn't get that treatment: ?? only fires when the whole object is absent, so a server that reports serverInfo: { name: "" } still yields serverInfo.name === "" at ViewHeader.tsx:246 and the header renders a nameless title while the tab bar works. name: serverInfo?.name || activeServer?.name || "" would make the header degrade the way the modal now does. I'd leave it: it's a different failure mode from the one this PR fixes (that one had no tabs, which is what made the server uninspectable), it needs a non-conforming-ish server to reach, and the fallback would then show a catalog name while serverInfoReported is true — muddying the invariant the flag exists to keep clean. Worth a follow-up issue at most. Fix this →

---

Things I re-checked that are fine

• Em-dash arithmetic still holds after the Protocol-row change. formatExtensions returns "—" for absent extensions (ConnectionInfoContent.tsx:152-157) and neither fullResult nor fullClientCaps advertises any, so the baseline is 2. Each of the four toHaveLength(3) assertions therefore pins exactly one added em dash — missing version, empty version, empty name, empty protocol — and none of them can be satisfied by SERVER_INFO_NOT_REPORTED_LABEL, since exact-match text queries don't conflate — with — (not reported by server).
• activeServer hoist is still safe. Grepped every activeServer reference in App.tsx: the memo is at :1449 and the earliest read is :1479 (the new fallback); servers/activeServerId are both defined well above. The only prior mention at :1199 is inside a comment.
• Exactly one non-test consumer of ConnectionInfoContent (ConnectionInfoModal.tsx:69) and one of ConnectionInfoModal (App.tsx:4443), so required-on-modal / optional-default-true-on-content is a complete guard — no third render path can skip the flag.
• The synthesized version: "" can't escape the modal. ViewHeader declares serverInfo: Implementation but reads only .name, into its #1450 exit snapshot (:246); InspectorView reads initializeResult?.capabilities (:837, :1334) and ?.protocolVersion (:1111) — never the version string.
• InspectorView.tsx:1107-1109's comment ("initializeResult … is only ever truthy when connected — it's derived from connectionStatus in the same memo") is more accurate after this PR, not less. Nice side effect; nothing to update.
• serverInfoReported is unconditional but can't go stale: the disconnect path clears serverInfo alongside status, and the modal is gated on initializeResult && activeServer (App.tsx:4441) with the open flag reset on disconnect (:1213), so it can never render true against a stale name.
• Story hygiene: import { expect, within } from "storybook/test" matches the convention used by the other group stories (e.g. AppControls.stories.tsx:5), and the meta-level serverInfoReported: true on ConnectionInfoModal.stories.tsx:57 satisfies the now-required prop for every story since Storybook merges meta args into story args.
• New-branch coverage isn't incidental: activeServer?.name vs. ?? "", serverInfoReported true/false, and the || arms for name/version/protocol are each pinned by a named assertion in App.test.tsx or ConnectionInfoContent.test.tsx; the default-true arm is exercised by every pre-existing Content test that omits the prop.

Caveat on verification

Same as all four previous rounds: this checkout has no installed dependencies (node_modules absent at the root and under clients/web), so I did not run validate, the ≥90 coverage gate, or Storybook — every claim above is a read of the code, including the em-dash arithmetic and the batching argument. You report 3482 tests / 457 stories / the gate green in the worktree; CI's coverage job is the authority.
· branch v2/fix-1772-modern-serverinfo-header

[cliffhall]

LGTM confirmed across rounds 4 and 5 — thanks for the thorough passes.

On the optional note (object-level ?? still blanks the header for a reported-but-empty name): I agree with your recommendation to leave it out of this PR. It's a materially milder failure mode than the "no tab bar" bug #1772 fixes (the header is nameless but the server stays inspectable), it needs a non-conforming server to reach, and — as you note — the one-line fix would surface a catalog name while serverInfoReported is true, muddying the exact invariant that flag exists to protect. That deserves a deliberate decision, not a reflexive one-liner. Filed as #1774 to track it.

This PR is ready to merge.