From 73bd67411b0db61ea7e2406a5c5c1e3b1c95065c Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 25 Jun 2026 11:54:27 -0400 Subject: [PATCH 1/2] fix(web): re-enable other server cards when a connection errors (#1521) When a server's connection settled into the terminal `error` state, the other server cards stayed dimmed/`inert` (Connect toggle + actions disabled), so the user couldn't interact with any other server. `ServerCard` dims any card whose id differs from `activeServer`, and App clears `activeServerId` only on the InspectorClient `disconnect` event. A terminal `error` (a connect-time handshake failure, or a mid-session transport `onerror` that fires without an `onclose`) sets status to `error` but never dispatches `disconnect`, so `activeServerId` lingers and every other card stays inert. We can't clear `activeServerId` on error because the errored card's status, the connected header, and the connection-info modal are all keyed off it. Fix: dim the other cards only while the active session is actually live (connecting or connected). InspectorView now passes the active id to the ServerListScreen as the dim source only when `connectionStatus` is `connecting`/`connected`; once it reaches a terminal state the others re-enable. The errored card still shows its real status via the merged `servers` list, which keys off the real `activeServer`. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../InspectorView/InspectorView.test.tsx | 70 +++++++++++++++++++ .../views/InspectorView/InspectorView.tsx | 18 ++++- 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/clients/web/src/components/views/InspectorView/InspectorView.test.tsx b/clients/web/src/components/views/InspectorView/InspectorView.test.tsx index 4b1f999a9..18e1be168 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.test.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.test.tsx @@ -920,6 +920,76 @@ describe("InspectorView", () => { ).toBeInTheDocument(); }); + it("dims the other server cards while a connection is live", () => { + const betaServer: ServerEntry = { + id: "beta", + name: "Beta", + config: { type: "stdio", command: "echo" }, + connection: { status: "disconnected" }, + }; + renderWithMantine( + , + ); + // The non-active card is inert while alpha holds a live session, so the + // user can't start a second connection mid-session. + const betaCard = screen.getByText("Beta").closest(".mantine-Card-root"); + expect(betaCard?.getAttribute("aria-disabled")).toBe("true"); + }); + + it("re-enables the other server cards when the active connection goes to error (#1521)", () => { + const betaServer: ServerEntry = { + id: "beta", + name: "Beta", + config: { type: "stdio", command: "echo" }, + connection: { status: "disconnected" }, + }; + // Live session on alpha → beta starts out dimmed/inert. + const { rerender } = renderWithMantine( + , + ); + expect( + screen + .getByText("Beta") + .closest(".mantine-Card-root") + ?.getAttribute("aria-disabled"), + ).toBe("true"); + + // alpha's connection errors. App does NOT clear `activeServer` here — a + // terminal `error` fires no InspectorClient `disconnect` event — so the + // id still points at alpha. The other cards must re-enable anyway; only a + // *live* session should dim them. + rerender( + , + ); + expect( + screen + .getByText("Beta") + .closest(".mantine-Card-root") + ?.getAttribute("aria-disabled"), + ).toBeNull(); + }); + describe("listChanged indicator wiring (#1402)", () => { // The indicator only mounts on the active screen, so each case connects, // navigates to the target tab, and asserts the "List updated" affordance. diff --git a/clients/web/src/components/views/InspectorView/InspectorView.tsx b/clients/web/src/components/views/InspectorView/InspectorView.tsx index 3ed89b6b9..74b518488 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.tsx @@ -576,6 +576,22 @@ export function InspectorView({ [serversInput, activeServer, connectionStatus, initializeResult], ); + // The other server cards are dimmed/`inert` only while a connection is + // actively live (connecting or connected), so the user can't kick off a + // second connection mid-session. Once the active session settles into a + // terminal state — `disconnected` or `error` — the rest must re-enable + // (#1521). A connect-time handshake failure (and a mid-session transport + // `onerror`) resolves to `error` *without* firing the InspectorClient + // `disconnect` event that App uses to clear `activeServerId`, so the id + // lingers; gating the dim source on liveness here is what un-dims the + // others. The errored card itself still shows its real status via the + // merged `servers` list above (keyed off the real `activeServer`), so + // passing `undefined` here lifts only the *other* cards' dimming, not the + // error indicator on the active one. + const sessionLive = + connectionStatus === "connecting" || connectionStatus === "connected"; + const dimCardsAgainst = sessionLive ? activeServer : undefined; + return ( // padding={0}: each screen fills `calc(100dvh - header)` and supplies its // own `xl` padding, so Main must contribute only the fixed-header offset. @@ -612,7 +628,7 @@ export function InspectorView({ Date: Thu, 25 Jun 2026 13:46:07 -0400 Subject: [PATCH 2/2] refactor(web): reuse isTerminalStatus for card-dim gate (#1521 review) Replace the hand-rolled `sessionLive` check with the existing `isTerminalStatus` predicate (the #1490 teardown convention) so the dim gate stays aligned with the canonical "session is over" branching and can't desync from a future status addition. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/views/InspectorView/InspectorView.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/clients/web/src/components/views/InspectorView/InspectorView.tsx b/clients/web/src/components/views/InspectorView/InspectorView.tsx index 74b518488..98195c54c 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.tsx @@ -18,6 +18,7 @@ import type { MessageEntry, ServerEntry, } from "@inspector/core/mcp/types.js"; +import { isTerminalStatus } from "@inspector/core/mcp/types.js"; import { isAppTool } from "@inspector/core/mcp/apps.js"; import { ViewHeader } from "../../groups/ViewHeader/ViewHeader"; import { ServerListScreen } from "../../screens/ServerListScreen/ServerListScreen"; @@ -587,10 +588,12 @@ export function InspectorView({ // others. The errored card itself still shows its real status via the // merged `servers` list above (keyed off the real `activeServer`), so // passing `undefined` here lifts only the *other* cards' dimming, not the - // error indicator on the active one. - const sessionLive = - connectionStatus === "connecting" || connectionStatus === "connected"; - const dimCardsAgainst = sessionLive ? activeServer : undefined; + // error indicator on the active one. `isTerminalStatus` (the #1490 teardown + // convention) is the single source of truth for "session is over" so this + // gate can't silently desync from a future status addition. + const dimCardsAgainst = isTerminalStatus(connectionStatus) + ? undefined + : activeServer; return ( // padding={0}: each screen fills `calc(100dvh - header)` and supplies its