* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
5de4e911e3
commit
7330b326f0
2 changed files with 90 additions and 1 deletions
|
|
@ -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(
|
||||
<InspectorView
|
||||
{...makeProps({
|
||||
servers: [sampleServer, betaServer],
|
||||
activeServer: "alpha",
|
||||
connectionStatus: "connected",
|
||||
initializeResult: connectedInit,
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
// 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(
|
||||
<InspectorView
|
||||
{...makeProps({
|
||||
servers: [sampleServer, betaServer],
|
||||
activeServer: "alpha",
|
||||
connectionStatus: "connected",
|
||||
initializeResult: connectedInit,
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
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(
|
||||
<InspectorView
|
||||
{...makeProps({
|
||||
servers: [sampleServer, betaServer],
|
||||
activeServer: "alpha",
|
||||
connectionStatus: "error",
|
||||
initializeResult: undefined,
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -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";
|
||||
|
|
@ -576,6 +577,24 @@ 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. `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
|
||||
// own `xl` padding, so Main must contribute only the fixed-header offset.
|
||||
|
|
@ -612,7 +631,7 @@ export function InspectorView({
|
|||
<ServerListScreen
|
||||
servers={servers}
|
||||
writable={serverListWritable}
|
||||
activeServer={activeServer}
|
||||
activeServer={dimCardsAgainst}
|
||||
onAddManually={onServerAdd}
|
||||
onImportConfig={onServerImportConfig}
|
||||
onImportServerJson={onServerImportJson}
|
||||
|
|
|
|||
Loading…
Reference in a new issue