diff --git a/README.md b/README.md index 1b813e38..29b74bd9 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,7 @@ Each client has its own README with client-specific detail: | [Publishing](./docs/publishing.md) | What ships in the tarball, the packaging invariants, and `pack:verify` | | [Docker](./docs/docker.md) | Running the container image — ports, volumes, and where secrets go | | [Migrating from v1 to v2](./docs/v1-to-v2-migration.md) | CLI flag mapping, `--config` vs. `--catalog`, the Node engine bump, env-var renames | +| [Environment variables](./docs/environment-variables.md) | Every variable that changes runtime behavior — auth, ports, storage, the secret store, logging, proxies — plus the Node TLS variables for a self-signed server | | [MCP server configuration](./docs/mcp-server-configuration.md) | Which server(s) the Inspector connects to, and the config file format | | [Reviewing an MCP App](./docs/mcp-app-review.md) | The CLI-first → one-shot-web recipe for automated App-tool review | | [Smoke-testing an MCP server](./docs/cli-smoke-testing.md) | The connect → list → call → assert workflow for a shell or CI job: `--format json` + `jq`, the exit-code map, and keeping OAuth non-interactive | diff --git a/clients/cli/README.md b/clients/cli/README.md index bd59c5d8..310f9e99 100644 --- a/clients/cli/README.md +++ b/clients/cli/README.md @@ -84,7 +84,7 @@ When a server is loaded from a `--catalog`/`--config` file, its per-server setti The file is the only durable way to give a run its roots: there is no roots flag, and `--method roots/set` applies only to that one short-lived connection. Roots configured for a server (the same field the web UI's Server Settings writes) are advertised at connect, so a server that asks for `roots/list` — `@modelcontextprotocol/server-filesystem` does, to learn its allowed directories — gets them. -**Environment-variable semantics.** `MCP_CATALOG_PATH` is honored only when no ad-hoc target is given (positional command, `--server-url`, or `--transport`) — so a shell that exports it can still run one-off ad-hoc invocations without hitting the catalog/ad-hoc conflict. `MCP_STORAGE_DIR` sets the storage directory used by the OAuth persist backend (`/oauth.json`); the per-file `MCP_INSPECTOR_OAUTH_STATE_PATH` override still takes precedence over it. +**Environment-variable semantics.** `MCP_CATALOG_PATH` is honored only when no ad-hoc target is given (positional command, `--server-url`, or `--transport`) — so a shell that exports it can still run one-off ad-hoc invocations without hitting the catalog/ad-hoc conflict. `MCP_STORAGE_DIR` sets the storage directory used by the OAuth persist backend (`/oauth.json`); the per-file `MCP_INSPECTOR_OAUTH_STATE_PATH` override still takes precedence over it. Every variable the CLI reads is listed in [Environment variables](../../docs/environment-variables.md). ### HTTP proxy support diff --git a/clients/launcher/README.md b/clients/launcher/README.md index 1f4409c1..9b4eec16 100644 --- a/clients/launcher/README.md +++ b/clients/launcher/README.md @@ -8,9 +8,9 @@ The launcher is the package that provides the global `mcp-inspector` binary (e.g - Forward all following arguments unchanged (including tokens that look like mode flags). - Dynamically import that app’s runner from `clients/{web,cli,tui}/build/index.js` (relative to the launcher build output) and call it **in-process** (no `spawn()`). -All configuration parsing, config-file loading, and server setup are handled by the app runners and by **core**; the launcher does not interpret config or env vars. +All configuration parsing, config-file loading, and server setup are handled by the app runners and by **core**; the launcher does not interpret server or client configuration, from files or from env vars. Its one exception is the debug switch described below. -**Error reporting.** A `--cli` failure is routed through the CLI's own error sink, so `mcp-inspector --cli` preserves the CLI exit-code map (`1` usage, `2` no-app, `3` auth-required, `4` unreachable, `5` tool-error) and its machine-readable `{"error":{…}}` stderr envelope — the same as invoking the CLI bin directly. `--web` / `--tui` failures print a human-readable `Error: ` and exit `1` (append `MCP_DEBUG=1` for the stack). +**Error reporting.** A `--cli` failure is routed through the CLI's own error sink, so `mcp-inspector --cli` preserves the CLI exit-code map (`1` usage, `2` no-app, `3` auth-required, `4` unreachable, `5` tool-error) and its machine-readable `{"error":{…}}` stderr envelope — the same as invoking the CLI bin directly. `--web` / `--tui` failures print a human-readable `Error: ` and exit `1` (append `MCP_DEBUG=1` for the stack). `MCP_DEBUG` (or `DEBUG`) is the only variable the launcher reads itself; the rest are listed in [Environment variables](../../docs/environment-variables.md). ## Web server-list flags (`--web`) diff --git a/clients/tui/README.md b/clients/tui/README.md index b604ad23..ba2c3c08 100644 --- a/clients/tui/README.md +++ b/clients/tui/README.md @@ -52,6 +52,8 @@ OAuth redirect URIs must match **exactly** what you register on the authorizatio #### Flags +Variables with no flag of their own — the secret store, `MCP_INSPECTOR_LOG_DIR`, `LOG_LEVEL`, proxies — are listed in [Environment variables](../../docs/environment-variables.md). + | Option | Env | Description | | ----------------------------- | ------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `--client-config ` | `MCP_CLIENT_CONFIG_PATH` | Install-level client config (default: `~/.mcp-inspector/storage/client.json`). | diff --git a/clients/web/README.md b/clients/web/README.md index 02d97a92..b2952821 100644 --- a/clients/web/README.md +++ b/clients/web/README.md @@ -384,7 +384,7 @@ Storybook is first-class here because the components are presentational — each ## Auth token -The dev/prod backend guards every `/api/*` route with `x-mcp-remote-auth: Bearer `. The browser recovers the token, in priority order (see `App.tsx` `getAuthToken()`): the `window.__INSPECTOR_API_TOKEN__` global injected into `index.html` on every page load (`server/inject-auth-token.ts`), then a `?MCP_INSPECTOR_API_TOKEN=…` query param, then `sessionStorage`. Injection is a no-op when auth is disabled (`DANGEROUSLY_OMIT_AUTH`). See the root [AGENTS.md](../../AGENTS.md) for the full rationale. +The dev/prod backend guards every `/api/*` route with `x-mcp-remote-auth: Bearer `. The browser recovers the token, in priority order (see `App.tsx` `getAuthToken()`): the `window.__INSPECTOR_API_TOKEN__` global injected into `index.html` on every page load (`server/inject-auth-token.ts`), then a `?MCP_INSPECTOR_API_TOKEN=…` query param, then `sessionStorage`. Injection is a no-op when auth is disabled (`DANGEROUSLY_OMIT_AUTH`). See the root [AGENTS.md](../../AGENTS.md) for the full rationale, and [Environment variables](../../docs/environment-variables.md) for every variable the backend reads. ## Host binding & the origin allow-list diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index 64bb3cfc..cc86b827 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -714,6 +714,7 @@ const DEFAULT_USE_INSPECTOR_CLIENT: ReturnType = { instructions: undefined, excludedTools: [], malformedListItems: [], + connectionDiagnostics: { capturedAt: 0, outstandingRequests: [] }, appRendererClient: null, connect: vi.fn().mockResolvedValue(undefined), disconnect: vi.fn().mockResolvedValue(undefined), diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index bc62376e..62959775 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -401,6 +401,7 @@ function App() { discoverResult, excludedTools, malformedListItems, + connectionDiagnostics, lastError, } = useInspectorClient(inspectorClient); // What every settings write that landed on disk actually wrote, so a later @@ -2028,6 +2029,7 @@ function App() { transport={connectionInfoTransport} protocolEra={protocolEra} discoverResult={discoverResult} + diagnostics={connectionDiagnostics} oauth={connectionInfoOAuth} onClearOAuth={ connectionInfoCanClearOAuth ? handleClearConnectionOAuth : undefined diff --git a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.stories.tsx b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.stories.tsx index 50549076..8e63e5a3 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.stories.tsx +++ b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.stories.tsx @@ -46,6 +46,51 @@ export const FullCapabilities: Story = { }, }; +// The snapshot clock, read once when the stories load. The rows start from it +// and then tick against the wall clock, so the fixture has to be "now" — a +// fixed historical date would read as months ago a second after mounting. +const STORY_NOW = Date.now(); + +/** + * The Connection Activity section, in the #2187 shape: the notification + * stream has been open the whole session and delivered nothing, `tools/list` + * has been outstanding for a minute, and nothing has been answered since + * `initialize`. The same facts a request timeout's message reports, shown + * while the request is still in flight (#2318). + */ +export const WithConnectionActivity: Story = { + args: { + initializeResult: fullResult, + clientCapabilities: fullClientCaps, + transport: "streamable-http", + diagnostics: { + capturedAt: STORY_NOW, + outstandingRequests: [ + { id: 2, method: "tools/list", sentAt: STORY_NOW - 60_000 }, + { id: 3, method: "ping", sentAt: STORY_NOW - 12_000 }, + ], + lastResponse: { method: "initialize", receivedAt: STORY_NOW - 61_000 }, + notificationStream: { + url: "http://127.0.0.1:9779/mcp", + openedAt: STORY_NOW - 252_000, + eventCount: 0, + }, + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.getByText("Connection Activity")).toBeInTheDocument(); + // Ticking: the rows may have advanced a second or two since the fixture + // was stamped, so match the shape rather than the exact second. + await expect( + canvas.getByText(/^tools\/list — sent 1m0\ds ago$/), + ).toBeInTheDocument(); + await expect( + canvas.getByText(/^GET \/mcp — open for 4m1\ds, 0 events delivered$/), + ).toBeInTheDocument(); + }, +}; + export const ModernEra: Story = { args: { initializeResult: { diff --git a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx index ba4fb132..00d6b61d 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx +++ b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx @@ -5,6 +5,13 @@ import type { InitializeResult, } from "@modelcontextprotocol/client"; import { renderWithMantine, screen } from "../../../test/renderWithMantine"; +import type { ConnectionDiagnostics } from "@inspector/core/mcp/connectionDiagnostics.js"; +import { + NO_NOTIFICATION_STREAM_LABEL, + NO_OUTSTANDING_REQUESTS_LABEL, + NO_RESPONSE_YET_LABEL, + NO_STREAM_ON_STDIO_LABEL, +} from "../../../utils/connectionActivity"; import { CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL, ConnectionInfoContent, @@ -934,3 +941,81 @@ describe("ConnectionInfoContent", () => { expect(onClearOAuth).toHaveBeenCalledTimes(1); }); }); + +describe("ConnectionInfoContent connection activity (#2318)", () => { + const NOW = 1_000_000; + const seconds = (n: number) => n * 1000; + + it("omits the section when no diagnostics are supplied", () => { + renderWithMantine( + , + ); + expect(screen.queryByText("Connection Activity")).not.toBeInTheDocument(); + }); + + it("renders the idle state: nothing unanswered, nothing received, stream not opened", () => { + renderWithMantine( + , + ); + expect(screen.getByText("Connection Activity")).toBeInTheDocument(); + expect(screen.getByText(NO_OUTSTANDING_REQUESTS_LABEL)).toBeInTheDocument(); + expect(screen.getByText(NO_RESPONSE_YET_LABEL)).toBeInTheDocument(); + expect(screen.getByText(NO_NOTIFICATION_STREAM_LABEL)).toBeInTheDocument(); + }); + + it("lists each unanswered request on its own line, with the last response and the open stream", () => { + const diagnostics: ConnectionDiagnostics = { + capturedAt: NOW, + outstandingRequests: [ + { id: 2, method: "tools/list", sentAt: NOW - seconds(60) }, + { id: 3, method: "ping", sentAt: NOW - seconds(12) }, + ], + lastResponse: { method: "initialize", receivedAt: NOW - seconds(61) }, + notificationStream: { + url: "http://127.0.0.1:9779/mcp", + openedAt: NOW - seconds(252), + eventCount: 0, + }, + }; + renderWithMantine( + , + ); + const outstanding = screen.getByTestId("connection-activity-outstanding"); + expect(outstanding.children).toHaveLength(2); + expect(outstanding.children[0]?.textContent).toBe( + "tools/list — sent 1m00s ago", + ); + expect(outstanding.children[1]?.textContent).toBe("ping — sent 12s ago"); + expect(screen.getByText("initialize — 1m01s ago")).toBeInTheDocument(); + expect( + screen.getByText("GET /mcp — open for 4m12s, 0 events delivered"), + ).toBeInTheDocument(); + }); + + it("reports the stream row as not applicable on stdio", () => { + renderWithMantine( + , + ); + // Both the Session row and the Notification stream row read N/A (stdio). + expect(screen.getAllByText(NO_STREAM_ON_STDIO_LABEL)).toHaveLength(2); + }); +}); diff --git a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.tsx b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.tsx index 1c04c951..6c604880 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.tsx +++ b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.tsx @@ -18,6 +18,14 @@ import type { ServerCapabilities, } from "@modelcontextprotocol/client"; import type { ServerType } from "@inspector/core/mcp/types.js"; +import type { ConnectionDiagnostics } from "@inspector/core/mcp/connectionDiagnostics.js"; +import { + formatLastResponse, + formatNotificationStream, + formatOutstandingRequests, + NO_OUTSTANDING_REQUESTS_LABEL, +} from "../../../utils/connectionActivity"; +import { useTickingClock } from "../../../hooks/useTickingClock"; import { TASKS_EXTENSION_KEY } from "@inspector/core/mcp/modernTaskSchemas.js"; import { getSkillsExtension } from "@inspector/core/mcp/skills.js"; import type { OAuthClientRegistrationKind } from "@inspector/core/auth/types.js"; @@ -75,6 +83,16 @@ export interface ConnectionInfoContentProps { * capabilities, and extensions learned up front. Undefined on legacy. (#1626) */ discoverResult?: DiscoverResult; + /** + * What the client is still waiting on, when it last heard back, and the + * state of the notification stream (#2318). Renders the Connection Activity + * section when present — the same facts a request timeout reports, shown + * here while the request is still in flight. Durations first read against + * the snapshot's own `capturedAt` and then tick once a second while the + * panel is open (`useTickingClock`), so an in-flight request's age keeps + * moving rather than freezing at the moment it went out. + */ + diagnostics?: ConnectionDiagnostics; oauth?: OAuthDetails; onClearOAuth?: () => void; } @@ -291,6 +309,7 @@ export function ConnectionInfoContent({ transport, protocolEra, discoverResult, + diagnostics, oauth, onClearOAuth, }: ConnectionInfoContentProps) { @@ -317,6 +336,15 @@ export function ConnectionInfoContent({ ? serverInfo.version?.trim() || "—" : SERVER_INFO_NOT_REPORTED_LABEL; + // The activity rows' clock: the snapshot's own on first paint (pure), then + // the wall clock once a second so "sent 5s ago" keeps counting. No timer + // at all when there is no activity section to tick. + const now = useTickingClock( + diagnostics?.capturedAt ?? 0, + 1000, + diagnostics !== undefined, + ); + const serverCaps = getServerCapabilityEntries(capabilities, protocolEra); const clientCaps = getCapabilityEntries( clientCapabilities, @@ -348,6 +376,42 @@ export function ConnectionInfoContent({ + {diagnostics && ( + + Connection Activity + + Unanswered requests + {/* A `Stack` so several in-flight requests read as a list, the + way the extension sections do; one line per request. */} + + {diagnostics.outstandingRequests.length === 0 ? ( + {NO_OUTSTANDING_REQUESTS_LABEL} + ) : ( + formatOutstandingRequests(diagnostics, now).map( + (line, index) => ( + + {line} + + ), + ) + )} + + + Last response + {formatLastResponse(diagnostics, now)} + + Notification stream + + {formatNotificationStream( + diagnostics.notificationStream, + transport, + now, + )} + + + + )} + {discoverResult && ( Discovery diff --git a/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.test.tsx b/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.test.tsx index c73acff5..041745f3 100644 --- a/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.test.tsx +++ b/clients/web/src/components/groups/ConnectionInfoModal/ConnectionInfoModal.test.tsx @@ -56,6 +56,27 @@ describe("ConnectionInfoModal", () => { expect(screen.getByText("Be excellent to each other.")).toBeInTheDocument(); }); + it("forwards connection diagnostics to ConnectionInfoContent (#2318)", () => { + renderWithMantine( + , + ); + expect(screen.getByText("Connection Activity")).toBeInTheDocument(); + expect(screen.getByText("tools/list — sent 1m00s ago")).toBeInTheDocument(); + }); + it("forwards oauth details to ConnectionInfoContent", () => { renderWithMantine( void; } @@ -51,6 +54,7 @@ export function ConnectionInfoModal({ transport, protocolEra, discoverResult, + diagnostics, oauth, onClearOAuth, }: ConnectionInfoModalProps) { @@ -79,6 +83,7 @@ export function ConnectionInfoModal({ transport={transport} protocolEra={protocolEra} discoverResult={discoverResult} + diagnostics={diagnostics} oauth={oauth} onClearOAuth={onClearOAuth} /> diff --git a/clients/web/src/components/groups/NetworkEntry/NetworkEntry.stories.tsx b/clients/web/src/components/groups/NetworkEntry/NetworkEntry.stories.tsx index 21349a41..725702d2 100644 --- a/clients/web/src/components/groups/NetworkEntry/NetworkEntry.stories.tsx +++ b/clients/web/src/components/groups/NetworkEntry/NetworkEntry.stories.tsx @@ -86,6 +86,31 @@ const streamingEntry: FetchRequestEntry = { category: "transport", }; +/** + * The standalone `GET /mcp` notification stream, still open, as the tracker + * watches it: the body is never captured, so the placeholder reports what the + * stream has delivered instead (#2318). + */ +const notificationStreamEntry: FetchRequestEntry = { + id: "n-notify", + timestamp: new Date("2026-03-17T10:30:01Z"), + method: "GET", + url: "http://localhost:3000/mcp", + requestHeaders: { + accept: "text/event-stream", + "mcp-session-id": "0a0b0a5-fd27-4c95-a805-c0fba67e00fb", + }, + responseStatus: 200, + responseStatusText: "OK", + responseHeaders: { + "cache-control": "no-cache", + "content-type": "text/event-stream", + }, + duration: 12, + stream: { eventCount: 0 }, + category: "transport", +}; + const transportError: FetchRequestEntry = { id: "n-4", timestamp: new Date("2026-03-17T10:30:15Z"), @@ -166,6 +191,18 @@ export const StreamingResponse: Story = { args: { entry: streamingEntry, isListExpanded: true }, }; +export const NotificationStreamOpen: Story = { + args: { entry: notificationStreamEntry, isListExpanded: true }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect( + canvas.getByText( + "Long-lived stream — 0 events delivered, still open; body not captured", + ), + ).toBeInTheDocument(); + }, +}; + export const FetchError: Story = { args: { entry: transportError, isListExpanded: true }, }; diff --git a/clients/web/src/components/groups/NetworkEntry/NetworkEntry.test.tsx b/clients/web/src/components/groups/NetworkEntry/NetworkEntry.test.tsx index e42d841c..e5fa1e70 100644 --- a/clients/web/src/components/groups/NetworkEntry/NetworkEntry.test.tsx +++ b/clients/web/src/components/groups/NetworkEntry/NetworkEntry.test.tsx @@ -150,6 +150,24 @@ describe("NetworkEntry", () => { ).toBeInTheDocument(); }); + it("shows the watched stream's event count and open state in place of the body (#2318)", async () => { + const user = userEvent.setup(); + const open: FetchRequestEntry = { + ...baseEntry, + method: "GET", + responseHeaders: { "content-type": "text/event-stream" }, + responseBody: undefined, + stream: { eventCount: 3 }, + }; + renderWithMantine(); + await user.click(screen.getByRole("button", { name: "Expand" })); + expect( + screen.getByText( + "Long-lived stream — 3 events delivered, still open; body not captured", + ), + ).toBeInTheDocument(); + }); + it("shows an SSE badge on long-lived GET event-stream entries", () => { const sse: FetchRequestEntry = { ...baseEntry, diff --git a/clients/web/src/components/groups/NetworkEntry/NetworkEntry.tsx b/clients/web/src/components/groups/NetworkEntry/NetworkEntry.tsx index 8aecb0fc..b19fe362 100644 --- a/clients/web/src/components/groups/NetworkEntry/NetworkEntry.tsx +++ b/clients/web/src/components/groups/NetworkEntry/NetworkEntry.tsx @@ -15,7 +15,11 @@ import { } from "@mantine/core"; import { RiErrorWarningLine } from "react-icons/ri"; import type { FetchRequestEntry } from "@inspector/core/mcp/types.js"; -import { isLongLivedStreamResponse } from "@inspector/core/mcp/fetchTracking.js"; +import { + isLongLivedStreamEntry, + longLivedStreamLabel, + uncapturedBodyNote, +} from "../../../utils/uncapturedBodyNote"; import { ContentViewer } from "../../elements/ContentViewer/ContentViewer"; import { getMimeKind } from "../../elements/ContentViewer/contentViewerUtils"; import { CopyButton } from "../../elements/CopyButton/CopyButton"; @@ -158,13 +162,6 @@ function statusLabel(entry: FetchRequestEntry): string { : `${entry.responseStatus}`; } -function isLongLivedStream(entry: FetchRequestEntry): boolean { - return isLongLivedStreamResponse( - entry.method, - entry.responseHeaders?.["content-type"], - ); -} - // Header-table cell text. A modern MCP-mirrored header name gets a violet accent // so the spec headers (Mcp-Method / Mcp-Name / Mcp-Param-* / MCP-Protocol-Version) // stand out from ordinary ones; a value that disagrees with the request body is @@ -217,6 +214,17 @@ const SentinelTooltip = Tooltip.withProps({ w: 280, }); +// The long-lived-stream marker on a GET entry, on the `status` variant like +// the status chip beside it. Amber cannot clear WCAG AA as either a filled +// (white on orange, 3.04:1) or a light (orange on tint, 3.03:1) badge at this +// size; `ThemeBadge` pins `status` + orange to a mid amber fill with black +// text for exactly that reason, which is what the a11y gate needs the moment a +// story renders one (#2318 added the first). +const StreamBadge = Badge.withProps({ + color: "orange", + variant: "status", +}); + const Base64Badge = Badge.withProps({ size: "xs", color: "gray", @@ -542,7 +550,9 @@ export function NetworkEntry({ {entry.duration != null && ( {formatDuration(entry.duration)} )} - {isLongLivedStream(entry) && SSE} + {isLongLivedStreamEntry(entry) && ( + {longLivedStreamLabel(entry)} + )} {statusLabel(entry)} @@ -647,11 +657,7 @@ export function NetworkEntry({ label="Response body" /> ) : ( - - {isLongLivedStream(entry) - ? "Long-lived stream — body not captured" - : "(empty)"} - + {uncapturedBodyNote(entry)} )} )} diff --git a/clients/web/src/hooks/useTickingClock.test.ts b/clients/web/src/hooks/useTickingClock.test.ts new file mode 100644 index 00000000..dc459353 --- /dev/null +++ b/clients/web/src/hooks/useTickingClock.test.ts @@ -0,0 +1,80 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { act, renderHook } from "@testing-library/react"; +import { useTickingClock } from "./useTickingClock"; + +describe("useTickingClock", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("reads the seed on the first render, before any tick", () => { + vi.useFakeTimers(); + vi.setSystemTime(5_000_000); + const { result } = renderHook(() => useTickingClock(1_000_000)); + expect(result.current).toBe(1_000_000); + }); + + it("advances to the wall clock on each interval", () => { + vi.useFakeTimers(); + vi.setSystemTime(5_000_000); + const { result } = renderHook(() => useTickingClock(1_000_000, 1000)); + act(() => { + vi.advanceTimersByTime(1000); + }); + expect(result.current).toBe(5_001_000); + act(() => { + vi.setSystemTime(5_010_000); + // Advancing the fake timers also advances the fake clock. + vi.advanceTimersByTime(1000); + }); + expect(result.current).toBe(5_011_000); + }); + + it("never reads behind a seed newer than the last tick", () => { + vi.useFakeTimers(); + vi.setSystemTime(5_000_000); + const { result, rerender } = renderHook( + ({ seed }) => useTickingClock(seed, 1000), + { initialProps: { seed: 1_000_000 } }, + ); + act(() => { + vi.advanceTimersByTime(1000); + }); + expect(result.current).toBe(5_001_000); + // A fresher snapshot arrives before the next tick: it is what the caller + // renders from, so it must not be measured against an older clock. + rerender({ seed: 5_002_500 }); + expect(result.current).toBe(5_002_500); + }); + + it("installs no timer and returns the seed when disabled", () => { + vi.useFakeTimers(); + vi.setSystemTime(5_000_000); + const setSpy = vi.spyOn(globalThis, "setInterval"); + const { result, rerender } = renderHook( + ({ enabled }) => useTickingClock(1_000_000, 1000, enabled), + { initialProps: { enabled: false } }, + ); + expect(setSpy).not.toHaveBeenCalled(); + act(() => { + vi.advanceTimersByTime(3000); + }); + expect(result.current).toBe(1_000_000); + // Enabling later starts the clock. + rerender({ enabled: true }); + act(() => { + vi.advanceTimersByTime(1000); + }); + expect(result.current).toBe(5_004_000); + setSpy.mockRestore(); + }); + + it("stops ticking on unmount", () => { + vi.useFakeTimers(); + const clearSpy = vi.spyOn(globalThis, "clearInterval"); + const { unmount } = renderHook(() => useTickingClock(0, 250)); + unmount(); + expect(clearSpy).toHaveBeenCalled(); + clearSpy.mockRestore(); + }); +}); diff --git a/clients/web/src/hooks/useTickingClock.ts b/clients/web/src/hooks/useTickingClock.ts new file mode 100644 index 00000000..4dc47123 --- /dev/null +++ b/clients/web/src/hooks/useTickingClock.ts @@ -0,0 +1,35 @@ +import { useEffect, useState } from "react"; + +/** + * A clock that starts at `seed` and advances to the wall clock once per + * `intervalMs` while the component is mounted (#2318). + * + * For rendering "N seconds ago" from a snapshot stamped with its own + * `capturedAt`: the first render reads the snapshot's clock (pure — no + * `Date.now()` in render, which `react-hooks/purity` reports), and the + * interval then keeps the displayed durations moving while the panel stays + * open. The seed also floors the result, so a snapshot newer than the last + * tick is never rendered against a clock that predates it — a duration + * cannot go negative, and a freshly captured snapshot reads as fresh until the + * next tick catches up. + * + * A timer is exactly the "synchronize with an external system" case an effect + * exists for; the state is set only from the interval callback, never in the + * effect body, so the first paint is not followed by a corrective re-render. + * `enabled: false` installs no timer at all and returns the seed. + */ +export function useTickingClock( + seed: number, + intervalMs = 1000, + enabled = true, +): number { + const [now, setNow] = useState(seed); + useEffect(() => { + // No timer for a consumer with nothing to tick — a component that + // renders no durations must not pay for a repeating interval. + if (!enabled) return; + const id = setInterval(() => setNow(Date.now()), intervalMs); + return () => clearInterval(id); + }, [intervalMs, enabled]); + return Math.max(now, seed); +} diff --git a/clients/web/src/test/core/mcp/authResyncPropagation.test.ts b/clients/web/src/test/core/mcp/authResyncPropagation.test.ts index 96e03fc3..604ac63b 100644 --- a/clients/web/src/test/core/mcp/authResyncPropagation.test.ts +++ b/clients/web/src/test/core/mcp/authResyncPropagation.test.ts @@ -117,7 +117,7 @@ describe("RemoteClientTransport send contract", () => { const started = Date.now(); await expect( transport.send({ jsonrpc: "2.0", id: 1, method: "tools/list" }), - ).rejects.toThrow(/Timed out waiting for MCP response on SSE/i); + ).rejects.toThrow(/Request timed out/); expect(Date.now() - started).toBeLessThan(5000); await transport.close(); diff --git a/clients/web/src/test/core/mcp/connectionDiagnostics.test.ts b/clients/web/src/test/core/mcp/connectionDiagnostics.test.ts new file mode 100644 index 00000000..d86a111c --- /dev/null +++ b/clients/web/src/test/core/mcp/connectionDiagnostics.test.ts @@ -0,0 +1,279 @@ +import { describe, it, expect } from "vitest"; +import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client"; +import { + annotateRequestTimeout, + describeConnectionDiagnostics, + describeStreamEndpoint, + formatDuration, + isRequestTimeoutError, + MAX_LISTED_OUTSTANDING_REQUESTS, + type AnnotatedRequestTimeoutData, + type ConnectionDiagnostics, +} from "@inspector/core/mcp/connectionDiagnostics.js"; + +// A fixed clock so every duration in these assertions is exact. +const NOW = 1_000_000; +const seconds = (n: number) => n * 1000; + +/** The SDK's own per-request timeout, exactly as `Protocol.request` throws it. */ +function sdkTimeout(timeout = 60_000): SdkError { + return new SdkError(SdkErrorCode.RequestTimeout, "Request timed out", { + timeout, + }); +} + +describe("formatDuration", () => { + it("renders sub-second values in milliseconds", () => { + expect(formatDuration(0)).toBe("0ms"); + expect(formatDuration(850)).toBe("850ms"); + expect(formatDuration(999.6)).toBe("1000ms"); + }); + + it("renders seconds, minutes and hours at increasing granularity", () => { + expect(formatDuration(seconds(1))).toBe("1s"); + expect(formatDuration(seconds(59))).toBe("59s"); + expect(formatDuration(seconds(60))).toBe("1m00s"); + expect(formatDuration(seconds(252))).toBe("4m12s"); + expect(formatDuration(seconds(3600))).toBe("1h00m"); + expect(formatDuration(seconds(3600 + 120 + 5))).toBe("1h02m"); + }); + + it("clamps a negative value to zero rather than rendering nonsense", () => { + // A clock skew between two timestamps must not print `-3s`. + expect(formatDuration(-3000)).toBe("0ms"); + }); +}); + +describe("describeStreamEndpoint", () => { + it("reduces a stream URL to its method and path", () => { + expect(describeStreamEndpoint("https://host.example:8443/mcp?x=1")).toBe( + "GET /mcp", + ); + }); + + it("falls back to the raw string for an unparseable URL", () => { + expect(describeStreamEndpoint("not a url")).toBe("GET not a url"); + }); +}); + +describe("describeConnectionDiagnostics", () => { + it("says so when nothing is outstanding and nothing has been received", () => { + expect( + describeConnectionDiagnostics({ + capturedAt: NOW, + outstandingRequests: [], + }), + ).toBe( + "No requests are unanswered. No response has been received this session.", + ); + }); + + it("names each unanswered request with how long ago it went out", () => { + const diagnostics: ConnectionDiagnostics = { + capturedAt: NOW, + outstandingRequests: [ + { id: 2, method: "tools/list", sentAt: NOW - seconds(60) }, + { id: 3, method: "ping", sentAt: NOW - seconds(12) }, + ], + lastResponse: { method: "initialize", receivedAt: NOW - seconds(61) }, + }; + expect(describeConnectionDiagnostics(diagnostics)).toBe( + "2 requests are unanswered: tools/list (sent 1m00s ago), ping (sent 12s ago). " + + "Last response received 1m01s ago (initialize).", + ); + }); + + it("uses the singular for exactly one unanswered request", () => { + const diagnostics: ConnectionDiagnostics = { + capturedAt: NOW, + outstandingRequests: [ + { id: 2, method: "tools/list", sentAt: NOW - seconds(60) }, + ], + }; + expect(describeConnectionDiagnostics(diagnostics)).toContain( + "1 request is unanswered: tools/list (sent 1m00s ago).", + ); + }); + + it("collapses the tail of a long outstanding list into a count", () => { + const outstandingRequests = Array.from({ length: 8 }, (_, i) => ({ + id: i, + method: `m${i}`, + sentAt: NOW - seconds(i + 1), + })); + const text = describeConnectionDiagnostics({ + capturedAt: NOW, + outstandingRequests, + }); + expect(text).toContain("8 requests are unanswered: "); + for (let i = 0; i < MAX_LISTED_OUTSTANDING_REQUESTS; i++) { + expect(text).toContain(`m${i} (sent`); + } + expect(text).not.toContain("m5 (sent"); + expect(text).toContain( + `, and ${8 - MAX_LISTED_OUTSTANDING_REQUESTS} more.`, + ); + }); + + it("describes an open notification stream by its endpoint, age and event count", () => { + const diagnostics: ConnectionDiagnostics = { + capturedAt: NOW, + outstandingRequests: [], + notificationStream: { + url: "http://127.0.0.1:9779/mcp", + openedAt: NOW - seconds(252), + eventCount: 0, + }, + }; + expect(describeConnectionDiagnostics(diagnostics)).toContain( + "Notification stream (GET /mcp) open for 4m12s, 0 events delivered.", + ); + }); + + it("describes a closed notification stream by when it closed and how long it lasted", () => { + const diagnostics: ConnectionDiagnostics = { + capturedAt: NOW, + outstandingRequests: [], + notificationStream: { + url: "http://127.0.0.1:9779/mcp", + openedAt: NOW - seconds(185), + eventCount: 1, + closedAt: NOW - seconds(5), + }, + }; + expect(describeConnectionDiagnostics(diagnostics)).toContain( + "Notification stream (GET /mcp) closed 5s ago after 3m00s, 1 event delivered.", + ); + }); + + it("measures every duration against the snapshot's own clock, not the wall clock", () => { + // A snapshot taken long ago still reads the way it read when it was + // taken — that is what makes a timeout message and a later render of the + // same snapshot agree. + const diagnostics: ConnectionDiagnostics = { + capturedAt: NOW, + outstandingRequests: [ + { id: 1, method: "tools/list", sentAt: NOW - seconds(5) }, + ], + }; + expect(describeConnectionDiagnostics(diagnostics)).toBe( + "1 request is unanswered: tools/list (sent 5s ago). " + + "No response has been received this session.", + ); + }); +}); + +describe("isRequestTimeoutError", () => { + it("recognises the SDK's per-request timeout", () => { + expect(isRequestTimeoutError(sdkTimeout())).toBe(true); + }); + + it("rejects the other RequestTimeout-coded rejections, which are not timeouts", () => { + // An abort through `options.signal` with a non-SdkError reason: the SDK + // wraps it in the same code with no `{ timeout }` data. + expect( + isRequestTimeoutError( + new SdkError(SdkErrorCode.RequestTimeout, "AbortError: cancelled"), + ), + ).toBe(false); + // The max-total-timeout variant carries different data. + expect( + isRequestTimeoutError( + new SdkError( + SdkErrorCode.RequestTimeout, + "Maximum total timeout exceeded", + { maxTotalTimeout: 1000, totalElapsed: 1200 }, + ), + ), + ).toBe(false); + // A non-numeric `timeout` is not the SDK's shape either. + expect( + isRequestTimeoutError( + new SdkError(SdkErrorCode.RequestTimeout, "Request timed out", { + timeout: "60s", + }), + ), + ).toBe(false); + }); + + it("rejects other SdkErrors, plain errors and non-errors", () => { + expect( + isRequestTimeoutError( + new SdkError(SdkErrorCode.InvalidResult, "bad", { timeout: 1 }), + ), + ).toBe(false); + expect(isRequestTimeoutError(new Error("Request timed out"))).toBe(false); + expect(isRequestTimeoutError("Request timed out")).toBe(false); + expect(isRequestTimeoutError(null)).toBe(false); + }); +}); + +describe("annotateRequestTimeout", () => { + const diagnostics: ConnectionDiagnostics = { + capturedAt: NOW, + outstandingRequests: [ + { id: 2, method: "tools/list", sentAt: NOW - seconds(60) }, + ], + lastResponse: { method: "initialize", receivedAt: NOW - seconds(61) }, + notificationStream: { + url: "http://127.0.0.1:9779/mcp", + openedAt: NOW - seconds(62), + eventCount: 0, + }, + }; + + it("rewrites the message to name the request, its budget and the connection state", () => { + const annotated = annotateRequestTimeout( + sdkTimeout(60_000), + "tools/list", + diagnostics, + ); + expect(SdkError.isInstance(annotated)).toBe(true); + expect((annotated as SdkError).message).toBe( + "Request timed out after 1m00s (tools/list). " + + "1 request is unanswered: tools/list (sent 1m00s ago). " + + "Last response received 1m01s ago (initialize). " + + "Notification stream (GET /mcp) open for 1m02s, 0 events delivered.", + ); + }); + + it("keeps the code, carries the method and snapshot in data, and links the original as cause", () => { + const original = sdkTimeout(300); + const annotated = annotateRequestTimeout( + original, + "ping", + diagnostics, + ) as SdkError; + expect(annotated).not.toBe(original); + expect(annotated.code).toBe(SdkErrorCode.RequestTimeout); + expect(annotated.cause).toBe(original); + const data = annotated.data as AnnotatedRequestTimeoutData; + expect(data.timeout).toBe(300); + expect(data.method).toBe("ping"); + expect(data.diagnostics).toBe(diagnostics); + // The rebuilt error still satisfies the predicate, so a consumer that + // checks before and after this runs sees the same answer. + expect(isRequestTimeoutError(annotated)).toBe(true); + }); + + it("returns anything that is not the per-request timeout untouched", () => { + const plain = new Error("boom"); + expect(annotateRequestTimeout(plain, "ping", diagnostics)).toBe(plain); + const abort = new SdkError(SdkErrorCode.RequestTimeout, "cancelled"); + expect(annotateRequestTimeout(abort, "ping", diagnostics)).toBe(abort); + expect(annotateRequestTimeout(undefined, "ping", diagnostics)).toBe( + undefined, + ); + }); + + it("reads cleanly when nothing else is known about the connection", () => { + const annotated = annotateRequestTimeout(sdkTimeout(1000), "ping", { + capturedAt: NOW, + outstandingRequests: [], + }) as SdkError; + expect(annotated.message).toBe( + "Request timed out after 1s (ping). No requests are unanswered. " + + "No response has been received this session.", + ); + }); +}); diff --git a/clients/web/src/test/core/mcp/fetchTracking.test.ts b/clients/web/src/test/core/mcp/fetchTracking.test.ts index 76abcb0f..0419b840 100644 --- a/clients/web/src/test/core/mcp/fetchTracking.test.ts +++ b/clients/web/src/test/core/mcp/fetchTracking.test.ts @@ -1,13 +1,17 @@ import { describe, it, expect, vi } from "vitest"; import { createFetchTracker, + findHeader, redactSensitiveHeaders, redactBody, redactUrlQuery, REDACTED_HEADER_VALUE, REDACTED_VALUE, } from "@inspector/core/mcp/fetchTracking.js"; -import type { FetchRequestEntryBase } from "@inspector/core/mcp/types.js"; +import type { + FetchRequestEntryBase, + FetchStreamState, +} from "@inspector/core/mcp/types.js"; // The tracker fires `trackRequest` synchronously with an entry whose // responseBody is always undefined, then reads the body in the background @@ -696,3 +700,389 @@ describe("redactSensitiveHeaders", () => { expect(input.authorization).toBe("Bearer x"); }); }); + +describe("findHeader", () => { + it("looks a header up case-insensitively and tolerates a missing record", () => { + expect(findHeader({ "Content-Type": "text/plain" }, "content-type")).toBe( + "text/plain", + ); + expect(findHeader({ "content-type": "text/plain" }, "CONTENT-TYPE")).toBe( + "text/plain", + ); + expect(findHeader({ accept: "*/*" }, "content-type")).toBeUndefined(); + expect(findHeader(undefined, "content-type")).toBeUndefined(); + }); +}); + +describe("createFetchTracker long-lived stream watching (#2318)", () => { + const encoder = new TextEncoder(); + + /** + * A server-controlled SSE body: `push` writes a chunk, `end` closes it, and + * `fail` errors it — the three ways a real stream advances. + */ + function controlledStream() { + let controller!: ReadableStreamDefaultController; + const stream = new ReadableStream({ + start(c) { + controller = c; + }, + }); + return { + stream, + push: (text: string) => controller.enqueue(encoder.encode(text)), + end: () => controller.close(), + fail: (err: Error) => controller.error(err), + }; + } + + /** Track a GET whose response is the given stream, collecting stream updates. */ + async function trackStream(stream: ReadableStream) { + const updates: Array<{ id: string; stream: FetchStreamState }> = []; + const tracked: FetchRequestEntryBase[] = []; + const fetcher = createFetchTracker( + (async () => + new Response(stream, { + headers: { "content-type": "text/event-stream" }, + })) as typeof fetch, + { + trackRequest: (entry) => tracked.push(entry), + updateStream: (id, state) => updates.push({ id, stream: state }), + streamUpdateIntervalMs: 0, + }, + ); + const response = await fetcher("https://example.com/mcp", { + method: "GET", + }); + return { response, updates, id: tracked[0]!.id }; + } + + it("announces the stream as open with zero events as soon as it is watched", async () => { + const server = controlledStream(); + const { updates, id } = await trackStream(server.stream); + // Before a byte arrives — this is the state the #2187 stall shows. + expect(updates).toEqual([{ id, stream: { eventCount: 0 } }]); + }); + + it("counts each data-carrying SSE event as it is delivered", async () => { + const server = controlledStream(); + const { updates, id } = await trackStream(server.stream); + server.push('event: message\ndata: {"jsonrpc":"2.0","method":"a"}\n\n'); + await flush(); + expect(updates.at(-1)).toEqual({ id, stream: { eventCount: 1 } }); + // An event split across chunks is still one event. + server.push('data: {"jsonrpc"'); + server.push(':"2.0","method":"b"}\n'); + await flush(); + expect(updates.map((u) => u.stream.eventCount)).toEqual([0, 1]); + server.push("\n"); + await flush(); + expect(updates.map((u) => u.stream.eventCount)).toEqual([0, 1, 2]); + }); + + it("ignores keepalive comments and blocks with no data field, and accepts a bare `data` line and CRLF framing", async () => { + const server = controlledStream(); + const { updates } = await trackStream(server.stream); + // A comment-only block (the keepalive shape) and an id-only block are + // not events; a bare `data` (no colon) is; so is a CRLF-delimited block. + server.push(": keepalive\n\n"); + server.push("id: 7\n\n"); + server.push("data\n\n"); + server.push("data: x\r\n\r\n"); + await flush(); + expect(updates.map((u) => u.stream.eventCount)).toEqual([0, 1, 2]); + }); + + it("counts every non-blank line on an NDJSON stream", async () => { + const server = controlledStream(); + const updates: Array<{ id: string; stream: FetchStreamState }> = []; + const fetcher = createFetchTracker( + (async () => + new Response(server.stream, { + headers: { "content-type": "application/x-ndjson" }, + })) as typeof fetch, + { + updateStream: (id, state) => updates.push({ id, stream: state }), + streamUpdateIntervalMs: 0, + }, + ); + await fetcher("https://example.com/mcp", { method: "GET" }); + server.push('{"jsonrpc":"2.0","method":"a"}\n\n'); + server.push('{"jsonrpc":"2.0","method":"b"}\r\n'); + // A partial line is not an event until its newline arrives. + server.push('{"jsonrpc":"2.0",'); + await flush(); + expect(updates.map((u) => u.stream.eventCount)).toEqual([0, 1, 2]); + server.push('"method":"c"}\n'); + await flush(); + expect(updates.at(-1)!.stream.eventCount).toBe(3); + }); + + it("drops a partial SSE block at end-of-stream, as the SDK's parser does", async () => { + const server = controlledStream(); + const { updates } = await trackStream(server.stream); + // A data line with no dispatching blank line before the stream ends. + server.push("data: one\n\ndata: two\n"); + server.end(); + await flush(); + const last = updates.at(-1)!.stream; + expect(last.eventCount).toBe(1); + expect(last.closedAt).toBeInstanceOf(Date); + }); + + it("accepts bare CR line endings, and a CRLF split across chunks", async () => { + const server = controlledStream(); + const { updates } = await trackStream(server.stream); + server.push("data: a\r\rdata: b\r"); + await flush(); + // The trailing CR is held: it may be the first half of a CRLF. + expect(updates.map((u) => u.stream.eventCount)).toEqual([0, 1]); + server.push("\n\r\n"); + await flush(); + expect(updates.map((u) => u.stream.eventCount)).toEqual([0, 1, 2]); + }); + + it("classifies the content type and the method case-insensitively", async () => { + const server = controlledStream(); + const updates: unknown[] = []; + const fetcher = createFetchTracker( + (async () => + new Response(server.stream, { + headers: { "content-type": "Text/Event-Stream" }, + })) as typeof fetch, + { + updateStream: (id, state) => updates.push({ id, state }), + streamUpdateIntervalMs: 0, + }, + ); + await fetcher("https://example.com/mcp", { method: "get" }); + expect(updates).toHaveLength(1); + }); + + it("takes the method from a Request input when init carries none", async () => { + // A bounded POST reply must go down the body-capture path, not the + // watcher, when the caller passed a `Request` rather than `init`. + const bodies: string[] = []; + const updates: unknown[] = []; + const tracked: FetchRequestEntryBase[] = []; + const fetcher = createFetchTracker( + (async () => + new Response("data: x\n\n", { + headers: { "content-type": "text/event-stream" }, + })) as typeof fetch, + { + trackRequest: (entry) => tracked.push(entry), + updateResponseBody: (_id, body) => bodies.push(body), + updateStream: (id, state) => updates.push({ id, state }), + streamUpdateIntervalMs: 0, + }, + ); + await fetcher(new Request("https://example.com/mcp", { method: "POST" })); + await flush(); + expect(tracked[0]?.method).toBe("POST"); + expect(bodies).toEqual(["data: x\n\n"]); + expect(updates).toEqual([]); + }); + + it("coalesces a moving count to one report per interval, and reports the close at once", async () => { + vi.useFakeTimers(); + try { + const server = controlledStream(); + const updates: Array<{ id: string; stream: FetchStreamState }> = []; + const fetcher = createFetchTracker( + (async () => + new Response(server.stream, { + headers: { "content-type": "text/event-stream" }, + })) as typeof fetch, + { + updateStream: (id, state) => updates.push({ id, stream: state }), + streamUpdateIntervalMs: 100, + }, + ); + await fetcher("https://example.com/mcp", { method: "GET" }); + // The open is immediate. + expect(updates.map((u) => u.stream.eventCount)).toEqual([0]); + server.push("data: a\n\ndata: b\n\ndata: c\n\n"); + await vi.advanceTimersByTimeAsync(0); + // Three events, no report yet — they are inside one window. + expect(updates.map((u) => u.stream.eventCount)).toEqual([0]); + await vi.advanceTimersByTimeAsync(100); + expect(updates.map((u) => u.stream.eventCount)).toEqual([0, 3]); + // A close cancels a pending window and reports immediately. + server.push("data: d\n\n"); + server.end(); + await vi.advanceTimersByTimeAsync(0); + expect(updates.map((u) => u.stream.eventCount)).toEqual([0, 3, 4]); + expect(updates.at(-1)!.stream.closedAt).toBeInstanceOf(Date); + await vi.advanceTimersByTimeAsync(200); + expect(updates).toHaveLength(3); + } finally { + vi.useRealTimers(); + } + }); + + it("counts an event whose bare-CR terminator is the last byte of the stream", async () => { + const server = controlledStream(); + const { updates } = await trackStream(server.stream); + server.push("data: x\r\r"); + await flush(); + // Held while the stream is open: the CR could be half of a CRLF. + expect(updates.map((u) => u.stream.eventCount)).toEqual([0]); + server.end(); + await flush(); + expect(updates.at(-1)!.stream).toMatchObject({ eventCount: 1 }); + expect(updates.at(-1)!.stream.closedAt).toBeInstanceOf(Date); + }); + + it("returns the response even when the listener throws on the open report", async () => { + // The open is reported synchronously inside the fetch wrapper; a throw + // there must not turn a successful response into a rejected fetch. + const server = controlledStream(); + const response = new Response(server.stream, { + headers: { "content-type": "text/event-stream" }, + }); + const fetcher = createFetchTracker((async () => response) as typeof fetch, { + updateStream: () => { + throw new Error("sink closed"); + }, + streamUpdateIntervalMs: 0, + }); + await expect( + fetcher("https://example.com/mcp", { method: "GET" }), + ).resolves.toBe(response); + // ...and a throw from a coalesced count does not escape the timer either. + server.push("data: a\n\n"); + server.end(); + await flush(); + }); + + it("survives a stream-update listener that throws on the close", async () => { + // The watcher's promise is discarded, so a throw from the final report + // would otherwise be an unhandled rejection — which fails the whole run. + const server = controlledStream(); + let reports = 0; + const fetcher = createFetchTracker( + (async () => + new Response(server.stream, { + headers: { "content-type": "text/event-stream" }, + })) as typeof fetch, + { + updateStream: (_id, state) => { + reports += 1; + if (state.closedAt) throw new Error("listener exploded"); + }, + streamUpdateIntervalMs: 0, + }, + ); + await fetcher("https://example.com/mcp", { method: "GET" }); + server.end(); + await flush(); + expect(reports).toBe(2); + }); + + it("reports the close when the server ends the stream", async () => { + const server = controlledStream(); + const { updates, id } = await trackStream(server.stream); + server.push("data: one\n\n"); + server.end(); + await flush(); + expect(updates).toHaveLength(3); + expect(updates[2]!.id).toBe(id); + expect(updates[2]!.stream.eventCount).toBe(1); + expect(updates[2]!.stream.closedAt).toBeInstanceOf(Date); + // Only the close carries `closedAt`. + expect(updates[0]!.stream.closedAt).toBeUndefined(); + expect(updates[1]!.stream.closedAt).toBeUndefined(); + }); + + it("reports the close when the stream errors, with the count so far", async () => { + const server = controlledStream(); + const { updates } = await trackStream(server.stream); + server.push("data: one\n\n"); + server.fail(new Error("connection reset")); + await flush(); + expect(updates).toHaveLength(3); + expect(updates[2]!.stream).toMatchObject({ eventCount: 1 }); + expect(updates[2]!.stream.closedAt).toBeInstanceOf(Date); + }); + + it("leaves the transport's own copy of the stream intact", async () => { + const server = controlledStream(); + const { response, updates } = await trackStream(server.stream); + server.push("data: hello\n\n"); + server.end(); + // The transport reads the original; the watcher read a tee'd clone. + expect(await response.text()).toBe("data: hello\n\n"); + await flush(); + expect(updates.at(-1)!.stream).toMatchObject({ eventCount: 1 }); + }); + + it("does not clone the response when nobody subscribed to stream updates", async () => { + const response = new Response(controlledStream().stream, { + headers: { "content-type": "text/event-stream" }, + }); + const clone = vi.spyOn(response, "clone"); + const fetcher = createFetchTracker((async () => response) as typeof fetch, { + trackRequest: () => {}, + }); + await fetcher("https://example.com/mcp", { method: "GET" }); + expect(clone).not.toHaveBeenCalled(); + }); + + it("does not watch a bounded response", async () => { + const updates: unknown[] = []; + const fetcher = createFetchTracker( + (async () => + new Response("data: x\n\n", { + headers: { "content-type": "text/event-stream" }, + })) as typeof fetch, + { + updateStream: (id, state) => updates.push({ id, state }), + streamUpdateIntervalMs: 0, + }, + ); + // A POST SSE reply is bounded and goes down the body-capture path. + await fetcher("https://example.com/mcp", { method: "POST" }); + await flush(); + expect(updates).toEqual([]); + }); + + it("gives up quietly when the response cannot be cloned", async () => { + const response = new Response(controlledStream().stream, { + headers: { "content-type": "text/event-stream" }, + }); + vi.spyOn(response, "clone").mockImplementation(() => { + throw new TypeError("already consumed"); + }); + const updates: unknown[] = []; + const fetcher = createFetchTracker((async () => response) as typeof fetch, { + updateStream: (id, state) => updates.push({ id, state }), + streamUpdateIntervalMs: 0, + }); + await expect( + fetcher("https://example.com/mcp", { method: "GET" }), + ).resolves.toBe(response); + await flush(); + expect(updates).toEqual([]); + }); + + it("gives up quietly when the clone has no body", async () => { + const response = new Response(controlledStream().stream, { + headers: { "content-type": "text/event-stream" }, + }); + vi.spyOn(response, "clone").mockImplementation( + () => + new Response(null, { + headers: { "content-type": "text/event-stream" }, + }), + ); + const updates: unknown[] = []; + const fetcher = createFetchTracker((async () => response) as typeof fetch, { + updateStream: (id, state) => updates.push({ id, state }), + streamUpdateIntervalMs: 0, + }); + await fetcher("https://example.com/mcp", { method: "GET" }); + await flush(); + expect(updates).toEqual([]); + }); +}); diff --git a/clients/web/src/test/core/mcp/inspectorClient-diagnostics-callbacks.test.ts b/clients/web/src/test/core/mcp/inspectorClient-diagnostics-callbacks.test.ts new file mode 100644 index 00000000..35adb3c6 --- /dev/null +++ b/clients/web/src/test/core/mcp/inspectorClient-diagnostics-callbacks.test.ts @@ -0,0 +1,81 @@ +import { describe, it, expect } from "vitest"; +import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client"; +import type { JSONRPCRequest } from "@modelcontextprotocol/client"; +import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; +import type { MessageTrackingCallbacks } from "@inspector/core/mcp/messageTrackingTransport.js"; + +/** + * The outstanding-request bookkeeping behind the connection diagnostics + * (#2318), driven through the message-tracking callbacks the client hands its + * transport — the same seam the transport uses, without a connection. + */ +describe("InspectorClient diagnostics tracking callbacks", () => { + interface Internals { + createMessageTrackingCallbacks: () => MessageTrackingCallbacks; + } + + function makeClient(): { + client: InspectorClient; + callbacks: MessageTrackingCallbacks; + } { + const client = new InspectorClient( + { type: "stdio", command: "noop", args: [] }, + // Never connects; the callbacks are exercised directly. + { environment: { transport: () => ({}) as never } }, + ); + // `createMessageTrackingCallbacks` is private: it is the seam the client + // hands its transport, and driving it directly is what lets this test + // exercise the bookkeeping without a connection. The double cast is the + // documented test-only exception for reaching a private member — the + // same one `inspectorClient-raw-wire.test.ts` uses — and `Internals` + // names exactly the one method it reaches. + const callbacks = ( + client as unknown as Internals + ).createMessageTrackingCallbacks(); + return { client, callbacks }; + } + + const request = (id: number): JSONRPCRequest => ({ + jsonrpc: "2.0", + id, + method: "tools/list", + }); + + const outstandingIds = (client: InspectorClient) => + client.getConnectionDiagnostics().outstandingRequests.map((r) => r.id); + + it("rolls a request back when its send failed before reaching the wire", () => { + const { client, callbacks } = makeClient(); + callbacks.trackRequest!(request(1), "client"); + expect(outstandingIds(client)).toEqual([1]); + callbacks.trackSendFailure!(request(1), new Error("connection closed")); + expect(outstandingIds(client)).toEqual([]); + }); + + it("keeps a request whose send failed with a request timeout", () => { + // The browser's remote transport awaits the response inside `send`, so + // its relay timeout surfaces as a send failure — the request reached the + // server and is still unanswered. + const { client, callbacks } = makeClient(); + callbacks.trackRequest!(request(1), "client"); + callbacks.trackSendFailure!( + request(1), + new SdkError(SdkErrorCode.RequestTimeout, "Request timed out", { + timeout: 60_000, + }), + ); + expect(outstandingIds(client)).toEqual([1]); + }); + + it("only tracks client-originated requests, and ignores a rollback for an unknown id", () => { + const { client, callbacks } = makeClient(); + callbacks.trackRequest!(request(7), "server"); + expect(outstandingIds(client)).toEqual([]); + let changes = 0; + client.addEventListener("connectionDiagnosticsChange", () => { + changes += 1; + }); + callbacks.trackSendFailure!(request(7), new Error("nope")); + expect(changes).toBe(0); + }); +}); diff --git a/clients/web/src/test/core/mcp/inspectorClient-raw-wire.test.ts b/clients/web/src/test/core/mcp/inspectorClient-raw-wire.test.ts index 2d9af6b1..638891b5 100644 --- a/clients/web/src/test/core/mcp/inspectorClient-raw-wire.test.ts +++ b/clients/web/src/test/core/mcp/inspectorClient-raw-wire.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi } from "vitest"; +import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client"; import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; import { ModernGetTaskResultSchema } from "@inspector/core/mcp/modernTaskSchemas.js"; @@ -134,7 +135,15 @@ describe("InspectorClient raw-wire channel (#1631)", () => { {}, ModernGetTaskResultSchema, ); - const assertion = expect(promise).rejects.toThrow(/timed out/); + // The same annotated shape an SDK request's timeout carries (#2318): + // the SDK's code, the budget, the method, and the connection state. + const assertion = expect(promise).rejects.toMatchObject({ + code: SdkErrorCode.RequestTimeout, + message: expect.stringMatching( + /^Request timed out after 10ms \(tasks\/get\)\. No requests are unanswered\./, + ), + data: expect.objectContaining({ timeout: 10, method: "tasks/get" }), + }); await vi.advanceTimersByTimeAsync(20); await assertion; } finally { @@ -142,6 +151,46 @@ describe("InspectorClient raw-wire channel (#1631)", () => { } }); + it("annotates a request timeout the transport's send rejects with (#2318)", async () => { + // The browser's remote transport awaits the response inside `send`, so + // its relay wait can expire before the local timer; it arrives here as + // the SDK's timeout shape and gets the same annotation. + const client = makeClient(); + internals(client).transport = { + send: vi.fn().mockRejectedValue( + new SdkError(SdkErrorCode.RequestTimeout, "Request timed out", { + timeout: 60_000, + }), + ), + }; + await expect( + internals(client).rawWireRequest( + "tasks/get", + {}, + ModernGetTaskResultSchema, + ), + ).rejects.toMatchObject({ + code: SdkErrorCode.RequestTimeout, + message: expect.stringMatching( + /^Request timed out after 1m00s \(tasks\/get\)\. /, + ), + data: expect.objectContaining({ timeout: 60_000, method: "tasks/get" }), + }); + }); + + it("leaves a non-timeout send failure untouched", async () => { + const client = makeClient(); + const boom = new Error("connection closed"); + internals(client).transport = { send: vi.fn().mockRejectedValue(boom) }; + await expect( + internals(client).rawWireRequest( + "tasks/get", + {}, + ModernGetTaskResultSchema, + ), + ).rejects.toBe(boom); + }); + it("rejects all pending requests on teardown", async () => { const client = makeClient(); internals(client).transport = { diff --git a/clients/web/src/test/core/mcp/messageTrackingTransport.test.ts b/clients/web/src/test/core/mcp/messageTrackingTransport.test.ts index 5f7204a2..ea69da47 100644 --- a/clients/web/src/test/core/mcp/messageTrackingTransport.test.ts +++ b/clients/web/src/test/core/mcp/messageTrackingTransport.test.ts @@ -33,6 +33,62 @@ function makeTracked() { } describe("MessageTrackingTransport.send", () => { + it("reports a request the base transport failed to send, after tracking it (#2318)", async () => { + const { callbacks, base, tracked } = makeTracked(); + const trackSendFailure = vi.fn(); + const failing = new MessageTrackingTransport(base, { + ...callbacks, + trackSendFailure, + }); + const boom = new Error("connection closed"); + base.send = async () => { + throw boom; + }; + const request = { jsonrpc: "2.0", id: 1, method: "tools/list" } as const; + await expect(failing.send(request)).rejects.toBe(boom); + expect(callbacks.trackRequest).toHaveBeenCalledWith(request, "client"); + expect(trackSendFailure).toHaveBeenCalledWith(request, boom); + // A signal already aborted before the send began: the frame never went + // out, so this is a failed send and rolls back. + trackSendFailure.mockClear(); + const preAborted = new AbortController(); + preAborted.abort(); + const preAbortedRequest = { + jsonrpc: "2.0", + id: 2, + method: "tools/call", + } as const; + await expect( + failing.send(preAbortedRequest, { requestSignal: preAborted.signal }), + ).rejects.toBe(boom); + expect(trackSendFailure).toHaveBeenCalledWith(preAbortedRequest, boom); + // A signal aborted while the send is in flight is a cancellation: the + // request may well have reached the server, and stays unanswered. + trackSendFailure.mockClear(); + const midSend = new AbortController(); + base.send = async () => { + midSend.abort(); + throw boom; + }; + await expect( + failing.send( + { jsonrpc: "2.0", id: 3, method: "tools/call" }, + { requestSignal: midSend.signal }, + ), + ).rejects.toBe(boom); + expect(trackSendFailure).not.toHaveBeenCalled(); + base.send = async () => { + throw boom; + }; + // Only requests roll back: a failed notification has no pending entry. + trackSendFailure.mockClear(); + await expect( + failing.send({ jsonrpc: "2.0", method: "notifications/initialized" }), + ).rejects.toBe(boom); + expect(trackSendFailure).not.toHaveBeenCalled(); + void tracked; + }); + it("tracks an outgoing request", async () => { const { callbacks, tracked } = makeTracked(); const request = { jsonrpc: "2.0", id: 1, method: "tools/list" } as const; diff --git a/clients/web/src/test/core/mcp/remote/remoteClientTransport.test.ts b/clients/web/src/test/core/mcp/remote/remoteClientTransport.test.ts index e0807093..d25eba92 100644 --- a/clients/web/src/test/core/mcp/remote/remoteClientTransport.test.ts +++ b/clients/web/src/test/core/mcp/remote/remoteClientTransport.test.ts @@ -791,9 +791,7 @@ describe("RemoteClientTransport", () => { }); // Hold the expected rejection now (as the RemoteSession timeout tests do) // so it is asserted rather than suppressed by an empty catch. - const rejection = expect(sent).rejects.toThrow( - /Timed out waiting for MCP response on SSE/, - ); + const rejection = expect(sent).rejects.toThrow(/Request timed out/); await flushSse(); // A log message before the deadline must not extend it. diff --git a/clients/web/src/test/core/mcp/remote/sessionStorage.test.ts b/clients/web/src/test/core/mcp/remote/sessionStorage.test.ts index a8fa25bb..a0b5249f 100644 --- a/clients/web/src/test/core/mcp/remote/sessionStorage.test.ts +++ b/clients/web/src/test/core/mcp/remote/sessionStorage.test.ts @@ -110,6 +110,45 @@ describe("RemoteInspectorClientStorage", () => { ); }); + it("loadSession revives a stream's closedAt alongside the entry timestamp (#2318)", async () => { + const fetchFn = vi.fn( + async () => + new Response( + JSON.stringify({ + fetchRequests: [ + { + id: "open", + timestamp: "2026-01-01T00:00:00.000Z", + method: "GET", + url: "http://example.com/mcp", + requestHeaders: {}, + stream: { eventCount: 2 }, + }, + { + id: "closed", + timestamp: "2026-01-01T00:00:00.000Z", + method: "GET", + url: "http://example.com/mcp", + requestHeaders: {}, + stream: { eventCount: 3, closedAt: "2026-01-01T00:00:05.000Z" }, + }, + ], + createdAt: 1735689600000, + updatedAt: 1735689600001, + }), + { status: 200 }, + ), + ); + const storage = makeStorage(fetchFn); + const state = await storage.loadSession("sid"); + // An open stream's state passes through untouched. + expect(state?.fetchRequests[0]?.stream).toEqual({ eventCount: 2 }); + const closed = state?.fetchRequests[1]?.stream; + expect(closed?.eventCount).toBe(3); + expect(closed?.closedAt).toBeInstanceOf(Date); + expect(closed?.closedAt?.toISOString()).toBe("2026-01-01T00:00:05.000Z"); + }); + it("loadSession preserves Date timestamps already provided as Date instances", async () => { const fetchFn = vi.fn( async () => diff --git a/clients/web/src/test/core/mcp/state/fetchRequestLogState.test.ts b/clients/web/src/test/core/mcp/state/fetchRequestLogState.test.ts index d8b239b3..5e00da33 100644 --- a/clients/web/src/test/core/mcp/state/fetchRequestLogState.test.ts +++ b/clients/web/src/test/core/mcp/state/fetchRequestLogState.test.ts @@ -146,6 +146,51 @@ describe("FetchRequestLogState", () => { expect(seen).toHaveLength(1); }); + it("patches the matching entry's stream state and re-emits on fetchRequestStreamUpdate (#2318)", () => { + client.dispatchTypedEvent("fetchRequest", entry("a")); + client.dispatchTypedEvent("fetchRequest", entry("b")); + const seen: FetchRequestEntry[][] = []; + state.addEventListener("fetchRequestsChange", (e) => seen.push(e.detail)); + + client.dispatchTypedEvent("fetchRequestStreamUpdate", { + id: "b", + stream: { eventCount: 2 }, + }); + const closedAt = new Date(2026, 4, 13, 0, 1); + client.dispatchTypedEvent("fetchRequestStreamUpdate", { + id: "b", + stream: { eventCount: 3, closedAt }, + }); + + const entries = state.getFetchRequests(); + expect(entries[0]?.stream).toBeUndefined(); + // Each update replaces the previous state wholesale. + expect(entries[1]?.stream).toEqual({ eventCount: 3, closedAt }); + expect(seen).toHaveLength(2); + }); + + it("ignores a stream update for an unknown id without emitting", () => { + client.dispatchTypedEvent("fetchRequest", entry("a")); + const seen: FetchRequestEntry[][] = []; + state.addEventListener("fetchRequestsChange", (e) => seen.push(e.detail)); + client.dispatchTypedEvent("fetchRequestStreamUpdate", { + id: "gone", + stream: { eventCount: 1 }, + }); + expect(seen).toHaveLength(0); + expect(state.getFetchRequests()[0]?.stream).toBeUndefined(); + }); + + it("stops patching stream state after destroy", () => { + client.dispatchTypedEvent("fetchRequest", entry("a")); + state.destroy(); + client.dispatchTypedEvent("fetchRequestStreamUpdate", { + id: "a", + stream: { eventCount: 1 }, + }); + expect(state.getFetchRequests()).toEqual([]); + }); + it("silently ignores a body update for an unknown id when the log is not full (benign straggler)", () => { // Below capacity, an idx === -1 means the entry was cleared or never // existed — not a rotation drop — so it must NOT warn or emit the dropped diff --git a/clients/web/src/test/core/react/useInspectorClient.test.tsx b/clients/web/src/test/core/react/useInspectorClient.test.tsx index 3f6c5793..107ba45e 100644 --- a/clients/web/src/test/core/react/useInspectorClient.test.tsx +++ b/clients/web/src/test/core/react/useInspectorClient.test.tsx @@ -68,6 +68,42 @@ describe("useInspectorClient", () => { expect(result.current.instructions).toBe("after"); }); + it("returns the client's connection diagnostics and re-reads them on change (#2318)", () => { + const client = new FakeInspectorClient(); + const { result } = renderHook(() => useInspectorClient(client)); + expect(result.current.connectionDiagnostics).toEqual({ + capturedAt: 0, + outstandingRequests: [], + }); + const next = { + capturedAt: 10, + outstandingRequests: [{ id: 1, method: "tools/list", sentAt: 5 }], + lastResponse: { method: "initialize", receivedAt: 4 }, + }; + act(() => { + client.setConnectionDiagnostics(next); + }); + expect(result.current.connectionDiagnostics).toBe(next); + }); + + it("falls back to an empty snapshot for a client without the optional accessor", () => { + const client = new FakeInspectorClient(); + // `getConnectionDiagnostics` is optional on the protocol; a double that + // predates it must still yield the one shape consumers expect. + Object.defineProperty(client, "getConnectionDiagnostics", { + value: undefined, + }); + const { result } = renderHook(() => useInspectorClient(client)); + expect(result.current.connectionDiagnostics).toEqual({ + capturedAt: 0, + outstandingRequests: [], + }); + const { result: absent } = renderHook(() => useInspectorClient(null)); + expect(absent.current.connectionDiagnostics).toBe( + result.current.connectionDiagnostics, + ); + }); + it("subscribes to protocolVersionChange and updates", () => { const client = new FakeInspectorClient(); const { result } = renderHook(() => useInspectorClient(client)); diff --git a/clients/web/src/test/integration/mcp/inspectorClient-timeout-diagnostics.test.ts b/clients/web/src/test/integration/mcp/inspectorClient-timeout-diagnostics.test.ts new file mode 100644 index 00000000..fd4649b6 --- /dev/null +++ b/clients/web/src/test/integration/mcp/inspectorClient-timeout-diagnostics.test.ts @@ -0,0 +1,366 @@ +import { describe, it, expect, afterEach } from "vitest"; +import { + createServer, + type IncomingMessage, + type Server, + type ServerResponse, +} from "node:http"; +import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client"; +import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; +import { FetchRequestLogState } from "@inspector/core/mcp/state/index.js"; +import { createTransportNode } from "@inspector/core/mcp/node/transport.js"; +import { eraToVersionNegotiation } from "@inspector/core/mcp/types.js"; +import type { + AnnotatedRequestTimeoutData, + ConnectionDiagnostics, +} from "@inspector/core/mcp/connectionDiagnostics.js"; + +/** + * Request-timeout diagnostics against a real connection (#2318). + * + * The #2187 shape: a server that answers `initialize`, holds the standalone + * `GET /mcp` notification stream open without ever emitting an event, and + * never answers `tools/list`. The SDK's per-request timeout fires and the + * Inspector's word on it used to be `Request timed out`. These tests drive the + * real transport, the real SDK client and the real timeout, and assert on what + * the rejection now says — plus the snapshot and the events it is built from. + * + * Hand-rolled rather than a composable test server because the fixture has + * to misbehave in ways the SDK's own server cannot be configured to: hold a + * request forever, and push raw bytes down the GET stream on cue. + */ + +/** A short budget so the timeout fires in test time rather than in 60s. */ +const REQUEST_TIMEOUT_MS = 300; + +type ToolsListMode = "hang" | "error"; + +interface HangingServer { + url: string; + /** Every JSON-RPC method the server was sent, in order. */ + calls: string[]; + /** Write one SSE event onto every open GET stream. */ + pushEvent: (message: Record) => void; + /** Write a comment-only keepalive block onto every open GET stream. */ + pushKeepalive: () => void; + /** How many GET streams have been opened so far. */ + streamsOpened: () => number; + setToolsListMode: (mode: ToolsListMode) => void; + stop: () => Promise; +} + +function startHangingServer(): Promise { + const calls: string[] = []; + const streams = new Set(); + const held = new Set(); + let streamsOpened = 0; + let toolsListMode: ToolsListMode = "hang"; + + const handler = async (req: IncomingMessage, res: ServerResponse) => { + if (req.method === "GET") { + streamsOpened += 1; + streams.add(res); + res.on("close", () => streams.delete(res)); + res.writeHead(200, { + "content-type": "text/event-stream", + "cache-control": "no-cache", + }); + // Headers out now, so the client's fetch resolves and the stream reads + // as open — without a byte of event data on it. + res.flushHeaders(); + return; + } + if (req.method === "DELETE") { + res.writeHead(200).end(); + return; + } + const chunks: Buffer[] = []; + for await (const chunk of req) chunks.push(chunk as Buffer); + const body = JSON.parse(Buffer.concat(chunks).toString()) as { + id?: unknown; + method: string; + }; + calls.push(body.method); + // Notifications (`notifications/initialized`, and the SDK's own + // `notifications/cancelled` when the timeout fires) get an empty 202. + if (body.id === undefined) { + res.writeHead(202).end(); + return; + } + const reply = (payload: Record) => { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ jsonrpc: "2.0", id: body.id, ...payload })); + }; + if (body.method === "initialize") { + reply({ + result: { + protocolVersion: "2025-06-18", + capabilities: { tools: {} }, + serverInfo: { name: "hanging-server", version: "1.0.0" }, + }, + }); + return; + } + if (body.method === "tools/list") { + if (toolsListMode === "error") { + reply({ error: { code: -32603, message: "tools/list exploded" } }); + return; + } + // Never answered. Kept so teardown can release the socket. + held.add(res); + res.on("close", () => held.delete(res)); + return; + } + reply({ result: {} }); + }; + + const server: Server = createServer((req, res) => { + void handler(req, res); + }); + + return new Promise((resolve) => { + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + /* v8 ignore next -- listen() on a fresh server always yields an AddressInfo */ + const port = typeof address === "object" && address ? address.port : 0; + resolve({ + url: `http://127.0.0.1:${port}/mcp`, + calls, + pushEvent: (message) => { + for (const res of streams) { + res.write(`event: message\ndata: ${JSON.stringify(message)}\n\n`); + } + }, + pushKeepalive: () => { + for (const res of streams) res.write(": keepalive\n\n"); + }, + streamsOpened: () => streamsOpened, + setToolsListMode: (mode) => { + toolsListMode = mode; + }, + stop: () => + new Promise((done) => { + for (const res of held) res.destroy(); + for (const res of streams) res.destroy(); + server.closeAllConnections(); + server.close(() => done()); + }), + }); + }); + }); +} + +/** Poll until `predicate` holds; the failure names what never became true. */ +async function waitUntil( + what: string, + predicate: () => boolean, + timeoutMs = 5000, +): Promise { + const deadline = Date.now() + timeoutMs; + while (!predicate()) { + if (Date.now() > deadline) { + throw new Error(`timed out waiting for ${what}`); + } + await new Promise((r) => setTimeout(r, 10)); + } +} + +describe("InspectorClient request-timeout diagnostics (#2318)", () => { + let client: InspectorClient | null = null; + let server: HangingServer | null = null; + + async function connectTo(url: string): Promise { + client = new InspectorClient( + { type: "streamable-http", url }, + { + environment: { transport: createTransportNode }, + // Pinned legacy so the connect is exactly `initialize` and nothing + // else — the assertions below name the last response by method. + versionNegotiation: eraToVersionNegotiation("legacy"), + timeout: REQUEST_TIMEOUT_MS, + }, + ); + await client.connect(); + return client; + } + + /** The SDK opens the GET stream after `notifications/initialized`, off the connect path. */ + async function waitForNotificationStream(c: InspectorClient): Promise { + await waitUntil( + "the notification stream to be tracked", + () => c.getConnectionDiagnostics().notificationStream !== undefined, + ); + } + + afterEach(async () => { + try { + await client?.disconnect(); + } catch { + // Teardown only — the assertions already ran. + } + client = null; + await server?.stop(); + server = null; + }); + + it("annotates the timeout with the request, the unanswered set, the last response and the stream", async () => { + server = await startHangingServer(); + const c = await connectTo(server.url); + await waitForNotificationStream(c); + server.pushKeepalive(); + + let caught: unknown; + try { + await c.listTools(); + } catch (err) { + caught = err; + } + + expect(SdkError.isInstance(caught)).toBe(true); + const error = caught as SdkError; + expect(error.code).toBe(SdkErrorCode.RequestTimeout); + expect(error.message).toMatch( + new RegExp( + "^Request timed out after 300ms \\(tools/list\\)\\. " + + "1 request is unanswered: tools/list \\(sent \\d+(ms|s) ago\\)\\. " + + "Last response received \\d+(ms|s) ago \\(initialize\\)\\. " + + "Notification stream \\(GET /mcp\\) open for \\d+(ms|s), 0 events delivered\\.$", + ), + ); + // The structured form rides along for consumers that want it. + const data = error.data as AnnotatedRequestTimeoutData; + expect(data.timeout).toBe(REQUEST_TIMEOUT_MS); + expect(data.method).toBe("tools/list"); + expect(data.diagnostics.outstandingRequests.map((r) => r.method)).toEqual([ + "tools/list", + ]); + expect(data.diagnostics.lastResponse?.method).toBe("initialize"); + expect(data.diagnostics.notificationStream?.eventCount).toBe(0); + // The SDK gave up on the request; the connection did not answer it. It is + // still unanswered, and the live snapshot keeps saying so. + expect( + c.getConnectionDiagnostics().outstandingRequests.map((r) => r.method), + ).toEqual(["tools/list"]); + // The keepalive comment reached the client and was not counted. + expect(c.getConnectionDiagnostics().notificationStream?.eventCount).toBe(0); + // The SDK's legacy cancel rode the wire and the server saw the whole + // exchange in order. The cancel is fire-and-forget on the SDK side — the + // rejection lands before the POST does — so wait for it to arrive. + const calls = server.calls; + await waitUntil("the cancellation to reach the server", () => + calls.includes("notifications/cancelled"), + ); + expect(calls).toEqual([ + "initialize", + "notifications/initialized", + "tools/list", + "notifications/cancelled", + ]); + }); + + it("emits connectionDiagnosticsChange as the state moves, and counts stream events", async () => { + server = await startHangingServer(); + const c = await connectTo(server.url); + const log = new FetchRequestLogState(c); + const snapshots: ConnectionDiagnostics[] = []; + c.addEventListener("connectionDiagnosticsChange", (event) => + snapshots.push(event.detail), + ); + try { + await waitForNotificationStream(c); + const stream = c.getConnectionDiagnostics().notificationStream!; + expect(stream.url).toBe(server.url); + expect(stream.closedAt).toBeUndefined(); + // Open from when the headers arrived, not from when the GET went out. + const getEntryAtOpen = log + .getFetchRequests() + .find((entry) => entry.method === "GET")!; + expect(stream.openedAt).toBe( + getEntryAtOpen.timestamp.getTime() + (getEntryAtOpen.duration ?? 0), + ); + // The snapshot is a copy: mutating it does not reach the client. + const snapshot = c.getConnectionDiagnostics(); + snapshot.outstandingRequests.push({ + id: "x", + method: "bogus", + sentAt: 0, + }); + expect(c.getConnectionDiagnostics().outstandingRequests).toEqual([]); + + // A server push on the notification stream is an event. + server.pushEvent({ + jsonrpc: "2.0", + method: "notifications/tools/list_changed", + }); + await waitUntil( + "the stream event to be counted", + () => c.getConnectionDiagnostics().notificationStream?.eventCount === 1, + ); + // ...and the Network log's entry for the GET carries the same state. + const getEntry = log + .getFetchRequests() + .find((entry) => entry.method === "GET"); + expect(getEntry?.stream).toEqual({ eventCount: 1 }); + + // The timeout path dispatched a snapshot with the request outstanding. + await expect(c.listTools()).rejects.toThrow(/Request timed out/); + expect( + snapshots.some((s) => + s.outstandingRequests.some((r) => r.method === "tools/list"), + ), + ).toBe(true); + + // Disconnecting aborts the stream: the watcher reports the close and + // the snapshot — which outlives the session until the next connect — + // records it. + await c.disconnect(); + await waitUntil( + "the stream close to be recorded", + () => + c.getConnectionDiagnostics().notificationStream?.closedAt !== + undefined, + ); + expect(c.getConnectionDiagnostics().notificationStream).toMatchObject({ + eventCount: 1, + }); + } finally { + log.destroy(); + } + }); + + it("starts the next session clean", async () => { + server = await startHangingServer(); + const c = await connectTo(server.url); + await waitForNotificationStream(c); + await expect(c.listTools()).rejects.toThrow(/Request timed out/); + expect(c.getConnectionDiagnostics().outstandingRequests).toHaveLength(1); + + await c.disconnect(); + await c.connect(); + + const fresh = c.getConnectionDiagnostics(); + expect(fresh.outstandingRequests).toEqual([]); + expect(fresh.lastResponse?.method).toBe("initialize"); + // Whether the new session's GET has landed yet is timing; what must not be + // there is the previous session's stream. + await waitForNotificationStream(c); + expect(c.getConnectionDiagnostics().notificationStream?.closedAt).toBe( + undefined, + ); + expect(server.streamsOpened()).toBe(2); + }); + + it("leaves a failure that is not a timeout exactly as the SDK threw it", async () => { + server = await startHangingServer(); + server.setToolsListMode("error"); + const c = await connectTo(server.url); + + await expect(c.listTools()).rejects.toThrow(/tools\/list exploded/); + await expect(c.listTools()).rejects.not.toThrow(/unanswered/); + // Answered (with an error), so nothing is outstanding. + expect(c.getConnectionDiagnostics().outstandingRequests).toEqual([]); + expect(c.getConnectionDiagnostics().lastResponse?.method).toBe( + "tools/list", + ); + }); +}); diff --git a/clients/web/src/test/integration/mcp/node/transport.test.ts b/clients/web/src/test/integration/mcp/node/transport.test.ts index c813fdf0..e5326b52 100644 --- a/clients/web/src/test/integration/mcp/node/transport.test.ts +++ b/clients/web/src/test/integration/mcp/node/transport.test.ts @@ -188,6 +188,35 @@ describe("Transport", () => { } }); + it("tracks fetches when only onFetchResponseBody is supplied (#2318)", async () => { + // Any one of the tracking callbacks earns the tracker — a caller that + // wants only bodies (or only stream updates) must not be handed the + // bare fetch and silently receive nothing. + const server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + serverType: "streamable-http", + }); + try { + await server.start(); + const bodies: string[] = []; + const result = createTransportNode( + { type: "streamable-http", url: server.url }, + { onFetchResponseBody: (_id, body) => bodies.push(body) }, + ); + const client = new Client( + { name: "test-client", version: "1.0.0" }, + { capabilities: {} }, + ); + await client.connect(result.transport); + await client.listTools(); + await client.close(); + expect(bodies.some((body) => body.includes("tools"))).toBe(true); + } finally { + await server.stop(); + } + }); + it("should call onFetchRequest callback for streamable-http transport", async () => { const server = createTestServerHttp({ serverInfo: createTestServerInfo(), diff --git a/clients/web/src/test/integration/mcp/remote/remote-session.test.ts b/clients/web/src/test/integration/mcp/remote/remote-session.test.ts index 66899eb5..61d55a1d 100644 --- a/clients/web/src/test/integration/mcp/remote/remote-session.test.ts +++ b/clients/web/src/test/integration/mcp/remote/remote-session.test.ts @@ -199,6 +199,30 @@ describe("RemoteSession", () => { }); }); + it("onFetchStreamUpdate forwards the event count, serializing closedAt to ISO when present (#2318)", () => { + const session = new RemoteSession("s10a"); + const received: { type: string; data: unknown }[] = []; + session.setEventConsumer((event) => + received.push({ type: event.type, data: event.data }), + ); + session.onFetchStreamUpdate("req-1", { eventCount: 2 }); + session.onFetchStreamUpdate("req-1", { + eventCount: 3, + closedAt: new Date("2026-01-01T00:00:05Z"), + }); + expect(received).toEqual([ + { type: "fetch_stream_update", data: { id: "req-1", eventCount: 2 } }, + { + type: "fetch_stream_update", + data: { + id: "req-1", + eventCount: 3, + closedAt: "2026-01-01T00:00:05.000Z", + }, + }, + ]); + }); + it("onFetchRequest serializes a Date timestamp to an ISO string", () => { const session = new RemoteSession("s11"); const received: { type: string; data: unknown }[] = []; diff --git a/clients/web/src/test/integration/mcp/remote/remoteClientTransport-unit.test.ts b/clients/web/src/test/integration/mcp/remote/remoteClientTransport-unit.test.ts index fddc10c9..51e93874 100644 --- a/clients/web/src/test/integration/mcp/remote/remoteClientTransport-unit.test.ts +++ b/clients/web/src/test/integration/mcp/remote/remoteClientTransport-unit.test.ts @@ -16,6 +16,7 @@ import type { RemoteTransportOptions } from "@inspector/core/mcp/remote/remoteCl import type { MCPServerConfig } from "@inspector/core/mcp/types.js"; import type { RemoteEvent } from "@inspector/core/mcp/remote/types.js"; import type { JSONRPCMessage } from "@modelcontextprotocol/client"; +import { SdkErrorCode } from "@modelcontextprotocol/client"; import { AuthChallengeError, AuthRecoveryRequiredError, @@ -340,6 +341,148 @@ describe("RemoteClientTransport (focused branch coverage)", () => { expect(onFetchResponseBody).toHaveBeenCalledWith("f1", "BODY"); }); + it("delivers fetch_stream_update events, rebuilding closedAt as a Date (#2318)", async () => { + const onFetchStreamUpdate = vi.fn(); + const t = makeTransport( + { + events: () => + sseResponse([ + sseFrame({ + type: "fetch_stream_update", + data: { id: "f1", eventCount: 2 }, + }), + sseFrame({ + type: "fetch_stream_update", + data: { + id: "f1", + eventCount: 3, + closedAt: "2026-01-01T00:00:05.000Z", + }, + }), + ]), + }, + { onFetchStreamUpdate }, + ); + await t.start(); + await tick(); + expect(onFetchStreamUpdate).toHaveBeenCalledTimes(2); + expect(onFetchStreamUpdate).toHaveBeenNthCalledWith(1, "f1", { + eventCount: 2, + }); + expect(onFetchStreamUpdate).toHaveBeenNthCalledWith(2, "f1", { + eventCount: 3, + closedAt: new Date("2026-01-01T00:00:05.000Z"), + }); + }); + + it("reports a still-open stream closed on close(), since the backend's own close cannot arrive (#2318)", async () => { + const onFetchStreamUpdate = vi.fn(); + const { response, push } = createPushableEventStream(); + const t = makeTransport( + { events: () => response }, + { onFetchStreamUpdate, onFetchRequest: vi.fn() }, + ); + await t.start(); + const entry = (id: string, method: string, contentType: string) => + ({ + id, + method, + url: "http://x/mcp", + timestamp: new Date().toISOString(), + requestHeaders: {}, + responseStatus: 200, + responseHeaders: { "Content-Type": contentType }, + }) as never; + push({ + type: "fetch_request", + data: entry("stream", "GET", "text/event-stream"), + }); + push({ + type: "fetch_request", + data: entry("post", "POST", "text/event-stream"), + }); + push({ + type: "fetch_stream_update", + data: { id: "stream", eventCount: 2 }, + }); + push({ + type: "fetch_request", + data: entry("earlier", "GET", "text/event-stream"), + }); + push({ + type: "fetch_stream_update", + data: { + id: "earlier", + eventCount: 1, + closedAt: new Date().toISOString(), + }, + }); + await tick(); + onFetchStreamUpdate.mockClear(); + await t.close(); + // Only the stream still open is reported closed, with its last count; + // the bounded POST and the already-closed stream are not. + expect(onFetchStreamUpdate).toHaveBeenCalledTimes(1); + expect(onFetchStreamUpdate).toHaveBeenCalledWith("stream", { + eventCount: 2, + closedAt: expect.any(Date), + }); + // A second close reports nothing more. + await t.close(); + expect(onFetchStreamUpdate).toHaveBeenCalledTimes(1); + }); + + it("synthesizes the close on close() for a consumer that registered only onFetchStreamUpdate", async () => { + const onFetchStreamUpdate = vi.fn(); + const { response, push } = createPushableEventStream(); + const t = makeTransport( + { events: () => response }, + { onFetchStreamUpdate }, + ); + await t.start(); + push({ + type: "fetch_request", + data: { + id: "stream", + method: "GET", + url: "http://x/mcp", + timestamp: new Date().toISOString(), + requestHeaders: {}, + responseStatus: 200, + responseHeaders: { "content-type": "text/event-stream" }, + } as never, + }); + push({ + type: "fetch_stream_update", + data: { id: "stream", eventCount: 3 }, + }); + await tick(); + await t.close(); + expect(onFetchStreamUpdate).toHaveBeenLastCalledWith("stream", { + eventCount: 3, + closedAt: expect.any(Date), + }); + }); + + it("drops fetch_stream_update events when no handler is registered", async () => { + const onFetchRequest = vi.fn(); + const t = makeTransport( + { + events: () => + sseResponse([ + sseFrame({ + type: "fetch_stream_update", + data: { id: "f1", eventCount: 1 }, + }), + ]), + }, + { onFetchRequest }, + ); + await t.start(); + await tick(); + expect(onFetchRequest).not.toHaveBeenCalled(); + }); + it("delivers stdio_log events to onStderr", async () => { const onStderr = vi.fn(); const t = makeTransport( @@ -562,6 +705,27 @@ describe("RemoteClientTransport (focused branch coverage)", () => { ).rejects.toThrow(/Transport is closed/); }); + it("rejects an expired SSE response wait with the SDK's own timeout shape (#2318)", async () => { + // The relay's budget, not the request's, can be the one that expires; + // shaped as the SDK's timeout so InspectorClient annotates it the same. + const t = makeTransport( + { + events: () => openEventsResponse(), + send: () => jsonResponse({ ok: true }), + }, + { sseResponseTimeoutMs: 50 }, + ); + await t.start(); + await expect( + t.send({ jsonrpc: "2.0", id: 1, method: "tools/list" }), + ).rejects.toMatchObject({ + code: SdkErrorCode.RequestTimeout, + message: "Request timed out", + data: { timeout: 50 }, + }); + await t.close(); + }); + it("does not wait for an SSE response on subscriptions/listen (long-lived stream)", async () => { // A `subscriptions/listen` never yields a JSON-RPC response — only an // acknowledged notification over the event channel — so send() must diff --git a/clients/web/src/utils/connectionActivity.test.ts b/clients/web/src/utils/connectionActivity.test.ts new file mode 100644 index 00000000..db31480e --- /dev/null +++ b/clients/web/src/utils/connectionActivity.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, it } from "vitest"; +import { + formatLastResponse, + formatNotificationStream, + formatOutstandingRequests, + NO_NOTIFICATION_STREAM_LABEL, + NO_RESPONSE_YET_LABEL, + NO_STREAM_ON_STDIO_LABEL, +} from "./connectionActivity"; + +const NOW = 1_000_000; +const seconds = (n: number) => n * 1000; + +describe("formatOutstandingRequests", () => { + it("renders one line per request, oldest first, against the given clock", () => { + expect( + formatOutstandingRequests( + { + capturedAt: NOW, + outstandingRequests: [ + { id: 1, method: "tools/list", sentAt: NOW - seconds(60) }, + { id: 2, method: "ping", sentAt: NOW - seconds(12) }, + ], + }, + NOW, + ), + ).toEqual(["tools/list — sent 1m00s ago", "ping — sent 12s ago"]); + }); + + it("renders nothing when nothing is outstanding", () => { + expect( + formatOutstandingRequests( + { capturedAt: NOW, outstandingRequests: [] }, + NOW, + ), + ).toEqual([]); + }); +}); + +describe("formatLastResponse", () => { + it("names the method and the age", () => { + expect( + formatLastResponse( + { + capturedAt: NOW, + outstandingRequests: [], + lastResponse: { method: "initialize", receivedAt: NOW - 500 }, + }, + NOW, + ), + ).toBe("initialize — 500ms ago"); + }); + + it("says none yet before any response", () => { + expect( + formatLastResponse({ capturedAt: NOW, outstandingRequests: [] }, NOW), + ).toBe(NO_RESPONSE_YET_LABEL); + }); +}); + +describe("the clock argument", () => { + it("moves every duration with it, so a ticking caller renders live values", () => { + const diagnostics = { + capturedAt: NOW, + outstandingRequests: [ + { id: 1, method: "tools/list", sentAt: NOW - seconds(5) }, + ], + lastResponse: { method: "initialize", receivedAt: NOW - seconds(6) }, + }; + expect(formatOutstandingRequests(diagnostics, NOW + seconds(55))).toEqual([ + "tools/list — sent 1m00s ago", + ]); + expect(formatLastResponse(diagnostics, NOW + seconds(55))).toBe( + "initialize — 1m01s ago", + ); + }); +}); + +describe("formatNotificationStream", () => { + const open = { + url: "http://h/mcp", + openedAt: NOW - seconds(252), + eventCount: 1, + }; + + it("is not applicable on stdio, whatever the snapshot says", () => { + expect(formatNotificationStream(open, "stdio", NOW)).toBe( + NO_STREAM_ON_STDIO_LABEL, + ); + }); + + it("says not opened when no stream has been tracked", () => { + expect(formatNotificationStream(undefined, "sse", NOW)).toBe( + NO_NOTIFICATION_STREAM_LABEL, + ); + }); + + it("describes an open stream by endpoint, age and event count", () => { + expect(formatNotificationStream(open, "streamable-http", NOW)).toBe( + "GET /mcp — open for 4m12s, 1 event delivered", + ); + }); + + it("describes a closed stream by when it closed and how long it lasted", () => { + expect( + formatNotificationStream( + { ...open, eventCount: 2, closedAt: NOW - seconds(5) }, + "streamable-http", + NOW, + ), + ).toBe("GET /mcp — closed 5s ago after 4m07s, 2 events delivered"); + }); +}); diff --git a/clients/web/src/utils/connectionActivity.ts b/clients/web/src/utils/connectionActivity.ts new file mode 100644 index 00000000..649785f4 --- /dev/null +++ b/clients/web/src/utils/connectionActivity.ts @@ -0,0 +1,63 @@ +// Row formatters for the Connection Info modal's Connection Activity section +// (#2318): the same snapshot a request timeout's message is written from, +// rendered while the request is still in flight. Pure functions over the +// snapshot and an explicit clock — the caller decides what "now" is (the +// snapshot's own `capturedAt` on first paint, a ticking clock afterwards; see +// `useTickingClock`), so nothing here reads the wall clock and a render stays +// a pure function of its inputs. + +import { + describeStreamEndpoint, + formatDuration, + type ConnectionDiagnostics, + type NotificationStreamState, +} from "@inspector/core/mcp/connectionDiagnostics.js"; +import type { ServerType } from "@inspector/core/mcp/types.js"; + +export const NO_OUTSTANDING_REQUESTS_LABEL = "None"; +export const NO_RESPONSE_YET_LABEL = "None this session"; +export const NO_NOTIFICATION_STREAM_LABEL = "Not opened"; +export const NO_STREAM_ON_STDIO_LABEL = "N/A (stdio)"; + +/** + * One line per unanswered request, oldest first — `tools/list — sent 60s + * ago`. A request the SDK has already given up on stays listed: it is still + * unanswered, which is the fact this section reports. + */ +export function formatOutstandingRequests( + diagnostics: ConnectionDiagnostics, + now: number, +): string[] { + return diagnostics.outstandingRequests.map( + (request) => + `${request.method} — sent ${formatDuration(now - request.sentAt)} ago`, + ); +} + +export function formatLastResponse( + diagnostics: ConnectionDiagnostics, + now: number, +): string { + const last = diagnostics.lastResponse; + if (!last) return NO_RESPONSE_YET_LABEL; + return `${last.method} — ${formatDuration(now - last.receivedAt)} ago`; +} + +/** + * The notification stream row. stdio has no HTTP stream at all, so the row + * says so rather than "Not opened", which would read as a fault. + */ +export function formatNotificationStream( + stream: NotificationStreamState | undefined, + transport: ServerType, + now: number, +): string { + if (transport === "stdio") return NO_STREAM_ON_STDIO_LABEL; + if (!stream) return NO_NOTIFICATION_STREAM_LABEL; + const endpoint = describeStreamEndpoint(stream.url); + const events = `${stream.eventCount} event${stream.eventCount === 1 ? "" : "s"} delivered`; + if (stream.closedAt !== undefined) { + return `${endpoint} — closed ${formatDuration(now - stream.closedAt)} ago after ${formatDuration(stream.closedAt - stream.openedAt)}, ${events}`; + } + return `${endpoint} — open for ${formatDuration(now - stream.openedAt)}, ${events}`; +} diff --git a/clients/web/src/utils/uncapturedBodyNote.test.ts b/clients/web/src/utils/uncapturedBodyNote.test.ts new file mode 100644 index 00000000..1bb88b0e --- /dev/null +++ b/clients/web/src/utils/uncapturedBodyNote.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from "vitest"; +import type { FetchRequestEntry } from "@inspector/core/mcp/types.js"; +import { + isLongLivedStreamEntry, + longLivedStreamLabel, + uncapturedBodyNote, +} from "./uncapturedBodyNote"; + +const streamEntry: FetchRequestEntry = { + id: "n-1", + timestamp: new Date("2026-03-17T10:00:00Z"), + method: "GET", + url: "https://example.com/mcp", + requestHeaders: {}, + responseStatus: 200, + responseHeaders: { "content-type": "text/event-stream" }, + category: "transport", +}; + +describe("isLongLivedStreamEntry", () => { + it("is true for a GET answered with an event stream and false otherwise", () => { + expect(isLongLivedStreamEntry(streamEntry)).toBe(true); + expect(isLongLivedStreamEntry({ ...streamEntry, method: "POST" })).toBe( + false, + ); + expect( + isLongLivedStreamEntry({ ...streamEntry, responseHeaders: undefined }), + ).toBe(false); + }); + + it("reads the content type whatever casing the entry recorded it with", () => { + expect( + isLongLivedStreamEntry({ + ...streamEntry, + responseHeaders: { "Content-Type": "text/event-stream" }, + }), + ).toBe(true); + }); +}); + +describe("longLivedStreamLabel", () => { + it("names the framing the stream uses", () => { + expect(longLivedStreamLabel(streamEntry)).toBe("SSE"); + expect( + longLivedStreamLabel({ + ...streamEntry, + responseHeaders: { "Content-Type": "application/x-ndjson" }, + }), + ).toBe("NDJSON"); + }); +}); + +describe("uncapturedBodyNote", () => { + it("says (empty) for a bounded response", () => { + expect(uncapturedBodyNote({ ...streamEntry, method: "POST" })).toBe( + "(empty)", + ); + }); + + it("says only that the body was not captured before the stream is watched", () => { + expect(uncapturedBodyNote(streamEntry)).toBe( + "Long-lived stream — body not captured", + ); + }); + + it("reports the event count and open state, with the singular for one event", () => { + expect( + uncapturedBodyNote({ ...streamEntry, stream: { eventCount: 3 } }), + ).toBe( + "Long-lived stream — 3 events delivered, still open; body not captured", + ); + expect( + uncapturedBodyNote({ + ...streamEntry, + stream: { eventCount: 1, closedAt: new Date() }, + }), + ).toBe("Long-lived stream — 1 event delivered, closed; body not captured"); + }); +}); diff --git a/clients/web/src/utils/uncapturedBodyNote.ts b/clients/web/src/utils/uncapturedBodyNote.ts new file mode 100644 index 00000000..e8e886fb --- /dev/null +++ b/clients/web/src/utils/uncapturedBodyNote.ts @@ -0,0 +1,50 @@ +// The Network entry's body placeholder for a response whose body was not +// captured (#2318). Pure: a string from the entry alone. + +import { + findHeader, + isLongLivedStreamResponse, + longLivedStreamFraming, +} from "@inspector/core/mcp/fetchTracking.js"; +import type { FetchRequestEntry } from "@inspector/core/mcp/types.js"; + +/** + * Whether the entry is an unbounded server-to-client stream (never + * buffered). The header lookup is case-insensitive: a recorded entry keeps + * the wire casing (`Content-Type` on some hosts, and in a restored session), + * and a miss here would silently reclassify the stream as a bounded body. + */ +export function isLongLivedStreamEntry(entry: FetchRequestEntry): boolean { + return isLongLivedStreamResponse( + entry.method, + findHeader(entry.responseHeaders, "content-type"), + ); +} + +/** + * The badge label for a long-lived stream entry — `SSE` or `NDJSON`, by the + * same content-type split the tracker counts events with. + */ +export function longLivedStreamLabel(entry: FetchRequestEntry): string { + return longLivedStreamFraming( + findHeader(entry.responseHeaders, "content-type"), + ) === "ndjson" + ? "NDJSON" + : "SSE"; +} + +/** + * What stands in for a body that was not captured. A long-lived stream is + * never buffered, so its placeholder reports what the tracker learned by + * watching it: how many events it delivered and whether it is still open. + * Before the first update, or for a log restored from a session that + * predates the field, the note says only that the body was not captured. + */ +export function uncapturedBodyNote(entry: FetchRequestEntry): string { + if (!isLongLivedStreamEntry(entry)) return "(empty)"; + const stream = entry.stream; + if (!stream) return "Long-lived stream — body not captured"; + const events = `${stream.eventCount} event${stream.eventCount === 1 ? "" : "s"} delivered`; + const state = stream.closedAt ? "closed" : "still open"; + return `Long-lived stream — ${events}, ${state}; body not captured`; +} diff --git a/core/mcp/__tests__/fakeInspectorClient.ts b/core/mcp/__tests__/fakeInspectorClient.ts index bf77d19c..41bcfcc1 100644 --- a/core/mcp/__tests__/fakeInspectorClient.ts +++ b/core/mcp/__tests__/fakeInspectorClient.ts @@ -40,6 +40,7 @@ import type { } from "../types.js"; import { INACTIVE_SUBSCRIPTION_STREAM_STATE } from "../types.js"; import type { MalformedListItem } from "../listSalvage.js"; +import type { ConnectionDiagnostics } from "../connectionDiagnostics.js"; import type { SkillEntry, SkillResource } from "../skillsSchemas.js"; import type { SkillsExtensionSupport } from "../skills.js"; import type { JsonValue } from "../../json/jsonUtils.js"; @@ -321,6 +322,21 @@ export class FakeInspectorClient return this.discoverResult; } + private connectionDiagnostics: ConnectionDiagnostics = { + capturedAt: 0, + outstandingRequests: [], + }; + + getConnectionDiagnostics(): ConnectionDiagnostics { + return this.connectionDiagnostics; + } + + /** Test helper: set the diagnostics snapshot and emit the change (#2318). */ + setConnectionDiagnostics(diagnostics: ConnectionDiagnostics): void { + this.connectionDiagnostics = diagnostics; + this.dispatchTypedEvent("connectionDiagnosticsChange", diagnostics); + } + resourceSubscriptionStreamState: ResourceSubscriptionStreamState = INACTIVE_SUBSCRIPTION_STREAM_STATE; diff --git a/core/mcp/connectionDiagnostics.ts b/core/mcp/connectionDiagnostics.ts new file mode 100644 index 00000000..9163f557 --- /dev/null +++ b/core/mcp/connectionDiagnostics.ts @@ -0,0 +1,252 @@ +/** + * Connection diagnostics: what the client is still waiting on, when it last + * heard back, and the state of the server-to-client notification stream — and + * the one place that turns the SDK's bare `Request timed out` into a message + * that reports those facts (#2318). + * + * The motivating case is #2187: a server that serves one request at a time. + * The standalone `GET /mcp` stream the client opens after `initialize` + * occupies that single slot, so `initialize` succeeds and every request after + * it hangs — and the Inspector's only word on it was `Request timed out`, + * which names the symptom the user can already see. Every fact needed to + * suspect the cause was already in-process at that moment: the stream had been + * open for the whole session and delivered nothing, `tools/list` had been + * outstanding for 60s, and nothing had been answered since `initialize`. + * + * Everything here is pure. `InspectorClient` owns the bookkeeping (it sees + * every outbound request, every inbound response and every tracked fetch) and + * hands a snapshot to {@link annotateRequestTimeout}; the UI clients render the + * same snapshot through `getConnectionDiagnostics()`. + * + * Deliberately factual: the summary reports what the connection's state *is* + * and lets the reader draw the conclusion. It does not assert a diagnosis the + * Inspector cannot verify from its own side. + */ + +import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client"; + +/** A request this client sent that no response has arrived for. */ +export interface OutstandingRequest { + id: string | number; + method: string; + /** Epoch ms at which the request went out. */ + sentAt: number; +} + +/** The most recent response received to one of this client's requests. */ +export interface LastResponse { + /** The method of the request it answered. */ + method: string; + /** Epoch ms at which it arrived. */ + receivedAt: number; +} + +/** + * The most recent long-lived server-to-client stream the transport opened — + * the standalone `GET` on Streamable HTTP, or the primary event stream on + * legacy SSE. Absent on stdio, and until the first such stream is opened. + */ +export interface NotificationStreamState { + /** The request URL, as recorded (query already redacted). */ + url: string; + /** Epoch ms at which the response headers arrived and the stream opened. */ + openedAt: number; + /** SSE events delivered so far — messages, not keepalive comments. */ + eventCount: number; + /** Epoch ms at which the stream ended, once it has. */ + closedAt?: number; +} + +export interface ConnectionDiagnostics { + /** + * Epoch ms at which this snapshot was taken. Every duration derived from + * the snapshot ("sent 60s ago", "open for 4m12s") is measured against this + * clock rather than against the moment of rendering, so the snapshot is + * self-contained: a message written from it, a component rendering it and + * a test asserting on it all read the same numbers. + */ + capturedAt: number; + /** + * Every request sent this session that has received no response, oldest + * first. A request the SDK gave up on (a timeout, a cancel) stays here: it + * is still unanswered, which is the fact being reported. + */ + outstandingRequests: OutstandingRequest[]; + lastResponse?: LastResponse; + notificationStream?: NotificationStreamState; +} + +/** + * The data the SDK attaches to a per-request timeout: `{ timeout }` in ms. + * Narrowed here rather than imported because the SDK types it `unknown`. + */ +interface RequestTimeoutData { + timeout: number; +} + +function isRequestTimeoutData(value: unknown): value is RequestTimeoutData { + return ( + typeof value === "object" && + value !== null && + typeof (value as { timeout?: unknown }).timeout === "number" + ); +} + +/** + * Whether `err` is the SDK's per-request timeout: an `SdkError` with + * `RequestTimeout` and a numeric `timeout` in its data. + * + * The data check is not decoration. `Protocol.request` reuses the + * `RequestTimeout` code for two other rejections that are not timeouts at + * all — an abort through `options.signal` whose reason is not an `SdkError` + * (the Cancel button), and "Maximum total timeout exceeded" — and neither + * carries `{ timeout }`. Only the genuine per-request timeout does. + */ +export function isRequestTimeoutError( + err: unknown, +): err is SdkError & { data: RequestTimeoutData } { + return ( + SdkError.isInstance(err) && + err.code === SdkErrorCode.RequestTimeout && + isRequestTimeoutData(err.data) + ); +} + +/** + * A compact human duration: `850ms`, `12s`, `4m12s`, `1h02m`. Sub-second + * values are shown in ms so a very short timeout still reads as a number + * rather than as `0s`. + */ +export function formatDuration(ms: number): string { + const clamped = Math.max(0, ms); + if (clamped < 1000) return `${Math.round(clamped)}ms`; + const totalSeconds = Math.round(clamped / 1000); + const hours = Math.floor(totalSeconds / 3600); + const minutes = Math.floor((totalSeconds % 3600) / 60); + const seconds = totalSeconds % 60; + if (hours > 0) return `${hours}h${String(minutes).padStart(2, "0")}m`; + if (minutes > 0) return `${minutes}m${String(seconds).padStart(2, "0")}s`; + return `${seconds}s`; +} + +/** + * How many outstanding requests the summary names individually before + * collapsing the rest into "and N more". Enough to show the shape of a stall + * (the one everything is queued behind, plus what queued behind it) without + * turning a toast into a page. + */ +export const MAX_LISTED_OUTSTANDING_REQUESTS = 5; + +/** + * `GET /mcp` for a stream URL: method plus path only. The origin is the + * server the user is already connected to, and the query string (already + * redacted at tracking time) adds nothing to a one-line summary. Falls back to + * the raw string for an unparseable URL rather than dropping the clause. + */ +export function describeStreamEndpoint(url: string): string { + try { + return `GET ${new URL(url).pathname}`; + } catch { + return `GET ${url}`; + } +} + +function plural(count: number, noun: string): string { + return `${count} ${noun}${count === 1 ? "" : "s"}`; +} + +function describeOutstanding( + requests: OutstandingRequest[], + now: number, +): string { + if (requests.length === 0) return "No requests are unanswered."; + const listed = requests + .slice(0, MAX_LISTED_OUTSTANDING_REQUESTS) + .map((r) => `${r.method} (sent ${formatDuration(now - r.sentAt)} ago)`); + const rest = requests.length - listed.length; + const tail = rest > 0 ? `, and ${rest} more` : ""; + const verb = requests.length === 1 ? "is" : "are"; + return `${plural(requests.length, "request")} ${verb} unanswered: ${listed.join(", ")}${tail}.`; +} + +function describeLastResponse( + lastResponse: LastResponse | undefined, + now: number, +): string { + if (!lastResponse) return "No response has been received this session."; + return `Last response received ${formatDuration(now - lastResponse.receivedAt)} ago (${lastResponse.method}).`; +} + +function describeStream( + stream: NotificationStreamState | undefined, + now: number, +): string | undefined { + if (!stream) return undefined; + const endpoint = describeStreamEndpoint(stream.url); + const events = plural(stream.eventCount, "event"); + if (stream.closedAt !== undefined) { + return `Notification stream (${endpoint}) closed ${formatDuration(now - stream.closedAt)} ago after ${formatDuration(stream.closedAt - stream.openedAt)}, ${events} delivered.`; + } + return `Notification stream (${endpoint}) open for ${formatDuration(now - stream.openedAt)}, ${events} delivered.`; +} + +/** + * One paragraph describing the connection's state as of the snapshot's own + * clock: outstanding requests, the last response, and the notification + * stream when there is one. Each clause is a sentence, so a consumer that + * wants only one of them can split on the sentence boundary rather than + * re-deriving it. + */ +export function describeConnectionDiagnostics( + diagnostics: ConnectionDiagnostics, +): string { + const now = diagnostics.capturedAt; + const parts = [ + describeOutstanding(diagnostics.outstandingRequests, now), + describeLastResponse(diagnostics.lastResponse, now), + describeStream(diagnostics.notificationStream, now), + ]; + return parts.filter((p): p is string => p !== undefined).join(" "); +} + +/** + * The data carried by an annotated timeout: the SDK's own `{ timeout }`, plus + * the method that timed out and the snapshot the message was written from. + * Exposed as a type so a consumer that wants the structured form (a details + * modal, a CLI's JSON output) can read it off `error.data` instead of parsing + * the message. + */ +export interface AnnotatedRequestTimeoutData extends RequestTimeoutData { + method: string; + diagnostics: ConnectionDiagnostics; +} + +/** + * Rebuild the SDK's per-request timeout with a message that says which + * request timed out and what the connection looked like at that moment. + * + * Anything that is not that exact error is returned untouched — the SDK's + * other `RequestTimeout` rejections (see {@link isRequestTimeoutError}), and + * every other failure — so this is safe to apply blindly on the rejection + * path of every request. + * + * The result is a fresh `SdkError` with the same code, so an `isInstance` or + * code check downstream sees exactly what it saw before; only the message and + * the data change. The original is kept as `cause`. + */ +export function annotateRequestTimeout( + err: unknown, + method: string, + diagnostics: ConnectionDiagnostics, +): unknown { + if (!isRequestTimeoutError(err)) return err; + const data: AnnotatedRequestTimeoutData = { + ...err.data, + method, + diagnostics, + }; + const message = `Request timed out after ${formatDuration(err.data.timeout)} (${method}). ${describeConnectionDiagnostics(diagnostics)}`; + const annotated = new SdkError(SdkErrorCode.RequestTimeout, message, data); + annotated.cause = err; + return annotated; +} diff --git a/core/mcp/fetchTracking.ts b/core/mcp/fetchTracking.ts index 7e1e5864..10eb1d00 100644 --- a/core/mcp/fetchTracking.ts +++ b/core/mcp/fetchTracking.ts @@ -1,4 +1,4 @@ -import type { FetchRequestEntryBase } from "./types.js"; +import type { FetchRequestEntryBase, FetchStreamState } from "./types.js"; /** * Header names whose values are replaced with `REDACTED_HEADER_VALUE` before a @@ -190,11 +190,18 @@ export function redactBody( return body; } -/** Case-insensitive lookup of a header value from a plain header record. */ -function findHeader( - headers: Record, +/** + * Case-insensitive lookup of a header value from a plain header record. + * Exported because a recorded entry's header casing is host-dependent (Node + * lowercases, happy-dom preserves the wire casing), so any consumer reading + * one back — the Network UI, the connection diagnostics — needs the same + * tolerance the tracker itself has. + */ +export function findHeader( + headers: Record | undefined, name: string, ): string | undefined { + if (!headers) return undefined; const target = name.toLowerCase(); for (const [key, value] of Object.entries(headers)) { if (key.toLowerCase() === target) return value; @@ -215,11 +222,13 @@ export function isLongLivedStreamResponse( method: string, contentType: string | null | undefined, ): boolean { - if (method !== "GET") return false; + if (method.toUpperCase() !== "GET") return false; if (!contentType) return false; + // Media types are case-insensitive (RFC 9110 §8.3.1); a server may send + // `Text/Event-Stream` and still be sending an event stream. + const type = contentType.toLowerCase(); return ( - contentType.includes("text/event-stream") || - contentType.includes("application/x-ndjson") + type.includes("text/event-stream") || type.includes("application/x-ndjson") ); } @@ -233,6 +242,194 @@ export interface FetchTrackingCallbacks { * never invoked and the entry's responseBody stays undefined. */ updateResponseBody?: (id: string, responseBody: string) => void; + /** + * Called for a long-lived stream response — the one case `updateResponseBody` + * never covers — each time an SSE event is delivered on it, and once more + * when it ends (with `closedAt` set). Lets the consumer show that the stream + * is open and what it has carried, which is the whole diagnostic when a + * server serializes requests behind it (#2318). + */ + updateStream?: (id: string, stream: FetchStreamState) => void; + /** + * How often, at most, `updateStream` reports a moving event count, in ms. + * The first report (open, zero events) and the close are immediate; counts + * in between are coalesced so a server pushing hundreds of notifications a + * second costs one update per window rather than one per event — each + * update re-publishes the whole fetch log and, on the web client, crosses + * the backend-to-browser event stream. Defaults to + * `DEFAULT_STREAM_UPDATE_INTERVAL_MS`; `0` reports every event, for tests. + */ + streamUpdateIntervalMs?: number; +} + +/** See `FetchTrackingCallbacks.streamUpdateIntervalMs`. */ +export const DEFAULT_STREAM_UPDATE_INTERVAL_MS = 250; + +/** + * The framing a long-lived stream's events arrive in — the two content types + * `isLongLivedStreamResponse` accepts. SSE dispatches on a blank line; + * NDJSON dispatches on every line. + */ +export type LongLivedStreamFraming = "sse" | "ndjson"; + +/** The framing for a long-lived stream's content type. */ +export function longLivedStreamFraming( + contentType: string | null | undefined, +): LongLivedStreamFraming { + return contentType?.toLowerCase().includes("application/x-ndjson") + ? "ndjson" + : "sse"; +} + +/** + * Count the events on a long-lived stream without buffering it, and report + * when it ends. + * + * Reads a `clone()` of the response through a reader and discards the bytes, + * so the transport keeps consuming the original at its own pace (the same + * tee the bounded-body path relies on). Counting follows the framing the + * transport's own parser applies. For SSE a blank line dispatches the block + * before it, and only a block carrying a `data` field is an event — + * keepalive comments (`: ping`) and bare `event:`/`id:` lines are not; a + * partial block at end-of-stream is dropped, exactly as the SSE spec and the + * SDK's `EventSourceParserStream` (which has no flush) drop it. For NDJSON + * every non-blank line is one event. + * + * Reports once as soon as watching starts (zero events, open), once per event, + * and once more on every exit from the read loop — a clean end-of-stream, a + * network error, or the transport aborting the fetch on disconnect (which + * errors both tee branches) — with `closedAt` set. The reader is never cancelled from here: + * cancelling one branch of a tee does not release the source, and + * `ReadableStream.cancel()` adopts the source's promise, which on a teardown + * path may never settle (the same hazard `core/mcp/remote/node/server.ts` + * documents at its own `cancel()` call). + */ +function watchLongLivedStream( + response: Response, + id: string, + framing: LongLivedStreamFraming, + callbacks: FetchTrackingCallbacks, +): void { + const listener = callbacks.updateStream; + if (!listener) return; + // Every report goes through here: the listener is consumer code (a relay + // sink that may already be closed, a state store), and it runs from three + // places none of which may throw outward — synchronously inside the fetch + // wrapper (the open), from a timer (a coalesced count), and off the + // discarded watcher promise (the close). A throw is the listener's + // problem; the stream's bookkeeping carries on. + const report = (state: FetchStreamState): void => { + try { + listener(id, state); + } catch { + // See above. + } + }; + const intervalMs = + callbacks.streamUpdateIntervalMs ?? DEFAULT_STREAM_UPDATE_INTERVAL_MS; + let body: ReadableStream | null; + try { + body = response.clone().body; + } catch { + // Clone failed (consumed body, transport quirks) — there is nothing to + // watch; the entry stays as tracked with no stream state. + return; + } + if (!body) return; + const reader = body.getReader(); + const decoder = new TextDecoder(); + let eventCount = 0; + let pending = ""; + let blockHasData = false; + // A moving count is reported at most once per interval (see + // `streamUpdateIntervalMs`); the open and the close go out at once. + let reportTimer: ReturnType | null = null; + const reportCount = (): void => { + if (intervalMs <= 0) { + report({ eventCount }); + return; + } + if (reportTimer !== null) return; + reportTimer = setTimeout(() => { + reportTimer = null; + report({ eventCount }); + }, intervalMs); + }; + // Announce the stream as open before a byte arrives. A stream that never + // delivers anything is the #2187 tell, and it would otherwise be the one + // stream with no state to show — the count only moves on an event. + report({ eventCount }); + + const consumeLine = (line: string): void => { + if (framing === "ndjson") { + if (line.trim() === "") return; + eventCount += 1; + reportCount(); + return; + } + if (line === "") { + if (blockHasData) { + eventCount += 1; + blockHasData = false; + reportCount(); + } + return; + } + // A field line is `name[:value]`; `data` with no colon is still a data + // field per the spec, so match the name rather than the `data:` prefix. + if (line === "data" || line.startsWith("data:")) blockHasData = true; + }; + + // Split off every complete line in `pending`. The SSE grammar (and the + // SDK's parser) accept LF, CRLF and a bare CR as a line ending, so all three + // end a line here; a CR that is the last byte so far is held back, since + // the LF of a CRLF pair may still be in flight in the next chunk. + const consumeCompleteLines = (): void => { + for (;;) { + const cr = pending.indexOf("\r"); + const lf = pending.indexOf("\n"); + const end = cr === -1 ? lf : lf === -1 ? cr : Math.min(cr, lf); + if (end === -1) return; + if (pending[end] === "\r") { + if (end === pending.length - 1) return; + const width = pending[end + 1] === "\n" ? 2 : 1; + consumeLine(pending.slice(0, end)); + pending = pending.slice(end + width); + } else { + consumeLine(pending.slice(0, end)); + pending = pending.slice(end + 1); + } + } + }; + + const pump = async (): Promise => { + try { + for (;;) { + const { value, done } = await reader.read(); + if (done) break; + pending += decoder.decode(value, { stream: true }); + consumeCompleteLines(); + } + // A CR held back as a possible half of a CRLF is, at end-of-stream, a + // whole line ending — `data: x\r\r` is a complete event. Anything after + // it is a partial block and is dropped, as above. + if (pending.endsWith("\r")) consumeLine(pending.slice(0, -1)); + } catch { + // An errored or aborted stream has still ended — fall through to report + // the close; the entry's `error` field is for a fetch that never got a + // response, which this one did. + } + if (reportTimer !== null) { + clearTimeout(reportTimer); + reportTimer = null; + } + report({ eventCount, closedAt: new Date() }); + }; + // Deliberately not awaited: the fetch wrapper has to return the response + // now, and this watcher lives as long as the stream does. It owns its own + // failures — the read loop is wrapped in a `try`, and every report goes + // through the non-throwing `report` above — so the promise cannot reject. + void pump(); } /** @@ -257,7 +454,12 @@ export function createFetchTracker( : input instanceof URL ? input.toString() : input.url; - const method = init?.method || "GET"; + // The method decides the long-lived-stream classification below, so it + // has to be the one the request actually carries: a `Request` input + // carries its own, and HTTP methods compare case-insensitively. + const method = ( + init?.method ?? (input instanceof Request ? input.method : "GET") + ).toUpperCase(); // Extract headers, redacting sensitive values BEFORE they reach any // downstream sink (logger, in-memory list, persisted session storage). @@ -378,6 +580,18 @@ export function createFetchTracker( callbacks.trackRequest?.(entry); + // A long-lived stream is watched instead of read: its body never ends, + // but its event count and its eventual close are exactly the state a + // stalled connection is diagnosed from (#2318). + if (isLongLivedStream && response.body && !response.bodyUsed) { + watchLongLivedStream( + response, + id, + longLivedStreamFraming(response.headers.get("content-type")), + callbacks, + ); + } + // Kick off a fire-and-forget read of the cloned body. The clone is an // independent tee'd stream so the transport keeps consuming the // original at its own pace. When the read resolves we patch the entry diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index c6fc0c64..72f3c192 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -13,6 +13,7 @@ import type { MessageOrigin, FetchRequestEntry, FetchRequestEntryBase, + FetchStreamState, InspectorServerSettings, ResourceReadInvocation, ResourceTemplateReadInvocation, @@ -265,10 +266,31 @@ import { import type { TokenRevocationOutcome } from "../auth/revocation.js"; import type { OAuthTokens } from "@modelcontextprotocol/client"; import { silentLogger, type InspectorLogger } from "../logging/logger.js"; -import { createFetchTracker } from "./fetchTracking.js"; +import { + createFetchTracker, + findHeader, + isLongLivedStreamResponse, +} from "./fetchTracking.js"; +import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client"; +import { + annotateRequestTimeout, + isRequestTimeoutError, + type ConnectionDiagnostics, + type LastResponse, + type NotificationStreamState, + type OutstandingRequest, +} from "./connectionDiagnostics.js"; import { OAuthManager, type OAuthManagerConfig } from "./oauthManager.js"; import { RemoteClientTransport } from "./remote/remoteClientTransport.js"; +/** + * The notification stream the diagnostics are following, plus the fetch entry + * id that ties later `fetchRequestStreamUpdate`s back to it (#2318). + */ +interface TrackedNotificationStream extends NotificationStreamState { + id: string; +} + /** Internal record for a receiver task (server polls us for status/result). */ interface ReceiverTaskRecord { task: Task; @@ -490,13 +512,23 @@ export class InspectorClient extends InspectorClientEventTarget { private outputValidator: AjvJsonSchemaValidator | null = null; private transport: Transport | MessageTrackingTransport | null = null; private baseTransport: Transport | null = null; - // Correlation for `markResponseRejected` (#1953): the method of each - // outbound request still awaiting a response, and — once one is answered — - // the id of the most recently answered request per method. Entries are - // dropped as responses arrive, so this holds at most one id per method - // rather than growing with the session. - private outboundRequestMethods = new Map(); + // Every outbound request still awaiting a response, keyed by JSON-RPC id. + // Serves two readers: the `markResponseRejected` correlation (#1953), which + // needs the method, and the connection diagnostics (#2318), which need the + // method and when it went out. Entries are dropped as responses arrive; a + // request the SDK gave up on (a timeout, a cancel) stays until the session + // resets, because it is still unanswered and that is what the diagnostics + // report. + private outstandingRequests = new Map(); + // The id of the most recently answered request per method, for + // `markResponseRejected` (#1953). At most one id per method, so it does not + // grow with the session. private lastAnsweredRequestByMethod = new Map(); + // Connection diagnostics (#2318): the most recent response to one of our + // requests, and the most recent long-lived transport stream (the standalone + // `GET` on Streamable HTTP). Both per-session, reset with the maps above. + private lastResponse: LastResponse | undefined; + private notificationStream: TrackedNotificationStream | undefined; /** True when the cached transport was built with an OAuth authProvider attached. */ private transportHasAuthProvider = false; /** Dedupes concurrent ambient auth challenges (reason + scopes). */ @@ -1021,6 +1053,47 @@ export class InspectorClient extends InspectorClientEventTarget { this.clientInfo, Object.keys(clientOptions).length > 0 ? clientOptions : undefined, ); + this.annotateSdkRequestTimeouts(this.client); + } + + /** + * Decorate the SDK Client's `request` so a per-request timeout rejects with + * the connection's state at that moment instead of a bare `Request timed + * out` (#2318). + * + * `Protocol.request` is the one funnel every SDK verb goes through — + * `listTools`, `callTool`, `readResource`, the handshake's `initialize`, + * and this class's own raw `client.request` calls all reach the wire + * through it — so decorating it here covers the whole surface without a + * `catch` at each of the several dozen call sites, and covers the next + * verb added without anyone remembering to. The SDK offers no hook on the + * rejection path: the timeout error is constructed and thrown inside + * `request()` itself. + * + * Replaced on the instance rather than through a subclass because + * `request` is declared with two overloads (spec method with a keyed + * result, custom method with an explicit schema), and an `override` would + * have to restate both against types that move with every SDK release. One + * implementation signature serves both at runtime; the single cast on the + * way back is what the overloads cost. + */ + private annotateSdkRequestTimeouts(client: Client): void { + type SdkRequest = Client["request"]; + const sdkRequest: SdkRequest = client.request.bind(client); + const decorated = async ( + ...args: Parameters + ): Promise => { + try { + return await sdkRequest(...args); + } catch (err) { + throw annotateRequestTimeout( + err, + args[0].method, + this.getConnectionDiagnostics(), + ); + } + }; + client.request = decorated as SdkRequest; } /** @@ -1058,7 +1131,12 @@ export class InspectorClient extends InspectorClientEventTarget { return { trackRequest: (message: JSONRPCRequest, origin: MessageOrigin) => { if (origin === "client") { - this.outboundRequestMethods.set(message.id, message.method); + this.outstandingRequests.set(message.id, { + id: message.id, + method: message.method, + sentAt: Date.now(), + }); + this.dispatchConnectionDiagnosticsChange(); } const entry: MessageEntry = { id: crypto.randomUUID(), @@ -1069,6 +1147,20 @@ export class InspectorClient extends InspectorClientEventTarget { }; this.dispatchTypedEvent("message", entry); }, + trackSendFailure: (message: JSONRPCRequest, error: unknown) => { + // A send that failed because the request timed out is not a send + // that failed: the browser's remote transport awaits the response + // inside `send`, so its relay timeout surfaces here — the request + // reached the server and went unanswered, which is the state to + // keep reporting. Every other failure means the frame never reached + // the wire, and leaving it would report a request the server never + // saw as unanswered, in every timeout message and Connection Info + // row, until the session resets (#2318). + if (isRequestTimeoutError(error)) return; + if (this.outstandingRequests.delete(message.id)) { + this.dispatchConnectionDiagnosticsChange(); + } + }, trackResponse: ( message: JSONRPCResultResponse | JSONRPCErrorResponse, origin: MessageOrigin, @@ -1080,11 +1172,13 @@ export class InspectorClient extends InspectorClientEventTarget { // frame answers no specific request, so it is skipped. const responseId = message.id; if (origin === "server" && responseId !== undefined) { - const method = this.outboundRequestMethods.get(responseId); - this.outboundRequestMethods.delete(responseId); + const method = this.outstandingRequests.get(responseId)?.method; + this.outstandingRequests.delete(responseId); if (method !== undefined) { this.lastAnsweredRequestByMethod.set(method, responseId); + this.lastResponse = { method, receivedAt: Date.now() }; } + this.dispatchConnectionDiagnosticsChange(); } const entry: MessageEntry = { id: crypto.randomUUID(), @@ -1943,8 +2037,11 @@ export class InspectorClient extends InspectorClientEventTarget { // start-clean path rather than in `disconnect()` for the reason documented // above: one route out (`onerror` with no `onclose`) tears down nothing // (#1953). - this.outboundRequestMethods.clear(); + this.outstandingRequests.clear(); this.lastAnsweredRequestByMethod.clear(); + this.lastResponse = undefined; + this.notificationStream = undefined; + this.dispatchConnectionDiagnosticsChange(); // Per-session for the same reason: both name entries of the PREVIOUS // server's list. Cleared here as well as in `disconnect()` because the // route out that tears down nothing (`onerror` with no `onclose`) would @@ -2113,10 +2210,14 @@ export class InspectorClient extends InspectorClientEventTarget { }, onFetchRequest: (entry: FetchRequestEntryBase) => { this.dispatchFetchRequest({ ...entry, category: "transport" }); + this.noteTransportStream(entry); }, onFetchResponseBody: (id: string, body: string) => { this.dispatchFetchRequestBodyUpdate(id, body); }, + onFetchStreamUpdate: (id: string, stream: FetchStreamState) => { + this.dispatchFetchRequestStreamUpdate(id, stream); + }, ...(this.serverSettings && { settings: this.serverSettings }), }; if (this.isHttpOAuthConfig() && oauthManager) { @@ -2857,8 +2958,19 @@ export class InspectorClient extends InspectorClientEventTarget { const raw = await new Promise((resolve, reject) => { const timer = setTimeout(() => { this.pendingRawWireRequests.delete(id); + // The same error, with the same annotation, as an SDK request that + // times out: this path bypasses `Protocol.request`, so it builds the + // SDK's own timeout shape and runs it through the decorator's + // annotation by hand — a raw-wire caller sees one kind of timeout, + // not two (#2318). reject( - new Error(`Raw request "${method}" timed out after ${timeoutMs} ms`), + annotateRequestTimeout( + new SdkError(SdkErrorCode.RequestTimeout, "Request timed out", { + timeout: timeoutMs, + }), + method, + this.getConnectionDiagnostics(), + ), ); }, timeoutMs); this.pendingRawWireRequests.set(id, { resolve, reject, timer }); @@ -2868,7 +2980,17 @@ export class InspectorClient extends InspectorClientEventTarget { clearTimeout(pending.timer); this.pendingRawWireRequests.delete(id); } - reject(err instanceof Error ? err : new Error(String(err))); + // The browser's remote transport awaits the response inside `send`, + // so its relay wait can expire here first, as the SDK's timeout + // shape; annotate it exactly as the local timer above does (a + // non-timeout error passes through untouched). + reject( + annotateRequestTimeout( + err instanceof Error ? err : new Error(String(err)), + method, + this.getConnectionDiagnostics(), + ), + ); }); }); return resultSchema.parse(raw); @@ -5989,6 +6111,87 @@ export class InspectorClient extends InspectorClientEventTarget { this.dispatchTypedEvent("fetchRequestBodyUpdate", { id, responseBody }); } + private dispatchFetchRequestStreamUpdate( + id: string, + stream: FetchStreamState, + ): void { + this.dispatchTypedEvent("fetchRequestStreamUpdate", { id, stream }); + // The diagnostics follow only the stream they are currently watching; + // an update for an older one (a reconnected stream's predecessor + // closing late) changes nothing they report. + if (this.notificationStream?.id !== id) return; + this.notificationStream = { + ...this.notificationStream, + eventCount: stream.eventCount, + ...(stream.closedAt && { closedAt: stream.closedAt.getTime() }), + }; + this.dispatchConnectionDiagnosticsChange(); + } + + /** + * Adopt a transport fetch as the notification stream when it is one — a + * `GET` answered with an unbounded event stream — so the diagnostics can + * say how long it has been open and what it has delivered (#2318). The + * newest such stream wins: the SDK reopens the standalone `GET` after a + * drop, and the current one is the one a stalled request is queued behind. + */ + private noteTransportStream(entry: FetchRequestEntryBase): void { + if ( + !isLongLivedStreamResponse( + entry.method, + findHeader(entry.responseHeaders, "content-type"), + ) + ) { + return; + } + this.notificationStream = { + id: entry.id, + url: entry.url, + // The entry's `timestamp` is when the request went out; the stream is + // open from when the response headers arrived, `duration` later. + openedAt: entry.timestamp.getTime() + (entry.duration ?? 0), + eventCount: 0, + }; + this.dispatchConnectionDiagnosticsChange(); + } + + /** + * What this client is still waiting on, when it last heard back, and the + * state of the notification stream (#2318). A fresh snapshot per call — + * the same one `annotateRequestTimeout` writes a timeout message from, and + * the one `connectionDiagnosticsChange` carries. + */ + getConnectionDiagnostics(): ConnectionDiagnostics { + return { + capturedAt: Date.now(), + // Copies, not the tracked records: the snapshot is handed to callers + // and carried on errors, and a mutation there must not reach the + // correlation state. + outstandingRequests: [...this.outstandingRequests.values()].map( + (request) => ({ ...request }), + ), + ...(this.lastResponse && { lastResponse: { ...this.lastResponse } }), + ...(this.notificationStream && { + notificationStream: this.publicStreamState(this.notificationStream), + }), + }; + } + + /** The stream state minus the entry id, which is bookkeeping, not a fact. */ + private publicStreamState( + stream: TrackedNotificationStream, + ): NotificationStreamState { + const { id: _id, ...state } = stream; + return state; + } + + private dispatchConnectionDiagnosticsChange(): void { + this.dispatchTypedEvent( + "connectionDiagnosticsChange", + this.getConnectionDiagnostics(), + ); + } + /** * Get current session ID (from OAuth state authId) */ diff --git a/core/mcp/inspectorClientEventTarget.ts b/core/mcp/inspectorClientEventTarget.ts index 58aa41f6..f9ce6c8c 100644 --- a/core/mcp/inspectorClientEventTarget.ts +++ b/core/mcp/inspectorClientEventTarget.ts @@ -20,6 +20,7 @@ import type { MessageEntry, StderrLogEntry, FetchRequestEntry, + FetchStreamState, PromptGetInvocation, ResourceReadInvocation, ResourceTemplateReadInvocation, @@ -28,6 +29,7 @@ import type { RequestMetadata, } from "./types.js"; import type { MalformedListItem } from "./listSalvage.js"; +import type { ConnectionDiagnostics } from "./connectionDiagnostics.js"; import type { Tool, ServerCapabilities, @@ -92,6 +94,20 @@ export interface InspectorClientEventMap { fetchRequest: FetchRequestEntry; /** Fired when an in-flight fetch's response body is read asynchronously. */ fetchRequestBodyUpdate: { id: string; responseBody: string }; + /** + * Fired as a tracked long-lived stream (the standalone `GET` on Streamable + * HTTP, the legacy SSE event stream) delivers events and when it ends. The + * body of such a stream is never captured, so this is the log's only view + * of its lifetime (#2318). + */ + fetchRequestStreamUpdate: { id: string; stream: FetchStreamState }; + /** + * Fired whenever the connection diagnostics snapshot changes — a request + * goes out, a response comes back, the notification stream opens, delivers + * an event or closes, or a new session resets it. Carries the fresh + * snapshot; `getConnectionDiagnostics()` returns the same one (#2318). + */ + connectionDiagnosticsChange: ConnectionDiagnostics; /** * Fired whenever the client transitions `status` to `"error"` from a path * that is NOT an awaited promise — i.e. a mid-session transport failure diff --git a/core/mcp/inspectorClientProtocol.ts b/core/mcp/inspectorClientProtocol.ts index 53c7134e..8eefd758 100644 --- a/core/mcp/inspectorClientProtocol.ts +++ b/core/mcp/inspectorClientProtocol.ts @@ -40,6 +40,7 @@ import type { } from "@modelcontextprotocol/client"; import type { JsonValue } from "../json/jsonUtils.js"; import type { MalformedListItem } from "./listSalvage.js"; +import type { ConnectionDiagnostics } from "./connectionDiagnostics.js"; import type { InspectorClientEventTarget } from "./inspectorClientEventTarget.js"; import type { SkillEntry } from "./skillsSchemas.js"; import type { SkillsExtensionSupport } from "./skills.js"; @@ -78,6 +79,14 @@ export interface InspectorClientProtocol extends InspectorClientEventTarget { getDiscoverResult(): DiscoverResult | undefined; getServerSettings(): InspectorServerSettings | undefined; setServerSettings(settings: InspectorServerSettings): void; + /** + * What the client is still waiting on, when it last heard back, and the + * state of the notification stream — the snapshot a request timeout is + * annotated with (#2318). Optional so existing test doubles satisfy the + * interface without implementing it; `connectionDiagnosticsChange` fires + * whenever it changes. + */ + getConnectionDiagnostics?(): ConnectionDiagnostics; getAppRendererClient(): AppRendererClient | null; // Connection control diff --git a/core/mcp/messageTrackingTransport.ts b/core/mcp/messageTrackingTransport.ts index 3402e244..c7645dcc 100644 --- a/core/mcp/messageTrackingTransport.ts +++ b/core/mcp/messageTrackingTransport.ts @@ -24,6 +24,19 @@ export interface MessageTrackingCallbacks { message: JSONRPCNotification, origin: MessageOrigin, ) => void; + /** + * An outgoing request the base transport failed to send (the connection + * closed, the fetch failed before the frame reached the wire). Fires after + * `trackRequest` already recorded it, so a consumer keeping "requests still + * awaiting a response" can roll that entry back — a request that never went + * out is not unanswered (#2318). Not fired when the caller aborted the + * request through `requestSignal` *during* the send: the modern transport + * aborts an in-flight request's own stream to cancel it (and the SDK does + * the same on a per-request timeout), and a request that was sent and then + * given up on is exactly the unanswered kind. A signal already aborted + * before the send began is a failed send like any other. + */ + trackSendFailure?: (message: JSONRPCRequest, error: unknown) => void; } /** @@ -95,7 +108,19 @@ export class MessageTrackingTransport implements Transport { "client", ); } else if ("method" in message) { - this.callbacks.trackRequest?.(message as JSONRPCRequest, "client"); + const request = message as JSONRPCRequest; + this.callbacks.trackRequest?.(request, "client"); + // A signal already aborted here means the frame will not go out at + // all — that is a failed send. One that becomes aborted while the + // send is in flight is a cancellation of a request that may well + // have reached the server, which stays unanswered. + const abortedBeforeSend = options?.requestSignal?.aborted === true; + return this.baseTransport.send(message, options).catch((err) => { + if (abortedBeforeSend || !options?.requestSignal?.aborted) { + this.callbacks.trackSendFailure?.(request, err); + } + throw err; + }); } } else if ("method" in message) { this.callbacks.trackNotification?.( diff --git a/core/mcp/node/transport.ts b/core/mcp/node/transport.ts index dd3c8358..c9a8ee54 100644 --- a/core/mcp/node/transport.ts +++ b/core/mcp/node/transport.ts @@ -49,11 +49,18 @@ export function createTransportNode( pipeStderr = false, onFetchRequest, onFetchResponseBody, + onFetchStreamUpdate, authProvider, settings, interceptAuthChallenges = false, onAuthChallengeObserved, } = options; + // Any of the three tracking callbacks earns a tracker: each is optional on + // the tracker itself, and a caller that wants only stream updates (or only + // bodies) must not be silently handed the bare fetch (#2318). + const wantsFetchTracking = Boolean( + onFetchRequest || onFetchResponseBody || onFetchStreamUpdate, + ); // `optionsFetchFn` is the caller's whole fetch stack and already sits on top of // a proxy-aware base when one is needed — the Node clients install @@ -114,10 +121,11 @@ export function createTransportNode( const sseFetch = configuredSseFetch ? withChallengeObserver(configuredSseFetch) : fetchWithOptionalAuthIntercept; - const trackedFetch = onFetchRequest + const trackedFetch = wantsFetchTracking ? createFetchTracker(sseFetch, { trackRequest: onFetchRequest, updateResponseBody: onFetchResponseBody, + updateStream: onFetchStreamUpdate, }) : sseFetch; @@ -134,10 +142,11 @@ export function createTransportNode( ...(headers && { headers }), }; - const postFetch = onFetchRequest + const postFetch = wantsFetchTracking ? createFetchTracker(fetchWithOptionalAuthIntercept, { trackRequest: onFetchRequest, updateResponseBody: onFetchResponseBody, + updateStream: onFetchStreamUpdate, }) : fetchWithOptionalAuthIntercept; @@ -161,10 +170,11 @@ export function createTransportNode( ...(headers && { headers }), }; - const transportFetch = onFetchRequest + const transportFetch = wantsFetchTracking ? createFetchTracker(fetchWithOptionalAuthIntercept, { trackRequest: onFetchRequest, updateResponseBody: onFetchResponseBody, + updateStream: onFetchStreamUpdate, }) : fetchWithOptionalAuthIntercept; diff --git a/core/mcp/remote/createRemoteTransport.ts b/core/mcp/remote/createRemoteTransport.ts index 96803d8b..d93f9b06 100644 --- a/core/mcp/remote/createRemoteTransport.ts +++ b/core/mcp/remote/createRemoteTransport.ts @@ -58,6 +58,7 @@ export function createRemoteTransport( onStderr: transportOptions.onStderr, onFetchRequest: transportOptions.onFetchRequest, onFetchResponseBody: transportOptions.onFetchResponseBody, + onFetchStreamUpdate: transportOptions.onFetchStreamUpdate, authProvider: transportOptions.authProvider, settings: transportOptions.settings, }, diff --git a/core/mcp/remote/node/remote-session.ts b/core/mcp/remote/node/remote-session.ts index 2ae68a7c..5e5be4e1 100644 --- a/core/mcp/remote/node/remote-session.ts +++ b/core/mcp/remote/node/remote-session.ts @@ -4,7 +4,7 @@ import type { Transport } from "@modelcontextprotocol/client"; import type { JSONRPCMessage } from "@modelcontextprotocol/client"; -import type { FetchRequestEntryBase } from "../../types.js"; +import type { FetchRequestEntryBase, FetchStreamState } from "../../types.js"; import type { RemoteEvent } from "../types.js"; import type { AuthChallenge } from "../../../auth/challenge.js"; import { AuthChallengeError } from "../../../auth/challenge.js"; @@ -328,6 +328,17 @@ export class RemoteSession { }); } + onFetchStreamUpdate(id: string, stream: FetchStreamState): void { + this.pushEvent({ + type: "fetch_stream_update", + data: { + id, + eventCount: stream.eventCount, + ...(stream.closedAt && { closedAt: stream.closedAt.toISOString() }), + }, + }); + } + onStderr(entry: { timestamp: Date; message: string }): void { this.pushEvent({ type: "stdio_log", diff --git a/core/mcp/remote/node/server.ts b/core/mcp/remote/node/server.ts index 15179eb1..f08a1f8b 100644 --- a/core/mcp/remote/node/server.ts +++ b/core/mcp/remote/node/server.ts @@ -868,6 +868,8 @@ export function createRemoteApp( onFetchRequest: (entry) => session.onFetchRequest(entry), onFetchResponseBody: (id, body) => session.onFetchResponseBody(id, body), + onFetchStreamUpdate: (id, stream) => + session.onFetchStreamUpdate(id, stream), authProvider: authHandle?.provider, settings: body.settings, // Always intercept 401/403 on the node MCP transport. Without this, the diff --git a/core/mcp/remote/remoteClientTransport.ts b/core/mcp/remote/remoteClientTransport.ts index e1480cd3..30dff297 100644 --- a/core/mcp/remote/remoteClientTransport.ts +++ b/core/mcp/remote/remoteClientTransport.ts @@ -11,8 +11,10 @@ import type { JSONRPCMessage, MessageExtraInfo, } from "@modelcontextprotocol/client"; +import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client"; +import { findHeader, isLongLivedStreamResponse } from "../fetchTracking.js"; import type { InspectorServerSettings, StderrLogEntry } from "../types.js"; -import type { FetchRequestEntryBase } from "../types.js"; +import type { FetchRequestEntryBase, FetchStreamState } from "../types.js"; import type { AuthChallenge, AuthChallengeOutcome, @@ -61,6 +63,9 @@ export interface RemoteTransportOptions { /** Callback for async response-body updates to a previously tracked fetch. */ onFetchResponseBody?: (id: string, responseBody: string) => void; + /** Callback for a previously tracked long-lived stream's events and close (#2318). */ + onFetchStreamUpdate?: (id: string, stream: FetchStreamState) => void; + /** Optional OAuth client provider for Bearer authentication */ authProvider?: import("@modelcontextprotocol/client").OAuthClientProvider; @@ -261,6 +266,14 @@ export class RemoteClientTransport implements Transport { private eventStreamConsumeTask: Promise | null = null; private restartingEventStream = false; private closed = false; + /** + * Long-lived streams the backend has reported open, by fetch entry id, with + * the last event count seen for each. `close()` reports them closed + * itself: it aborts the event channel before asking the backend to + * disconnect, so the backend watcher's own close report lands on an + * unconsumed queue and is lost with the session (#2318). + */ + private openStreams = new Map(); private readonly sseResponseWaits = new Map< string | number, SseResponseWait @@ -539,12 +552,23 @@ export class RemoteClientTransport implements Transport { this.resetSseResponseWait(progressToken); } this.onmessage?.(msg, undefined); - } else if ( - parsed.type === "fetch_request" && - this.options.onFetchRequest - ) { + } else if (parsed.type === "fetch_request") { const entry = parsed.data; - this.options.onFetchRequest({ + // Stream bookkeeping runs whether or not the consumer logs + // requests: `onFetchStreamUpdate` is documented to work on its + // own, and `close()` needs to know which streams are open. It + // exists only to serve that callback, though — without one the + // set would only ever grow, one entry per reconnect. + if ( + this.options.onFetchStreamUpdate && + isLongLivedStreamResponse( + entry.method, + findHeader(entry.responseHeaders, "content-type"), + ) + ) { + this.openStreams.set(entry.id, 0); + } + this.options.onFetchRequest?.({ ...entry, timestamp: typeof entry.timestamp === "string" @@ -559,6 +583,19 @@ export class RemoteClientTransport implements Transport { parsed.data.id, parsed.data.responseBody, ); + } else if ( + parsed.type === "fetch_stream_update" && + this.options.onFetchStreamUpdate + ) { + const { id, eventCount, closedAt } = parsed.data; + if (closedAt !== undefined) this.openStreams.delete(id); + else if (this.openStreams.has(id)) { + this.openStreams.set(id, eventCount); + } + this.options.onFetchStreamUpdate(id, { + eventCount, + ...(closedAt !== undefined && { closedAt: new Date(closedAt) }), + }); } else if (parsed.type === "stdio_log" && this.options.onStderr) { this.options.onStderr({ timestamp: new Date(parsed.data.timestamp), @@ -693,10 +730,17 @@ export class RemoteClientTransport implements Transport { const arm = () => { timer = setTimeout(() => { this.sseResponseWaits.delete(requestId); + // The SDK's own per-request timeout shape, not a plain `Error`: + // this wait can expire before the SDK's timer does (its budget is + // the relay's, not the request's), and the rejection travels up + // through `Client.request`, where `InspectorClient` annotates a + // timeout of exactly this shape with the connection's state + // (#2318). A plain error would pass that decorator untouched and + // reach the user as a bare timeout on the web client alone. reject( - new Error( - `Timed out waiting for MCP response on SSE (${this.sseResponseTimeoutMs}ms)`, - ), + new SdkError(SdkErrorCode.RequestTimeout, "Request timed out", { + timeout: this.sseResponseTimeoutMs, + }), ); }, this.sseResponseTimeoutMs); }; @@ -836,6 +880,13 @@ export class RemoteClientTransport implements Transport { this.closed = true; this.cancelAllSseWaits(new Error("Transport closed")); await this.stopEventStream(); + // The disconnect below ends every stream the backend still holds open, + // and its close reports cannot reach us any more (see `openStreams`). + const closedAt = new Date(); + for (const [id, eventCount] of this.openStreams) { + this.options.onFetchStreamUpdate?.(id, { eventCount, closedAt }); + } + this.openStreams.clear(); if (this._sessionId) { try { diff --git a/core/mcp/remote/sessionStorage.ts b/core/mcp/remote/sessionStorage.ts index 9c24ee59..7d3764fb 100644 --- a/core/mcp/remote/sessionStorage.ts +++ b/core/mcp/remote/sessionStorage.ts @@ -119,7 +119,10 @@ export class RemoteInspectorClientStorage implements InspectorClientStorage { const data = (await res.json()) as InspectorClientSessionState; - // Deserialize state (convert ISO strings back to Date objects) + // Deserialize state (convert ISO strings back to Date objects). The + // nested `stream.closedAt` is a Date too, and `JSON.stringify` turned it + // into an ISO string on the way out; left as a string it would fail the + // `getTime()` every consumer of the type performs (#2318). return { ...data, fetchRequests: data.fetchRequests.map((req) => ({ @@ -130,6 +133,9 @@ export class RemoteInspectorClientStorage implements InspectorClientStorage { : req.timestamp instanceof Date ? req.timestamp : new Date(req.timestamp), + ...(req.stream?.closedAt !== undefined && { + stream: { ...req.stream, closedAt: new Date(req.stream.closedAt) }, + }), })), }; } diff --git a/core/mcp/remote/types.ts b/core/mcp/remote/types.ts index 9e948401..661f1522 100644 --- a/core/mcp/remote/types.ts +++ b/core/mcp/remote/types.ts @@ -137,6 +137,7 @@ export type RemoteEventType = | "message" | "fetch_request" | "fetch_request_body_update" + | "fetch_stream_update" | "stdio_log" | "transport_error" | "auth_challenge"; @@ -156,6 +157,16 @@ export interface RemoteEventFetchRequestBodyUpdate { data: { id: string; responseBody: string }; } +/** + * A long-lived stream's lifecycle, forwarded from the backend's fetch tracker + * (#2318). `closedAt` is an ISO string on the wire; the browser transport + * rebuilds the `Date` the way it does for `fetch_request`'s timestamp. + */ +export interface RemoteEventFetchStreamUpdate { + type: "fetch_stream_update"; + data: { id: string; eventCount: number; closedAt?: string }; +} + export interface RemoteEventStdioLog { type: "stdio_log"; data: { timestamp: string; message: string }; @@ -178,6 +189,7 @@ export type RemoteEvent = | RemoteEventMessage | RemoteEventFetchRequest | RemoteEventFetchRequestBodyUpdate + | RemoteEventFetchStreamUpdate | RemoteEventStdioLog | RemoteEventTransportError | RemoteEventAuthChallenge; diff --git a/core/mcp/state/fetchRequestLogState.ts b/core/mcp/state/fetchRequestLogState.ts index 080fd39b..2e547ad6 100644 --- a/core/mcp/state/fetchRequestLogState.ts +++ b/core/mcp/state/fetchRequestLogState.ts @@ -151,6 +151,28 @@ export class FetchRequestLogState extends TypedEventTarget, + ): void => { + const { id, stream } = event.detail; + const idx = this.fetchRequests.findIndex((e) => e.id === id); + if (idx === -1) return; + this.fetchRequests[idx] = { ...this.fetchRequests[idx]!, stream }; + this.dispatchTypedEvent("fetchRequestsChange", this.getFetchRequests()); + }; + this.client.addEventListener( + "fetchRequestStreamUpdate", + onFetchRequestStreamUpdate, + ); + const sessionStorage = options.sessionStorage; const sessionId = options.sessionId; @@ -194,6 +216,10 @@ export class FetchRequestLogState extends TypedEventTarget; @@ -986,6 +1008,14 @@ export interface CreateTransportOptions { */ onFetchResponseBody?: (id: string, responseBody: string) => void; + /** + * Optional callback fired asynchronously as a previously tracked long-lived + * stream (GET + `text/event-stream`) delivers events and when it ends. The + * tracker never buffers such a body, so this is the only view the consumer + * gets of the stream's lifetime (#2318). + */ + onFetchStreamUpdate?: (id: string, stream: FetchStreamState) => void; + /** * Optional OAuth client provider for Bearer authentication (SSE, streamable-http). * When set, the SDK injects tokens and handles 401 via the provider. diff --git a/core/react/useInspectorClient.ts b/core/react/useInspectorClient.ts index 1d22a94c..448144c5 100644 --- a/core/react/useInspectorClient.ts +++ b/core/react/useInspectorClient.ts @@ -12,6 +12,7 @@ import type { } from "@modelcontextprotocol/client"; import type { ExcludedTool } from "../mcp/types.js"; import type { MalformedListItem } from "../mcp/listSalvage.js"; +import type { ConnectionDiagnostics } from "../mcp/connectionDiagnostics.js"; import { useStoreSnapshot } from "./useStoreSnapshot.js"; // Module-scope frozen object so the `?? EMPTY_CLIENT_CAPABILITIES` @@ -27,6 +28,13 @@ const EMPTY_CLIENT_CAPABILITIES: ClientCapabilities = Object.freeze({}); */ const NO_EXCLUDED_TOOLS: ExcludedTool[] = []; const NO_MALFORMED_LIST_ITEMS: MalformedListItem[] = []; +// Also the fallback for a client that does not implement the optional +// accessor (a test double), so the field is always a snapshot and never +// `undefined` — one shape for consumers, whatever client is attached. +const NO_CONNECTION_DIAGNOSTICS: ConnectionDiagnostics = Object.freeze({ + capturedAt: 0, + outstandingRequests: [], +}); const readStatus = (client: InspectorClientProtocol): ConnectionStatus => client.getStatus(); @@ -53,6 +61,10 @@ const readExcludedTools = (client: InspectorClientProtocol): ExcludedTool[] => const readMalformedListItems = ( client: InspectorClientProtocol, ): MalformedListItem[] => client.getMalformedListItems(); +const readConnectionDiagnostics = ( + client: InspectorClientProtocol, +): ConnectionDiagnostics => + client.getConnectionDiagnostics?.() ?? NO_CONNECTION_DIAGNOSTICS; export interface UseInspectorClientResult { status: ConnectionStatus; @@ -86,6 +98,13 @@ export interface UseInspectorClientResult { * non-empty set means that list rendered without them (#1909). */ malformedListItems: MalformedListItem[]; + /** + * What the client is still waiting on, when it last heard back, and the + * state of the notification stream — re-read on every request, response + * and stream event. The Connection Info modal renders it; a request + * timeout's message is written from the same snapshot (#2318). + */ + connectionDiagnostics: ConnectionDiagnostics; /** * Message from the most recent mid-session transport failure (the client's * `error` event — stdio crash, SSE drop, HTTP 5xx). Stays set until the next @@ -174,6 +193,12 @@ export function useInspectorClient( readMalformedListItems, NO_MALFORMED_LIST_ITEMS, ); + const connectionDiagnostics = useStoreSnapshot( + inspectorClient, + "connectionDiagnosticsChange", + readConnectionDiagnostics, + NO_CONNECTION_DIAGNOSTICS, + ); // `lastError` is the one value here that is NOT a snapshot of something the // client stores: there is no `getLastError()`, because the client emits the @@ -243,6 +268,7 @@ export function useInspectorClient( discoverResult, excludedTools, malformedListItems, + connectionDiagnostics, lastError, appRendererClient: inspectorClient?.getAppRendererClient() ?? null, connect, diff --git a/docs/docker.md b/docs/docker.md index ed93353c..46d6278a 100644 --- a/docs/docker.md +++ b/docs/docker.md @@ -77,7 +77,7 @@ If another process holds the lock and will not let go, the save **fails** rather It is also what covers the lock being unavailable. This store exists for boxes where the usual mechanism isn't there, so a directory that can't hold a lock file — a read-only `$HOME`, a mount owned by another uid — makes the save proceed unlocked with a warning, rather than turning every `set` into a failure on exactly the deployments the store was written for. -Three env vars affect where the file lands. `MCP_INSPECTOR_SECRET_STORE=keyring|file|memory` picks the store outright, bypassing the probe. `MCP_INSPECTOR_SECRET_FILE` names the file. Failing both, the file follows `MCP_STORAGE_DIR` — the same variable that relocates OAuth tokens and `client.json` — so mounting a volume at your configured storage directory is enough to make secrets durable there. +Three env vars affect where the file lands. `MCP_INSPECTOR_SECRET_STORE=keyring|file|memory` picks the store outright, bypassing the probe. `MCP_INSPECTOR_SECRET_FILE` names the file. Failing both, the file follows `MCP_STORAGE_DIR` — the same variable that relocates OAuth tokens and `client.json` — so mounting a volume at your configured storage directory is enough to make secrets durable there. These variables apply outside a container too; every runtime variable is listed in [Environment variables](./environment-variables.md). **Upgrading from an image before this fix?** Earlier images did not create `/home/node/.mcp-inspector`, so Docker created the volume's mount point as `root` and the non-root `node` user couldn't write to it. An **empty** volume repairs itself on the first run of a current image (Docker applies the image directory's ownership to an empty volume), but one that already has files in it keeps its old `root` ownership and still fails with `EACCES`. Fix it once: diff --git a/docs/environment-variables.md b/docs/environment-variables.md new file mode 100644 index 00000000..70546de2 --- /dev/null +++ b/docs/environment-variables.md @@ -0,0 +1,116 @@ +# Environment variables + +Every environment variable that changes how the Inspector behaves at runtime, in one place: the Inspector's own variables, plus the standard system and Node variables it reads (`HOME`, the proxy variables, the Node TLS variables). Set them in the shell that launches `mcp-inspector` (or with `-e` for the [Docker image](./docker.md)). + +The **Read by** column names the client whose process reads the variable: **web** is the Node backend that `--web` starts (the browser itself reads no environment), **CLI** and **TUI** are those clients, and **launcher** is the `mcp-inspector` bin that picks one of them. A variable read in shared `core/` code is marked with every client that reaches it. + +⚠️ **Unset a variable rather than setting it to an empty string.** The two are not interchangeable: `HOST=""` is read as an all-interfaces bind and refused, and an empty path variable such as `MCP_STORAGE_DIR=` or `MCP_INSPECTOR_LOG_DIR=` can resolve relative to the working directory instead of falling back to the default. A row says so explicitly where an empty value is treated as unset. + +A `~` in a default below means the home directory as described under [Home directory](#home-directory). + +## Authentication and network exposure + +These guard the web backend, which spawns processes on request. Read [Host binding and the origin allow-list](../clients/web/README.md#host-binding--the-origin-allow-list) before widening any of them. + +| Variable | Read by | Default | Effect | +| --------------------------------- | -------- | ------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `MCP_INSPECTOR_API_TOKEN` | web, CLI | a random token per launch | Bearer token guarding every `/api/*` route (`x-mcp-remote-auth: Bearer `). Set it to use a known token instead of the generated one printed in the launch banner. The CLI reads it only to fill the `autoConnect` parameter of the deep link it emits. | +| `MCP_PROXY_AUTH_TOKEN` | web | — | **Deprecated** v1 name for `MCP_INSPECTOR_API_TOKEN`, used only when the new name is unset. | +| `DANGEROUSLY_OMIT_AUTH` | web | unset | Disables the API token entirely. ⚠️ **Any non-empty value turns auth off, including `false` and `0`** — unset the variable to keep auth on. | +| `HOST` | web, CLI | `127.0.0.1` | Address the web server binds. An all-interfaces host (`0.0.0.0`, `::`, an empty string, and equivalent spellings) is **refused** unless `DANGEROUSLY_BIND_ALL_INTERFACES` is enabled. The CLI reads it only to build its deep link. | +| `DANGEROUSLY_BIND_ALL_INTERFACES` | web | off | Opts in to an all-interfaces `HOST`. Only `true` or `1` (case-insensitive) enable it, so `false` reads as off. The Docker image sets it. | +| `ALLOWED_ORIGINS` | web | derived from `HOST` | Comma-separated origins allowed to call the API. Unset, the list follows `HOST` at `CLIENT_PORT`: the loopback origins for a loopback host, the loopback origins plus `http://0.0.0.0` and `http://[::]` for an all-interfaces bind, and otherwise only the configured host's own origin (so binding a LAN address does **not** also allow `localhost`). **Replaces** the default list rather than adding to it, so list every form you browse from. Each entry must include the scheme (`http://localhost:6274`). | + +## Ports + +| Variable | Read by | Default | Effect | +| --------------------- | -------- | ------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `CLIENT_PORT` | web, CLI | `6274` | Web UI port. Must be a **fixed** integer in 1–65535: `0` (an OS-assigned port) is rejected at startup, because the origin allow-list and the MCP Apps sandbox CSP are derived from it. | +| `MCP_SANDBOX_PORT` | web, CLI | `6275` | Port of the MCP Apps sandbox server, 0–65535. `0` asks the OS for a free port. An invalid value is ignored with a warning. | +| `SERVER_PORT` | web | — | v1's proxy port, now only a fallback for the sandbox port: used whenever `MCP_SANDBOX_PORT` does not yield a valid port — unset, empty, **or invalid**. | +| `MCP_APP_ORIGIN_PORT` | web, CLI | `6278` | Port of the dedicated app-origin server, used only by an MCP App whose UI resource declares `_meta.ui.domain`; 0–65535, where `0` asks the OS for a free port. An invalid value is ignored with a warning. Pin it if your app's backend allowlists that origin. | + +The sandbox port resolves as: a valid `MCP_SANDBOX_PORT`, else a valid `SERVER_PORT`, else `6275`. The CLI reads `CLIENT_PORT`, `MCP_SANDBOX_PORT`, `MCP_APP_ORIGIN_PORT` and `HOST` only to build the deep link and port list it hands to a web session; it binds none of them. It normalizes `HOST` (an all-interfaces host becomes `localhost`, any other host is canonicalized), but it validates none of the three **port** variables: any non-empty port value is copied into the URLs and port-forwarding command as-is, with no range check, no warning, and no `SERVER_PORT` fallback, so a malformed value produces a broken hand-off rather than an error. + +## Behavior + +| Variable | Read by | Default | Effect | +| ------------------------ | ------------- | -------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `MCP_AUTO_OPEN_ENABLED` | web, CLI | see effect | Whether a browser is opened for you. `false` never opens one; `true` always does. Unset (or any other value), the web client opens the UI at launch, and the CLI opens the OAuth authorization page only when stderr is a TTY. In the CLI, `true` also lets interactive OAuth **start** when neither stdin nor stderr is a TTY; otherwise that case fails with an auth-required error pointing at `--stored-auth-only`. **The TUI does not read it** and always opens the OAuth page. | +| `MCP_CATALOG_PATH` | web, CLI, TUI | `~/.mcp-inspector/mcp.json` | Default writable catalog, used when no `--catalog` is given. The CLI honors it only when no ad-hoc target (positional command, `--server-url`, or `--transport`) is given. See [MCP server configuration](./mcp-server-configuration.md). | +| `MCP_OAUTH_CALLBACK_URL` | CLI, TUI | `http://127.0.0.1:6276/oauth/callback` | Loopback redirect URL for the CLI/TUI OAuth flow. `--callback-url` takes precedence. | +| `NO_COLOR` | CLI | unset | Any non-empty value disables ANSI styling in the CLI's human-readable output. An empty `NO_COLOR=` counts as unset. | + +## Storage and state + +| Variable | Read by | Default | Effect | +| -------------------------------- | ------------- | -------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `MCP_STORAGE_DIR` | web, CLI, TUI | `~/.mcp-inspector/storage` | Storage directory. Relocates the OAuth state file (`oauth.json`) and the secrets file (`secrets.json`) for every client. For the **web** backend it also relocates `client.json`; the CLI and TUI find `client.json` through `MCP_CLIENT_CONFIG_PATH` instead. | +| `MCP_INSPECTOR_OAUTH_STATE_PATH` | CLI, TUI | `~/.mcp-inspector/storage/oauth.json` | Names the OAuth state file outright. Lookup order: this variable, then `/oauth.json`, then `~/.mcp-inspector/storage/oauth.json`. ⚠️ Setting `MCP_STORAGE_DIR` alone does not isolate a CLI or TUI run if this variable is also exported. **The web backend does not read it** — it always uses `/oauth.json`. | +| `MCP_CLIENT_CONFIG_PATH` | CLI, TUI | `~/.mcp-inspector/storage/client.json` | Install-level client config (CIMD, enterprise IdP). `--client-config` takes precedence. | + +### Home directory + +Every default above that starts with `~` is built from the home directory the process sees, not from the OS account database: + +| Variable | Read by | Effect | +| ------------- | ------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `HOME` | web, CLI, TUI | Base of `~/.mcp-inspector`: the default catalog, storage directory (`oauth.json`, `client.json`), secrets file, and TUI log directory. | +| `USERPROFILE` | web, CLI, TUI | Used in place of `HOME` when `HOME` is unset or empty — the normal case on Windows. ⚠️ If **neither** is set, as under some service managers, those defaults resolve against the **current working directory** instead. Set `HOME` or the specific path variables above. | + +## Secret store + +Where server secrets (headers, client secrets) are kept. The details — the keychain probe, the file format, encryption and locking — are in the [Docker guide](./docker.md); these variables apply to every install, not only containers. + +| Variable | Read by | Default | Effect | +| ---------------------------- | ------------- | --------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `MCP_INSPECTOR_SECRET_STORE` | web, CLI, TUI | probe the OS keychain | `keyring`, `file`, or `memory` (case-insensitive) picks the store outright and skips the probe. An empty or whitespace-only value counts as unset and silently runs automatic selection; any other value is ignored with a warning and also falls back to automatic selection. | +| `MCP_INSPECTOR_SECRET_FILE` | web, CLI, TUI | `~/.mcp-inspector/secrets.json` | Path of the file store. Lookup order: this variable, then `secrets.json` in `MCP_STORAGE_DIR` when that is set, then `~/.mcp-inspector/secrets.json`. ⚠️ The default sits **beside** the storage directory, not inside it. | +| `MCP_INSPECTOR_SECRET_KEY` | web, CLI, TUI | unset (file is plaintext, `0600`) | Passphrase that encrypts the file store; an empty or whitespace-only value counts as unset. Use a generated, high-entropy value. ⚠️ Changing or losing it makes the existing file unreadable; see the Docker guide before rotating it. | + +When no store is configured, the choice also depends on whether the Inspector is running in a container, which it detects from `KUBERNETES_SERVICE_HOST` (or Docker's and Podman's marker files). That variable is set by the orchestrator, not by you. + +## Logging and debugging + +| Variable | Read by | Default | Effect | +| ----------------------- | ------------- | ------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `MCP_DEBUG` / `DEBUG` | launcher, TUI | off | Prints the full stack trace when the process exits on an error — the launcher for any `--web`/`--tui` failure, and the standalone TUI entry point for a startup failure. Any value other than empty, `0` or `false` (case-insensitive) turns it on. | +| `MCP_LOG_FILE` | web | unset (no log) | Appends the web backend's structured (pino, JSON lines) log to this file, creating its directory if needed. An empty value counts as unset. | +| `MCP_INSPECTOR_LOG_DIR` | TUI | `~/.mcp-inspector` | Directory of the TUI's `auth.log`. The TUI logs to a file so its output does not corrupt the terminal UI. | +| `LOG_LEVEL` | TUI | `info` | Level of the TUI's `auth.log`: one of `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `silent`. An empty value is not replaced by `info`. | + +## Outbound proxy + +| Variable | Read by | Default | Effect | +| ---------------------------- | ------------- | ------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `HTTPS_PROXY` / `HTTP_PROXY` | web, CLI, TUI | unset | Route connections to remote HTTP/SSE servers, including OAuth discovery and token requests, through a proxy. Lowercase forms are honored too. See [HTTP proxy support](../clients/cli/README.md#http-proxy-support). | +| `NO_PROXY` | web, CLI, TUI | unset | Hosts exempted from the proxy. | + +## Node.js variables + +These belong to Node, not the Inspector, but they are the answer to some common connection problems. Because the web client connects to MCP servers **from its Node backend**, not from the browser, they apply to all three clients. + +### Connecting to a server with a self-signed certificate + +A server at `https://localhost`, or anywhere else with a self-signed or privately issued certificate, fails to connect with a generic fetch error, because Node does not trust the certificate. Trusting the browser's exception does not help — the browser is not the one connecting ([#1936](https://github.com/modelcontextprotocol/inspector/issues/1936)). + +**Prefer trusting the certificate's CA.** `NODE_EXTRA_CA_CERTS` adds certificates to the ones Node trusts, and leaves verification on for every other connection: + +```sh +NODE_EXTRA_CA_CERTS=/path/to/your-ca.pem npx @modelcontextprotocol/inspector +``` + +For a self-signed certificate with no separate CA, point it at the certificate itself. Node reads the variable once at startup, so it must be set before launching. + +**Or turn verification off, for local development only:** + +```sh +NODE_TLS_REJECT_UNAUTHORIZED=0 npx @modelcontextprotocol/inspector +``` + +⚠️ This disables certificate verification for **every** TLS connection the Inspector process makes — every MCP server, every OAuth authorization server and token endpoint — not just the one you are testing, and Node prints a warning saying so. A connection that would have been refused for impersonation is then accepted. Never set it in a shell profile, a shared environment, or a deployment. + +| Variable | Default | Effect | +| ------------------------------ | ------- | ----------------------------------------------------------------------------------------------------- | +| `NODE_EXTRA_CA_CERTS` | unset | PEM file of extra CA certificates to trust, in addition to Node's bundled ones. Read once at startup. | +| `NODE_TLS_REJECT_UNAUTHORIZED` | `1` | `0` disables TLS certificate verification for the whole process. Development only. | diff --git a/docs/v1-to-v2-migration.md b/docs/v1-to-v2-migration.md index 752ecc12..f552c9e3 100644 --- a/docs/v1-to-v2-migration.md +++ b/docs/v1-to-v2-migration.md @@ -268,6 +268,8 @@ Stdout is otherwise compatible: the default `text` format still pretty-prints th ## Environment variables +This table maps v1 names to v2. Defaults, accepted values, and the variables with no v1 counterpart are in [Environment variables](./environment-variables.md). + | v1 | v2 | Notes | | ------------------------ | ----------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `MCP_PROXY_AUTH_TOKEN` | **`MCP_INSPECTOR_API_TOKEN`** | Renamed, but the **old name still works** as a deprecated fallback when the new one is unset — an existing deployment keeps running while you migrate. Guards `/api/*` via `x-mcp-remote-auth: Bearer `; the browser also receives it injected into `index.html`, so a bare reload keeps working. (The `?MCP_PROXY_AUTH_TOKEN=` **query param** has no such fallback — see [Web UI](#web-ui).) | @@ -277,7 +279,7 @@ Stdout is otherwise compatible: the default `text` format still pretty-prints th | `HOST` | same, **guarded** | An all-interfaces host (`0.0.0.0`, `::`, and equivalent spellings) is now **refused** unless `DANGEROUSLY_BIND_ALL_INTERFACES=true`. Binding a specific IP or hostname needs no opt-in | | `ALLOWED_ORIGINS` | same | Still comma-separated, still **replaces** the default list rather than merging. Entries must include the scheme | | `DANGEROUSLY_OMIT_AUTH` | same | | -| `MCP_AUTO_OPEN_ENABLED` | same | Also gates non-TTY interactive OAuth in the CLI/TUI | +| `MCP_AUTO_OPEN_ENABLED` | same | Also governs opening the OAuth page in the CLI (`true` opens it even when stderr is not a TTY). The TUI does not read it | | — | `DANGEROUSLY_BIND_ALL_INTERFACES` | New opt-in for a wildcard bind (the Docker image sets it) | | — | `MCP_CATALOG_PATH` | Default catalog path | | — | `MCP_SANDBOX_PORT` | MCP Apps sandbox server port (fixed `6275` by default) |