modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

revert(web): drop custom headers from the manual add form (#1915)

Browse source 
Server Settings already has a Custom Headers editor, so adding one to the
manual add form created a second place to edit the same field. The reporter
had also conditionally withdrawn #1915 — "editing that information from the
server settings dialog is sufficient" once #1848 closed, which it has.

Reverts the add-form editor, the shared KeyValueRows element, the optional
`settings` argument on addServer / updateServer, and the App-level merge
helper. What remains are two defects found while working in
ServerSettingsForm, which this PR now closes instead (#2040): the Custom
Headers hint misstating OAuth precedence, and the key/value rows' missing
accessible names.

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-16 22:57:34 -04:00
commit 4dd8ed4498
13 changed files with 152 additions and 975 deletions
Download patch file Download diff file Expand all files Collapse all files

61
clients/web/src/App.tsx
View file

@ -138,8 +138,6 @@ import {
ServerConfigModal,
type ServerConfigModalMode,
} from "./components/groups/ServerConfigModal/ServerConfigModal";
import type { KeyValuePair } from "./components/elements/KeyValueRows/KeyValueRows";
import { buildHeaderSettingsPatch } from "./utils/serverSettingsPatch";
import { ServerSettingsModal } from "./components/groups/ServerSettingsModal/ServerSettingsModal";
import { ClientSettingsModal } from "./components/groups/ClientSettingsModal/ClientSettingsModal";
import {
@ -3897,17 +3895,30 @@ function App() {
// each id), deduped. The batch is reset to empty when an add/import modal
// opens (see the menu handlers).
const addServerHighlighted = useCallback(
async (
id: string,
config: MCPServerConfig,
settings?: InspectorServerSettings,
) => {
await addServer(id, config, settings);
async (id: string, config: MCPServerConfig) => {
await addServer(id, config);
setHighlightedServerIds((ids) => (ids.includes(id) ? ids : [...ids, id]));
},
[addServer],
);
// On rename of the active server, keep activeServerId pointed at the new id.
const onConfigSubmit = useCallback(
async (id: string, config: MCPServerConfig) => {
if (configModal?.mode === "edit" && configModal.targetId) {
const originalId = configModal.targetId;
await updateServer(originalId, id, config);
if (originalId === activeServerId && id !== originalId) {
setActiveServerId(id);
}
return;
}
// add or clone
await addServerHighlighted(id, config);
},
[configModal, addServerHighlighted, updateServer, activeServerId],
);
// Derive the existingIds list the modal uses for uniqueness validation.
// In edit mode the target's own id must be excluded so saving without
// renaming doesn't trip the "already exists" check.
@ -3924,39 +3935,6 @@ function App() {
return servers.find((s) => s.id === configModal.targetId);
}, [configModal, servers]);
// On rename of the active server, keep activeServerId pointed at the new id.
const onConfigSubmit = useCallback(
async (id: string, config: MCPServerConfig, headers: KeyValuePair[]) => {
// Headers live on the entry's `settings`, not on the transport config,
// so they're folded back in here (#1915). The edit-vs-clone rules live
// in the helper, which is unit-tested — this file is outside the
// coverage gate and that distinction is where credentials can leak.
const settings = buildHeaderSettingsPatch(
configModal?.mode ?? "add",
configModalTarget?.settings,
headers,
EMPTY_SETTINGS,
);
if (configModal?.mode === "edit" && configModal.targetId) {
const originalId = configModal.targetId;
await updateServer(originalId, id, config, settings);
if (originalId === activeServerId && id !== originalId) {
setActiveServerId(id);
}
return;
}
// add or clone
await addServerHighlighted(id, config, settings);
},
[
configModal,
configModalTarget,
addServerHighlighted,
updateServer,
activeServerId,
],
);
const settingsModalTarget = useMemo(() => {
if (!settingsModalTargetId) return undefined;
return servers.find((s) => s.id === settingsModalTargetId);
@ -4572,7 +4550,6 @@ function App() {
mode={configModal?.mode ?? "add"}
initialId={configModalTarget?.id}
initialConfig={configModalTarget?.config}
initialHeaders={configModalTarget?.settings?.headers}
existingIds={existingIds}
onClose={() => setConfigModal(null)}
onSubmit={onConfigSubmit}

60
clients/web/src/components/elements/KeyValueRows/KeyValueRows.stories.tsx
View file

@ -1,60 +0,0 @@
import type { Meta, StoryObj } from "@storybook/react-vite";
import { expect, fn, userEvent, within } from "storybook/test";
import { KeyValueRows } from "./KeyValueRows";
const meta: Meta<typeof KeyValueRows> = {
title: "Elements/KeyValueRows",
component: KeyValueRows,
args: {
entityLabel: "header",
onChange: fn(),
onRemove: fn(),
},
};
export default meta;
type Story = StoryObj<typeof KeyValueRows>;
// Each row's controls carry a row-scoped accessible name, so a screen reader
// can tell one row's key box from another's.
export const Populated: Story = {
args: {
items: [
{ key: "Cookie", value: "branch=feature-x" },
{ key: "X-Env", value: "dev" },
],
},
play: async ({ args, canvasElement }) => {
const canvas = within(canvasElement);
const value = await canvas.findByRole("textbox", {
name: "header value, Cookie, row 1",
});
await expect(value).toHaveValue("branch=feature-x");
await userEvent.click(
canvas.getByRole("button", { name: "Remove header, X-Env, row 2" }),
);
await expect(args.onRemove).toHaveBeenCalledWith(1);
},
};
// A row whose key is still blank is named by position alone rather than
// announcing nothing.
export const BlankKeyRow: Story = {
args: { items: [{ key: "", value: "" }] },
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await expect(
await canvas.findByRole("textbox", { name: "header name, row 1" }),
).toBeInTheDocument();
},
};
// An empty list renders nothing — callers draw their own empty-state hint.
export const Empty: Story = {
args: { items: [] },
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await expect(canvas.queryAllByRole("textbox")).toHaveLength(0);
},
};

126
clients/web/src/components/elements/KeyValueRows/KeyValueRows.test.tsx
View file

@ -1,126 +0,0 @@
import { describe, it, expect, vi } from "vitest";
import userEvent from "@testing-library/user-event";
import { renderWithMantine, screen } from "../../../test/renderWithMantine";
import { KeyValueRows } from "./KeyValueRows";
describe("KeyValueRows", () => {
function setup(items: { key: string; value: string }[], disabled?: boolean) {
const onChange = vi.fn();
const onRemove = vi.fn();
renderWithMantine(
<KeyValueRows
items={items}
entityLabel="header"
disabled={disabled}
onChange={onChange}
onRemove={onRemove}
/>,
);
return { onChange, onRemove };
}
it("renders nothing for an empty list", () => {
setup([]);
expect(screen.queryAllByRole("textbox")).toHaveLength(0);
});
it("names each control by entity and row so rows are distinguishable", () => {
setup([
{ key: "Cookie", value: "a=1" },
{ key: "", value: "" },
]);
expect(
screen.getByRole("textbox", { name: "header name, Cookie, row 1" }),
).toHaveValue("Cookie");
expect(
screen.getByRole("textbox", { name: "header value, Cookie, row 1" }),
).toHaveValue("a=1");
// A blank (or whitespace-only) key leaves the position alone as the name.
expect(
screen.getByRole("textbox", { name: "header name, row 2" }),
).toBeInTheDocument();
});
it("reports key and value edits with the row index", async () => {
const user = userEvent.setup({ delay: null });
const { onChange } = setup([{ key: "X", value: "1" }]);
await user.type(
screen.getByRole("textbox", { name: "header name, X, row 1" }),
"Y",
);
expect(onChange).toHaveBeenLastCalledWith(0, "XY", "1");
await user.type(
screen.getByRole("textbox", { name: "header value, X, row 1" }),
"2",
);
expect(onChange).toHaveBeenLastCalledWith(0, "X", "12");
});
it("clears a key or a value through its own named Clear button", async () => {
const user = userEvent.setup({ delay: null });
const { onChange } = setup([{ key: "X", value: "1" }]);
// A bare "Clear" repeated per field would be indistinguishable across
// rows, so each clear button names the field it empties.
await user.click(
screen.getByRole("button", { name: "Clear header name, X, row 1" }),
);
expect(onChange).toHaveBeenLastCalledWith(0, "", "1");
await user.click(
screen.getByRole("button", { name: "Clear header value, X, row 1" }),
);
expect(onChange).toHaveBeenLastCalledWith(0, "X", "");
});
it("omits the Clear button for an empty key or value", () => {
setup([{ key: "", value: "" }]);
expect(screen.queryAllByRole("button", { name: /^Clear/ })).toHaveLength(0);
});
it("locks every control when disabled", () => {
setup([{ key: "X", value: "1" }], true);
expect(
screen.getByRole("textbox", { name: "header name, X, row 1" }),
).toBeDisabled();
expect(
screen.getByRole("textbox", { name: "header value, X, row 1" }),
).toBeDisabled();
expect(
screen.getByRole("button", { name: "Clear header name, X, row 1" }),
).toBeDisabled();
expect(
screen.getByRole("button", { name: "Remove header, X, row 1" }),
).toBeDisabled();
});
it("distinguishes two rows that share a key", () => {
// Duplicate keys happen mid-edit, and a server can persist duplicate
// metadata, so the key alone cannot identify a row.
setup([
{ key: "Set-Cookie", value: "a=1" },
{ key: "Set-Cookie", value: "b=2" },
]);
expect(
screen.getByRole("textbox", { name: "header value, Set-Cookie, row 1" }),
).toHaveValue("a=1");
expect(
screen.getByRole("textbox", { name: "header value, Set-Cookie, row 2" }),
).toHaveValue("b=2");
});
it("removes the clicked row", async () => {
const user = userEvent.setup({ delay: null });
const { onRemove } = setup([
{ key: "A", value: "1" },
{ key: "B", value: "2" },
]);
await user.click(
screen.getByRole("button", { name: "Remove header, B, row 2" }),
);
expect(onRemove).toHaveBeenCalledWith(1);
});
});

114
clients/web/src/components/elements/KeyValueRows/KeyValueRows.tsx
View file

@ -1,114 +0,0 @@
import { ActionIcon, Group, TextInput } from "@mantine/core";
import { ClearButton } from "../ClearButton/ClearButton";
import type { KeyValuePair } from "../../../utils/keyValuePairs";
// Re-exported for call sites that already import this component, so they need
// not know the type lives a layer down.
export type { KeyValuePair };
export interface KeyValueRowsProps {
items: KeyValuePair[];
/**
* Singular noun for one row ("header", "environment variable", …). Used only
* to build each control's `aria-label`: the section heading and the "Key" /
* "Value" placeholders are not programmatically associated with the inputs,
* so without it an assistive technology cannot tell one list's key box from
* another's, and the remove button announces only "X". The clear buttons are
* named the same way — a bare "Clear" repeated six times across three rows
* tells a screen-reader user nothing about which field it empties.
*/
entityLabel: string;
/**
* Lock every control in the list — used while a submit is in flight, so the
* rows can't drift out of sync with the payload the caller already captured.
*/
disabled?: boolean;
onChange: (index: number, key: string, value: string) => void;
onRemove: (index: number) => void;
}
// Optional (non-required) clearable field — keeps the ClearButton clickable.
const ClearableTextInput = TextInput.withProps({
rightSectionPointerEvents: "auto",
});
const RemoveIcon = ActionIcon.withProps({
color: "red",
variant: "subtle",
});
/**
* Controlled editor for a list of `{ key, value }` pairs — the shape the
* Inspector persists for custom headers, request metadata, and stdio
* environment variables. Owns no state: every keystroke is reported through
* `onChange(index, key, value)` and the caller re-renders with the new list.
*
* Shared by ServerSettingsForm (headers / metadata / env) and ServerConfigModal
* (headers on the manual add form, #1915) so the two cannot drift.
*/
export function KeyValueRows({
items,
entityLabel,
disabled,
onChange,
onRemove,
}: KeyValueRowsProps) {
return (
<>
{items.map((item, index) => {
// The row number is always part of the label, not just a fallback for
// a blank key: two rows can carry the SAME key (mid-edit, or a
// duplicate a server legitimately persisted), and a key-only label
// would give both rows' controls identical accessible names — the very
// thing this labelling exists to prevent.
const key = item.key.trim();
const rowLabel = key ? `${key}, row ${index + 1}` : `row ${index + 1}`;
return (
<Group key={index} grow>
<ClearableTextInput
placeholder="Key"
aria-label={`${entityLabel} name, ${rowLabel}`}
value={item.key}
disabled={disabled}
onChange={(e) =>
onChange(index, e.currentTarget.value, item.value)
}
rightSection={
item.key ? (
<ClearButton
aria-label={`Clear ${entityLabel} name, ${rowLabel}`}
disabled={disabled}
onClick={() => onChange(index, "", item.value)}
/>
) : null
}
/>
<ClearableTextInput
placeholder="Value"
aria-label={`${entityLabel} value, ${rowLabel}`}
value={item.value}
disabled={disabled}
onChange={(e) => onChange(index, item.key, e.currentTarget.value)}
rightSection={
item.value ? (
<ClearButton
aria-label={`Clear ${entityLabel} value, ${rowLabel}`}
disabled={disabled}
onClick={() => onChange(index, item.key, "")}
/>
) : null
}
/>
<RemoveIcon
aria-label={`Remove ${entityLabel}, ${rowLabel}`}
disabled={disabled}
onClick={() => onRemove(index)}
>
X
</RemoveIcon>
</Group>
);
})}
</>
);
}

40
clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.stories.tsx
View file

@ -19,8 +19,8 @@ function InteractiveRender(args: ServerConfigModalProps) {
args.onClose();
updateArgs({ opened: false });
}}
onSubmit={async (id, config, headers) => {
await args.onSubmit(id, config, headers);
onSubmit={async (id, config) => {
await args.onSubmit(id, config);
updateArgs({ opened: false });
}}
/>
@ -134,39 +134,7 @@ export const EditSse: Story = {
const body = within(canvasElement.ownerDocument.body);
const dialog = within(await findDialog(body, "Edit server"));
await expect(await dialog.findByLabelText(/^URL/)).toBeInTheDocument();
// The headers editor starts collapsed to a bare "+ Add Header" control
// when the server has none stored.
await expect(
dialog.getByRole("button", { name: "+ Add Header" }),
).toBeInTheDocument();
await expect(
dialog.queryAllByRole("textbox", { name: /header name/ }),
).toHaveLength(0);
},
};
// Custom headers on the manual form (#1915) — the reason the field exists is a
// cookie routing requests to one developer's branch on a shared dev server.
export const EditSseWithHeaders: Story = {
args: {
mode: "edit",
initialId: "remote",
initialConfig: sseConfig,
initialHeaders: [{ key: "Cookie", value: "branch=feature-x" }],
existingIds: [],
},
play: async ({ canvasElement }) => {
const body = within(canvasElement.ownerDocument.body);
const dialog = within(await findDialog(body, "Edit server"));
const value = (await dialog.findByRole("textbox", {
name: "header value, Cookie, row 1",
})) as HTMLInputElement;
await expect(value.value).toBe("branch=feature-x");
// Adding a row appends an empty pair, named by position until it is keyed.
await userEvent.click(dialog.getByRole("button", { name: "+ Add Header" }));
await expect(
await dialog.findByRole("textbox", { name: "header name, row 2" }),
).toBeInTheDocument();
// Headers are no longer entered here — they live in ServerSettingsForm.
await expect(dialog.queryByLabelText(/Headers/i)).toBeNull();
},
};

180
clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.test.tsx
View file

@ -66,11 +66,11 @@ describe("ServerConfigModal", () => {
await user.click(screen.getByRole("button", { name: /^Add$/ }));
await waitFor(() => expect(props.onSubmit).toHaveBeenCalledOnce());
expect(props.onSubmit).toHaveBeenCalledWith(
"alpha",
{ type: "stdio", command: "node", args: ["x.js", "--port=3000"] },
[],
);
expect(props.onSubmit).toHaveBeenCalledWith("alpha", {
type: "stdio",
command: "node",
args: ["x.js", "--port=3000"],
});
});
it("requires a command for stdio submission", async () => {
@ -117,7 +117,7 @@ describe("ServerConfigModal", () => {
expect(screen.queryByLabelText(/^Command/)).not.toBeInTheDocument();
});
it("submits an sse config with just the url and no headers", async () => {
it("submits an sse config with just the url (headers move to the settings form)", async () => {
const user = userEvent.setup({ delay: null });
const props = base();
renderWithMantine(
@ -133,11 +133,10 @@ describe("ServerConfigModal", () => {
await user.click(screen.getByRole("button", { name: /^Save$/ }));
await waitFor(() => expect(props.onSubmit).toHaveBeenCalledOnce());
expect(props.onSubmit).toHaveBeenCalledWith(
"remote",
{ type: "sse", url: "https://x.test/sse" },
[],
);
expect(props.onSubmit).toHaveBeenCalledWith("remote", {
type: "sse",
url: "https://x.test/sse",
});
});
it("submits a streamable-http config", async () => {
@ -153,11 +152,10 @@ describe("ServerConfigModal", () => {
);
await user.click(screen.getByRole("button", { name: /^Save$/ }));
await waitFor(() => expect(props.onSubmit).toHaveBeenCalledOnce());
expect(props.onSubmit).toHaveBeenCalledWith(
"http-srv",
{ type: "streamable-http", url: "https://x.test/mcp" },
[],
);
expect(props.onSubmit).toHaveBeenCalledWith("http-srv", {
type: "streamable-http",
url: "https://x.test/mcp",
});
});
it("loads the url from a streamable-http config", () => {
@ -277,11 +275,11 @@ describe("ServerConfigModal", () => {
await user.click(screen.getByRole("button", { name: /^Add$/ }));
await waitFor(() => expect(props.onSubmit).toHaveBeenCalledOnce());
expect(props.onSubmit).toHaveBeenCalledWith(
"alpha",
{ type: "stdio", command: "node", cwd: "/tmp/cwd" },
[],
);
expect(props.onSubmit).toHaveBeenCalledWith("alpha", {
type: "stdio",
command: "node",
cwd: "/tmp/cwd",
});
});
it("clears the Server ID field via its Clear button", async () => {
@ -472,144 +470,4 @@ describe("ServerConfigModal", () => {
await user.click(screen.getByRole("button", { name: /Cancel/ }));
expect(props.onClose).toHaveBeenCalledOnce();
});
describe("custom headers (#1915)", () => {
it("hides the headers section for stdio", () => {
renderWithMantine(<ServerConfigModal {...base()} />);
expect(screen.queryByText("Custom headers")).not.toBeInTheDocument();
});
// Every keystroke re-renders the whole modal, and this case types four
// fields, so it runs several times slower under the coverage project's v8
// instrumentation than in a plain unit run — enough to trip the 5s default.
// The extra ceiling is headroom for the instrumented run, not a hang guard.
const TYPING_HEAVY_TIMEOUT_MS = 20000;
it(
"submits headers added on the manual add form",
async () => {
const user = userEvent.setup({ delay: null });
const props = base();
// Seeded on the http transport rather than driven through the
// Transport select — that select has its own test above, and walking
// its combobox here only added to the cost described above.
renderWithMantine(
<ServerConfigModal
{...props}
initialConfig={{ type: "streamable-http", url: "" }}
/>,
);
await user.type(screen.getByLabelText(/Server ID/i), "remote");
await user.type(screen.getByLabelText(/^URL/), "https://x.test/mcp");
await user.click(screen.getByRole("button", { name: "+ Add Header" }));
await user.type(
screen.getByRole("textbox", { name: /header name, row 1/ }),
"Cookie",
);
await user.type(
screen.getByRole("textbox", { name: /header value, Cookie, row 1/ }),
"branch=feature-x",
);
await user.click(screen.getByRole("button", { name: /^Add$/ }));
await waitFor(() => expect(props.onSubmit).toHaveBeenCalledOnce());
expect(props.onSubmit).toHaveBeenCalledWith(
"remote",
{ type: "streamable-http", url: "https://x.test/mcp" },
[{ key: "Cookie", value: "branch=feature-x" }],
);
},
TYPING_HEAVY_TIMEOUT_MS,
);
it("pre-populates the rows from initialHeaders and submits an edit", async () => {
const user = userEvent.setup({ delay: null });
const props = base();
renderWithMantine(
<ServerConfigModal
{...props}
mode="edit"
initialId="remote"
initialConfig={{ type: "sse", url: "https://x.test/sse" }}
initialHeaders={[{ key: "X-Env", value: "dev" }]}
/>,
);
const valueInput = screen.getByRole("textbox", {
name: /header value, X-Env, row 1/,
});
expect(valueInput).toHaveValue("dev");
await user.type(valueInput, "-2");
await user.click(screen.getByRole("button", { name: /^Save$/ }));
await waitFor(() => expect(props.onSubmit).toHaveBeenCalledOnce());
expect(props.onSubmit).toHaveBeenCalledWith(
"remote",
{ type: "sse", url: "https://x.test/sse" },
[{ key: "X-Env", value: "dev-2" }],
);
});
it("drops blank-key rows and removes a row on X", async () => {
const user = userEvent.setup({ delay: null });
const props = base();
renderWithMantine(
<ServerConfigModal
{...props}
mode="edit"
initialId="remote"
initialConfig={{ type: "sse", url: "https://x.test/sse" }}
initialHeaders={[
{ key: "Keep", value: "1" },
{ key: "Drop", value: "2" },
]}
/>,
);
// An empty row is the form's "still typing" placeholder — it must not
// reach the caller.
await user.click(screen.getByRole("button", { name: "+ Add Header" }));
// Editing one row of several must leave its siblings untouched.
await user.type(
screen.getByRole("textbox", { name: "header value, Keep, row 1" }),
"9",
);
await user.click(
screen.getByRole("button", { name: "Remove header, Drop, row 2" }),
);
await user.click(screen.getByRole("button", { name: /^Save$/ }));
await waitFor(() => expect(props.onSubmit).toHaveBeenCalledOnce());
expect(props.onSubmit).toHaveBeenCalledWith(
"remote",
{ type: "sse", url: "https://x.test/sse" },
[{ key: "Keep", value: "19" }],
);
});
it("submits no headers for stdio even when initialHeaders is set", async () => {
const user = userEvent.setup({ delay: null });
const props = base();
renderWithMantine(
<ServerConfigModal
{...props}
mode="edit"
initialId="local"
initialConfig={{ type: "stdio", command: "node" }}
initialHeaders={[{ key: "X-Env", value: "dev" }]}
/>,
);
await user.click(screen.getByRole("button", { name: /^Save$/ }));
await waitFor(() => expect(props.onSubmit).toHaveBeenCalledOnce());
expect(props.onSubmit).toHaveBeenCalledWith(
"local",
{ type: "stdio", command: "node" },
[],
);
});
});
});

129
clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.tsx
View file

@ -10,10 +10,6 @@ import {
Textarea,
} from "@mantine/core";
import { ClearButton } from "../../elements/ClearButton/ClearButton";
import {
KeyValueRows,
type KeyValuePair,
} from "../../elements/KeyValueRows/KeyValueRows";
import { useValueChange } from "../../../hooks/useValueChange";
import type {
MCPServerConfig,
@ -32,27 +28,11 @@ export interface ServerConfigModalProps {
initialId?: string;
/** When editing or cloning, the existing config to pre-populate. */
initialConfig?: MCPServerConfig;
/**
* Custom HTTP headers already stored for the target server, pre-populated
* when editing or cloning. Headers are not part of `MCPServerConfig` — they
* live on the entry's `settings`, so they arrive (and leave, via `onSubmit`)
* as their own value rather than folded into the config. (#1915)
*/
initialHeaders?: KeyValuePair[];
/** Ids already in use — drives the uniqueness check (caller excludes the
* target id from this list when in 'edit' mode). */
existingIds: string[];
onClose: () => void;
/**
* `headers` carries the edited custom-header rows (blank-key rows already
* dropped). It is always `[]` for stdio, which has no HTTP request to attach
* headers to.
*/
onSubmit: (
id: string,
config: MCPServerConfig,
headers: KeyValuePair[],
) => Promise<void> | void;
onSubmit: (id: string, config: MCPServerConfig) => Promise<void> | void;
}
type TransportChoice = "stdio" | "sse" | "streamable-http";
@ -67,8 +47,6 @@ interface FormState {
cwd: string;
// sse / streamable-http
url: string;
/** Custom HTTP headers — sse / streamable-http only. (#1915) */
headers: KeyValuePair[];
}
// The `string`-valued FormState keys, which all share the same text-input
@ -106,10 +84,6 @@ const EnvTextarea = Textarea.withProps({
minRows: 2,
rightSectionPointerEvents: "auto",
});
const AddHeaderButton = Button.withProps({ size: "xs", variant: "light" });
const HeadersHeader = Group.withProps({ justify: "space-between", gap: "sm" });
const HeadersLabel = Text.withProps({ size: "sm", fw: 500 });
const HeadersHint = Text.withProps({ size: "xs", c: "dimmed" });
const MODE_TITLES: Record<ServerConfigModalMode, string> = {
add: "Add server",
@ -120,14 +94,9 @@ const MODE_TITLES: Record<ServerConfigModalMode, string> = {
function configToFormState(
initialId: string | undefined,
initialConfig: MCPServerConfig | undefined,
initialHeaders: KeyValuePair[] | undefined,
mode: ServerConfigModalMode,
): FormState {
const id = mode === "edit" ? (initialId ?? "") : "";
// Copy the rows rather than aliasing the caller's array — the form mutates
// this list as its own state, and a clone keeps a cancelled edit from
// reaching the entry the caller passed in.
const headers = (initialHeaders ?? []).map((h) => ({ ...h }));
const transport: TransportChoice =
initialConfig?.type === undefined ? "stdio" : initialConfig.type;
if (!initialConfig) {
@ -139,7 +108,6 @@ function configToFormState(
envText: "",
cwd: "",
url: "",
headers,
};
}
if (transport === "stdio") {
@ -154,9 +122,9 @@ function configToFormState(
.join("\n"),
cwd: c.cwd ?? "",
url: "",
headers,
};
}
// sse / streamable-http — custom headers live in ServerSettingsForm now.
const url =
initialConfig.type === "sse" || initialConfig.type === "streamable-http"
? initialConfig.url
@ -169,7 +137,6 @@ function configToFormState(
envText: "",
cwd: "",
url: url ?? "",
headers,
};
}
@ -218,14 +185,13 @@ export function ServerConfigModal({
mode,
initialId,
initialConfig,
initialHeaders,
existingIds,
onClose,
onSubmit,
}: ServerConfigModalProps) {
const initial = useMemo(
() => configToFormState(initialId, initialConfig, initialHeaders, mode),
[initialId, initialConfig, initialHeaders, mode],
() => configToFormState(initialId, initialConfig, mode),
[initialId, initialConfig, mode],
);
const [form, setForm] = useState<FormState>(initial);
const [submitError, setSubmitError] = useState<string | undefined>(undefined);
@ -244,22 +210,6 @@ export function ServerConfigModal({
const clearTextField = (field: TextField) => () =>
setForm((f) => ({ ...f, [field]: "" }));
// Header rows are a pair array rather than a plain string, so they get their
// own handlers instead of riding `setTextField` (which `TextField` excludes
// by type).
const addHeader = () =>
setForm((f) => ({ ...f, headers: [...f.headers, { key: "", value: "" }] }));
const removeHeader = (index: number) =>
setForm((f) => ({
...f,
headers: f.headers.filter((_, i) => i !== index),
}));
const changeHeader = (index: number, key: string, value: string) =>
setForm((f) => ({
...f,
headers: f.headers.map((h, i) => (i === index ? { key, value } : h)),
}));
// Reset the form whenever the modal opens, or whenever `initial` changes
// while it is open. Keying on `opened ? initial : undefined` collapses both
// triggers into one value: it flips to `initial` on open, tracks `initial`
@ -313,9 +263,9 @@ export function ServerConfigModal({
return { ok: false, error: "URL is required for sse / streamable-http." };
}
const base = { url: form.url.trim() };
// Custom headers are persisted under `settings.headers` on the entry, not
// on the config — they leave through `onSubmit`'s third argument, so the
// SSE / streamable-http config here carries only transport fields.
// Custom headers live in ServerSettingsForm now (persisted under
// settings.headers on the entry); the SSE / streamable-http config here
// only carries the canonical transport fields.
const config: MCPServerConfig =
form.transport === "sse"
? { type: "sse", ...base }
@ -338,19 +288,9 @@ export function ServerConfigModal({
setSubmitError(built.error);
return;
}
// Blank-key rows are the form's placeholder for "row being typed" and mean
// nothing on the wire — the persist layer drops them anyway, so drop them
// here too and keep the caller's payload honest. stdio has no HTTP request
// to carry headers, so it always submits none.
const headers =
form.transport === "stdio"
? []
: form.headers
.map((h) => ({ key: h.key.trim(), value: h.value }))
.filter((h) => h.key.length > 0);
setSubmitting(true);
try {
await onSubmit(trimmedId, built.config, headers);
await onSubmit(trimmedId, built.config);
onClose();
} catch (err) {
setSubmitError(err instanceof Error ? err.message : String(err));
@ -472,44 +412,21 @@ export function ServerConfigModal({
/>
</>
) : (
<>
<RequiredTextInput
label="URL"
placeholder="https://example.com/mcp"
value={form.url}
onChange={setTextField("url")}
disabled={submitting}
rightSection={
form.url ? (
<ClearButton
disabled={submitting}
onClick={clearTextField("url")}
/>
) : null
}
/>
<FieldGrid>
<HeadersHeader>
<HeadersLabel>Custom headers</HeadersLabel>
<AddHeaderButton onClick={addHeader} disabled={submitting}>
+ Add Header
</AddHeaderButton>
</HeadersHeader>
<HeadersHint>
Sent with every HTTP request to this server — cookies
included. A custom `Authorization` header takes precedence
over an OAuth access token, so remove it if you configure
OAuth for this server later.
</HeadersHint>
<KeyValueRows
items={form.headers}
entityLabel="header"
disabled={submitting}
onChange={changeHeader}
onRemove={removeHeader}
/>
</FieldGrid>
</>
<RequiredTextInput
label="URL"
placeholder="https://example.com/mcp"
value={form.url}
onChange={setTextField("url")}
disabled={submitting}
rightSection={
form.url ? (
<ClearButton
disabled={submitting}
onClick={clearTextField("url")}
/>
) : null
}
/>
)}
</FieldGrid>

79
clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx
View file

@ -13,7 +13,6 @@ import {
TextInput,
} from "@mantine/core";
import { ClearButton } from "../../elements/ClearButton/ClearButton";
import { KeyValueRows } from "../../elements/KeyValueRows/KeyValueRows";
import type { ProtocolEra } from "@modelcontextprotocol/client";
import type {
InspectorServerSettings,
@ -224,6 +223,84 @@ const ClearStoredOAuthHint = Text.withProps({
miw: "12rem",
});
function KeyValueRows({
items,
entityLabel,
onChange,
onRemove,
}: {
items: { key: string; value: string }[];
/**
* Singular noun for one row ("header", "environment variable", …). Used only
* to build each control's `aria-label`: the section heading and the "Key" /
* "Value" placeholders are not programmatically associated with the inputs,
* so without it an assistive technology cannot tell one section's key box
* from another's, and every remove button announces only "X". The row number
* rides along because two rows can carry the same key — mid-edit, or a
* duplicate a server persisted — and a key-only name would leave both rows'
* controls indistinguishable, which is the thing this exists to prevent.
*/
entityLabel: string;
onChange: (index: number, key: string, value: string) => void;
onRemove: (index: number) => void;
}) {
/* v8 ignore next 3 -- unreachable: every caller guards with `length === 0`
and renders an EmptyHint instead, so KeyValueRows is only mounted with
a non-empty list. */
if (items.length === 0) {
return null;
}
return (
<>
{items.map((item, index) => {
const key = item.key.trim();
const rowLabel = key ? `${key}, row ${index + 1}` : `row ${index + 1}`;
return (
<Group key={index} grow>
<ClearableTextInput
placeholder="Key"
aria-label={`${entityLabel} name, ${rowLabel}`}
value={item.key}
onChange={(e) =>
onChange(index, e.currentTarget.value, item.value)
}
rightSection={
item.key ? (
<ClearButton
aria-label={`Clear ${entityLabel} name, ${rowLabel}`}
onClick={() => onChange(index, "", item.value)}
/>
) : null
}
/>
<ClearableTextInput
placeholder="Value"
aria-label={`${entityLabel} value, ${rowLabel}`}
value={item.value}
onChange={(e) => onChange(index, item.key, e.currentTarget.value)}
rightSection={
item.value ? (
<ClearButton
aria-label={`Clear ${entityLabel} value, ${rowLabel}`}
onClick={() => onChange(index, item.key, "")}
/>
) : null
}
/>
<RemoveIcon
aria-label={`Remove ${entityLabel}, ${rowLabel}`}
onClick={() => onRemove(index)}
>
X
</RemoveIcon>
</Group>
);
})}
</>
);
}
// Reserved-key rejection is inline per row (#2018): the reason is passed as the
// key input's `error` **string**, so Mantine renders it in the input's own error
// slot and wires the `aria-describedby` association — a bare boolean would mark

89
clients/web/src/test/core/react/useServers.test.tsx
View file

@ -123,95 +123,6 @@ describe("useServers", () => {
});
});
it("addServer persists a settings node when one is supplied (#1915)", async () => {
const { result } = renderHook(() =>
useServers({ baseUrl: "http://test.local", fetchFn: h.fetchFn }),
);
await waitFor(() => expect(result.current.loading).toBe(false));
await act(async () => {
// The manual add form carries custom headers here — they live on
// `settings`, not on the transport config.
await result.current.addServer(
"remote",
{ type: "streamable-http", url: "https://x.test/mcp" },
{
headers: [{ key: "Cookie", value: "branch=feature-x" }],
env: [],
metadata: [],
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,
maxFetchRequests: 1000,
roots: [],
},
);
});
await waitFor(() => {
const added = result.current.servers.find((srv) => srv.id === "remote");
expect(added?.settings?.headers).toEqual([
{ key: "Cookie", value: "branch=feature-x" },
]);
});
// On disk headers are the flat `Record<string, string>` form, a direct key
// on the entry (post-#1358) rather than a nested settings node.
expect(readConfig(h.configPath).mcpServers.remote?.headers).toEqual({
Cookie: "branch=feature-x",
});
});
it("updateServer replaces the settings node when one is supplied (#1915)", async () => {
writeFileSync(
h.configPath,
JSON.stringify({
mcpServers: {
alpha: {
type: "streamable-http",
url: "https://x.test/mcp",
headers: { "X-Old": "1" },
metadata: [{ key: "trace", value: "abc" }],
},
},
}),
);
const { result } = renderHook(() =>
useServers({ baseUrl: "http://test.local", fetchFn: h.fetchFn }),
);
await waitFor(() => expect(result.current.loading).toBe(false));
await act(async () => {
// Editing headers in ServerConfigModal sends the whole settings node,
// so the caller is responsible for carrying the untouched fields
// (here: metadata) forward.
await result.current.updateServer(
"alpha",
"alpha",
{ type: "streamable-http", url: "https://x.test/mcp" },
{
headers: [{ key: "X-New", value: "2" }],
env: [],
metadata: [{ key: "trace", value: "abc" }],
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,
maxFetchRequests: 1000,
roots: [],
},
);
});
await waitFor(() => {
expect(result.current.servers[0]?.settings?.headers).toEqual([
{ key: "X-New", value: "2" },
]);
});
expect(result.current.servers[0]?.settings?.metadata).toEqual([
{ key: "trace", value: "abc" },
]);
});
it("importSource returns a result for a known source type", async () => {
const { result } = renderHook(() =>
useServers({ baseUrl: "http://test.local", fetchFn: h.fetchFn }),

15
clients/web/src/utils/keyValuePairs.ts
View file

@ -1,15 +0,0 @@
/**
* One editable `{ key, value }` row — the in-memory shape the Inspector uses
* for custom headers, request metadata, and stdio environment variables
* (`InspectorServerSettings.headers` and friends). A pair *array* rather than
* a record specifically so a controlled form can hold a half-typed row with a
* blank key; the persist layer collapses it to a record and drops the blanks.
*
* Declared here, in `utils`, because it is a pure domain type shared by a
* component (`KeyValueRows`, which re-exports it) and non-UI logic
* (`serverSettingsPatch`). Imports point `components -> utils`, never back.
*/
export interface KeyValuePair {
key: string;
value: string;
}

135
clients/web/src/utils/serverSettingsPatch.test.ts
View file

@ -1,135 +0,0 @@
import { describe, it, expect } from "vitest";
import type { InspectorServerSettings } from "@inspector/core/mcp/types.js";
import { buildHeaderSettingsPatch } from "./serverSettingsPatch";
const EMPTY: InspectorServerSettings = {
headers: [],
env: [],
metadata: [],
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,
autoRefreshOnListChanged: false,
paginatedLists: false,
maxFetchRequests: 1000,
roots: [],
};
// A server carrying settings the modal never shows — the fields an edit must
// preserve and a clone must not copy.
const POPULATED: InspectorServerSettings = {
...EMPTY,
headers: [{ key: "X-Old", value: "1" }],
metadata: [{ key: "trace", value: "abc" }],
connectionTimeout: 5000,
oauthClientId: "cid",
oauthClientSecret: "shhh",
roots: [{ uri: "file:///project", name: "Project" }],
};
const NEW_HEADERS = [{ key: "Cookie", value: "branch=x" }];
describe("buildHeaderSettingsPatch", () => {
it("sends nothing when there are no headers on either side", () => {
expect(
buildHeaderSettingsPatch("add", undefined, [], EMPTY),
).toBeUndefined();
expect(buildHeaderSettingsPatch("edit", EMPTY, [], EMPTY)).toBeUndefined();
});
it("carries the target's other settings forward on an edit", () => {
const patch = buildHeaderSettingsPatch(
"edit",
POPULATED,
NEW_HEADERS,
EMPTY,
);
expect(patch).toEqual({ ...POPULATED, headers: NEW_HEADERS });
// The node is replaced wholesale, so the fields the modal doesn't show
// have to travel with it.
expect(patch?.metadata).toEqual([{ key: "trace", value: "abc" }]);
expect(patch?.oauthClientSecret).toBe("shhh");
expect(patch?.connectionTimeout).toBe(5000);
});
it("sends nothing when an edit leaves the headers untouched", () => {
// The modal holds a snapshot taken when it opened, so writing it back on
// an id/URL-only save would clobber a metadata or OAuth change made in the
// settings form since. Omitting the key is what preserves it.
expect(
buildHeaderSettingsPatch(
"edit",
POPULATED,
// A fresh array with equal contents — the modal always rebuilds it.
[{ key: "X-Old", value: "1" }],
EMPTY,
),
).toBeUndefined();
});
it("sends when an edit only reorders the headers", () => {
const two: InspectorServerSettings = {
...POPULATED,
headers: [
{ key: "A", value: "1" },
{ key: "B", value: "2" },
],
};
expect(
buildHeaderSettingsPatch(
"edit",
two,
[
{ key: "B", value: "2" },
{ key: "A", value: "1" },
],
EMPTY,
),
).toEqual({
...two,
headers: [
{ key: "B", value: "2" },
{ key: "A", value: "1" },
],
});
});
it("still sends on an edit that clears the last header", () => {
expect(buildHeaderSettingsPatch("edit", POPULATED, [], EMPTY)).toEqual({
...POPULATED,
headers: [],
});
});
it("does not copy the source server's settings on a clone", () => {
// The regression this guards: `configModalTarget` in clone mode is the
// SOURCE entry, so spreading it put that server's OAuth client secret on a
// brand-new one.
const patch = buildHeaderSettingsPatch(
"clone",
POPULATED,
NEW_HEADERS,
EMPTY,
);
expect(patch).toEqual({ ...EMPTY, headers: NEW_HEADERS });
expect(patch?.oauthClientSecret).toBeUndefined();
expect(patch?.oauthClientId).toBeUndefined();
expect(patch?.metadata).toEqual([]);
expect(patch?.roots).toEqual([]);
expect(patch?.connectionTimeout).toBe(0);
});
it("builds from the empty shape on an add", () => {
expect(
buildHeaderSettingsPatch("add", undefined, NEW_HEADERS, EMPTY),
).toEqual({ ...EMPTY, headers: NEW_HEADERS });
});
it("ignores a stale target on a clone with no headers", () => {
// A clone of a server that HAS headers, submitted with none: nothing about
// the new entry needs a settings node.
expect(
buildHeaderSettingsPatch("clone", POPULATED, [], EMPTY),
).toBeUndefined();
});
});

55
clients/web/src/utils/serverSettingsPatch.ts
View file

@ -1,55 +0,0 @@
import type { InspectorServerSettings } from "@inspector/core/mcp/types.js";
import type { KeyValuePair } from "./keyValuePairs";
/**
* Decide what `settings` node ServerConfigModal's submit should send for the
* custom headers it just collected (#1915).
*
* Two rules, both load-bearing:
*
* 1. **Only an edit carries the target's other settings forward.** The
* backend replaces the whole node when one is sent, so an edit must
* re-send the fields the modal doesn't expose (metadata, timeouts, OAuth
* credentials, roots) or they'd be dropped. An **add or clone must not**:
* the modal's target in clone mode is the *source* server, so spreading it
* would copy that server's OAuth client secret and behavior flags onto a
* new entry the user only gave a URL and some headers.
* 2. **`undefined` means "don't send the key at all", and that is the
* default.** Omitting `settings` makes the backend preserve the node it
* already has, which matters beyond convenience: what this modal holds is
* a snapshot taken when it opened, so writing it back on a save that did
* not touch a header would overwrite a metadata or OAuth change made in
* the settings form in the meantime. So the patch is sent only when the
* submitted headers actually differ from the stored ones — including the
* case where the last one was cleared, which does need the node rewritten.
*
* Lives here rather than inline in App.tsx so this seam is unit-testable:
* App.tsx is outside the coverage gate, and the edit-vs-clone distinction is
* exactly where credentials leaked before.
*
* @param emptySettings the app's blank settings shape, used as the base for an
* add or clone.
*/
export function buildHeaderSettingsPatch(
// Spelled as a literal union rather than imported from ServerConfigModal:
// `utils` must not depend on `components`. It stays honest because
// `ServerConfigModalMode` is checked against it at the call site — adding a
// fourth mode there is a compile error here.
mode: "add" | "edit" | "clone",
existingSettings: InspectorServerSettings | undefined,
headers: KeyValuePair[],
emptySettings: InspectorServerSettings,
): InspectorServerSettings | undefined {
const existing = mode === "edit" ? existingSettings : undefined;
if (sameHeaders(existing?.headers ?? [], headers)) return undefined;
return { ...(existing ?? emptySettings), headers };
}
/**
* Order-sensitive pair-list equality. Order matters because it is what the
* form round-trips and what the user sees, so a reorder is a real edit.
*/
function sameHeaders(a: KeyValuePair[], b: KeyValuePair[]): boolean {
if (a.length !== b.length) return false;
return a.every((h, i) => h.key === b[i]?.key && h.value === b[i]?.value);
}

44
core/react/useServers.ts
View file

@ -29,28 +29,11 @@ export interface UseServersResult {
loading: boolean;
error: string | undefined;
refresh: () => Promise<void>;
/**
* `settings` is optional: omitted, no settings node is persisted for the new
* entry. Passed, it is written alongside the config in the same POST — the
* add form needs this to carry custom headers, which live on `settings`
* rather than on the transport config (#1915).
*/
addServer: (
id: string,
config: MCPServerConfig,
settings?: InspectorServerSettings,
) => Promise<void>;
/**
* `settings` is optional and asymmetric with `addServer`'s: **omitting** it
* tells the backend to preserve the entry's existing settings node, so a
* config-only save cannot silently wipe persisted headers / metadata / OAuth
* credentials. Pass a full settings object to replace it.
*/
addServer: (id: string, config: MCPServerConfig) => Promise<void>;
updateServer: (
originalId: string,
newId: string,
config: MCPServerConfig,
settings?: InspectorServerSettings,
) => Promise<void>;
/**
* Patch only the `settings` node on an existing server entry, leaving the
@ -219,17 +202,11 @@ export function useServers(opts: UseServersOptions): UseServersResult {
}, [base, authToken, doFetch, refreshInternal]);
const addServer = useCallback(
async (
id: string,
config: MCPServerConfig,
settings?: InspectorServerSettings,
): Promise<void> => {
async (id: string, config: MCPServerConfig): Promise<void> => {
const res = await doFetch(`${base}/api/servers`, {
method: "POST",
headers: buildHeaders(authToken, true),
// `settings` is omitted from the body when undefined (JSON.stringify
// drops undefined values), which the route reads as "no settings node".
body: JSON.stringify({ id, config, settings }),
body: JSON.stringify({ id, config }),
});
if (!res.ok) {
throw new Error(await readErrorMessage(res));
@ -258,21 +235,18 @@ export function useServers(opts: UseServersOptions): UseServersResult {
originalId: string,
newId: string,
config: MCPServerConfig,
settings?: InspectorServerSettings,
): Promise<void> => {
// With `settings` undefined, `JSON.stringify` drops the key entirely and
// the backend route treats that omission as "preserve the existing
// settings node on disk" — so a config-only save cannot silently wipe
// persisted headers / metadata / OAuth credentials. A caller editing a
// field that lives on settings (e.g. ServerConfigModal's custom headers)
// passes the full settings object it wants written, which replaces the
// node wholesale — so it must carry forward the fields it isn't editing.
// `settings` is intentionally omitted from the body. The backend route
// treats omission as "preserve the existing settings node on disk", so
// a config-only save (e.g. ServerConfigModal) cannot silently wipe
// persisted headers / metadata / OAuth credentials. To explicitly
// clear settings, send `settings: null`.
const res = await doFetch(
`${base}/api/servers/${encodeURIComponent(originalId)}`,
{
method: "PUT",
headers: buildHeaders(authToken, true),
body: JSON.stringify({ id: newId, config, settings }),
body: JSON.stringify({ id: newId, config }),
},
);
if (!res.ok) {