feat(connection-info): wire Connection Info modal to the connection handshake (#1377)
* feat(server-info): wire Server Info modal to the connection handshake Closes #1364 - Add ServerInfoModal wrapping the existing ServerInfoContent, mirroring the ServerSettingsModal layout. - Gate the Server Info button on ServerCard to render only when the connection is "connected". - Expose `getClientCapabilities()` on InspectorClient (and protocol + FakeInspectorClient), and surface it through useInspectorClient so the modal can render the client side of the capability handshake without poking the SDK Client's private state. - App.tsx: open the modal from the gated Server Info button, derive the active server's transport and OAuth details (synchronously, from the guided-OAuth state snapshot + persisted settings), and reset the open flag on InspectorClient `disconnect` so the modal does not auto-reopen on a future reconnect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(server-info): dedupe modal header and render instructions in ContentViewer - Drop the in-content "Server Information" Title from ServerInfoContent and rename the modal title to "Server Information" so the modal renders one heading instead of two stacked ones. - Swap the instructions Blockquote for ContentViewer (copyable, with the existing wrapping Code variant) so long instructions wrap and the user can copy the full payload. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(server-info): rename ServerInfo -> ConnectionInfo The popup covers server identity AND client capabilities AND OAuth state — it's a snapshot of the live connection, not just the server. Rename to match what it actually shows. Also add a "Server Implementation" heading above the top section (name/version/protocol/transport) to balance the existing capability headings. - ServerInfoContent -> ConnectionInfoContent (directory, file, exports) - ServerInfoModal -> ConnectionInfoModal (directory, file, exports) - ServerCard button label "Server Info" -> "Connection Info"; prop onServerInfo -> onConnectionInfo - Modal title -> "Connection Info" - App.tsx / ServerListScreen / InspectorView wiring updated to match Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(connection-info): cap instructions block height so it scrolls in place A long server prompt (e.g. an Everything-style instruction document) was pushing the OAuth section and the modal chrome off-screen — users had to scroll the whole modal to reach the close button. Wrap the ContentViewer in ScrollArea.Autosize with mah=280 so the instructions section scrolls internally and the rest of the modal stays put. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(connection-info): add subtle background to section headings Add a `section` variant for Title that paints the heading in the existing `--inspector-surface-subtle` token (same surface token already used for modal call-out panels). Apply it to each section heading in ConnectionInfoContent (Server Implementation, Server Capabilities, Client Capabilities, Server Instructions, OAuth Details) via a SectionHeading subcomponent constant, so the sections read as distinct cards against the modal background instead of bleeding into one stream of text. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(stories): wrap long import line in ConnectionInfoModal.stories.tsx Prettier flagged the import in CI. The line exceeded printWidth=80 and needed to wrap onto multiple lines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(agents): require `npm run format` before committing Add a pre-commit instruction to run the auto-fixing `npm run format` ahead of `npm run validate`. `validate` runs `format:check` (non-fixing) which will fail CI on any unformatted file — running the auto-fixer first eliminates that class of failure entirely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(favicon): use theme/assets/MCP.svg as the favicon source of truth Point index.html's icon link at /src/theme/assets/MCP.svg so the same MCP logo asset already imported by ViewHeader is also the favicon. Delete the now-unused clients/web/public/favicon.svg. Vite resolves the source path during build and emits a hashed copy under dist/assets/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(server-card): surface failed-handshake errors with a 5s auto-dismiss Two related fixes so the user actually sees what went wrong when a connect attempt fails (e.g. OAuth-backed server, bad URL, missing dev auth token): - App.tsx: add `connectInFlightRef` and skip clearing `activeServerId` in the InspectorClient `disconnect` listener while a connect attempt is in flight. The transport closes during a failing handshake and dispatches `disconnect` before the catch block sets `errorMessage`; without this guard InspectorView would map the card to plain "disconnected" and the InlineError would never render. - InlineError: opt-in `autoDismissMs` prop that runs a Mantine `slide-up` Transition exit animation after the message has been visible for the given duration. Visibility is derived from a "dismissed message" state (rather than a mounted boolean + setMounted-in-effect), so the timer re-arms whenever `error.message` changes without tripping the react-hooks/set-state-in-effect rule. ServerCard passes autoDismissMs={5000}. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(server-card): own InlineError visibility + animate parent-driven removal Move the 5s auto-dismiss timer and visibility state out of InlineError and into ServerCard so the slide-up exit animation runs in both cases: - Timer-driven: after 5s the alert animates out (existing behavior). - Parent-driven: when `connection.error` is cleared mid-flight (e.g. a successful reconnect), the alert ALSO slides up instead of vanishing. To keep the message painted during the parent-driven exit, ServerCard tracks a `lastError` state that mirrors `connection.error` only on its truthy edges. The conditional setState during render is the supported React pattern for "remember the last seen value of a prop"; gated on message inequality so it can't loop. The `useRef` version of this pattern failed `react-hooks/refs` (no ref writes/reads during render). InlineError is now a pure presentational component: `mounted` prop controls a Mantine `slide-up` Transition. The internal `autoDismissMs` prop is gone — its single caller has moved to the controlled `mounted` pattern. Closes the "error vanishes the moment the parent removes it" half of #1364 (the timer half landed in 6d147e0f). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(server-card): surface connection errors as a toast, drop InlineError Move handshake-failure messaging out of the ServerCard body and onto a Mantine toast (`notifications.show`). The inline alert had a 5-second visibility contract entangled with InspectorView's status pipeline, and the active card was flipping through error → disconnected → error during a failing handshake; surfacing the error as a toast sidesteps that whole timing problem and keeps the card focused on the live status indicator. - App.tsx: toast on the connect-catch path; drop `errorMessage` state + `connectInFlightRef` (the latter was only there to keep the inline alert visible past the transport's close event). - InspectorView: drop the `errorMessage` prop and the `connectionStatus === "error" && errorMessage` mapping branch. - ServerCard: drop the InlineError rendering + the displayedError / errorVisible visibility state machine. - Delete the InlineError component — no other consumers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(notifications): anchor toasts to the bottom right Move the Notifications portal from top-right to bottom-right so a connection-failure toast doesn't overlap the ViewHeader / tabs row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: address PR feedback on the Connection Info modal 1. Hoist `EMPTY_CLIENT_CAPABILITIES = Object.freeze({})` to module scope in `useInspectorClient` so the fallback returns a stable reference (avoids invalidating downstream `useMemo`/`useEffect` deps when no client is attached). 4. Drop the `?? "stdio"` silent fallback on `connectionInfoTransport` in the JSX — gate the modal render on `activeServer` too. The "stdio" default still lives at the derivation site (config.type is schema-optional and `ServerType` is required by the modal prop), but the gate makes the invariant explicit. 6. Replace the raw `document.querySelector("button.mantine-CloseButton-root")` in `ConnectionInfoModal.test.tsx` with `screen.getByRole("button", { name: /close/i })`. Add `aria-label="Close"` to the CloseButton so the assertion (and screen readers) have a name to match on. 7. Switch the OAuth scope splitter from `/\s+/` to the literal `" "` per OAuth 2.1 §3.3. Also clarify in comments: 2. Note that `connectionInfoOAuth` uses snapshot-at-open semantics on purpose — token refreshes mid-session won't re-render the modal. 5. Note that we prefer the AS-advertised `authorization_endpoint` over `authorizationUrl` because the latter is per-flight noise (state, code_challenge) rather than a stable identifier. Finding 3 (modal exit animation short-circuited by the `{initializeResult && …}` guard) left as intentional — we want the snap-shut on disconnect so a stale info dialog can't keep rendering with no live client behind it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(connection-info): tighten the OAuth memo comment per re-review The reviewer noted that "snapshot-at-open" was imprecise — the memo re-runs on any of its deps (connectionStatus, inspectorClient, activeServer), which includes settings edits that re-reference the active server. Reframe as "snapshot-at-last-derivation" and call out the natural triggers (server switch, reconnect, settings edit) so a future reader doesn't conclude the memo is open-only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall
GitHub
parent
dd0f5de820
commit
e3b0b6a908
3 changed files with 569 additions and 331 deletions
|
|
@ -1,52 +0,0 @@
|
|||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { InlineError } from "./InlineError";
|
||||
|
||||
const meta: Meta<typeof InlineError> = {
|
||||
title: "Elements/InlineError",
|
||||
component: InlineError,
|
||||
};
|
||||
|
||||
export default meta;
|
||||
type Story = StoryObj<typeof InlineError>;
|
||||
|
||||
export const ShortError: Story = {
|
||||
args: {
|
||||
error: { message: "Connection timeout after 20s" },
|
||||
},
|
||||
};
|
||||
|
||||
export const WithRetryCount: Story = {
|
||||
args: {
|
||||
error: { message: "Connection failed" },
|
||||
retryCount: 3,
|
||||
maxRetries: 5,
|
||||
},
|
||||
};
|
||||
|
||||
export const WithDetails: Story = {
|
||||
args: {
|
||||
error: {
|
||||
message: "Connection refused",
|
||||
data: "Error: ECONNREFUSED 127.0.0.1:3000\n at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1494:16)",
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
export const WithDocLink: Story = {
|
||||
args: {
|
||||
error: { message: "Connection failed" },
|
||||
docLink: "https://example.com/troubleshooting",
|
||||
},
|
||||
};
|
||||
|
||||
export const FullError: Story = {
|
||||
args: {
|
||||
error: {
|
||||
message: "Connection refused",
|
||||
data: "Error: ECONNREFUSED 127.0.0.1:3000\n at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1494:16)",
|
||||
},
|
||||
retryCount: 3,
|
||||
maxRetries: 5,
|
||||
docLink: "https://example.com/troubleshooting",
|
||||
},
|
||||
};
|
||||
|
|
@ -1,64 +0,0 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { renderWithMantine, screen } from "../../../test/renderWithMantine";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { InlineError } from "./InlineError";
|
||||
|
||||
describe("InlineError", () => {
|
||||
it("renders the error message", () => {
|
||||
renderWithMantine(<InlineError error={{ message: "boom" }} />);
|
||||
expect(screen.getByText("boom")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders retry attempt count without max", () => {
|
||||
renderWithMantine(
|
||||
<InlineError error={{ message: "boom" }} retryCount={2} />,
|
||||
);
|
||||
expect(screen.getByText("Retry attempt 2")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders retry attempt count with max", () => {
|
||||
renderWithMantine(
|
||||
<InlineError error={{ message: "boom" }} retryCount={2} maxRetries={5} />,
|
||||
);
|
||||
expect(screen.getByText("Retry attempt 2 of 5")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("does not show expand button when there is nothing to expand", () => {
|
||||
renderWithMantine(<InlineError error={{ message: "boom" }} />);
|
||||
expect(screen.queryByText("Show more")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("expands details on click", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(
|
||||
<InlineError error={{ message: "boom", data: "extra" }} />,
|
||||
);
|
||||
await user.click(screen.getByRole("button", { name: "Show more" }));
|
||||
expect(
|
||||
screen.getByRole("button", { name: "Show less" }),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.getByText("extra")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("formats non-string data as JSON", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(
|
||||
<InlineError error={{ message: "boom", data: { code: 42 } }} />,
|
||||
);
|
||||
await user.click(screen.getByRole("button", { name: "Show more" }));
|
||||
expect(screen.getByText(/"code": 42/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders a doc link button when docLink is provided and expanded", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(
|
||||
<InlineError
|
||||
error={{ message: "boom" }}
|
||||
docLink="https://example.com/docs"
|
||||
/>,
|
||||
);
|
||||
await user.click(screen.getByRole("button", { name: "Show more" }));
|
||||
const link = screen.getByRole("link", { name: /Troubleshooting/ });
|
||||
expect(link).toHaveAttribute("href", "https://example.com/docs");
|
||||
});
|
||||
});
|
||||
|
|
@ -1,96 +0,0 @@
|
|||
import { Alert, Button, Collapse, Group, Stack, Text } from "@mantine/core";
|
||||
import { useState } from "react";
|
||||
|
||||
export interface InlineErrorProps {
|
||||
error: { message: string; data?: unknown };
|
||||
retryCount?: number;
|
||||
maxRetries?: number;
|
||||
docLink?: string;
|
||||
}
|
||||
|
||||
const HeaderRow = Group.withProps({
|
||||
justify: "space-between",
|
||||
wrap: "nowrap",
|
||||
});
|
||||
|
||||
const MessageGroup = Group.withProps({
|
||||
gap: "sm",
|
||||
wrap: "nowrap",
|
||||
});
|
||||
|
||||
const ErrorMessage = Text.withProps({
|
||||
size: "sm",
|
||||
fw: 500,
|
||||
c: "red",
|
||||
});
|
||||
|
||||
const RetryText = Text.withProps({
|
||||
size: "sm",
|
||||
c: "dimmed",
|
||||
});
|
||||
|
||||
const ExpandButton = Button.withProps({
|
||||
variant: "transparent",
|
||||
size: "compact-xs",
|
||||
c: "var(--inspector-text-primary)",
|
||||
});
|
||||
|
||||
const DocLinkButton = Button.withProps({
|
||||
variant: "light",
|
||||
size: "xs",
|
||||
component: "a",
|
||||
target: "_blank",
|
||||
});
|
||||
|
||||
function formatRetryLabel(retryCount: number, maxRetries?: number): string {
|
||||
return maxRetries !== undefined
|
||||
? `Retry attempt ${retryCount} of ${maxRetries}`
|
||||
: `Retry attempt ${retryCount}`;
|
||||
}
|
||||
|
||||
function formatDetails(data: unknown): string {
|
||||
if (typeof data === "string") return data;
|
||||
return JSON.stringify(data, null, 2);
|
||||
}
|
||||
|
||||
export function InlineError({
|
||||
error,
|
||||
retryCount,
|
||||
maxRetries,
|
||||
docLink,
|
||||
}: InlineErrorProps) {
|
||||
const [expanded, setExpanded] = useState(false);
|
||||
const details =
|
||||
error.data !== undefined ? formatDetails(error.data) : undefined;
|
||||
const hasExpandable = details !== undefined || docLink !== undefined;
|
||||
|
||||
return (
|
||||
<Alert color="red" variant="light">
|
||||
<Stack gap="xs">
|
||||
<HeaderRow>
|
||||
<MessageGroup>
|
||||
<ErrorMessage>{error.message}</ErrorMessage>
|
||||
{retryCount !== undefined && (
|
||||
<RetryText>{formatRetryLabel(retryCount, maxRetries)}</RetryText>
|
||||
)}
|
||||
</MessageGroup>
|
||||
{hasExpandable && (
|
||||
<ExpandButton onClick={() => setExpanded((v) => !v)}>
|
||||
{expanded ? "Show less" : "Show more"}
|
||||
</ExpandButton>
|
||||
)}
|
||||
</HeaderRow>
|
||||
<Collapse in={expanded}>
|
||||
<Stack gap="xs">
|
||||
{details && <Text size="sm">{details}</Text>}
|
||||
{docLink && (
|
||||
<DocLinkButton href={docLink}>
|
||||
View Troubleshooting Guide
|
||||
</DocLinkButton>
|
||||
)}
|
||||
</Stack>
|
||||
</Collapse>
|
||||
</Stack>
|
||||
</Alert>
|
||||
);
|
||||
}
|
||||
Loading…
Reference in a new issue