From c2d1c407b6a9e0ee1398791c1e93b9c29d3bed0c Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 25 Jul 2026 09:33:57 -0400 Subject: [PATCH 1/5] fix(web): render the connected header when a modern server omits serverInfo (#1772) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A modern-era `server/discover` makes `serverInfo` OPTIONAL (SHOULD, not MUST — it's stamped in `_meta["io.modelcontextprotocol/serverInfo"]`). A conforming modern server may omit it, so the client stays `connected` with `serverInfo === undefined`. `initializeResult` was gated on `serverInfo`, returning `undefined` in that case, and the connected header (with its entire tab bar) is gated on `initializeResult` downstream (`InspectorView` `connectionStatus === "connected" && initializeResult`) — so such a server connected but showed NO top menu at all. Fix: gate `initializeResult` on `connectionStatus` alone and fall back to the active server's catalog name when `serverInfo` is absent, so the header + tabs render. Everything downstream already tolerates an empty version (and now an inferred name); legacy `initialize` always carries serverInfo, so the fallback only fires for a modern server that skipped the optional field. Adds App tests: initializeResult is built when connected without serverInfo, is absent while disconnected, and uses the reported serverInfo name when present. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --- clients/web/src/App.test.tsx | 43 ++++++++++++++++++++++++++++++++++++ clients/web/src/App.tsx | 27 ++++++++++++++++------ 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index b1d650093..e0048e137 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -358,6 +358,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({ currentLogLevel?: string; activeTab?: string; erroredServerId?: string; + initializeResult?: { serverInfo: { name: string; version: string } }; onActiveTabChange: (tab: string) => void; onToggleConnection: (id: string) => void; onToolsUiChange: (next: { @@ -425,6 +426,11 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({ {props.currentLogLevel} {props.activeTab ?? "none"} + + {props.initializeResult + ? `name:${props.initializeResult.serverInfo.name || "(empty)"}` + : "none"} + {props.erroredServerId ?? "none"} @@ -625,6 +631,43 @@ describe("App failed-connection card border (#1621)", () => { }); }); +describe("App initializeResult when connected without serverInfo (#1772)", () => { + beforeEach(() => { + clientInstances.length = 0; + vi.mocked(useInspectorClient).mockReturnValue(DEFAULT_USE_INSPECTOR_CLIENT); + }); + + // A modern-era `server/discover` makes `serverInfo` optional, so a conforming + // modern server can be `connected` with `serverInfo === undefined`. The header + // (and its whole tab bar) is gated on `initializeResult` downstream, so it must + // still be built in that case — otherwise the connected server shows no menu. + it("builds initializeResult when connected even though serverInfo is undefined", () => { + // DEFAULT_USE_INSPECTOR_CLIENT is exactly this case: connected + no serverInfo. + renderWithMantine(); + expect(screen.getByTestId("init-result")).not.toHaveTextContent("none"); + }); + + it("does not build initializeResult while disconnected", () => { + vi.mocked(useInspectorClient).mockReturnValue({ + ...DEFAULT_USE_INSPECTOR_CLIENT, + status: "disconnected", + }); + renderWithMantine(); + expect(screen.getByTestId("init-result")).toHaveTextContent("none"); + }); + + it("uses the reported serverInfo name when present (legacy / stamped modern)", () => { + vi.mocked(useInspectorClient).mockReturnValue({ + ...DEFAULT_USE_INSPECTOR_CLIENT, + serverInfo: { name: "real-server", version: "2.0.0" }, + }); + renderWithMantine(); + expect(screen.getByTestId("init-result")).toHaveTextContent( + "name:real-server", + ); + }); +}); + describe("App session-scoped state reset on disconnect", () => { beforeEach(() => { clientInstances.length = 0; diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 8faaebfdd..2db49e127 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -1446,17 +1446,28 @@ function App() { // modal expect from the hook's split fields. `protocolVersion` is the value // the InspectorClient negotiated during initialize (#1324); it's dispatched // alongside serverInfo, so in practice it's present whenever we're connected. - // We deliberately gate only on serverInfo (not protocolVersion): this object - // also drives the connected header and Connection Info modal, so a - // missing/edge-case version must not hide those. It flows through as the - // optional field it is everywhere downstream (the ServerCard label and the - // modal value both tolerate an empty string), so "" reads as "unknown". + // We gate only on `connectionStatus`, never on serverInfo or protocolVersion: + // this object also drives the connected header (and its whole tab bar) and the + // Connection Info modal, so a missing field must not hide those. + // + // A modern-era server's `server/discover` makes `serverInfo` OPTIONAL (SHOULD, + // not MUST — it's stamped in `_meta["io.modelcontextprotocol/serverInfo"]`), so + // a conforming modern server may omit it and `serverInfo` stays undefined even + // while connected. Falling back to the catalog name (rather than returning + // `undefined`) keeps the header + tabs rendered for those servers (#1772); + // everything downstream already tolerates an empty version, and now an + // inferred name. Legacy `initialize` always carries serverInfo, so this + // fallback only ever fires for a modern server that skipped it. const initializeResult = useMemo(() => { - if (connectionStatus !== "connected" || !serverInfo) return undefined; + if (connectionStatus !== "connected") return undefined; + const resolvedServerInfo = serverInfo ?? { + name: servers.find((s) => s.id === activeServerId)?.name ?? "", + version: "", + }; return { protocolVersion: protocolVersion ?? "", capabilities: capabilities ?? {}, - serverInfo, + serverInfo: resolvedServerInfo, ...(instructions ? { instructions } : {}), }; }, [ @@ -1465,6 +1476,8 @@ function App() { serverInfo, instructions, protocolVersion, + servers, + activeServerId, ]); // The Server Info modal needs the active server's transport and (optional) From 9865afc9f8be38c7861885ce647622c4c94807d8 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 25 Jul 2026 10:45:58 -0400 Subject: [PATCH 2/5] fix(web): address review of the modern-serverInfo header fix (#1772) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of PR #1773: 1. Fidelity — the synthesized catalog name was rendered under Connection Info → "Server Implementation → Name" indistinguishably from a server-reported value, hiding exactly what a user opens that modal to check (did the modern server stamp serverInfo?). Thread a `serverInfoReported` flag App → ConnectionInfoModal → ConnectionInfoContent; when false, Name/Version render "— (not reported by server)" instead of the inferred name. Required on the Modal (app path stays type-safe), optional-default-true on the presentational Content (leaf fixtures with a real serverInfo need no change). 2. `version ?? "—"` → `version || "—"` so a reported-but-empty version reads as unknown rather than a blank row (the empty-string path this PR makes reachable). 3. Update the now-stale App.test mock comment (initializeResult is built when connected as of this PR). 4. Add an App test that actually exercises the fallback — connect server "A" and assert the catalog name ("PlotRocket") surfaces, not just the degenerate empty case. 5. Hoist `activeServer` above `initializeResult` and reuse it, removing the duplicate `servers.find` and shrinking the dep list to `[activeServer]` so initializeResult stops re-identifying on every catalog change. 6. Soften the "only ever fires for a modern server" comment — there's also the batched instant on any connect between the status and serverInfo dispatches. Adds ConnectionInfoContent tests for the "not reported" and empty-version paths. Verified: web validate (3478 tests), the >=90 coverage gate, and Storybook (456) all green. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --- clients/web/src/App.test.tsx | 20 +++++++++- clients/web/src/App.tsx | 37 +++++++++++-------- .../ConnectionInfoContent.test.tsx | 36 ++++++++++++++++++ .../ConnectionInfoContent.tsx | 28 +++++++++++++- .../ConnectionInfoModal.stories.tsx | 1 + .../ConnectionInfoModal.test.tsx | 5 +++ .../ConnectionInfoModal.tsx | 9 +++++ 7 files changed, 117 insertions(+), 19 deletions(-) diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index e0048e137..dad713ad3 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -220,8 +220,10 @@ vi.mock("@inspector/core/react/useInspectorClient.js", () => ({ status: "connected", capabilities: {}, clientCapabilities: {}, - // Left undefined so `initializeResult` stays undefined and the - // ConnectionInfoModal (gated on it) never mounts during the test. + // Undefined models a modern server that omitted the optional serverInfo. As + // of #1772 `initializeResult` is still built when connected (with a + // catalog-name fallback), so this exercises that path — see the "#1772" + // describe block. serverInfo: undefined, instructions: undefined, })), @@ -647,6 +649,20 @@ describe("App initializeResult when connected without serverInfo (#1772)", () => expect(screen.getByTestId("init-result")).not.toHaveTextContent("none"); }); + it("falls back to the active server's catalog name when serverInfo is undefined", async () => { + const user = userEvent.setup(); + renderWithMantine(); + // Connect server "A" (PlotRocket) via the mocked InspectorView control so it + // becomes the active server; serverInfo is still undefined (default mock), so + // the synthesized name must come from the catalog entry. + await user.click(screen.getByText("connect")); + await waitFor(() => + expect(screen.getByTestId("init-result")).toHaveTextContent( + "name:PlotRocket", + ), + ); + }); + it("does not build initializeResult while disconnected", () => { vi.mocked(useInspectorClient).mockReturnValue({ ...DEFAULT_USE_INSPECTOR_CLIENT, diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 2db49e127..1f1caef9f 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -1442,6 +1442,20 @@ function App() { }; }, [inspectorClient]); + // The Server Info modal needs the active server's transport and (optional) + // OAuth details — both are co-located here so the modal opens against the + // same connection snapshot the header is reading. Also feeds the + // `initializeResult` serverInfo fallback below. + const activeServer = useMemo( + () => servers.find((s) => s.id === activeServerId), + [servers, activeServerId], + ); + + // Whether the server actually reported `serverInfo`, vs. the catalog-name + // fallback synthesized below. Threaded to the Connection Info modal so it can + // show "not reported" instead of an inferred name that looks server-sent. + const serverInfoReported = serverInfo !== undefined; + // Build the InitializeResult the connected ViewHeader / Connection Info // modal expect from the hook's split fields. `protocolVersion` is the value // the InspectorClient negotiated during initialize (#1324); it's dispatched @@ -1454,14 +1468,15 @@ function App() { // not MUST — it's stamped in `_meta["io.modelcontextprotocol/serverInfo"]`), so // a conforming modern server may omit it and `serverInfo` stays undefined even // while connected. Falling back to the catalog name (rather than returning - // `undefined`) keeps the header + tabs rendered for those servers (#1772); - // everything downstream already tolerates an empty version, and now an - // inferred name. Legacy `initialize` always carries serverInfo, so this - // fallback only ever fires for a modern server that skipped it. + // `undefined`) keeps the header + tabs rendered for those servers (#1772). It + // fires for such a modern server — and, harmlessly, in the batched instant on + // any connect (legacy included) between the `connected` status dispatch and the + // `serverInfo` dispatch, which land in a single React render. The modal uses + // `serverInfoReported` (above), not this name, to stay faithful. const initializeResult = useMemo(() => { if (connectionStatus !== "connected") return undefined; const resolvedServerInfo = serverInfo ?? { - name: servers.find((s) => s.id === activeServerId)?.name ?? "", + name: activeServer?.name ?? "", version: "", }; return { @@ -1476,18 +1491,9 @@ function App() { serverInfo, instructions, protocolVersion, - servers, - activeServerId, + activeServer, ]); - // The Server Info modal needs the active server's transport and (optional) - // OAuth details — both are co-located here so the modal opens against the - // same connection snapshot the header is reading. - const activeServer = useMemo( - () => servers.find((s) => s.id === activeServerId), - [servers, activeServerId], - ); - // Mirror the active server's name into a ref so a mid-session failure toast // can still name the server: a transport crash dispatches `disconnect`, // which clears `activeServerId` (and thus `activeServer`) before the @@ -4437,6 +4443,7 @@ function App() { opened={connectionInfoModalOpen} onClose={() => setConnectionInfoModalOpen(false)} initializeResult={initializeResult} + serverInfoReported={serverInfoReported} clientCapabilities={clientCapabilities} transport={connectionInfoTransport} protocolEra={protocolEra} diff --git a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx index 51565697b..4358a7276 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx +++ b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx @@ -62,6 +62,42 @@ describe("ConnectionInfoContent", () => { expect(screen.getAllByText("—")).toHaveLength(3); }); + it("renders an em-dash when the reported version is an empty string", () => { + renderWithMantine( + , + ); + // An empty version reads as unknown ("—"), not a blank row (#1772). + expect(screen.getByText("Empty Version Server")).toBeInTheDocument(); + expect(screen.getAllByText("—")).toHaveLength(3); + }); + + it("shows 'not reported' for name and version when serverInfo was not reported", () => { + renderWithMantine( + , + ); + // The inferred catalog name is NOT shown as the server's reported name... + expect(screen.queryByText("my-catalog-name")).not.toBeInTheDocument(); + // ...both Name and Version read as not reported instead. + expect(screen.getAllByText("— (not reported by server)")).toHaveLength(2); + }); + it("renders server and client capability sections", () => { renderWithMantine( Server Implementation Name - {serverInfo.name} + {displayName} Version - {serverInfo.version ?? "—"} + {displayVersion} Protocol {protocolVersion} diff --git a/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.stories.tsx b/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.stories.tsx index 90390711e..1c07c91b3 100644 --- a/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.stories.tsx +++ b/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.stories.tsx @@ -54,6 +54,7 @@ const meta: Meta = { args: { opened: true, onClose: fn(), + serverInfoReported: true, }, }; diff --git a/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.test.tsx b/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.test.tsx index dcbe05a4b..c73acff56 100644 --- a/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.test.tsx +++ b/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.test.tsx @@ -29,6 +29,7 @@ describe("ConnectionInfoModal", () => { opened={false} onClose={vi.fn()} initializeResult={initializeResult} + serverInfoReported clientCapabilities={clientCapabilities} transport="stdio" />, @@ -42,6 +43,7 @@ describe("ConnectionInfoModal", () => { opened onClose={vi.fn()} initializeResult={initializeResult} + serverInfoReported clientCapabilities={clientCapabilities} transport="stdio" />, @@ -60,6 +62,7 @@ describe("ConnectionInfoModal", () => { opened onClose={vi.fn()} initializeResult={initializeResult} + serverInfoReported clientCapabilities={clientCapabilities} transport="streamable-http" oauth={{ @@ -87,6 +90,7 @@ describe("ConnectionInfoModal", () => { opened onClose={onClose} initializeResult={initializeResult} + serverInfoReported clientCapabilities={clientCapabilities} transport="stdio" />, @@ -103,6 +107,7 @@ describe("ConnectionInfoModal", () => { opened onClose={onClose} initializeResult={initializeResult} + serverInfoReported clientCapabilities={clientCapabilities} transport="stdio" />, diff --git a/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.tsx b/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.tsx index fb4f9ace3..d19ad8797 100644 --- a/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.tsx +++ b/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.tsx @@ -15,6 +15,13 @@ export interface ConnectionInfoModalProps { opened: boolean; onClose: () => void; initializeResult: InitializeResult; + /** + * Whether the server actually reported `serverInfo`. False for a modern server + * that omitted the optional `_meta` stamp — in which case `initializeResult`'s + * name is a client-side catalog fallback, and the modal shows "not reported" + * rather than passing it off as server-sent (#1772). + */ + serverInfoReported: boolean; clientCapabilities: ClientCapabilities; transport: ServerType; protocolEra?: ProtocolEra; @@ -27,6 +34,7 @@ export function ConnectionInfoModal({ opened, onClose, initializeResult, + serverInfoReported, clientCapabilities, transport, protocolEra, @@ -60,6 +68,7 @@ export function ConnectionInfoModal({ Date: Sat, 25 Jul 2026 10:56:14 -0400 Subject: [PATCH 3/5] fix(web): symmetric name em-dash + story + wiring test for #1772 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review of PR #1773: 1. `displayName` had the same blank-row bug just fixed for version — a reported `serverInfo: { name: "" }` (legacy `initialize` mandates the field, not a non-empty value) rendered a blank Name row. `serverInfo.name || "—"` makes the two rows symmetric. Adds a test for the empty reported name. 2. Adds a `ServerInfoNotReported` ConnectionInfoContent story (modern era, `serverInfoReported: false`) with a play function asserting Name/Version render "— (not reported by server)" and the catalog name is not shown — documents the modern-omitted state and doubles as a browser guard. 3. Pins the App → ConnectionInfoModal `serverInfoReported` wiring: adds an open-connection-info control to the mocked InspectorView and two App tests that open the real modal — one asserts "not reported" for the default (serverInfo undefined) mock, the other asserts the reported name for a stamped serverInfo. Without this, hard-coding the flag to `true` would keep the suite green and silently reintroduce the fidelity bug. Verified: web validate (3481 tests), the >=90 coverage gate, and Storybook (457) all green. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --- clients/web/src/App.test.tsx | 36 +++++++++++++++++++ .../ConnectionInfoContent.stories.tsx | 25 +++++++++++++ .../ConnectionInfoContent.test.tsx | 17 +++++++++ .../ConnectionInfoContent.tsx | 9 +++-- 4 files changed, 84 insertions(+), 3 deletions(-) diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index dad713ad3..7f9dba1b2 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -362,6 +362,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({ erroredServerId?: string; initializeResult?: { serverInfo: { name: string; version: string } }; onActiveTabChange: (tab: string) => void; + onConnectionInfo: () => void; onToggleConnection: (id: string) => void; onToolsUiChange: (next: { selectedToolName?: string; @@ -440,6 +441,9 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({ switch-servers-tab +