From 53dc4549be408f6e85360ad139cadc51005e6090 Mon Sep 17 00:00:00 2001 From: Cliff Hall Date: Sun, 14 Jun 2026 17:35:52 -0400 Subject: [PATCH] fix(core): trace dropped fetch body update when entry rotates out (#1390) (#1480) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(core): trace dropped fetch body update when entry rotates out (#1390) FetchRequestLogState.onFetchRequestBodyUpdate silently dropped a deferred response-body update when the matching entry had already been evicted by log rotation (>= maxFetchRequests newer requests appended between the append and the fire-and-forget body read). The user saw an empty Response Body with no way to distinguish "body never came back" from "body dropped after eviction". Plumb an optional pino logger into FetchRequestLogState (defaulting to the silent logger) and emit a logger.debug at the early-return site so the drop is traceable. The web client wires the existing remote logger from createWebEnvironment into the state manager. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(core): emit dropped-body trace at warn level so it is observable (#1390) Review feedback: the web remote logger (createRemoteLogger) defaults to level 'info' with no override at the environmentFactory call site, so a logger.debug at the drop site is filtered out by both the pino level gate and transmit.level — the diagnostic never reaches /api/log and the drop stays invisible in the shipped web app, defeating the issue's goal. Promote the trace to logger.warn (>= info, so it transmits by default). A response body lost to log rotation is unexpected data loss, not routine diagnostic noise, so warn is the appropriate severity. Update the comment to explain the level choice and switch the test stub from debug to warn, extracting a small typed makeWarnLogger helper per review. Co-Authored-By: Claude Opus 4.8 (1M context) * feat(web): configurable Network Log Size per server + body-dropped toast (#1390) Turn the otherwise-invisible "response body dropped after rotation" case into an actionable signal: 1. Per-server `maxFetchRequests` setting (default 1000, 0 = unlimited), editable as "Network Log Size" in the first (Options) accordion section of the Server Settings form. Plumbed through the full settings pipeline: InspectorServerSettings + StoredMCPServer types, the serverList disk converters (read defaults to 1000, write omits the default to keep diffs minimal — 0 is a meaningful non-default), and the node server's GET-sanitize + write-side validateSettings (non-negative number). Applied live to the active connection on settings-modal close via a new FetchRequestLogState.setMaxFetchRequests (shrinking trims immediately), and at connect time for the next session. 2. FetchRequestLogState now emits a `fetchRequestBodyDropped` event, but only for genuine rotation drops (log at capacity with a finite cap) — not for benign post-clear stragglers or unlimited logs, which previously also hit the warn path. App shows a deduped, persistent toast (stable per-server id, no auto-close) explaining what happened and the likely causes, with a link that opens this server's settings on the Options section (modal remounts via key) so the user can raise the limit if it's just a high-traffic server. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(web): don't treat a cleared Network Log Size field as unlimited (#1390) Review feedback: clearing the NumberInput emits "", and `parseInt("") || 0` coerced that to 0 (unlimited). Since the value applies live on modal close, a user who cleared the field to retype and then closed the modal would silently switch the log to unlimited. Fall back to the current value on an empty/NaN parse and reserve 0 for an explicit numeric entry. Adds a cleared-input test. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- clients/web/src/App.test.tsx | 95 ++++++++++++++- clients/web/src/App.tsx | 109 +++++++++++++++++- .../ServerSettingsForm.stories.tsx | 9 ++ .../ServerSettingsForm.test.tsx | 51 ++++++++ .../ServerSettingsForm/ServerSettingsForm.tsx | 25 ++++ .../ServerSettingsModal.stories.tsx | 2 + .../ServerSettingsModal.test.tsx | 2 + .../ServerSettingsModal.tsx | 10 ++ .../mcp/remote/remoteClientTransport.test.ts | 1 + .../web/src/test/core/mcp/serverList.test.ts | 59 ++++++++++ .../mcp/state/fetchRequestLogState.test.ts | 107 ++++++++++++++++- .../mcp/state/managedPromptsState.test.ts | 1 + .../managedResourceTemplatesState.test.ts | 1 + .../mcp/state/managedResourcesState.test.ts | 1 + .../core/mcp/state/managedToolsState.test.ts | 1 + .../src/test/core/react/useServers.test.tsx | 4 + .../integration/mcp/inspectorClient.test.ts | 2 + .../integration/mcp/node/transport.test.ts | 3 + .../mcp/remote/servers-route.test.ts | 107 +++++++++++++++++ .../integration/mcp/remote/transport.test.ts | 1 + core/mcp/remote/node/server.ts | 31 ++++- core/mcp/serverList.ts | 17 ++- core/mcp/state/fetchRequestLogState.ts | 67 ++++++++++- core/mcp/types.ts | 24 ++++ 24 files changed, 717 insertions(+), 13 deletions(-) diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index ccd6ae56..68747ec3 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -122,11 +122,25 @@ vi.mock("@inspector/core/mcp/state/messageLogState.js", () => ({ return { destroy: vi.fn(), clearMessages: messageLogClear }; }), })); -vi.mock("@inspector/core/mcp/state/fetchRequestLogState.js", () => ({ - FetchRequestLogState: vi.fn(function () { - return { destroy: vi.fn(), getFetchRequests: vi.fn(() => []) }; - }), -})); +// Extends EventTarget so the App's `fetchRequestBodyDropped` subscription is +// real; the test fires `dispatchEvent(new CustomEvent("fetchRequestBodyDropped", +// { detail }))` on the tracked instance to drive the body-dropped toast. +vi.mock("@inspector/core/mcp/state/fetchRequestLogState.js", () => { + class FakeFetchRequestLogState extends EventTarget { + destroy = vi.fn(); + getFetchRequests = vi.fn(() => []); + setMaxFetchRequests = vi.fn(); + } + const instances: FakeFetchRequestLogState[] = []; + return { + FetchRequestLogState: vi.fn(function () { + const inst = new FakeFetchRequestLogState(); + instances.push(inst); + return inst; + }), + __fetchLogInstances: instances, + }; +}); vi.mock("@inspector/core/mcp/state/stderrLogState.js", () => ({ StderrLogState: vi.fn(function () { return { destroy: vi.fn() }; @@ -387,6 +401,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({ import App from "./App"; import * as McpIndex from "@inspector/core/mcp/index.js"; +import * as FetchLogModule from "@inspector/core/mcp/state/fetchRequestLogState.js"; import { useManagedRequestorTasks } from "@inspector/core/react/useManagedRequestorTasks.js"; import { useMessageLog } from "@inspector/core/react/useMessageLog.js"; import { useInspectorClient } from "@inspector/core/react/useInspectorClient.js"; @@ -413,6 +428,10 @@ const clientInstances = ( McpIndex as unknown as { __clientInstances: EventTarget[] } ).__clientInstances; +const fetchLogInstances = ( + FetchLogModule as unknown as { __fetchLogInstances: EventTarget[] } +).__fetchLogInstances; + describe("App session-scoped state reset on disconnect", () => { beforeEach(() => { clientInstances.length = 0; @@ -623,6 +642,71 @@ describe("App tool progress toasts", () => { }); }); +describe("App network-log body-dropped toast", () => { + beforeEach(() => { + clientInstances.length = 0; + fetchLogInstances.length = 0; + notificationsMock.show.mockClear(); + notificationsMock.hide.mockClear(); + }); + + it("shows a deduped toast when the fetch log emits fetchRequestBodyDropped", async () => { + const user = userEvent.setup(); + renderWithMantine(); + + await user.click(screen.getByText("connect")); + await waitFor(() => expect(fetchLogInstances).toHaveLength(1)); + + act(() => { + fetchLogInstances[0].dispatchEvent( + new CustomEvent("fetchRequestBodyDropped", { + detail: { id: "req-1", maxFetchRequests: 1000 }, + }), + ); + }); + + expect(notificationsMock.show).toHaveBeenCalledTimes(1); + const shown = notificationsMock.show.mock.calls[0][0]; + expect(shown.title).toBe("Network log: response body dropped"); + // Stable per-server id + no auto-close so a storm dedupes into one toast. + expect(typeof shown.id).toBe("string"); + expect(shown.autoClose).toBe(false); + }); + + it("opens the settings modal (Options/Network Log Size) from the toast link", async () => { + const user = userEvent.setup(); + renderWithMantine(); + + await user.click(screen.getByText("connect")); + await waitFor(() => expect(fetchLogInstances).toHaveLength(1)); + + act(() => { + fetchLogInstances[0].dispatchEvent( + new CustomEvent("fetchRequestBodyDropped", { + detail: { id: "req-1", maxFetchRequests: 500 }, + }), + ); + }); + + // The toast message is a React node carrying the "Adjust" link; render it + // and click the link to exercise the onAdjust handler (hide toast + open + // settings modal for the active server). + const message = notificationsMock.show.mock.calls[0][0].message; + renderWithMantine(message); + await user.click( + screen.getByRole("button", { + name: /Adjust Network Log Size for this server/, + }), + ); + + expect(notificationsMock.hide).toHaveBeenCalled(); + // The settings modal is now open on the Options section, showing the field. + await waitFor(() => + expect(screen.getByLabelText(/Network Log Size/)).toBeInTheDocument(), + ); + }); +}); + describe("App pending server-initiated request modal", () => { beforeEach(() => { clientInstances.length = 0; @@ -1127,6 +1211,7 @@ const settingsWithRoots = ( connectionTimeout: 0, requestTimeout: 0, taskTtl: 60000, + maxFetchRequests: 1000, roots, }); diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index a482bee5..ad7814de 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -1,6 +1,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { Anchor, + List, Stack, Text, useComputedColorScheme, @@ -36,7 +37,10 @@ import type { ServerEntry, ServerType, } from "@inspector/core/mcp/types.js"; -import { DEFAULT_TASK_TTL_MS } from "@inspector/core/mcp/types.js"; +import { + DEFAULT_MAX_FETCH_REQUESTS, + DEFAULT_TASK_TTL_MS, +} from "@inspector/core/mcp/types.js"; import { API_SERVER_ENV_VARS, INSPECTOR_API_TOKEN_GLOBAL, @@ -53,6 +57,7 @@ import { } from "@inspector/core/mcp/serverList.js"; import { MessageLogState } from "@inspector/core/mcp/state/messageLogState.js"; import { FetchRequestLogState } from "@inspector/core/mcp/state/fetchRequestLogState.js"; +import type { FetchRequestLogStateEventMap } from "@inspector/core/mcp/state/fetchRequestLogState.js"; import { StderrLogState } from "@inspector/core/mcp/state/stderrLogState.js"; import type { RedirectUrlProvider } from "@inspector/core/auth/index.js"; import { @@ -279,9 +284,53 @@ const EMPTY_SETTINGS: InspectorServerSettings = { requestTimeout: 0, taskTtl: DEFAULT_TASK_TTL_MS, autoRefreshOnListChanged: false, + maxFetchRequests: DEFAULT_MAX_FETCH_REQUESTS, roots: [], }; +// Stable toast id for the "response body dropped" warning, keyed per server so +// a request storm updates one persistent toast rather than stacking thousands +// (the drop event can fire rapidly). Mirrors the progress-toast dedupe pattern. +function bodyDroppedToastId(serverId: string): string { + return `fetch-body-dropped-${serverId}`; +} + +// Body of the "response body dropped" warning toast: a one-line summary of what +// happened, the likely causes, and a link that opens this server's settings +// (on the Options section) so the user can raise the Network Log Size if it's +// just a high-traffic server. Surfaces the otherwise-invisible rotation drop +// described in #1390. +const FetchBodyDroppedToastMessage = ({ + maxFetchRequests, + onAdjust, +}: { + maxFetchRequests: number; + onAdjust: () => void; +}) => ( + + + A response body arrived after its Network log entry had already rotated + out (the log hit its {maxFetchRequests}-request limit), so the body + couldn't be shown. This usually indicates: + + + + a chatty or misbehaving server (notification storms, rapid polling) + + an SSE/transport reconnect or retry storm + + a slow streaming call racing against high request volume + + + the Network Log Size set too low for this server's traffic + + + + Adjust Network Log Size for this server + + +); + // Body of the output-schema-mismatch warning toast: a one-line summary plus a // link that opens the full validation details in a modal (the raw error is far // too long for a toast). @@ -645,6 +694,50 @@ function App() { ); const { messages } = useMessageLog(messageLogState); const { fetchRequests } = useFetchRequestLog(fetchRequestLogState); + + // Surface the otherwise-invisible "response body dropped after rotation" case + // (#1390) as a deduped toast that links to this server's Network Log Size + // setting. The state manager only emits this when the drop is genuinely due + // to rotation (log at capacity), not for benign post-clear stragglers. + useEffect(() => { + if (!fetchRequestLogState || activeServerId === undefined) return; + const onBodyDropped = ( + event: TypedEventGeneric< + FetchRequestLogStateEventMap, + "fetchRequestBodyDropped" + >, + ) => { + notifications.show({ + id: bodyDroppedToastId(activeServerId), + title: "Network log: response body dropped", + color: "yellow", + // Stays until dismissed (or the user opens settings via the link) so a + // single toast represents an ongoing condition rather than flashing per + // drop; the stable id dedupes a storm into this one toast. + autoClose: false, + message: ( + { + notifications.hide(bodyDroppedToastId(activeServerId)); + setSettingsModalTargetId(activeServerId); + }} + /> + ), + }); + }; + fetchRequestLogState.addEventListener( + "fetchRequestBodyDropped", + onBodyDropped, + ); + return () => { + fetchRequestLogState.removeEventListener( + "fetchRequestBodyDropped", + onBodyDropped, + ); + }; + }, [fetchRequestLogState, activeServerId]); + // Server-initiated sampling / elicitation requests. These arrive while a call // (e.g. a tool execution) is in flight and block it until the user responds. const { pendingSamples, pendingElicitations } = @@ -1133,7 +1226,7 @@ function App() { fetchRequestLogState?.destroy(); stderrLogState?.destroy(); - const { environment } = createWebEnvironment( + const { environment, logger } = createWebEnvironment( getAuthToken(), redirectUrlProvider, onBeforeOAuthRedirect, @@ -1221,6 +1314,9 @@ function App() { // reads the current log. const nextFetchLog = new FetchRequestLogState(client, { sessionStorage: sessionStorageAdapter, + logger, + maxFetchRequests: + savedSettings?.maxFetchRequests ?? DEFAULT_MAX_FETCH_REQUESTS, ...(sessionId && { sessionId }), }); fetchLogRef.current = nextFetchLog; @@ -2122,6 +2218,11 @@ function App() { // effect without a reconnect (#1444). Connection-time inputs (transport, // OAuth, timeouts) still only apply on the next connect. inspectorClient.setServerSettings(settingsDraft); + // Resize the Network log buffer live so a maxFetchRequests edit takes + // effect without a reconnect (shrinking trims immediately). Connect-time + // construction also reads this, so a reconnect would apply it anyway — + // this just makes the toast→adjust flow responsive. + fetchLogRef.current?.setMaxFetchRequests(settingsDraft.maxFetchRequests); const nextRoots = cleanRoots(settingsDraft.roots); const currentRoots = cleanRoots(inspectorClient.getRoots()); if (JSON.stringify(nextRoots) !== JSON.stringify(currentRoots)) { @@ -2357,6 +2458,10 @@ function App() { onSubmit={onConfigSubmit} /> { + args.onMaxFetchRequestsChange(value); + updateArgs({ + settings: { ...args.settings, maxFetchRequests: value }, + }); + }} onOAuthChange={(oauth) => { args.onOAuthChange(oauth); updateArgs({ @@ -126,6 +133,7 @@ const meta: Meta = { onMetadataChange: fn(), onTimeoutChange: fn(), onAutoRefreshChange: fn(), + onMaxFetchRequestsChange: fn(), onOAuthChange: fn(), }, }; @@ -177,6 +185,7 @@ export const AllConfigured: Story = { connectionTimeout: 15000, requestTimeout: 45000, taskTtl: 45000, + maxFetchRequests: 5000, oauthClientId: "my-client-id", oauthClientSecret: "super-secret-value", oauthScopes: "read write", diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx index 101e78ac..dc9a3388 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx @@ -22,6 +22,7 @@ const emptySettings: InspectorServerSettings = { connectionTimeout: 30000, requestTimeout: 60000, taskTtl: 60000, + maxFetchRequests: 1000, roots: [], }; @@ -31,6 +32,7 @@ const populatedSettings: InspectorServerSettings = { connectionTimeout: 30000, requestTimeout: 60000, taskTtl: 60000, + maxFetchRequests: 1000, oauthClientId: "cid", oauthClientSecret: "secret", oauthScopes: "read", @@ -55,6 +57,7 @@ const baseHandlers = { onMetadataChange: vi.fn(), onTimeoutChange: vi.fn(), onAutoRefreshChange: vi.fn(), + onMaxFetchRequestsChange: vi.fn(), onOAuthChange: vi.fn(), onAddRoot: vi.fn(), onRemoveRoot: vi.fn(), @@ -294,6 +297,54 @@ describe("ServerSettingsForm", () => { expect(onAutoRefreshChange).toHaveBeenCalledWith(true); }); + it("renders the Network Log Size field reflecting maxFetchRequests", () => { + renderWithMantine( + , + ); + expect(screen.getByLabelText(/Network Log Size/)).toHaveValue("2500"); + }); + + it("invokes onMaxFetchRequestsChange when the Network Log Size value changes", async () => { + const user = userEvent.setup(); + const onMaxFetchRequestsChange = vi.fn(); + renderWithMantine( + , + ); + const input = screen.getByLabelText(/Network Log Size/); + await user.type(input, "5"); + // Appends to the existing value via the NumberInput; the handler receives a + // number, not the raw string. + expect(onMaxFetchRequestsChange).toHaveBeenCalled(); + const lastArg = onMaxFetchRequestsChange.mock.calls.at(-1)?.[0]; + expect(typeof lastArg).toBe("number"); + }); + + it("keeps the current Network Log Size (not 0) when the field is cleared", async () => { + // Clearing the input must not silently mean "unlimited" (0) — it falls back + // to the current value so a clear-then-close doesn't change the cap. + const user = userEvent.setup(); + const onMaxFetchRequestsChange = vi.fn(); + renderWithMantine( + , + ); + await user.clear(screen.getByLabelText(/Network Log Size/)); + expect(onMaxFetchRequestsChange).toHaveBeenLastCalledWith(2500); + }); + it("invokes onOAuthChange when typing in client id", async () => { const user = userEvent.setup(); const onOAuthChange = vi.fn(); diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx index 3a9267fa..ed4452ed 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx @@ -39,6 +39,7 @@ export interface ServerSettingsFormProps { value: number, ) => void; onAutoRefreshChange: (value: boolean) => void; + onMaxFetchRequestsChange: (value: number) => void; onOAuthChange: (oauth: OAuthSettings) => void; onAddRoot: () => void; onRemoveRoot: (index: number) => void; @@ -184,11 +185,27 @@ export function ServerSettingsForm({ onMetadataChange, onTimeoutChange, onAutoRefreshChange, + onMaxFetchRequestsChange, onOAuthChange, onAddRoot, onRemoveRoot, onRootChange, }: ServerSettingsFormProps) { + const handleMaxFetchRequestsChange = (value: number | string) => { + if (typeof value === "number") { + onMaxFetchRequestsChange(value); + return; + } + // Mantine emits "" when the field is cleared. Don't coerce that to 0 — 0 + // means "unlimited", and since the value applies live on modal close, + // clearing-to-retype-then-closing would silently switch the log to + // unlimited. Keep the current value on an empty/NaN parse; reserve 0 for an + // explicit numeric entry. + const parsed = parseInt(value, 10); + onMaxFetchRequestsChange( + Number.isNaN(parsed) ? settings.maxFetchRequests : parsed, + ); + }; const handleTimeoutChange = (field: "connectionTimeout" | "requestTimeout" | "taskTtl") => (value: number | string) => { @@ -224,6 +241,14 @@ export function ServerSettingsForm({ checked={settings.autoRefreshOnListChanged ?? false} onChange={(e) => onAutoRefreshChange(e.currentTarget.checked)} /> + diff --git a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.stories.tsx b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.stories.tsx index 16d56efb..56111497 100644 --- a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.stories.tsx +++ b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.stories.tsx @@ -17,6 +17,7 @@ const initialSettings: InspectorServerSettings = { connectionTimeout: 30000, requestTimeout: 60000, taskTtl: 60000, + maxFetchRequests: 5000, oauthClientId: "my-client-id", oauthClientSecret: "super-secret-value", oauthScopes: "read write", @@ -73,6 +74,7 @@ export const EmptySettings: Story = { connectionTimeout: 30000, requestTimeout: 60000, taskTtl: 60000, + maxFetchRequests: 1000, roots: [], }, }, diff --git a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx index 54025427..b6b83cb8 100644 --- a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx @@ -13,6 +13,7 @@ const initialSettings: InspectorServerSettings = { oauthClientId: "cid", oauthClientSecret: "secret", oauthScopes: "read", + maxFetchRequests: 1000, roots: [{ uri: "file:///project", name: "Project" }], }; @@ -22,6 +23,7 @@ const emptySettings: InspectorServerSettings = { connectionTimeout: 30000, requestTimeout: 60000, taskTtl: 60000, + maxFetchRequests: 1000, roots: [], }; diff --git a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx index 8dfb1ab4..a0ea603b 100644 --- a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx +++ b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx @@ -32,6 +32,11 @@ export function ServerSettingsModal({ onClose, onSettingsChange, }: ServerSettingsModalProps) { + // Initial expansion is the first ("options") section — where Network Log + // Size lives, so a deep-link from the body-dropped toast lands on the + // relevant control. The parent remounts this modal per open (via `key`), so + // this initial state re-applies on each open rather than persisting a + // user's prior expand/collapse across opens. const [expandedSections, setExpandedSections] = useState< ServerSettingsSection[] >(["options"]); @@ -104,6 +109,10 @@ export function ServerSettingsModal({ onSettingsChange({ ...settings, autoRefreshOnListChanged: value }); } + function handleMaxFetchRequestsChange(value: number) { + onSettingsChange({ ...settings, maxFetchRequests: value }); + } + function handleAddRoot() { onSettingsChange({ ...settings, @@ -159,6 +168,7 @@ export function ServerSettingsModal({ onMetadataChange={handleMetadataChange} onTimeoutChange={handleTimeoutChange} onAutoRefreshChange={handleAutoRefreshChange} + onMaxFetchRequestsChange={handleMaxFetchRequestsChange} onOAuthChange={handleOAuthChange} onAddRoot={handleAddRoot} onRemoveRoot={handleRemoveRoot} 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 34a08faa..243cd298 100644 --- a/clients/web/src/test/core/mcp/remote/remoteClientTransport.test.ts +++ b/clients/web/src/test/core/mcp/remote/remoteClientTransport.test.ts @@ -342,6 +342,7 @@ describe("RemoteClientTransport", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, roots: [], }; const transport = new RemoteClientTransport( diff --git a/clients/web/src/test/core/mcp/serverList.test.ts b/clients/web/src/test/core/mcp/serverList.test.ts index 7dda8342..5684ef31 100644 --- a/clients/web/src/test/core/mcp/serverList.test.ts +++ b/clients/web/src/test/core/mcp/serverList.test.ts @@ -247,6 +247,58 @@ describe("serverEntriesToMcpConfig", () => { expect(round).toEqual(original); }); + it("round-trips maxFetchRequests: lifts a non-default value to settings and back to disk", () => { + const original: MCPConfig = { + mcpServers: { + zeta: { + type: "streamable-http", + url: "https://x.test/mcp", + maxFetchRequests: 5000, + }, + }, + }; + const [entry] = mcpConfigToServerEntries(original); + expect(entry?.settings?.maxFetchRequests).toBe(5000); + const round = serverEntriesToMcpConfig(mcpConfigToServerEntries(original)); + expect(round).toEqual(original); + }); + + it("round-trips maxFetchRequests: 0 (unlimited) — a meaningful non-default value", () => { + const original: MCPConfig = { + mcpServers: { + eta: { + type: "streamable-http", + url: "https://x.test/mcp", + maxFetchRequests: 0, + }, + }, + }; + const [entry] = mcpConfigToServerEntries(original); + expect(entry?.settings?.maxFetchRequests).toBe(0); + const round = serverEntriesToMcpConfig(mcpConfigToServerEntries(original)); + expect(round).toEqual(original); + }); + + it("omits maxFetchRequests from disk when it equals the default", () => { + // Absent on disk lifts to the product default (1000) in memory; writing it + // back must NOT inject the field, keeping the diff minimal. A benign + // inspector field (connectionTimeout) is present so `settings` is built. + const original: MCPConfig = { + mcpServers: { + theta: { + type: "streamable-http", + url: "https://x.test/mcp", + connectionTimeout: 5000, + }, + }, + }; + const [entry] = mcpConfigToServerEntries(original); + expect(entry?.settings?.maxFetchRequests).toBe(1000); + const round = serverEntriesToMcpConfig(mcpConfigToServerEntries(original)); + expect("maxFetchRequests" in (round.mcpServers.theta ?? {})).toBe(false); + expect(round).toEqual(original); + }); + it("lifts top-level Inspector-extension fields onto ServerEntry.settings (form shape)", () => { const cfg: MCPConfig = { mcpServers: { @@ -269,6 +321,8 @@ describe("serverEntriesToMcpConfig", () => { taskTtl: 60000, // Absent autoRefreshOnListChanged on disk → false in memory (for the form) autoRefreshOnListChanged: false, + // Absent maxFetchRequests on disk → product default in memory (for the form) + maxFetchRequests: 1000, // Absent roots on disk → empty list in memory (for the form) roots: [], // Nested oauth on disk → flat oauthClientId in memory @@ -343,6 +397,7 @@ describe("serverEntriesToMcpConfig", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, roots: [], }, connection: { status: "disconnected" }, @@ -368,6 +423,7 @@ describe("serverEntriesToMcpConfig", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, roots: [], }, connection: { status: "disconnected" }, @@ -395,6 +451,7 @@ describe("serverEntriesToMcpConfig", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, roots: [ { uri: "file:///project", name: "Project" }, { uri: "file:///tmp" }, @@ -432,6 +489,7 @@ describe("serverEntriesToMcpConfig", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, roots: [ { uri: "file:///keep", name: " " }, { uri: " " }, @@ -457,6 +515,7 @@ describe("serverEntriesToMcpConfig", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, roots: [{ uri: "" }], }, connection: { status: "disconnected" }, 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 6af743e8..d8b239b3 100644 --- a/clients/web/src/test/core/mcp/state/fetchRequestLogState.test.ts +++ b/clients/web/src/test/core/mcp/state/fetchRequestLogState.test.ts @@ -44,6 +44,32 @@ describe("FetchRequestLogState", () => { let client: FakeInspectorClient; let state: FetchRequestLogState; + type LoggerOption = NonNullable< + ConstructorParameters[1] + >["logger"]; + + // Stub logger exposing just the `warn` method exercised on the drop path, + // cast to the FetchRequestLogStateOptions["logger"] type. Returns both the + // logger and the spy so a test can assert the diagnostic was emitted. + function makeWarnLogger(): { + logger: LoggerOption; + warn: ReturnType; + } { + const warn = vi.fn(); + return { logger: { warn } as unknown as LoggerOption, warn }; + } + + // Build a state wired to a stub logger so the diagnostic on the rotated-out + // drop path can be asserted. + function makeLoggedState( + logger: LoggerOption, + extra: Partial< + NonNullable[1]> + > = {}, + ): FetchRequestLogState { + return new FetchRequestLogState(client, { logger, ...extra }); + } + beforeEach(() => { client = new FakeInspectorClient(); state = new FetchRequestLogState(client); @@ -120,15 +146,92 @@ describe("FetchRequestLogState", () => { expect(seen).toHaveLength(1); }); - it("ignores fetchRequestBodyUpdate for unknown ids", () => { + 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 + // event (which would mislead the user into raising the log size). + const { logger, warn } = makeWarnLogger(); + const logged = makeLoggedState(logger, { maxFetchRequests: 1000 }); client.dispatchTypedEvent("fetchRequest", entry("a")); let changes = 0; - state.addEventListener("fetchRequestsChange", () => changes++); + let dropped = 0; + logged.addEventListener("fetchRequestsChange", () => changes++); + logged.addEventListener("fetchRequestBodyDropped", () => dropped++); client.dispatchTypedEvent("fetchRequestBodyUpdate", { id: "nonexistent", responseBody: "x", }); expect(changes).toBe(0); + expect(dropped).toBe(0); + expect(warn).not.toHaveBeenCalled(); + }); + + it("traces and emits when the entry rotated out before the body arrived", () => { + const { logger, warn } = makeWarnLogger(); + const logged = makeLoggedState(logger, { maxFetchRequests: 1 }); + const dropped: { id: string; maxFetchRequests: number }[] = []; + logged.addEventListener("fetchRequestBodyDropped", (e) => + dropped.push(e.detail), + ); + client.dispatchTypedEvent("fetchRequest", entry("a")); + // A newer request evicts "a" before its deferred body update arrives. + client.dispatchTypedEvent("fetchRequest", entry("b")); + client.dispatchTypedEvent("fetchRequestBodyUpdate", { + id: "a", + responseBody: "late", + }); + expect(logged.getFetchRequests().map((e) => e.id)).toEqual(["b"]); + expect(warn).toHaveBeenCalledWith( + expect.objectContaining({ + fetchRequestId: "a", + maxFetchRequests: 1, + }), + expect.stringContaining("rotated out"), + ); + expect(dropped).toEqual([{ id: "a", maxFetchRequests: 1 }]); + }); + + it("does not flag a rotation drop when the log is unlimited (maxFetchRequests = 0)", () => { + // With no cap, entries never rotate out, so an unknown id is always a + // benign straggler — never a data-loss event. + const { logger, warn } = makeWarnLogger(); + const logged = makeLoggedState(logger, { maxFetchRequests: 0 }); + let dropped = 0; + logged.addEventListener("fetchRequestBodyDropped", () => dropped++); + client.dispatchTypedEvent("fetchRequest", entry("a")); + client.dispatchTypedEvent("fetchRequestBodyUpdate", { + id: "gone", + responseBody: "x", + }); + expect(warn).not.toHaveBeenCalled(); + expect(dropped).toBe(0); + }); + + it("setMaxFetchRequests trims live when shrunk and re-emits", () => { + const sized = new FetchRequestLogState(client, { maxFetchRequests: 5 }); + for (let i = 0; i < 5; i++) { + client.dispatchTypedEvent("fetchRequest", entry(`e${i}`)); + } + const seen: string[][] = []; + sized.addEventListener("fetchRequestsChange", (e) => + seen.push(e.detail.map((x) => x.id)), + ); + sized.setMaxFetchRequests(2); + // Trims to the newest 2 and emits once. + expect(sized.getFetchRequests().map((e) => e.id)).toEqual(["e3", "e4"]); + expect(seen).toEqual([["e3", "e4"]]); + }); + + it("setMaxFetchRequests is a no-op when unchanged or when growing within bounds", () => { + const sized = new FetchRequestLogState(client, { maxFetchRequests: 2 }); + client.dispatchTypedEvent("fetchRequest", entry("a")); + client.dispatchTypedEvent("fetchRequest", entry("b")); + let changes = 0; + sized.addEventListener("fetchRequestsChange", () => changes++); + sized.setMaxFetchRequests(2); // unchanged + sized.setMaxFetchRequests(10); // grown — nothing to trim + expect(changes).toBe(0); + expect(sized.getFetchRequests().map((e) => e.id)).toEqual(["a", "b"]); }); it("does NOT clear on connect or disconnect", () => { diff --git a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts index d4f597d9..9b660f19 100644 --- a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts @@ -14,6 +14,7 @@ const AUTO_REFRESH_SETTINGS: InspectorServerSettings = { connectionTimeout: 0, requestTimeout: 0, taskTtl: 60000, + maxFetchRequests: 1000, autoRefreshOnListChanged: true, roots: [], }; diff --git a/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts b/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts index 41538af9..ab13a646 100644 --- a/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts @@ -14,6 +14,7 @@ const AUTO_REFRESH_SETTINGS: InspectorServerSettings = { connectionTimeout: 0, requestTimeout: 0, taskTtl: 60000, + maxFetchRequests: 1000, autoRefreshOnListChanged: true, roots: [], }; diff --git a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts index 3a6119a8..729fb6eb 100644 --- a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts @@ -14,6 +14,7 @@ const AUTO_REFRESH_SETTINGS: InspectorServerSettings = { connectionTimeout: 0, requestTimeout: 0, taskTtl: 60000, + maxFetchRequests: 1000, autoRefreshOnListChanged: true, roots: [], }; diff --git a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts index 73b5bed2..b5cca48f 100644 --- a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts @@ -14,6 +14,7 @@ const AUTO_REFRESH_SETTINGS: InspectorServerSettings = { connectionTimeout: 0, requestTimeout: 0, taskTtl: 60000, + maxFetchRequests: 1000, autoRefreshOnListChanged: true, roots: [], }; diff --git a/clients/web/src/test/core/react/useServers.test.tsx b/clients/web/src/test/core/react/useServers.test.tsx index d1ffc7a6..a63212fd 100644 --- a/clients/web/src/test/core/react/useServers.test.tsx +++ b/clients/web/src/test/core/react/useServers.test.tsx @@ -392,6 +392,7 @@ describe("useServers", () => { connectionTimeout: 5000, requestTimeout: 30000, taskTtl: 30000, + maxFetchRequests: 1000, roots: [], }); }); @@ -403,6 +404,7 @@ describe("useServers", () => { connectionTimeout: 5000, requestTimeout: 30000, taskTtl: 30000, + maxFetchRequests: 1000, autoRefreshOnListChanged: false, roots: [], }); @@ -436,6 +438,7 @@ describe("useServers", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, roots: [], }); }), @@ -485,6 +488,7 @@ describe("useServers", () => { requestTimeout: 0, // Absent taskTtl on disk reads back as the product default for the form. taskTtl: 60000, + maxFetchRequests: 1000, // Absent autoRefreshOnListChanged on disk reads back as false. autoRefreshOnListChanged: false, // Absent roots on disk reads back as an empty list for the form. diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts index 7993000d..4772d6d5 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts @@ -322,6 +322,7 @@ describe("InspectorClient", () => { connectionTimeout: 50, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, roots: [], }, }, @@ -663,6 +664,7 @@ describe("InspectorClient", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, autoRefreshOnListChanged: false, roots: [], }; 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 060a93f7..c5237e7f 100644 --- a/clients/web/src/test/integration/mcp/node/transport.test.ts +++ b/clients/web/src/test/integration/mcp/node/transport.test.ts @@ -217,6 +217,7 @@ describe("Transport", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, roots: [], }; @@ -270,6 +271,7 @@ describe("Transport", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, roots: [], }; @@ -313,6 +315,7 @@ describe("Transport", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, roots: [], }; const result = createTransportNode(config, { settings }); diff --git a/clients/web/src/test/integration/mcp/remote/servers-route.test.ts b/clients/web/src/test/integration/mcp/remote/servers-route.test.ts index 1b954e6c..6a5113df 100644 --- a/clients/web/src/test/integration/mcp/remote/servers-route.test.ts +++ b/clients/web/src/test/integration/mcp/remote/servers-route.test.ts @@ -533,6 +533,113 @@ describe("/api/servers routes", () => { expect(res.status).toBe(400); }); + it("persists a non-default maxFetchRequests through the PUT write path", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + delta: { type: "streamable-http", url: "https://x.test/mcp" }, + }, + }), + ); + const res = await fetch(`${h.baseUrl}/api/servers/delta`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + maxFetchRequests: 5000, + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers + .delta as unknown as Record; + expect(stored.maxFetchRequests).toBe(5000); + }); + + it("persists maxFetchRequests: 0 (unlimited) — a meaningful non-default value", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + delta: { type: "streamable-http", url: "https://x.test/mcp" }, + }, + }), + ); + const res = await fetch(`${h.baseUrl}/api/servers/delta`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + maxFetchRequests: 0, + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers + .delta as unknown as Record; + expect(stored.maxFetchRequests).toBe(0); + }); + + it("omits maxFetchRequests from disk when it equals the default (1000)", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + delta: { + type: "streamable-http", + url: "https://x.test/mcp", + maxFetchRequests: 5000, + }, + }, + }), + ); + const res = await fetch(`${h.baseUrl}/api/servers/delta`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + maxFetchRequests: 1000, + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers + .delta as unknown as Record; + expect(stored).not.toHaveProperty("maxFetchRequests"); + }); + + it("rejects a negative maxFetchRequests", async () => { + const res = await fetch(`${h.baseUrl}/api/servers`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + id: "bad-maxfetch", + config: { type: "stdio", command: "node" }, + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + maxFetchRequests: -5, + }, + }), + }); + expect(res.status).toBe(400); + }); + it("rejects a non-object settings field", async () => { const res = await fetch(`${h.baseUrl}/api/servers`, { method: "POST", diff --git a/clients/web/src/test/integration/mcp/remote/transport.test.ts b/clients/web/src/test/integration/mcp/remote/transport.test.ts index e3082a43..06084555 100644 --- a/clients/web/src/test/integration/mcp/remote/transport.test.ts +++ b/clients/web/src/test/integration/mcp/remote/transport.test.ts @@ -411,6 +411,7 @@ describe("Remote transport e2e", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + maxFetchRequests: 1000, roots: [], }; diff --git a/core/mcp/remote/node/server.ts b/core/mcp/remote/node/server.ts index 70f0dd9f..ad2c99ef 100644 --- a/core/mcp/remote/node/server.ts +++ b/core/mcp/remote/node/server.ts @@ -25,7 +25,7 @@ import { streamSSE } from "hono/streaming"; import { watch as chokidarWatch, type FSWatcher } from "chokidar"; import { createTransportNode } from "../../node/transport.js"; import type { RemoteConnectRequest, RemoteSendRequest } from "../types.js"; -import { DEFAULT_TASK_TTL_MS } from "../../types.js"; +import { DEFAULT_MAX_FETCH_REQUESTS, DEFAULT_TASK_TTL_MS } from "../../types.js"; import type { InspectorServerSettings, MCPConfig, @@ -962,6 +962,16 @@ export function createRemoteApp( ); delete valObj.taskTtl; } + if ( + "maxFetchRequests" in valObj && + !isNonNegNumber(valObj.maxFetchRequests) + ) { + logWarn( + { route: "/api/servers", id, droppedKey: "maxFetchRequests" }, + "Dropping malformed `maxFetchRequests` field — expected non-negative number.", + ); + delete valObj.maxFetchRequests; + } if ("oauth" in valObj && !isOauthObject(valObj.oauth)) { logWarn( { route: "/api/servers", id, droppedKey: "oauth" }, @@ -1120,6 +1130,18 @@ export function createRemoteApp( error: "settings.autoRefreshOnListChanged must be a boolean", }; } + // maxFetchRequests is optional on the wire (older clients won't send it); + // when present it must be a non-negative number (0 = unlimited), otherwise + // it defaults below. + if ( + obj.maxFetchRequests !== undefined && + (typeof obj.maxFetchRequests !== "number" || obj.maxFetchRequests < 0) + ) { + return { + ok: false, + error: "settings.maxFetchRequests must be a non-negative number", + }; + } for (const optional of [ "oauthClientId", "oauthClientSecret", @@ -1160,6 +1182,13 @@ export function createRemoteApp( // Absent → false, matching the read side. The omit-on-false logic lives // in inspectorSettingsToStoredFields, so a false value writes nothing. autoRefreshOnListChanged: obj.autoRefreshOnListChanged === true, + // Absent → product default, matching the read side. The default is the + // omit-sentinel in inspectorSettingsToStoredFields, so a client that + // didn't send one writes no spurious maxFetchRequests to disk. + maxFetchRequests: + typeof obj.maxFetchRequests === "number" + ? obj.maxFetchRequests + : DEFAULT_MAX_FETCH_REQUESTS, // Absent → empty list, matching the read side // (storedFieldsToInspectorSettings). Empty rows are dropped on the way // to disk by inspectorSettingsToStoredFields, so an empty array here diff --git a/core/mcp/serverList.ts b/core/mcp/serverList.ts index 13c33f01..4bd76692 100644 --- a/core/mcp/serverList.ts +++ b/core/mcp/serverList.ts @@ -5,7 +5,7 @@ * remote-server route handlers. */ -import { DEFAULT_TASK_TTL_MS } from "./types.js"; +import { DEFAULT_MAX_FETCH_REQUESTS, DEFAULT_TASK_TTL_MS } from "./types.js"; import type { Root } from "@modelcontextprotocol/sdk/types.js"; import type { InspectorServerSettings, @@ -93,6 +93,7 @@ type StoredInspectorFields = Pick< | "requestTimeout" | "taskTtl" | "autoRefreshOnListChanged" + | "maxFetchRequests" | "oauth" | "roots" >; @@ -119,6 +120,7 @@ export function storedFieldsToInspectorSettings( stored.requestTimeout !== undefined || stored.taskTtl !== undefined || stored.autoRefreshOnListChanged !== undefined || + stored.maxFetchRequests !== undefined || stored.oauth !== undefined || stored.roots !== undefined; if (!hasAny) return undefined; @@ -136,6 +138,10 @@ export function storedFieldsToInspectorSettings( // default so the form shows it and "Run as task" has a value to send. taskTtl: stored.taskTtl ?? DEFAULT_TASK_TTL_MS, autoRefreshOnListChanged: stored.autoRefreshOnListChanged ?? false, + // Concrete default like taskTtl (not a 0-sentinel): the form needs a value + // to render and the log state needs one to size its buffer. An absent + // on-disk field reads back as the default, which the write side then omits. + maxFetchRequests: stored.maxFetchRequests ?? DEFAULT_MAX_FETCH_REQUESTS, // Defaults to an empty list so the form always has a concrete array to // render controlled rows from. An absent on-disk `roots` reads back as // `[]`, which `inspectorSettingsToStoredFields` then omits on write. @@ -203,6 +209,14 @@ export function inspectorSettingsToStoredFields( out.autoRefreshOnListChanged = true; } + // Persist only when it differs from the default. Unlike the timeouts, 0 is a + // meaningful value here (unlimited), so the omit-sentinel is the default + // itself rather than 0 — writing the default would inject the field into + // hand-edited files that never had it and break byte-stable round-trips. + if (settings.maxFetchRequests !== DEFAULT_MAX_FETCH_REQUESTS) { + out.maxFetchRequests = settings.maxFetchRequests; + } + const oauthFields: { clientId?: string; clientSecret?: string; @@ -248,6 +262,7 @@ const INSPECTOR_FIELD_KEY_MAP = { requestTimeout: true, taskTtl: true, autoRefreshOnListChanged: true, + maxFetchRequests: true, oauth: true, roots: true, } as const satisfies Record; diff --git a/core/mcp/state/fetchRequestLogState.ts b/core/mcp/state/fetchRequestLogState.ts index 9761a0d8..eb539f58 100644 --- a/core/mcp/state/fetchRequestLogState.ts +++ b/core/mcp/state/fetchRequestLogState.ts @@ -13,6 +13,8 @@ * concrete `InspectorClient` since the runtime class is not yet ported. */ +import type pino from "pino"; +import { silentLogger } from "../../logging/logger.js"; import type { InspectorClientProtocol } from "../inspectorClientProtocol.js"; import type { FetchRequestEntry } from "../types.js"; import type { @@ -25,9 +27,22 @@ import { type TypedEventGeneric, } from "../typedEventTarget.js"; +/** + * Detail for `fetchRequestBodyDropped`: a deferred response body arrived after + * its log entry had already rotated out (>= `maxFetchRequests` newer requests + * appended in between), so the body was unrecoverable. Carries the dropped + * request id and the cap that was in force, so the UI can explain the drop and + * offer to raise the limit. + */ +export interface FetchRequestBodyDropped { + id: string; + maxFetchRequests: number; +} + export interface FetchRequestLogStateEventMap { fetchRequest: FetchRequestEntry; fetchRequestsChange: FetchRequestEntry[]; + fetchRequestBodyDropped: FetchRequestBodyDropped; } export interface FetchRequestLogStateOptions { @@ -44,13 +59,19 @@ export interface FetchRequestLogStateOptions { sessionStorage?: InspectorClientStorage; /** Session ID for load/save; required for sessionStorage to have effect. */ sessionId?: string; + /** + * Logger for diagnostic traces (e.g. a deferred body update arriving after + * its entry rotated out of the log). Defaults to the silent logger. + */ + logger?: pino.Logger; } export class FetchRequestLogState extends TypedEventTarget { private fetchRequests: FetchRequestEntry[] = []; private client: InspectorClientProtocol | null = null; private unsubscribe: (() => void) | null = null; - private readonly maxFetchRequests: number; + private maxFetchRequests: number; + private readonly logger: pino.Logger; constructor( client: InspectorClientProtocol, @@ -58,6 +79,7 @@ export class FetchRequestLogState extends TypedEventTarget { const { id, responseBody } = event.detail; const idx = this.fetchRequests.findIndex((e) => e.id === id); - if (idx === -1) return; + if (idx === -1) { + // The deferred body read (fire-and-forget tee'd stream in + // fetchTracking) found no matching entry. Distinguish the two causes: + // a *rotation* drop — the entry was evicted because the log is at + // capacity (>= maxFetchRequests, with a finite cap) — versus a benign + // straggler whose entry was simply cleared (length below the cap, or + // an unlimited log where rotation can't happen). Only the rotation + // case is unexpected data loss worth surfacing; a post-clear straggler + // is expected and stays silent. + const rotatedOut = + this.maxFetchRequests > 0 && + this.fetchRequests.length >= this.maxFetchRequests; + if (rotatedOut) { + // `warn` (not `debug`) so the trace clears the web remote logger's + // default `info` level and is observable without opt-in. + this.logger.warn( + { fetchRequestId: id, maxFetchRequests: this.maxFetchRequests }, + "fetchRequestBodyUpdate dropped: entry rotated out before body arrived", + ); + // Let the UI surface the drop (toast) and offer to raise the cap. + this.dispatchTypedEvent("fetchRequestBodyDropped", { + id, + maxFetchRequests: this.maxFetchRequests, + }); + } + return; + } this.fetchRequests[idx] = { ...this.fetchRequests[idx]!, responseBody, @@ -202,6 +250,21 @@ export class FetchRequestLogState extends TypedEventTarget 0 && this.fetchRequests.length > max) { + this.fetchRequests = this.fetchRequests.slice(-max); + this.dispatchTypedEvent("fetchRequestsChange", this.getFetchRequests()); + } + } + /** * Clear all fetch requests. Dispatches fetchRequestsChange only if the list * was non-empty. diff --git a/core/mcp/types.ts b/core/mcp/types.ts index a432f399..a61c7639 100644 --- a/core/mcp/types.ts +++ b/core/mcp/types.ts @@ -111,6 +111,13 @@ export type StoredMCPServer = MCPServerConfig & { * false (the default). (#1402) */ autoRefreshOnListChanged?: boolean; + /** + * Maximum number of HTTP fetch requests retained in the Network log for this + * server (oldest rotate out past the cap). Inspector-specific. Omitted on + * disk when it equals `DEFAULT_MAX_FETCH_REQUESTS` (the default), keeping the + * file diff minimal for servers that never tuned it. `0` means unlimited. + */ + maxFetchRequests?: number; /** * Pre-configured OAuth client credentials for HTTP transports. Nested to * match Claude Code's `.mcp.json` shape; lifted into the flat `oauthClientId` @@ -381,6 +388,15 @@ export interface OAuthSettings { */ export const DEFAULT_TASK_TTL_MS = 60000; +/** + * Default maximum number of HTTP fetch requests retained in the Network log + * (per server). When exceeded, the oldest entries rotate out. A larger value + * keeps more history at the cost of memory; `0` means unlimited (not + * recommended). Mirrors `FetchRequestLogState`'s built-in default so the form + * and the log state agree on the omit-sentinel. + */ +export const DEFAULT_MAX_FETCH_REQUESTS = 1000; + /** * Runtime settings for a configured server. A subset of * InspectorClientOptions (v1.5) relevant to the settings form: @@ -402,6 +418,14 @@ export interface InspectorServerSettings { * and the user pulls the new list via Refresh. (#1402) */ autoRefreshOnListChanged?: boolean; + /** + * Maximum number of HTTP fetch requests retained in the Network log for this + * server. When exceeded, the oldest entries rotate out (and any deferred + * response body that arrives after its entry rotated out is dropped — see + * `FetchRequestLogState`). Concrete value so the form always has something to + * render; defaults to `DEFAULT_MAX_FETCH_REQUESTS`. `0` means unlimited. + */ + maxFetchRequests: number; /** * Roots advertised to the server via the `roots` client capability. Each * root carries a required `uri` and an optional `name` (SDK `Root`). The