From 51d35d864b97497b80a7b54b2227724cd25abff3 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 16 Aug 2026 22:14:40 -0400 Subject: [PATCH] fix(web): correct the OAuth header hint, keep utils off components (#1915) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-4 review follow-ups on #2038. The Custom Headers hint claimed a custom `Authorization` value is "ignored" once OAuth is configured. Verified against the installed @modelcontextprotocol/client: both SSE and streamable-HTTP `_commonHeaders()` set the bearer first and then spread `requestInit.headers`, so a custom value *overrides* the OAuth token. The hint told users the opposite of what would happen, and could have them silently break their own auth. Corrected in both places — the modal's new hint and the pre-existing one in ServerSettingsForm, which carried the same wrong claim. `utils/serverSettingsPatch.ts` imported both its shared types from components, reversing the one-way `components -> utils` direction AGENTS.md sets. `KeyValuePair` moves to `utils/keyValuePairs.ts` (KeyValueRows re-exports it), and the mode is spelled as a literal union — still checked, since `ServerConfigModalMode` must stay assignable to it at the call site. Signed-off-by: cliffhall --- .../elements/KeyValueRows/KeyValueRows.tsx | 8 ++++---- .../ServerConfigModal/ServerConfigModal.tsx | 6 +++--- .../ServerSettingsForm/ServerSettingsForm.tsx | 8 +++++--- clients/web/src/utils/keyValuePairs.ts | 15 +++++++++++++++ clients/web/src/utils/serverSettingsPatch.ts | 9 ++++++--- 5 files changed, 33 insertions(+), 13 deletions(-) create mode 100644 clients/web/src/utils/keyValuePairs.ts diff --git a/clients/web/src/components/elements/KeyValueRows/KeyValueRows.tsx b/clients/web/src/components/elements/KeyValueRows/KeyValueRows.tsx index 9a44585d..776ce5e9 100644 --- a/clients/web/src/components/elements/KeyValueRows/KeyValueRows.tsx +++ b/clients/web/src/components/elements/KeyValueRows/KeyValueRows.tsx @@ -1,10 +1,10 @@ import { ActionIcon, Group, TextInput } from "@mantine/core"; import { ClearButton } from "../ClearButton/ClearButton"; +import type { KeyValuePair } from "../../../utils/keyValuePairs"; -export interface KeyValuePair { - key: string; - value: string; -} +// 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[]; diff --git a/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.tsx b/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.tsx index 0361aa32..105bb121 100644 --- a/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.tsx +++ b/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.tsx @@ -497,9 +497,9 @@ export function ServerConfigModal({ Sent with every HTTP request to this server — cookies - included. If OAuth is configured later, the `Authorization` - header is owned by the OAuth flow and any value set here is - ignored. + included. A custom `Authorization` header takes precedence + over an OAuth access token, so remove it if you configure + OAuth for this server later. - Headers sent with every HTTP request to this server. If OAuth is - configured below, the `Authorization` header is owned by the - OAuth flow and any value set here is ignored. + Headers sent with every HTTP request to this server. A custom + `Authorization` header takes precedence over an OAuth access + token — the SDK transports apply these headers last — so remove + it once OAuth is configured, or the flow's token never gets + sent. + Add Header diff --git a/clients/web/src/utils/keyValuePairs.ts b/clients/web/src/utils/keyValuePairs.ts new file mode 100644 index 00000000..2625e651 --- /dev/null +++ b/clients/web/src/utils/keyValuePairs.ts @@ -0,0 +1,15 @@ +/** + * 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; +} diff --git a/clients/web/src/utils/serverSettingsPatch.ts b/clients/web/src/utils/serverSettingsPatch.ts index 25fe7709..b7500c44 100644 --- a/clients/web/src/utils/serverSettingsPatch.ts +++ b/clients/web/src/utils/serverSettingsPatch.ts @@ -1,6 +1,5 @@ import type { InspectorServerSettings } from "@inspector/core/mcp/types.js"; -import type { ServerConfigModalMode } from "../components/groups/ServerConfigModal/ServerConfigModal"; -import type { KeyValuePair } from "../components/elements/KeyValueRows/KeyValueRows"; +import type { KeyValuePair } from "./keyValuePairs"; /** * Decide what `settings` node ServerConfigModal's submit should send for the @@ -32,7 +31,11 @@ import type { KeyValuePair } from "../components/elements/KeyValueRows/KeyValueR * add or clone. */ export function buildHeaderSettingsPatch( - mode: ServerConfigModalMode, + // 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,