fix(web): correct the OAuth header hint, keep utils off components (#1915)
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 <cliff@futurescale.com>
This commit is contained in:
parent
d60a5c713d
commit
51d35d864b
5 changed files with 33 additions and 13 deletions
|
|
@ -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[];
|
||||
|
|
|
|||
|
|
@ -497,9 +497,9 @@ export function ServerConfigModal({
|
|||
</HeadersHeader>
|
||||
<HeadersHint>
|
||||
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.
|
||||
</HeadersHint>
|
||||
<KeyValueRows
|
||||
items={form.headers}
|
||||
|
|
|
|||
|
|
@ -611,9 +611,11 @@ export function ServerSettingsForm({
|
|||
<Stack gap="md">
|
||||
<Group justify="space-between">
|
||||
<HintText>
|
||||
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.
|
||||
</HintText>
|
||||
<AddButton onClick={onAddHeader}>+ Add Header</AddButton>
|
||||
</Group>
|
||||
|
|
|
|||
15
clients/web/src/utils/keyValuePairs.ts
Normal file
15
clients/web/src/utils/keyValuePairs.ts
Normal file
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in a new issue