Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 103 additions & 2 deletions clients/web/src/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})),
Expand Down Expand Up @@ -358,7 +360,9 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({
currentLogLevel?: string;
activeTab?: string;
erroredServerId?: string;
initializeResult?: { serverInfo: { name: string; version: string } };
onActiveTabChange: (tab: string) => void;
onConnectionInfo: () => void;
onToggleConnection: (id: string) => void;
onToolsUiChange: (next: {
selectedToolName?: string;
Expand Down Expand Up @@ -425,13 +429,21 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({
</span>
<span data-testid="log-level">{props.currentLogLevel}</span>
<span data-testid="active-tab">{props.activeTab ?? "none"}</span>
<span data-testid="init-result">
{props.initializeResult
? `name:${props.initializeResult.serverInfo.name || "(empty)"}`
: "none"}
</span>
<span data-testid="errored-server">
{props.erroredServerId ?? "none"}
</span>
<button onClick={() => props.onActiveTabChange("Servers")}>
switch-servers-tab
</button>
<button onClick={() => props.onToggleConnection("A")}>connect</button>
<button onClick={() => props.onConnectionInfo()}>
open-connection-info
</button>
<button
onClick={() =>
props.onToolsUiChange({
Expand Down Expand Up @@ -537,6 +549,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({
}));

import App from "./App";
import { SERVER_INFO_NOT_REPORTED_LABEL } from "./components/groups/ConnectionInfoContent/ConnectionInfoContent";
import { OAUTH_CALLBACK_PATH } from "./utils/oauthFlow.js";
import { INSPECTOR_SERVERS_TAB } from "./utils/inspectorTabs.js";
import {
Expand Down Expand Up @@ -625,6 +638,94 @@ 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(<App />);
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(<App />);
// 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,
status: "disconnected",
});
renderWithMantine(<App />);
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(<App />);
expect(screen.getByTestId("init-result")).toHaveTextContent(
"name:real-server",
);
});

// Pins the App → ConnectionInfoModal wiring of `serverInfoReported`: without
// this, hard-coding it to `true` would keep the suite green and silently
// reintroduce the fidelity bug.
it("passes serverInfoReported=false to the modal, which shows 'not reported' (serverInfo omitted)", async () => {
const user = userEvent.setup();
renderWithMantine(<App />);
await user.click(screen.getByText("connect")); // active server = A
await user.click(screen.getByText("open-connection-info"));
await waitFor(() =>
expect(screen.getAllByText(SERVER_INFO_NOT_REPORTED_LABEL)).toHaveLength(
2,
),
);
// The synthesized catalog name is not presented as the server's report.
// Exact-match query on purpose: the mocked InspectorView's `init-result` span
// prints "name:PlotRocket" (prefixed), so this only matches a bare "PlotRocket"
// — i.e. the modal's Name cell — not the harness span.
expect(screen.queryByText("PlotRocket")).not.toBeInTheDocument();
});

it("passes serverInfoReported=true to the modal, which shows the reported name", async () => {
vi.mocked(useInspectorClient).mockReturnValue({
...DEFAULT_USE_INSPECTOR_CLIENT,
serverInfo: { name: "real-server", version: "2.0.0" },
});
const user = userEvent.setup();
renderWithMantine(<App />);
await user.click(screen.getByText("connect"));
await user.click(screen.getByText("open-connection-info"));
await waitFor(() =>
expect(screen.getByText("real-server")).toBeInTheDocument(),
);
expect(
screen.queryByText(SERVER_INFO_NOT_REPORTED_LABEL),
).not.toBeInTheDocument();
});
});

describe("App session-scoped state reset on disconnect", () => {
beforeEach(() => {
clientInstances.length = 0;
Expand Down
50 changes: 35 additions & 15 deletions clients/web/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1442,21 +1442,47 @@ 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<ServerEntry | undefined>(
() => 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
// 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). 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<InitializeResult | undefined>(() => {
if (connectionStatus !== "connected" || !serverInfo) return undefined;
if (connectionStatus !== "connected") return undefined;
const resolvedServerInfo = serverInfo ?? {
name: activeServer?.name ?? "",
version: "",
};
return {
protocolVersion: protocolVersion ?? "",
capabilities: capabilities ?? {},
serverInfo,
serverInfo: resolvedServerInfo,
...(instructions ? { instructions } : {}),
};
}, [
Expand All @@ -1465,16 +1491,9 @@ function App() {
serverInfo,
instructions,
protocolVersion,
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<ServerEntry | undefined>(
() => 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
Expand Down Expand Up @@ -4424,6 +4443,7 @@ function App() {
opened={connectionInfoModalOpen}
onClose={() => setConnectionInfoModalOpen(false)}
initializeResult={initializeResult}
serverInfoReported={serverInfoReported}
clientCapabilities={clientCapabilities}
transport={connectionInfoTransport}
protocolEra={protocolEra}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import type {
InitializeResult,
} from "@modelcontextprotocol/client";
import type { Meta, StoryObj } from "@storybook/react-vite";
import { ConnectionInfoContent } from "./ConnectionInfoContent";
import { expect, within } from "storybook/test";
import {
ConnectionInfoContent,
SERVER_INFO_NOT_REPORTED_LABEL,
} from "./ConnectionInfoContent";

const fullResult: InitializeResult = {
protocolVersion: "2025-03-26",
Expand Down Expand Up @@ -75,6 +79,38 @@ export const ModernEra: Story = {
},
};

// A modern server that omitted the optional `_meta` serverInfo stamp (#1772).
// `initializeResult.serverInfo` here is App's client-side catalog fallback, and
// `serverInfoReported: false` tells the modal not to present it as server-sent —
// Name and Version read "— (not reported by server)".
export const ServerInfoNotReported: Story = {
args: {
initializeResult: {
protocolVersion: "2026-07-28",
// The catalog name App synthesizes; must NOT surface as the reported name.
serverInfo: { name: "my-catalog-name", version: "" },
capabilities: { tools: { listChanged: true } },
},
serverInfoReported: false,
clientCapabilities: { roots: { listChanged: true } },
transport: "streamable-http",
protocolEra: "modern",
// A real modern connection that skipped the `_meta` serverInfo stamp still
// has a discover result — with `supportedVersions` + capabilities, just no
// `serverInfo`. Including it makes the story accurate and shows the Discovery
// section doesn't leak a name either.
discoverResult: {
supportedVersions: ["2026-07-28", "2025-11-25"],
capabilities: { tools: { listChanged: true } },
},
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
expect(canvas.queryByText("my-catalog-name")).not.toBeInTheDocument();
expect(canvas.getAllByText(SERVER_INFO_NOT_REPORTED_LABEL)).toHaveLength(2);
},
};

export const MinimalCapabilities: Story = {
args: {
initializeResult: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { renderWithMantine, screen } from "../../../test/renderWithMantine";
import {
CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL,
ConnectionInfoContent,
SERVER_INFO_NOT_REPORTED_LABEL,
} from "./ConnectionInfoContent";

const fullResult: InitializeResult = {
Expand Down Expand Up @@ -62,6 +63,72 @@ describe("ConnectionInfoContent", () => {
expect(screen.getAllByText("—")).toHaveLength(3);
});

it("renders an em-dash when the reported version is an empty string", () => {
renderWithMantine(
<ConnectionInfoContent
initializeResult={{
...fullResult,
serverInfo: { name: "Empty Version Server", version: "" },
}}
clientCapabilities={fullClientCaps}
transport="stdio"
/>,
);
// An empty version reads as unknown ("—"), not a blank row (#1772).
expect(screen.getByText("Empty Version Server")).toBeInTheDocument();
expect(screen.getAllByText("—")).toHaveLength(3);
});

it("renders an em-dash when the reported name is an empty string", () => {
renderWithMantine(
<ConnectionInfoContent
initializeResult={{
...fullResult,
serverInfo: { name: "", version: "1.0.0" },
}}
clientCapabilities={fullClientCaps}
transport="stdio"
/>,
);
// `initialize` mandates the name field, not a non-empty value, so an empty
// reported name also reads as unknown ("—") — symmetric with version.
expect(screen.getByText("1.0.0")).toBeInTheDocument();
expect(screen.getAllByText("—")).toHaveLength(3);
});

it("shows 'not reported' for name and version when serverInfo was not reported", () => {
renderWithMantine(
<ConnectionInfoContent
initializeResult={{
...fullResult,
// App synthesizes a catalog-name fallback here when a modern server
// omits serverInfo; the modal must not present it as server-reported.
serverInfo: { name: "my-catalog-name", version: "" },
}}
serverInfoReported={false}
clientCapabilities={fullClientCaps}
transport="streamable-http"
/>,
);
// 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(SERVER_INFO_NOT_REPORTED_LABEL)).toHaveLength(2);
});

it("renders an em-dash when the protocol version is empty", () => {
renderWithMantine(
<ConnectionInfoContent
initializeResult={{ ...fullResult, protocolVersion: "" }}
clientCapabilities={fullClientCaps}
transport="stdio"
/>,
);
// App supplies `protocolVersion ?? ""`; an empty one reads as unknown ("—"),
// symmetric with Name/Version (#1772).
expect(screen.getAllByText("—")).toHaveLength(3);
});

it("renders server and client capability sections", () => {
renderWithMantine(
<ConnectionInfoContent
Expand Down
Loading
Loading