diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index daac033c..55559799 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -1261,6 +1261,7 @@ const settingsWithRoots = ( roots: InspectorServerSettings["roots"], ): InspectorServerSettings => ({ headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 9bdf0b18..7b07379d 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -20,6 +20,7 @@ import type { Tool, } from "@modelcontextprotocol/sdk/types.js"; import { InspectorClient } from "@inspector/core/mcp/index.js"; +import { getServerType } from "@inspector/core/mcp/config.js"; import type { InspectorClientEventMap, JsonValue, @@ -282,6 +283,7 @@ async function replayHistoryRequest( // React doesn't re-allocate on every render. const EMPTY_SETTINGS: InspectorServerSettings = { headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, @@ -2257,6 +2259,16 @@ function App() { const settingsModalValue: InspectorServerSettings = settingsDraft ?? EMPTY_SETTINGS; + // Gate the stdio-only Working Directory / Environment Variables controls in + // the settings modal. Resolve the target server's transport from its config; + // default to non-stdio when the target isn't resolvable (modal closed). + const settingsModalTarget = servers.find( + (s) => s.id === settingsModalTargetId, + ); + const settingsModalIsStdio = settingsModalTarget + ? getServerType(settingsModalTarget.config) === "stdio" + : false; + const onSettingsModalClose = useCallback(() => { flushSettingsDraft(); // Apply root edits to the live client once, on close — not on every @@ -2551,6 +2563,7 @@ function App() { key={settingsModalTargetId ?? "closed"} opened={settingsModalTargetId !== undefined} settings={settingsModalValue} + isStdio={settingsModalIsStdio} onClose={onSettingsModalClose} onSettingsChange={onSettingsChange} /> diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.stories.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.stories.tsx index 442ef3e5..ed710900 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.stories.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.stories.tsx @@ -9,6 +9,7 @@ import { const defaultSettings: InspectorServerSettings = { headers: [], + env: [], metadata: [], connectionTimeout: 30000, requestTimeout: 60000, @@ -85,6 +86,39 @@ function InteractiveRender(args: ServerSettingsFormProps) { }, }); }} + onAddEnv={() => { + args.onAddEnv(); + updateArgs({ + settings: { + ...args.settings, + env: [...args.settings.env, { key: "", value: "" }], + }, + }); + }} + onRemoveEnv={(index) => { + args.onRemoveEnv(index); + updateArgs({ + settings: { + ...args.settings, + env: args.settings.env.filter((_, i) => i !== index), + }, + }); + }} + onEnvChange={(index, key, value) => { + args.onEnvChange(index, key, value); + updateArgs({ + settings: { + ...args.settings, + env: args.settings.env.map((e, i) => + i === index ? { key, value } : e, + ), + }, + }); + }} + onCwdChange={(value) => { + args.onCwdChange(value); + updateArgs({ settings: { ...args.settings, cwd: value } }); + }} onTimeoutChange={(field, value) => { args.onTimeoutChange(field, value); updateArgs({ @@ -123,11 +157,16 @@ const meta: Meta = { component: ServerSettingsForm, render: InteractiveRender, args: { + isStdio: false, expandedSections: ["options"], onExpandedSectionsChange: fn(), onAddHeader: fn(), onRemoveHeader: fn(), onHeaderChange: fn(), + onAddEnv: fn(), + onRemoveEnv: fn(), + onEnvChange: fn(), + onCwdChange: fn(), onAddMetadata: fn(), onRemoveMetadata: fn(), onMetadataChange: fn(), @@ -178,6 +217,7 @@ export const AllConfigured: Story = { { key: "Authorization", value: "Bearer token-abc-123" }, { key: "X-Request-Id", value: "req-456" }, ], + env: [], metadata: [ { key: "userId", value: "user-789" }, { key: "sessionId", value: "session-012" }, @@ -196,3 +236,18 @@ export const AllConfigured: Story = { }, }, }; + +export const StdioEnvironment: Story = { + args: { + isStdio: true, + expandedSections: ["options", "environment"], + settings: { + ...defaultSettings, + cwd: "/srv/my-server", + env: [ + { key: "API_KEY", value: "abc-123" }, + { key: "LOG_LEVEL", value: "debug" }, + ], + }, + }, +}; diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx index dc9a3388..29ac0c97 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx @@ -18,6 +18,7 @@ function clearButtonFor(input: HTMLElement): HTMLElement { const emptySettings: InspectorServerSettings = { headers: [], + env: [], metadata: [], connectionTimeout: 30000, requestTimeout: 60000, @@ -28,6 +29,7 @@ const emptySettings: InspectorServerSettings = { const populatedSettings: InspectorServerSettings = { headers: [{ key: "Authorization", value: "Bearer abc" }], + env: [], metadata: [{ key: "userId", value: "u-1" }], connectionTimeout: 30000, requestTimeout: 60000, @@ -48,10 +50,16 @@ const allSections: ServerSettingsSection[] = [ ]; const baseHandlers = { + // Default to non-stdio; the stdio-only env / cwd tests override this to true. + isStdio: false, onExpandedSectionsChange: vi.fn(), onAddHeader: vi.fn(), onRemoveHeader: vi.fn(), onHeaderChange: vi.fn(), + onAddEnv: vi.fn(), + onRemoveEnv: vi.fn(), + onEnvChange: vi.fn(), + onCwdChange: vi.fn(), onAddMetadata: vi.fn(), onRemoveMetadata: vi.fn(), onMetadataChange: vi.fn(), @@ -345,6 +353,128 @@ describe("ServerSettingsForm", () => { expect(onMaxFetchRequestsChange).toHaveBeenLastCalledWith(2500); }); + describe("stdio Working Directory (Options) / Environment Variables section", () => { + it("hides the Working Directory field and the Environment Variables section for non-stdio servers", () => { + renderWithMantine( + , + ); + expect( + screen.queryByLabelText(/Working Directory/), + ).not.toBeInTheDocument(); + // The whole Environment Variables accordion section is absent. + expect( + screen.queryByRole("button", { name: "Environment Variables" }), + ).not.toBeInTheDocument(); + expect( + screen.queryByText(/No environment variables configured/), + ).not.toBeInTheDocument(); + }); + + it("shows the Working Directory field in Options and the Environment Variables section for stdio servers", () => { + renderWithMantine( + , + ); + expect(screen.getByLabelText(/Working Directory/)).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "Environment Variables" }), + ).toBeInTheDocument(); + expect( + screen.getByText(/No environment variables configured/), + ).toBeInTheDocument(); + }); + + it("reflects the cwd value and invokes onCwdChange when typing", async () => { + const user = userEvent.setup(); + const onCwdChange = vi.fn(); + renderWithMantine( + , + ); + const input = screen.getByLabelText(/Working Directory/); + expect(input).toHaveValue("/srv"); + await user.type(input, "X"); + expect(onCwdChange).toHaveBeenLastCalledWith("/srvX"); + }); + + it("clears the cwd via its Clear button", async () => { + const user = userEvent.setup(); + const onCwdChange = vi.fn(); + renderWithMantine( + , + ); + await user.click( + clearButtonFor(screen.getByLabelText(/Working Directory/)), + ); + expect(onCwdChange).toHaveBeenCalledWith(""); + }); + + it("invokes onAddEnv when the Add Environment Variable button is clicked", async () => { + const user = userEvent.setup(); + const onAddEnv = vi.fn(); + renderWithMantine( + , + ); + await user.click( + screen.getByRole("button", { name: /Add Environment Variable/ }), + ); + expect(onAddEnv).toHaveBeenCalledTimes(1); + }); + + it("renders env rows and invokes onEnvChange / onRemoveEnv", async () => { + const user = userEvent.setup(); + const onEnvChange = vi.fn(); + const onRemoveEnv = vi.fn(); + renderWithMantine( + , + ); + const keyInput = screen.getByDisplayValue("API_KEY"); + await user.type(keyInput, "2"); + expect(onEnvChange).toHaveBeenLastCalledWith(0, "API_KEY2", "secret"); + + // The remove ("X") button sits alongside the row's key/value inputs. + const removeButtons = screen.getAllByRole("button", { name: "X" }); + await user.click(removeButtons[removeButtons.length - 1]!); + expect(onRemoveEnv).toHaveBeenCalledWith(0); + }); + }); + 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 cbf3e615..aa511eae 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx @@ -18,6 +18,7 @@ import type { Root } from "@modelcontextprotocol/sdk/types.js"; export type ServerSettingsSection = | "options" + | "environment" | "headers" | "metadata" | "timeouts" @@ -26,11 +27,21 @@ export type ServerSettingsSection = export interface ServerSettingsFormProps { settings: InspectorServerSettings; + /** + * Whether the target server uses the stdio transport. Gates the Working + * Directory field and Environment Variables section in the Options area — + * both are stdio-only config concepts and hidden for sse / streamable-http. + */ + isStdio: boolean; expandedSections: ServerSettingsSection[]; onExpandedSectionsChange: (sections: ServerSettingsSection[]) => void; onAddHeader: () => void; onRemoveHeader: (index: number) => void; onHeaderChange: (index: number, key: string, value: string) => void; + onAddEnv: () => void; + onRemoveEnv: (index: number) => void; + onEnvChange: (index: number, key: string, value: string) => void; + onCwdChange: (value: string) => void; onAddMetadata: () => void; onRemoveMetadata: (index: number) => void; onMetadataChange: (index: number, key: string, value: string) => void; @@ -165,11 +176,16 @@ function RootRows({ export function ServerSettingsForm({ settings, + isStdio, expandedSections, onExpandedSectionsChange, onAddHeader, onRemoveHeader, onHeaderChange, + onAddEnv, + onRemoveEnv, + onEnvChange, + onCwdChange, onAddMetadata, onRemoveMetadata, onMetadataChange, @@ -239,10 +255,52 @@ export function ServerSettingsForm({ value={settings.maxFetchRequests} onChange={handleMaxFetchRequestsChange} /> + {isStdio ? ( + onCwdChange(e.currentTarget.value)} + rightSectionPointerEvents="auto" + rightSection={ + settings.cwd ? ( + onCwdChange("")} /> + ) : null + } + /> + ) : null} + {isStdio ? ( + + Environment Variables + + + + + Environment variables passed to the stdio server process. + + + + Add Environment Variable + + + {settings.env.length === 0 ? ( + No environment variables configured + ) : ( + + )} + + + + ) : null} + Custom Headers diff --git a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.stories.tsx b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.stories.tsx index 56111497..0d249114 100644 --- a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.stories.tsx +++ b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.stories.tsx @@ -13,6 +13,7 @@ const initialSettings: InspectorServerSettings = { { key: "Authorization", value: "Bearer token-abc-123" }, { key: "X-Request-Id", value: "req-456" }, ], + env: [], metadata: [{ key: "userId", value: "user-789" }], connectionTimeout: 30000, requestTimeout: 60000, @@ -52,6 +53,7 @@ const meta: Meta = { render: InteractiveRender, args: { opened: true, + isStdio: false, onClose: fn(), onSettingsChange: fn(), }, @@ -70,6 +72,7 @@ export const EmptySettings: Story = { args: { settings: { headers: [], + env: [], metadata: [], connectionTimeout: 30000, requestTimeout: 60000, @@ -79,3 +82,25 @@ export const EmptySettings: Story = { }, }, }; + +// A stdio server surfaces the Working Directory field (Options) and a dedicated +// Environment Variables section. +export const StdioServer: Story = { + args: { + isStdio: true, + settings: { + headers: [], + env: [ + { key: "API_KEY", value: "abc-123" }, + { key: "LOG_LEVEL", value: "debug" }, + ], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + taskTtl: 60000, + maxFetchRequests: 1000, + cwd: "/srv/my-server", + roots: [], + }, + }, +}; diff --git a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx index b6b83cb8..7454d721 100644 --- a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx @@ -6,6 +6,7 @@ import { ServerSettingsModal } from "./ServerSettingsModal"; const initialSettings: InspectorServerSettings = { headers: [{ key: "Authorization", value: "Bearer abc" }], + env: [], metadata: [{ key: "userId", value: "u-1" }], connectionTimeout: 30000, requestTimeout: 60000, @@ -19,6 +20,7 @@ const initialSettings: InspectorServerSettings = { const emptySettings: InspectorServerSettings = { headers: [], + env: [], metadata: [], connectionTimeout: 30000, requestTimeout: 60000, @@ -33,6 +35,7 @@ describe("ServerSettingsModal", () => { , @@ -45,6 +48,7 @@ describe("ServerSettingsModal", () => { , @@ -60,6 +64,7 @@ describe("ServerSettingsModal", () => { , @@ -80,6 +85,7 @@ describe("ServerSettingsModal", () => { , @@ -99,6 +105,7 @@ describe("ServerSettingsModal", () => { , @@ -119,6 +126,7 @@ describe("ServerSettingsModal", () => { , @@ -140,6 +148,7 @@ describe("ServerSettingsModal", () => { , @@ -159,6 +168,7 @@ describe("ServerSettingsModal", () => { , @@ -181,6 +191,7 @@ describe("ServerSettingsModal", () => { , @@ -201,6 +212,7 @@ describe("ServerSettingsModal", () => { , @@ -220,6 +232,7 @@ describe("ServerSettingsModal", () => { , @@ -239,6 +252,7 @@ describe("ServerSettingsModal", () => { , @@ -258,6 +272,7 @@ describe("ServerSettingsModal", () => { , @@ -279,6 +294,7 @@ describe("ServerSettingsModal", () => { , @@ -299,6 +315,7 @@ describe("ServerSettingsModal", () => { , @@ -333,4 +350,112 @@ describe("ServerSettingsModal", () => { expect(optionsControl.getAttribute("aria-expanded")).toBe("false"); expect(headersControl.getAttribute("aria-expanded")).toBe("false"); }); + + describe("stdio env / cwd handlers", () => { + // The Options section is expanded on open, so the stdio fields are visible + // immediately when isStdio is true. + it("does not render the stdio fields when isStdio is false", () => { + renderWithMantine( + , + ); + expect( + screen.queryByLabelText(/Working Directory/), + ).not.toBeInTheDocument(); + }); + + it("calls onSettingsChange when adding an environment variable", async () => { + const user = userEvent.setup(); + const onSettingsChange = vi.fn(); + renderWithMantine( + , + ); + // The Environment Variables section is collapsed on open — expand it. + await user.click( + screen.getByRole("button", { name: "Environment Variables" }), + ); + await user.click( + screen.getByRole("button", { name: "+ Add Environment Variable" }), + ); + expect(onSettingsChange).toHaveBeenCalledWith({ + ...emptySettings, + env: [{ key: "", value: "" }], + }); + }); + + it("calls onSettingsChange when removing an environment variable", async () => { + const user = userEvent.setup(); + const onSettingsChange = vi.fn(); + renderWithMantine( + , + ); + await user.click( + screen.getByRole("button", { name: "Environment Variables" }), + ); + const removeButtons = screen.getAllByRole("button", { name: "X" }); + await user.click(removeButtons[removeButtons.length - 1]); + expect(onSettingsChange).toHaveBeenCalledWith({ + ...emptySettings, + env: [], + }); + }); + + it("calls onSettingsChange with the updated env value when typing", async () => { + const user = userEvent.setup(); + const onSettingsChange = vi.fn(); + renderWithMantine( + , + ); + await user.click( + screen.getByRole("button", { name: "Environment Variables" }), + ); + await user.type(screen.getByDisplayValue("1"), "2"); + const lastCall = onSettingsChange.mock.calls.at(-1)?.[0]; + expect(lastCall.env[0]).toEqual({ key: "A", value: "12" }); + }); + + it("calls onSettingsChange when editing the working directory", async () => { + const user = userEvent.setup(); + const onSettingsChange = vi.fn(); + renderWithMantine( + , + ); + // Controlled input: the value prop stays "/srv" (the parent vi.fn does not + // feed edits back), so a single keystroke appends to that base. + await user.type(screen.getByLabelText(/Working Directory/), "X"); + expect(onSettingsChange).toHaveBeenLastCalledWith({ + ...emptySettings, + cwd: "/srvX", + }); + }); + }); }); diff --git a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx index a0ea603b..0a410aa5 100644 --- a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx +++ b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx @@ -10,18 +10,30 @@ import { type ServerSettingsSection, } from "../ServerSettingsForm/ServerSettingsForm"; -const ALL_SECTIONS: ServerSettingsSection[] = [ - "options", - "headers", - "metadata", - "timeouts", - "oauth", - "roots", -]; +// The "environment" section only renders for stdio servers, so it joins the +// expand/collapse-all set only then — otherwise `allExpanded` could never be +// reached (the section isn't in the DOM to expand). +function allSectionsFor(isStdio: boolean): ServerSettingsSection[] { + return [ + "options", + ...(isStdio ? (["environment"] as const) : []), + "headers", + "metadata", + "timeouts", + "oauth", + "roots", + ]; +} export interface ServerSettingsModalProps { opened: boolean; settings: InspectorServerSettings; + /** + * Whether the target server uses the stdio transport. Forwarded to the form + * to gate the stdio-only Working Directory field and Environment Variables + * section. + */ + isStdio: boolean; onClose: () => void; onSettingsChange: (settings: InspectorServerSettings) => void; } @@ -29,6 +41,7 @@ export interface ServerSettingsModalProps { export function ServerSettingsModal({ opened, settings, + isStdio, onClose, onSettingsChange, }: ServerSettingsModalProps) { @@ -41,10 +54,11 @@ export function ServerSettingsModal({ ServerSettingsSection[] >(["options"]); - const allExpanded = expandedSections.length === ALL_SECTIONS.length; + const allSections = allSectionsFor(isStdio); + const allExpanded = expandedSections.length === allSections.length; function handleToggleAll() { - setExpandedSections(allExpanded ? [] : ALL_SECTIONS); + setExpandedSections(allExpanded ? [] : allSections); } function handleAddHeader() { @@ -68,6 +82,29 @@ export function ServerSettingsModal({ onSettingsChange({ ...settings, headers }); } + function handleAddEnv() { + onSettingsChange({ + ...settings, + env: [...settings.env, { key: "", value: "" }], + }); + } + + function handleRemoveEnv(index: number) { + onSettingsChange({ + ...settings, + env: settings.env.filter((_, i) => i !== index), + }); + } + + function handleEnvChange(index: number, key: string, value: string) { + const env = settings.env.map((e, i) => (i === index ? { key, value } : e)); + onSettingsChange({ ...settings, env }); + } + + function handleCwdChange(value: string) { + onSettingsChange({ ...settings, cwd: value }); + } + function handleAddMetadata() { onSettingsChange({ ...settings, @@ -158,11 +195,16 @@ export function ServerSettingsModal({ { }); const settings = { headers: [{ key: "X-Tenant", value: "acme" }], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, diff --git a/clients/web/src/test/core/mcp/serverList.test.ts b/clients/web/src/test/core/mcp/serverList.test.ts index 5684ef31..03465d2b 100644 --- a/clients/web/src/test/core/mcp/serverList.test.ts +++ b/clients/web/src/test/core/mcp/serverList.test.ts @@ -2,13 +2,17 @@ import { describe, it, expect } from "vitest"; import { cleanRoots, DEFAULT_SEED_CONFIG, + envPairsToRecord, + envRecordToPairs, expectedSecretFields, extractSecretsFromStored, + inspectorSettingsToStoredFields, mcpConfigToServerEntries, mergeSecretsIntoStored, normalizeServerType, serverEntriesToMcpConfig, serializeMcpConfig, + storedFieldsToInspectorSettings, } from "@inspector/core/mcp/serverList.js"; import { SECRET_FIELD_OAUTH_CLIENT_SECRET, @@ -314,6 +318,8 @@ describe("serverEntriesToMcpConfig", () => { expect(entry?.settings).toEqual({ // Object headers on disk → pair-array headers in memory headers: [{ key: "X-Tenant", value: "acme" }], + // Non-stdio server → empty env mirror in memory (for the form) + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, @@ -393,6 +399,7 @@ describe("serverEntriesToMcpConfig", () => { { key: "", value: "stub" }, { key: " ", value: "whitespace" }, ], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, @@ -419,6 +426,7 @@ describe("serverEntriesToMcpConfig", () => { config: { type: "streamable-http", url: "https://x.test" }, settings: { headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, @@ -447,6 +455,7 @@ describe("serverEntriesToMcpConfig", () => { config: { type: "stdio", command: "node" }, settings: { headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, @@ -485,6 +494,7 @@ describe("serverEntriesToMcpConfig", () => { config: { type: "stdio", command: "node" }, settings: { headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, @@ -511,6 +521,7 @@ describe("serverEntriesToMcpConfig", () => { config: { type: "stdio", command: "node" }, settings: { headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, @@ -777,3 +788,130 @@ describe("expectedSecretFields", () => { ).toEqual([SECRET_FIELD_OAUTH_CLIENT_SECRET]); }); }); + +describe("envRecordToPairs / envPairsToRecord", () => { + it("envRecordToPairs preserves key insertion order", () => { + expect(envRecordToPairs({ B: "2", A: "1" })).toEqual([ + { key: "B", value: "2" }, + { key: "A", value: "1" }, + ]); + }); + + it("envRecordToPairs returns an empty array for undefined", () => { + expect(envRecordToPairs(undefined)).toEqual([]); + }); + + it("envPairsToRecord drops empty / whitespace-only keys", () => { + expect( + envPairsToRecord([ + { key: "API_KEY", value: "secret" }, + { key: "", value: "ignored" }, + { key: " ", value: "ignored" }, + { key: "DEBUG", value: "1" }, + ]), + ).toEqual({ API_KEY: "secret", DEBUG: "1" }); + }); + + it("round-trips a populated env through both converters", () => { + const record = { API_KEY: "secret", DEBUG: "1" }; + expect(envPairsToRecord(envRecordToPairs(record))).toEqual(record); + }); +}); + +describe("stdio env / cwd mirroring", () => { + it("lifts stdio env (object → pair-array) and cwd onto settings while keeping them on config", () => { + const cfg: MCPConfig = { + mcpServers: { + alpha: { + type: "stdio", + command: "node", + args: ["server.js"], + env: { API_KEY: "secret", DEBUG: "1" }, + cwd: "/srv/app", + }, + }, + }; + const [entry] = mcpConfigToServerEntries(cfg); + expect(entry?.settings?.env).toEqual([ + { key: "API_KEY", value: "secret" }, + { key: "DEBUG", value: "1" }, + ]); + expect(entry?.settings?.cwd).toBe("/srv/app"); + // env / cwd remain on the SDK config so the transport still sees them. + expect(entry?.config).toMatchObject({ + env: { API_KEY: "secret", DEBUG: "1" }, + cwd: "/srv/app", + }); + }); + + it("materializes a settings node for a bare stdio entry carrying only env", () => { + const cfg: MCPConfig = { + mcpServers: { + alpha: { type: "stdio", command: "node", env: { A: "1" } }, + }, + }; + const [entry] = mcpConfigToServerEntries(cfg); + expect(entry?.settings?.env).toEqual([{ key: "A", value: "1" }]); + }); + + it("leaves env an empty list and cwd unset for a stdio entry without them", () => { + const settings = storedFieldsToInspectorSettings({ + headers: { "X-A": "b" }, + }); + expect(settings?.env).toEqual([]); + expect(settings?.cwd).toBeUndefined(); + }); + + it("mirrors empty env for non-stdio servers", () => { + const cfg: MCPConfig = { + mcpServers: { + alpha: { + type: "streamable-http", + url: "https://x.test/mcp", + headers: { "X-Tenant": "acme" }, + }, + }, + }; + const [entry] = mcpConfigToServerEntries(cfg); + expect(entry?.settings?.env).toEqual([]); + }); + + it("does NOT re-emit env / cwd from inspectorSettingsToStoredFields (config owns them on disk)", () => { + // env / cwd are config fields; the write side is the PUT route's + // write-through, not this converter. So even a populated settings.env must + // not leak back as a settings delta that would clobber config on merge. + const out = inspectorSettingsToStoredFields({ + headers: [], + env: [{ key: "API_KEY", value: "secret" }], + cwd: "/srv/app", + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + taskTtl: 60000, + maxFetchRequests: 1000, + roots: [], + }); + expect(out).not.toHaveProperty("env"); + expect(out).not.toHaveProperty("cwd"); + }); + + it("preserves stdio env / cwd through a config serialize round-trip", () => { + const cfg: MCPConfig = { + mcpServers: { + alpha: { + type: "stdio", + command: "node", + env: { API_KEY: "secret" }, + cwd: "/srv/app", + }, + }, + }; + const entries = mcpConfigToServerEntries(cfg); + const back = serverEntriesToMcpConfig(entries); + expect(back.mcpServers.alpha).toMatchObject({ + command: "node", + env: { API_KEY: "secret" }, + cwd: "/srv/app", + }); + }); +}); 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 79f9d4a4..2a775d15 100644 --- a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts @@ -11,6 +11,7 @@ function prompt(name: string): Prompt { const AUTO_REFRESH_SETTINGS: InspectorServerSettings = { headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, 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 c0cf63b6..980d3a9f 100644 --- a/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts @@ -11,6 +11,7 @@ function template(name: string): ResourceTemplate { const AUTO_REFRESH_SETTINGS: InspectorServerSettings = { headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, 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 33d82545..e738f516 100644 --- a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts @@ -11,6 +11,7 @@ function resource(uri: string): Resource { const AUTO_REFRESH_SETTINGS: InspectorServerSettings = { headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, 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 dad8d80a..a8865922 100644 --- a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts @@ -11,6 +11,7 @@ function tool(name: string): Tool { const AUTO_REFRESH_SETTINGS: InspectorServerSettings = { headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, diff --git a/clients/web/src/test/core/react/useServers.test.tsx b/clients/web/src/test/core/react/useServers.test.tsx index f2669dff..2bd98d85 100644 --- a/clients/web/src/test/core/react/useServers.test.tsx +++ b/clients/web/src/test/core/react/useServers.test.tsx @@ -411,6 +411,7 @@ describe("useServers", () => { await act(async () => { await result.current.updateServerSettings("alpha", { headers: [{ key: "X-Tenant", value: "acme" }], + env: [], metadata: [{ key: "trace", value: "abc" }], connectionTimeout: 5000, requestTimeout: 30000, @@ -423,6 +424,7 @@ describe("useServers", () => { await waitFor(() => { expect(result.current.servers[0]?.settings).toEqual({ headers: [{ key: "X-Tenant", value: "acme" }], + env: [], metadata: [{ key: "trace", value: "abc" }], connectionTimeout: 5000, requestTimeout: 30000, @@ -457,6 +459,7 @@ describe("useServers", () => { act(async () => { await result.current.updateServerSettings("nonexistent", { headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, @@ -506,6 +509,7 @@ describe("useServers", () => { }); expect(result.current.servers[0]?.settings).toEqual({ headers: [{ key: "X-Keep", value: "yes" }], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, diff --git a/clients/web/src/test/integration/mcp/import/clientConfig.test.ts b/clients/web/src/test/integration/mcp/import/clientConfig.test.ts index d5531826..a8a1fa22 100644 --- a/clients/web/src/test/integration/mcp/import/clientConfig.test.ts +++ b/clients/web/src/test/integration/mcp/import/clientConfig.test.ts @@ -22,13 +22,14 @@ describe("parseMcpServersConfig", () => { expect(config.mcpServers.sse.type).toBe("sse"); }); - it("preserves extension fields (env, headers) on entries", () => { + it("preserves extension fields (env, cwd, headers) on entries", () => { const raw = JSON.stringify({ mcpServers: { s: { command: "node", args: ["server.js"], env: { API_KEY: "abc" }, + cwd: "/srv/app", }, }, }); @@ -37,6 +38,7 @@ describe("parseMcpServersConfig", () => { expect(entry.type).toBe("stdio"); if (entry.type === "stdio") { expect(entry.env).toEqual({ API_KEY: "abc" }); + expect(entry.cwd).toBe("/srv/app"); } }); diff --git a/clients/web/src/test/integration/mcp/import/serverJson.test.ts b/clients/web/src/test/integration/mcp/import/serverJson.test.ts index b1f335f2..b7bf4acf 100644 --- a/clients/web/src/test/integration/mcp/import/serverJson.test.ts +++ b/clients/web/src/test/integration/mcp/import/serverJson.test.ts @@ -64,6 +64,9 @@ describe("parseServerJson — packages", () => { // surfaced for the user to fill. if (opt.baseConfig.type === "stdio") { expect(opt.baseConfig.env).toEqual({ LOG_LEVEL: "info" }); + // The registry server.json schema carries no working-directory concept, + // so a parsed package never sets cwd (documented in serverJson.ts). + expect(opt.baseConfig.cwd).toBeUndefined(); } expect(opt.envVars).toEqual([ { diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts index b369e556..a92ee91a 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts @@ -318,6 +318,7 @@ describe("InspectorClient", () => { environment: { transport: fakeFactory }, serverSettings: { headers: [], + env: [], metadata: [], connectionTimeout: 50, requestTimeout: 0, @@ -660,6 +661,7 @@ describe("InspectorClient", () => { it("getServerSettings returns the constructor settings; setServerSettings replaces them live (#1444)", () => { const initial = { headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, 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 c5237e7f..f887149d 100644 --- a/clients/web/src/test/integration/mcp/node/transport.test.ts +++ b/clients/web/src/test/integration/mcp/node/transport.test.ts @@ -213,6 +213,7 @@ describe("Transport", () => { { key: "X-Trace", value: "abc123" }, { key: "", value: "ignored-empty-key" }, ], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, @@ -267,6 +268,7 @@ describe("Transport", () => { const config: MCPServerConfig = { type: "sse", url: server.url }; const settings: InspectorServerSettings = { headers: [{ key: "X-Tenant", value: "acme" }], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, @@ -311,6 +313,7 @@ describe("Transport", () => { }; const settings: InspectorServerSettings = { headers: [], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, 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 83435b12..ca67a721 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 @@ -1233,6 +1233,282 @@ describe("/api/servers routes", () => { }); }); + describe("stdio env / cwd write-through (settings modal)", () => { + it("writes settings.env / settings.cwd onto a stdio config on a settings-only PUT", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { srv: { type: "stdio", command: "node" } }, + }), + ); + const res = await fetch(`${h.baseUrl}/api/servers/srv`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + env: [{ key: "API_KEY", value: "abc-123" }], + cwd: "/srv/app", + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers.srv as { + env?: Record; + cwd?: string; + }; + // env key persisted on the config (value stripped to keychain), cwd set. + expect(stored.env).toEqual({ API_KEY: "" }); + expect(stored.cwd).toBe("/srv/app"); + expect(await h.secretStore.get("srv", envSecretField("API_KEY"))).toBe( + "abc-123", + ); + // env / cwd are NOT persisted as a settings wrapper — they live on config. + expect(stored).not.toHaveProperty("settings"); + }); + + it("preserves config.env / cwd and the keychain when a settings-only PUT omits them", async () => { + // An env-unaware caller patching just a timeout must not wipe the stored + // env/cwd. Absent env/cwd on the wire => preserve, distinct from explicit + // empty => clear. + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + srv: { + type: "stdio", + command: "node", + env: { API_KEY: "" }, + cwd: "/srv/app", + }, + }, + }), + ); + await h.secretStore.set("srv", envSecretField("API_KEY"), "abc-123"); + const res = await fetch(`${h.baseUrl}/api/servers/srv`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + // env / cwd intentionally omitted; only a timeout is patched. + body: JSON.stringify({ + settings: { + headers: [], + metadata: [], + connectionTimeout: 5000, + requestTimeout: 0, + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers.srv as { + env?: Record; + cwd?: string; + }; + // Disk keys + cwd preserved (env value blanked, as always — it lives in + // the keychain). + expect(stored.env).toEqual({ API_KEY: "" }); + expect(stored.cwd).toBe("/srv/app"); + // Keychain value survives: the omit path rehydrates it so the reconcile + // doesn't sweep the untouched secret. + expect(await h.secretStore.get("srv", envSecretField("API_KEY"))).toBe( + "abc-123", + ); + }); + + it("GET lifts the stored env / cwd back into the wire config (round-trip)", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + srv: { + type: "stdio", + command: "node", + env: { API_KEY: "" }, + cwd: "/srv/app", + }, + }, + }), + ); + await h.secretStore.set("srv", envSecretField("API_KEY"), "abc-123"); + const res = await fetch(`${h.baseUrl}/api/servers`); + const body = (await res.json()) as MCPConfig; + const srv = body.mcpServers.srv as { + env?: Record; + cwd?: string; + }; + expect(srv.env).toEqual({ API_KEY: "abc-123" }); + expect(srv.cwd).toBe("/srv/app"); + }); + + it("clears env / cwd from the config when the settings-only PUT sends an empty list / blank cwd", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + srv: { + type: "stdio", + command: "node", + env: { API_KEY: "" }, + cwd: "/srv/app", + }, + }, + }), + ); + await h.secretStore.set("srv", envSecretField("API_KEY"), "abc-123"); + const res = await fetch(`${h.baseUrl}/api/servers/srv`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + env: [], + cwd: "", + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers + .srv as unknown as Record; + expect(stored).not.toHaveProperty("env"); + expect(stored).not.toHaveProperty("cwd"); + // The removed env key reconciles out of the keychain. + expect(await h.secretStore.get("srv", envSecretField("API_KEY"))).toBe( + null, + ); + }); + + it("drops empty-key env rows on a settings-only PUT", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { srv: { type: "stdio", command: "node" } }, + }), + ); + const res = await fetch(`${h.baseUrl}/api/servers/srv`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + env: [ + { key: "KEEP", value: "1" }, + { key: "", value: "drop-me" }, + { key: " ", value: "drop-me-too" }, + ], + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers.srv as { + env?: Record; + }; + expect(Object.keys(stored.env ?? {})).toEqual(["KEEP"]); + }); + + it("a config-providing PUT owns env/cwd; the preserved settings mirror does not revert it", async () => { + // Pre-seed a stdio server with env A=1 (keychain-backed). + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + srv: { + type: "stdio", + command: "node", + env: { A: "" }, + cwd: "/old", + }, + }, + }), + ); + await h.secretStore.set("srv", envSecretField("A"), "1"); + // Config-only PUT (settings omitted → preserved). The new config's env/cwd + // must win even though the preserved settings mirror still carries the old. + const res = await fetch(`${h.baseUrl}/api/servers/srv`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + config: { + type: "stdio", + command: "node", + env: { A: "2" }, + cwd: "/new", + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers.srv as { + env?: Record; + cwd?: string; + }; + expect(stored.cwd).toBe("/new"); + expect(stored.env).toEqual({ A: "" }); + expect(await h.secretStore.get("srv", envSecretField("A"))).toBe("2"); + }); + + it("rejects a non-array settings.env with 400", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { srv: { type: "stdio", command: "node" } }, + }), + ); + const res = await fetch(`${h.baseUrl}/api/servers/srv`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + env: "nope", + }, + }), + }); + expect(res.status).toBe(400); + const body = (await res.json()) as { error?: string }; + expect(body.error).toMatch(/settings\.env/); + }); + + it("ignores settings env/cwd for a non-stdio server (no leak onto config)", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + srv: { type: "streamable-http", url: "https://x.test/mcp" }, + }, + }), + ); + const res = await fetch(`${h.baseUrl}/api/servers/srv`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + env: [{ key: "API_KEY", value: "abc" }], + cwd: "/srv/app", + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers + .srv as unknown as Record; + expect(stored).not.toHaveProperty("env"); + expect(stored).not.toHaveProperty("cwd"); + }); + }); + describe("concurrent mutations", () => { it("does not lose updates when many adds fire in parallel (write-lock)", async () => { // Without the in-process mutex, concurrent POSTs would all read the 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 06084555..1f8c2d44 100644 --- a/clients/web/src/test/integration/mcp/remote/transport.test.ts +++ b/clients/web/src/test/integration/mcp/remote/transport.test.ts @@ -407,6 +407,7 @@ describe("Remote transport e2e", () => { { key: "X-Tenant", value: "acme" }, { key: "X-Trace", value: "abc123" }, ], + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, diff --git a/core/mcp/import/serverJson.ts b/core/mcp/import/serverJson.ts index 3652cd43..a076b99c 100644 --- a/core/mcp/import/serverJson.ts +++ b/core/mcp/import/serverJson.ts @@ -228,6 +228,12 @@ function parsePackage(raw: Record): ServerJsonOption | null { ? [...runtimeArgs, "-y", runtime.ref, ...packageArgs] : [...runtimeArgs, runtime.ref, ...packageArgs]; + // No `cwd`: the registry `server.json` package schema (2025-12-11) carries no + // working-directory concept — its package fields are registryType, + // registryBaseUrl, identifier, version, fileSha256, runtimeHint, + // runtimeArguments, packageArguments, environmentVariables, transport. So + // there is nothing to map onto `config.cwd` here; the user can set one + // afterward via the Add/Edit or Server Settings modal. const baseConfig: StdioServerConfig = { type: "stdio", command: runtime.command, diff --git a/core/mcp/node/servers.ts b/core/mcp/node/servers.ts index 0ee89e20..10180a04 100644 --- a/core/mcp/node/servers.ts +++ b/core/mcp/node/servers.ts @@ -42,6 +42,7 @@ export function headersToServerSettings( } return { headers: Object.entries(headers).map(([key, value]) => ({ key, value })), + env: [], metadata: [], connectionTimeout: 0, requestTimeout: 0, diff --git a/core/mcp/remote/node/server.ts b/core/mcp/remote/node/server.ts index e5700f8f..85a410e7 100644 --- a/core/mcp/remote/node/server.ts +++ b/core/mcp/remote/node/server.ts @@ -35,10 +35,12 @@ import type { InspectorServerSettings, MCPConfig, MCPServerConfig, + StdioServerConfig, StoredMCPServer, } from "../../types.js"; import { DEFAULT_SEED_CONFIG, + envPairsToRecord, expectedSecretFields, extractSecretsFromStored, INSPECTOR_FIELD_KEYS, @@ -57,6 +59,7 @@ import { KeyringSecretStore, type SecretStore, } from "../../../auth/node/secret-store.js"; +import { envSecretField } from "../../../auth/secret-fields.js"; /** * Shape of the initial config returned by GET /api/config (defaults for client). @@ -1100,6 +1103,47 @@ export function createRemoteApp( return normalized; }; + // Write-through for the stdio `env` / `cwd` config fields edited via the + // Server Settings modal. They are NOT Inspector-extension fields (the + // transport reads them off `config`), so `inspectorSettingsToStoredFields` + // doesn't emit them. Instead, on a settings-only patch the settings mirror is + // authoritative: this maps `settings.env` / `settings.cwd` back onto the + // stored stdio config, including clearing (empty list / blank cwd → field + // removed) so a user can delete a value through the modal. + // + // Each field is only written when the caller actually *sent* it (`provided`). + // An absent field on the wire means "leave the stored value alone" — without + // this gate an `env`-unaware client doing a settings patch (e.g. just bumping + // a timeout) would silently wipe the server's `env`/`cwd`, since + // `validateSettings` coerces a missing `env` to `[]` (which otherwise reads as + // "clear"). The integrated web client always resends the full GET-rehydrated + // `env`, so it is unaffected either way; this protects the raw HTTP contract. + // No-op for non-stdio entries. Mutates `entry` in place. + const applyStdioSettingsToConfig = ( + entry: StoredMCPServer, + settings: InspectorServerSettings, + provided: { env: boolean; cwd: boolean }, + ): void => { + if (!(entry.type === "stdio" || entry.type === undefined)) return; + const stdio = entry as StdioServerConfig & StoredMCPServer; + if (provided.env) { + const env = envPairsToRecord(settings.env); + if (Object.keys(env).length > 0) { + stdio.env = env; + } else { + delete stdio.env; + } + } + if (provided.cwd) { + const cwd = settings.cwd?.trim(); + if (cwd) { + stdio.cwd = cwd; + } else { + delete stdio.cwd; + } + } + }; + // Structurally validates an InspectorServerSettings payload off the wire so // a malformed body can't persist to disk and crash the UI later (e.g. // `settings: []` or `settings: { headers: "oops" }`). Mirrors the lenient @@ -1108,7 +1152,16 @@ export function createRemoteApp( const validateSettings = ( raw: unknown, ): - | { ok: true; value: InspectorServerSettings } + | { + ok: true; + value: InspectorServerSettings; + // Whether the caller actually sent `env` / `cwd` (vs. them being absent + // and defaulted). The write-through uses this to preserve a stored + // `config.env`/`cwd` on a patch that omits the field, rather than + // treating "absent" as "clear". + envProvided: boolean; + cwdProvided: boolean; + } | { ok: false; error: string } => { if (raw === null || typeof raw !== "object" || Array.isArray(raw)) { return { ok: false, error: "settings must be an object" }; @@ -1136,6 +1189,19 @@ export function createRemoteApp( error: "settings.metadata must be an array of { key, value }", }; } + // env is optional on the wire (older clients / non-stdio servers won't send + // it); when present it must be an array of { key, value } like headers. + if (obj.env !== undefined && !isKvArray(obj.env)) { + return { + ok: false, + error: "settings.env must be an array of { key, value }", + }; + } + // cwd is optional; when present it must be a string. Empty coerces to absent + // below (matching the read side), which the write-through reads as "clear". + if (obj.cwd !== undefined && typeof obj.cwd !== "string") { + return { ok: false, error: "settings.cwd must be a string" }; + } if ( typeof obj.connectionTimeout !== "number" || obj.connectionTimeout < 0 @@ -1213,6 +1279,9 @@ export function createRemoteApp( // later be misread as "OAuth configured." const value: InspectorServerSettings = { headers: obj.headers as { key: string; value: string }[], + // Absent → empty list, matching the read side. The write-through drops + // empty-key rows and clears `config.env` when the list is empty. + env: isKvArray(obj.env) ? obj.env : [], metadata: obj.metadata as { key: string; value: string }[], connectionTimeout: obj.connectionTimeout as number, requestTimeout: obj.requestTimeout as number, @@ -1250,7 +1319,18 @@ export function createRemoteApp( if (typeof obj.oauthScopes === "string" && obj.oauthScopes !== "") { value.oauthScopes = obj.oauthScopes; } - return { ok: true, value }; + // Empty cwd coerces to absent on the value (matching the read side); the + // write-through distinguishes "sent cwd: '' " (clear) from "cwd omitted" + // (preserve) via `cwdProvided` below rather than the value alone. + if (typeof obj.cwd === "string" && obj.cwd !== "") { + value.cwd = obj.cwd; + } + return { + ok: true, + value, + envProvided: obj.env !== undefined, + cwdProvided: obj.cwd !== undefined, + }; }; // In-process serialization for the read-modify-write flow on the @@ -1747,7 +1827,12 @@ export function createRemoteApp( type SettingsIntent = | { kind: "preserve" } | { kind: "clear" } - | { kind: "apply"; value: InspectorServerSettings }; + | { + kind: "apply"; + value: InspectorServerSettings; + envProvided: boolean; + cwdProvided: boolean; + }; let settingsIntent: SettingsIntent; if (body.settings === undefined) { settingsIntent = { kind: "preserve" }; @@ -1756,7 +1841,12 @@ export function createRemoteApp( } else { const validated = validateSettings(body.settings); if (!validated.ok) return c.json({ error: validated.error }, 400); - settingsIntent = { kind: "apply", value: validated.value }; + settingsIntent = { + kind: "apply", + value: validated.value, + envProvided: validated.envProvided, + cwdProvided: validated.cwdProvided, + }; } const newId = typeof body.id === "string" ? body.id : originalId; if (!validateStoreId(newId)) { @@ -1819,6 +1909,48 @@ export function createRemoteApp( // key dropped, OAuth secret cleared) gets cleaned up rather // than orphaned. const built = buildStoredEntry(newId, nextConfig, nextSettings); + // stdio env/cwd are config fields editable from both the Add/Edit modal + // (patches `config`) and the Server Settings modal (patches `settings`). + // On a settings-only *apply* (config preserved) the settings mirror is + // authoritative for the fields the caller actually sent — write those + // through onto the stored config so edits and explicit clears take + // effect, while a field the caller omitted leaves the stored value + // untouched. When a config body was provided it owns env/cwd and the + // settings mirror is ignored; a preserve/clear settings intent never + // touches config env/cwd. Runs before secret extraction so a removed env + // key reconciles out of the keychain. + if (body.config === undefined && settingsIntent.kind === "apply") { + applyStdioSettingsToConfig(built, settingsIntent.value, { + env: settingsIntent.envProvided, + cwd: settingsIntent.cwdProvided, + }); + // When the caller omitted `env`, the stored env is preserved + // structurally but its values are blanked on disk (the real values + // live in the keychain). Rehydrate those onto `built` so the secret + // extraction + reconcile below re-persist them, instead of treating + // the blanked keys as "removed" and sweeping them from the keychain. + // The integrated web client always resends the full GET-rehydrated + // env, so this only matters for env-unaware raw HTTP callers. + if ( + !settingsIntent.envProvided && + (built.type === "stdio" || built.type === undefined) + ) { + const stdio = built as StdioServerConfig & StoredMCPServer; + if (stdio.env && Object.keys(stdio.env).length > 0) { + const keys = Object.keys(stdio.env); + const existingSecrets = await readKeychainEntriesFor( + originalId, + keys.map((k) => envSecretField(k)), + ); + const rehydrated: Record = { ...stdio.env }; + for (const k of keys) { + const v = existingSecrets[envSecretField(k)]; + if (v !== undefined) rehydrated[k] = v; + } + stdio.env = rehydrated; + } + } + } const { stripped, secrets } = extractSecretsFromStored(built); const next: MCPConfig = { mcpServers: {} }; for (const [key, val] of Object.entries(current.mcpServers)) { diff --git a/core/mcp/serverList.ts b/core/mcp/serverList.ts index 4bd76692..93b350f5 100644 --- a/core/mcp/serverList.ts +++ b/core/mcp/serverList.ts @@ -98,6 +98,36 @@ type StoredInspectorFields = Pick< | "roots" >; +/** + * Convert a stored stdio `env` record into the controlled key/value rows the + * settings form edits, preserving the object's key insertion order. Empty when + * absent. Inverse of `envPairsToRecord`. + */ +export function envRecordToPairs( + env: Record | undefined, +): { key: string; value: string }[] { + return env + ? Object.entries(env).map(([key, value]) => ({ key, value })) + : []; +} + +/** + * Collapse the form's controlled `env` rows back into a `Record`, dropping rows + * with an empty/whitespace key (the form lets users leave a new row blank + * mid-edit). Inverse of `envRecordToPairs`. Used by the `/api/servers` PUT + * write-through that maps `settings.env` back onto `config.env`. + */ +export function envPairsToRecord( + pairs: { key: string; value: string }[], +): Record { + const out: Record = {}; + for (const { key, value } of pairs) { + if (key.trim() === "") continue; + out[key] = value; + } + return out; +} + /** * Lift the Inspector-extension fields off a freshly-read `StoredMCPServer` * into the pair-array / flat-OAuth `InspectorServerSettings` shape the form @@ -109,9 +139,20 @@ type StoredInspectorFields = Pick< * `oauth.*` becomes the flat `oauthClientId` / `oauthClientSecret` / * `oauthScopes` fields. Numeric timeouts default to 0 when absent — the form * needs concrete values to render and 0 is the SDK's "no timeout" signal. + * + * `env` / `cwd` are SDK config fields (not Inspector-extension keys), but they + * are mirrored into the settings here so the Server Settings modal can edit + * them for stdio servers. They are NOT re-emitted by + * `inspectorSettingsToStoredFields` — the write side lives in the PUT route's + * write-through (config stays the single on-disk owner). Their presence alone + * is enough to materialize a settings node so a bare `{ command, env }` entry + * surfaces its env in the form. */ export function storedFieldsToInspectorSettings( - stored: StoredInspectorFields, + stored: StoredInspectorFields & { + env?: Record; + cwd?: string; + }, ): InspectorServerSettings | undefined { const hasAny = stored.headers !== undefined || @@ -122,7 +163,9 @@ export function storedFieldsToInspectorSettings( stored.autoRefreshOnListChanged !== undefined || stored.maxFetchRequests !== undefined || stored.oauth !== undefined || - stored.roots !== undefined; + stored.roots !== undefined || + stored.env !== undefined || + stored.cwd !== undefined; if (!hasAny) return undefined; const headersPairs: { key: string; value: string }[] = stored.headers @@ -131,6 +174,7 @@ export function storedFieldsToInspectorSettings( const settings: InspectorServerSettings = { headers: headersPairs, + env: envRecordToPairs(stored.env), metadata: stored.metadata ?? [], connectionTimeout: stored.connectionTimeout ?? 0, requestTimeout: stored.requestTimeout ?? 0, @@ -155,6 +199,9 @@ export function storedFieldsToInspectorSettings( if (stored.oauth?.clientSecret) settings.oauthClientSecret = stored.oauth.clientSecret; if (stored.oauth?.scopes) settings.oauthScopes = stored.oauth.scopes; + // Mirror the stdio working directory for the form. Like the OAuth fields, an + // empty string coerces to absent so the form's "(inherit)" placeholder shows. + if (stored.cwd) settings.cwd = stored.cwd; return settings; } @@ -323,7 +370,20 @@ export function mcpConfigToServerEntries(config: MCPConfig): ServerEntry[] { config: normalizedConfig, connection: { status: "disconnected" }, }; - const settings = storedFieldsToInspectorSettings(inspectorFields); + // Mirror the stdio `env` / `cwd` (SDK config fields) into the settings so + // the Server Settings modal can edit them. They stay on `config` for the + // transport. Gate on the stdio type rather than blindly casting — a non- + // stdio config carries neither field, matching the modal's stdio-only UI. + const isStdio = + normalizedConfig.type === "stdio" || normalizedConfig.type === undefined; + const stdioConfig = isStdio + ? (normalizedConfig as StdioServerConfig) + : undefined; + const settings = storedFieldsToInspectorSettings({ + ...inspectorFields, + env: stdioConfig?.env, + cwd: stdioConfig?.cwd, + }); if (settings !== undefined) entry.settings = settings; return entry; }); diff --git a/core/mcp/types.ts b/core/mcp/types.ts index 7741812e..473c5b9a 100644 --- a/core/mcp/types.ts +++ b/core/mcp/types.ts @@ -422,6 +422,22 @@ export const DEFAULT_MAX_FETCH_REQUESTS = 1000; export interface InspectorServerSettings { headers: { key: string; value: string }[]; metadata: { key: string; value: string }[]; + /** + * Environment variables for stdio servers, edited as controlled key/value + * rows (mirrors `headers`). Only meaningful for stdio transports; non-stdio + * servers keep this an empty list. These do NOT live on disk as a settings + * field — they round-trip through the SDK config's `env` (the standard + * mcp.json location). The settings layer mirrors them for the form, and the + * `/api/servers` PUT route writes an edited list back onto `config.env` when + * the caller patches settings only. Empty-key rows are dropped on persist. + */ + env: { key: string; value: string }[]; + /** + * Working directory for stdio servers (`config.cwd`). Like `env`, this is a + * mirror of the SDK config field rather than a persisted settings field; + * empty/unset means "inherit". Only meaningful for stdio transports. + */ + cwd?: string; connectionTimeout: number; requestTimeout: number; /** TTL (ms) for tasks created via "Run as task". Defaults to 60000. */