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
5 changes: 5 additions & 0 deletions clients/web/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1504,6 +1504,11 @@ function App() {
// 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.
//
// This `??` only covers an *absent* serverInfo. A server that *reports* a
// blank name (`{ name: "" }`) is degraded for display a layer below, in
// InspectorView's `resolveHeaderServerInfo` (#1774) — kept there so this
// faithful object (and thus the modal) never carries a borrowed name.
const initializeResult = useMemo<InitializeResult | undefined>(() => {
if (connectionStatus !== "connected") return undefined;
const resolvedServerInfo = serverInfo ?? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,42 @@ describe("ConnectionInfoContent", () => {
expect(screen.getAllByText("—")).toHaveLength(3);
});

it("renders an em-dash when the reported name is missing", () => {
renderWithMantine(
<ConnectionInfoContent
initializeResult={{
...fullResult,
// Non-conforming server: `name` is runtime-absent (the field is typed
// non-null). Pins the `?.trim()` tolerance — symmetric with the
// "version is missing" test above — so it reads "—", not a crash.
serverInfo: { version: "1.0.0" } as never,
}}
clientCapabilities={fullClientCaps}
transport="stdio"
/>,
);
expect(screen.getByText("1.0.0")).toBeInTheDocument();
expect(screen.getAllByText("—")).toHaveLength(3);
});

it("renders an em-dash when the reported name is whitespace-only", () => {
renderWithMantine(
<ConnectionInfoContent
initializeResult={{
...fullResult,
serverInfo: { name: " ", version: "1.0.0" },
}}
clientCapabilities={fullClientCaps}
transport="stdio"
/>,
);
// A whitespace-only reported name is the same non-conforming class as an
// empty one (#1774): it must read as unknown ("—"), not a visually blank
// row. Stays faithful — never borrows the catalog name.
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,14 +206,20 @@ export function ConnectionInfoContent({
initializeResult;

// Only trust `serverInfo` when the server actually reported it; otherwise the
// name is a catalog fallback. Both rows use `||` (not `??`) so a reported-but-
// empty name/version reads as unknown ("—") rather than a blank row —
// `initialize` mandates the fields, not non-empty values.
// name is a catalog fallback. Both rows `?.trim()` before the `||` (not `??`)
// so a reported-but-blank name/version — empty, whitespace-only (" "), or a
// non-conforming runtime-absent field — reads as unknown ("—") rather than a
// blank row. The optional chain preserves the prior tolerance of a missing
// field (the field is typed non-null, but a non-conforming server can omit
// it); whitespace-only is the same class InspectorView's
// `resolveHeaderServerInfo` handles for the header (#1774). `initialize`
// mandates the fields, not non-empty values. Stays faithful: the fallback is
// "—", never a borrowed catalog name.
const displayName = serverInfoReported
? serverInfo.name || "—"
? serverInfo.name?.trim() || "—"
: SERVER_INFO_NOT_REPORTED_LABEL;
const displayVersion = serverInfoReported
? serverInfo.version || "—"
? serverInfo.version?.trim() || "—"
: SERVER_INFO_NOT_REPORTED_LABEL;

const serverCaps = getCapabilityEntries(capabilities, SERVER_CAPABILITY_KEYS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,103 @@ describe("InspectorView", () => {
expect(screen.getByRole("switch")).toBeChecked();
});

it("falls back to the catalog name in the header when the reported serverInfo name is empty (#1774)", () => {
// A non-conforming server reports serverInfo with an empty name string.
// `App`'s `??` fallback only fires when the whole object is absent, so the
// header would otherwise render a nameless title. The view degrades to the
// active server's catalog name ("Alpha") so the header still identifies the
// server. Scoped to the header (role="banner") to exclude the ServerCard,
// which shows the catalog name unconditionally.
renderWithMantine(
<StatefulInspectorViewHost
{...makeProps({
servers: [sampleServer],
activeServer: "alpha",
connectionStatus: "connected",
initializeResult: {
...connectedInit,
serverInfo: { name: "", version: "1.0.0" },
},
})}
/>,
);
expect(
within(screen.getByRole("banner")).getByText("Alpha"),
).toBeInTheDocument();
});

it("falls back to the catalog name in the header when the reported serverInfo name is missing (#1774)", () => {
// Non-conforming server: `serverInfo` omits `name` entirely (the field is
// typed non-null). Pins the `?.trim()` tolerance in resolveHeaderServerInfo
// — a runtime-absent name degrades to the catalog name, it doesn't throw.
renderWithMantine(
<StatefulInspectorViewHost
{...makeProps({
servers: [sampleServer],
activeServer: "alpha",
connectionStatus: "connected",
initializeResult: {
...connectedInit,
serverInfo: { version: "1.0.0" } as never,
},
})}
/>,
);
expect(
within(screen.getByRole("banner")).getByText("Alpha"),
).toBeInTheDocument();
});

it("falls back to the catalog name in the header when the reported serverInfo name is whitespace-only (#1774)", () => {
// A whitespace-only reported name (" ") is the same non-conforming class
// as an empty string — truthy, so a naive `if (serverInfo.name)` guard would
// let it through and render a blank-looking title. The `.trim()` guard
// degrades it to the catalog name like the empty case.
renderWithMantine(
<StatefulInspectorViewHost
{...makeProps({
servers: [sampleServer],
activeServer: "alpha",
connectionStatus: "connected",
initializeResult: {
...connectedInit,
serverInfo: { name: " ", version: "1.0.0" },
},
})}
/>,
);
expect(
within(screen.getByRole("banner")).getByText("Alpha"),
).toBeInTheDocument();
});

it("still renders the connected header when the reported name is blank and no catalog entry matches (#1774)", () => {
// Blank reported name AND no catalog server to borrow from: the connected
// header must still render (the connection is live) rather than crash or
// invent a label — it just shows no server name. Asserting the Disconnect
// control inside the banner makes this a real regression guard for the
// no-catalog-match branch, not merely a coverage-only test.
renderWithMantine(
<StatefulInspectorViewHost
{...makeProps({
servers: [],
activeServer: "ghost",
connectionStatus: "connected",
initializeResult: {
...connectedInit,
serverInfo: { name: "", version: "1.0.0" },
},
})}
/>,
);
const header = screen.getByRole("banner");
expect(
within(header).getByRole("button", { name: "Disconnect from server" }),
).toBeInTheDocument();
// No reported name and nothing to borrow, so the header shows no catalog name.
expect(within(header).queryByText("ghost")).not.toBeInTheDocument();
});

it("surfaces the negotiated protocol version on the active connected card", () => {
renderWithMantine(
<StatefulInspectorViewHost
Expand Down
28 changes: 27 additions & 1 deletion clients/web/src/components/views/InspectorView/InspectorView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
} from "@mantine/core";
import { useLocalStorage } from "@mantine/hooks";
import type {
Implementation,
InitializeResult,
LoggingLevel,
Prompt,
Expand Down Expand Up @@ -170,6 +171,27 @@ const PROTOCOL_TAB = "Protocol";
const NETWORK_TAB = "Network";
const CONSOLE_TAB = "Console";

// Resolve the server name the connected header renders. A server may report
// `serverInfo` with a blank `name` (#1774) — empty or whitespace-only — and the
// header must never show a nameless title, so fall back to the active server's
// catalog name when the reported name is blank (`.trim()` catches " ", the
// same non-conforming class an empty string is). This is a display-only
// fallback: the Connection Info modal stays faithful to the raw report because
// it reads App's untouched `initializeResult` + `serverInfoReported`, not this
// resolved value. (App already folds the catalog name into
// `initializeResult.serverInfo` for the modern-omitted case where serverInfo is
// absent entirely (#1772); this covers the reported-but-blank-name case that
// `??` fallback doesn't reach.)
function resolveHeaderServerInfo(
serverInfo: Implementation,
servers: ServerEntry[],
activeServerId: string | undefined,
): Implementation {
if (serverInfo.name?.trim()) return serverInfo;
const catalogName = servers.find((s) => s.id === activeServerId)?.name;
return catalogName ? { ...serverInfo, name: catalogName } : serverInfo;
}

const ALL_TABS: string[] = [
SERVERS_TAB,
"Apps",
Expand Down Expand Up @@ -1274,7 +1296,11 @@ export function InspectorView({
{connectionStatus === "connected" && initializeResult ? (
<ViewHeader
connected
serverInfo={initializeResult.serverInfo}
serverInfo={resolveHeaderServerInfo(
initializeResult.serverInfo,
serversInput,
activeServer,
)}
status={connectionStatus}
latencyMs={latencyMs}
activeTab={activeTab}
Expand Down
Loading