diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx
index 1314a597..ad6a5a1e 100644
--- a/clients/web/src/App.test.tsx
+++ b/clients/web/src/App.test.tsx
@@ -3283,6 +3283,94 @@ describe("App paginated list pagination toggle (#1721)", () => {
expect(fetchLogOptions?.maxFetchRequests).toBe(42);
});
+ it("carries a landed stdio env/cwd onto the config it connects with (#2096)", async () => {
+ // The sibling case above covers the client *options*, which are derived
+ // from the settings. `env` and `cwd` are the two fields the modal edits as
+ // settings while the transport reads them off the **config** — and the
+ // config came straight off the same frozen `servers` entry. So a save that
+ // landed while list reads were failing kept spawning the child process with
+ // the pre-save environment, while the modal (re-seeded from the tracker
+ // since #2089) showed the new one. Credentials live here, so the symptom is
+ // the inspected server failing to authorize rather than a lost edit.
+ const user = userEvent.setup();
+ renderWithMantine();
+
+ const draftOptions = vi.mocked(useSettingsDraft).mock.calls.at(-1)?.[0];
+ if (!draftOptions) throw new Error("useSettingsDraft was never called");
+ await act(async () => {
+ await draftOptions.onPersist("A", {
+ ...settingsWithRoots([]),
+ env: [{ key: "FOO", value: "after" }],
+ cwd: "/tmp/after",
+ });
+ });
+
+ // `servers` still reports `{ type: "stdio", command: "node" }` with no
+ // settings node at all — what a failed reload leaves behind.
+ await user.click(screen.getByText("connect"));
+ await waitFor(() => expect(clientInstances).toHaveLength(1));
+ expect(vi.mocked(McpIndex.InspectorClient).mock.calls.at(-1)?.[0]).toEqual({
+ type: "stdio",
+ command: "node",
+ env: { FOO: "after" },
+ cwd: "/tmp/after",
+ });
+ });
+
+ it("clears a stdio env/cwd the last write removed (#2096)", async () => {
+ // Clearing has to travel too, and it is the direction a naive merge gets
+ // wrong: an empty env list and a blank cwd mean "remove the field", which
+ // is how the modal deletes a value. Spreading the settings over the config
+ // would leave the stale one in place instead.
+ const user = userEvent.setup();
+ const previousUseServers = vi.mocked(useServers).getMockImplementation();
+ try {
+ vi.mocked(useServers).mockReturnValue({
+ servers: [
+ {
+ ...SERVER_A,
+ config: {
+ type: "stdio",
+ command: "node",
+ env: { FOO: "before" },
+ cwd: "/tmp/before",
+ },
+ } as ServerEntry,
+ ],
+ loading: false,
+ error: undefined,
+ refresh: vi.fn().mockResolvedValue(undefined),
+ addServer: vi.fn(),
+ updateServer: vi.fn(),
+ updateServerSettings: updateServerSettingsSpy,
+ removeServer: vi.fn(),
+ reorderServers: vi.fn(),
+ importSource: vi.fn().mockResolvedValue({ servers: {} }),
+ });
+ renderWithMantine();
+
+ const draftOptions = vi.mocked(useSettingsDraft).mock.calls.at(-1)?.[0];
+ if (!draftOptions) throw new Error("useSettingsDraft was never called");
+ await act(async () => {
+ await draftOptions.onPersist("A", {
+ ...settingsWithRoots([]),
+ env: [],
+ cwd: "",
+ });
+ });
+
+ await user.click(screen.getByText("connect"));
+ await waitFor(() => expect(clientInstances).toHaveLength(1));
+ expect(
+ vi.mocked(McpIndex.InspectorClient).mock.calls.at(-1)?.[0],
+ ).toEqual({ type: "stdio", command: "node" });
+ } finally {
+ if (previousUseServers) {
+ vi.mocked(useServers).mockImplementation(previousUseServers);
+ }
+ }
+ });
+
it("lets a successful list read supersede a concrete rollback override (#2089)", async () => {
// A rollback sets the override to a value rather than clearing it, so the
// effect that drops it cannot key on the persisted boolean alone: if a
diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx
index 45b7f9b3..4c8280f4 100644
--- a/clients/web/src/App.tsx
+++ b/clients/web/src/App.tsx
@@ -70,6 +70,7 @@ import { ManagedResourceTemplatesState } from "@inspector/core/mcp/state/managed
import { ManagedRequestorTasksState } from "@inspector/core/mcp/state/managedRequestorTasksState.js";
import { ResourceSubscriptionsState } from "@inspector/core/mcp/state/resourceSubscriptionsState.js";
import {
+ applyStdioSettingsToConfig,
cleanRoots,
oauthAuthorizationParamsFromSettings,
oauthEndpointOverridesFromSettings,
@@ -2656,7 +2657,19 @@ function App() {
...(activeCimdUrl && { clientMetadataUrl: activeCimdUrl }),
}
: undefined;
- const client = new InspectorClient(server.config, {
+ // The stdio `env` / `cwd` are edited as *settings* but stored on — and
+ // read by the transport from — *config*, so the tracker's account of the
+ // former has to be carried onto the latter. `server.config` comes off the
+ // same frozen `servers` entry `server.settings` does, so without this a
+ // save that landed while list reads were failing spawns the child process
+ // with the pre-save environment while the modal, re-seeded from the
+ // tracker, shows the new one (#2096). Same mapping the PUT route applies
+ // when persisting it, from the same helper.
+ const effectiveConfig = applyStdioSettingsToConfig(
+ server.config,
+ savedSettings,
+ );
+ const client = new InspectorClient(effectiveConfig, {
environment,
// The Tasks tab needs the receiver-task pipeline; the
// requestor-task list comes from the client's task store.
diff --git a/clients/web/src/test/core/mcp/serverList.test.ts b/clients/web/src/test/core/mcp/serverList.test.ts
index de7b53e0..8f8ca44e 100644
--- a/clients/web/src/test/core/mcp/serverList.test.ts
+++ b/clients/web/src/test/core/mcp/serverList.test.ts
@@ -12,12 +12,18 @@ import {
mergeSecretsIntoStored,
normalizeServerType,
normalizeStoredMetadata,
+ applyStdioSettingsToConfig,
+ stdioConfigFieldsFromSettings,
oauthAuthorizationParamsFromSettings,
oauthEndpointOverridesFromSettings,
serverEntriesToMcpConfig,
serializeMcpConfig,
storedFieldsToInspectorSettings,
} from "@inspector/core/mcp/serverList.js";
+import type {
+ InspectorServerSettings,
+ MCPServerConfig,
+} from "@inspector/core/mcp/types.js";
import {
SECRET_FIELD_OAUTH_CLIENT_SECRET,
envSecretField,
@@ -1833,3 +1839,122 @@ describe("cleanAuthorizationParams", () => {
]);
});
});
+
+describe("stdio env/cwd settings → config mapping (#2096)", () => {
+ // `env` and `cwd` are edited as settings but read by the transport off the
+ // config, so both sides of the wire have to derive one from the other. This
+ // is the single mapping they share — the PUT write-through persists it, the
+ // web client applies it when constructing an InspectorClient.
+ const settings = (
+ over: Partial = {},
+ ): InspectorServerSettings => ({
+ headers: [],
+ env: [],
+ metadata: {},
+ connectionTimeout: 0,
+ requestTimeout: 0,
+ taskTtl: 60000,
+ maxFetchRequests: 1000,
+ roots: [],
+ ...over,
+ });
+
+ describe("stdioConfigFieldsFromSettings", () => {
+ it("collapses the pair rows into a record and trims the cwd", () => {
+ expect(
+ stdioConfigFieldsFromSettings(
+ settings({
+ env: [
+ { key: "FOO", value: "after" },
+ { key: "BAR", value: "" },
+ ],
+ cwd: " /tmp/after ",
+ }),
+ ),
+ ).toEqual({ env: { FOO: "after", BAR: "" }, cwd: "/tmp/after" });
+ });
+
+ it("reports an empty list and a blank cwd as unset, i.e. cleared", () => {
+ // The modal deletes a value by emptying it, so "no rows" cannot mean
+ // "leave it alone" — it has to remove the field.
+ expect(
+ stdioConfigFieldsFromSettings(settings({ env: [], cwd: " " })),
+ ).toEqual({ env: undefined, cwd: undefined });
+ // A row the user started and left blank is dropped, so it does not
+ // materialize an env consisting of one nameless variable.
+ expect(
+ stdioConfigFieldsFromSettings(
+ settings({ env: [{ key: " ", value: "x" }] }),
+ ).env,
+ ).toBeUndefined();
+ });
+ });
+
+ describe("applyStdioSettingsToConfig", () => {
+ const stdio: MCPServerConfig = {
+ type: "stdio",
+ command: "node",
+ args: ["server.js"],
+ env: { FOO: "before" },
+ cwd: "/tmp/before",
+ };
+
+ it("overwrites env/cwd and leaves the rest of the config alone", () => {
+ expect(
+ applyStdioSettingsToConfig(
+ stdio,
+ settings({
+ env: [{ key: "FOO", value: "after" }],
+ cwd: "/tmp/after",
+ }),
+ ),
+ ).toEqual({
+ type: "stdio",
+ command: "node",
+ args: ["server.js"],
+ env: { FOO: "after" },
+ cwd: "/tmp/after",
+ });
+ });
+
+ it("removes the fields the settings cleared", () => {
+ expect(applyStdioSettingsToConfig(stdio, settings())).toEqual({
+ type: "stdio",
+ command: "node",
+ args: ["server.js"],
+ });
+ });
+
+ it("does not mutate the config it was given", () => {
+ const before = { ...stdio };
+ applyStdioSettingsToConfig(stdio, settings());
+ expect(stdio).toEqual(before);
+ });
+
+ it("treats a typeless entry as stdio, matching the on-disk default", () => {
+ // An `mcp.json` entry written without a `type` key is stdio (the Claude
+ // Desktop convention), and the modal edits its env like any other.
+ const applied = applyStdioSettingsToConfig(
+ { command: "node" } as MCPServerConfig,
+ settings({ env: [{ key: "FOO", value: "after" }] }),
+ );
+ expect(applied).toEqual({ command: "node", env: { FOO: "after" } });
+ });
+
+ it("returns a non-stdio config untouched", () => {
+ // An HTTP server carries neither field, matching the modal's stdio-only
+ // UI — applying an empty mirror to it must not invent one.
+ const http: MCPServerConfig = {
+ type: "streamable-http",
+ url: "https://mcp.example.com/mcp",
+ };
+ expect(applyStdioSettingsToConfig(http, settings())).toBe(http);
+ });
+
+ it("returns the config untouched when there are no settings", () => {
+ // A server with no settings node has nothing to apply; treating that as
+ // an empty mirror would clear an env the file does hold.
+ expect(applyStdioSettingsToConfig(stdio, undefined)).toBe(stdio);
+ });
+ });
+});
diff --git a/core/mcp/remote/node/server.ts b/core/mcp/remote/node/server.ts
index 2c6d4fa3..33d57e6d 100644
--- a/core/mcp/remote/node/server.ts
+++ b/core/mcp/remote/node/server.ts
@@ -51,7 +51,6 @@ import type {
} from "../../types.js";
import {
DEFAULT_SEED_CONFIG,
- envPairsToRecord,
expectedSecretFields,
extractSecretsFromStored,
INSPECTOR_FIELD_KEYS,
@@ -59,6 +58,7 @@ import {
isProtocolEra,
mergeSecretsIntoStored,
normalizeServerType,
+ stdioConfigFieldsFromSettings,
storedFieldsToInspectorSettings,
stripInspectorFields,
} from "../../serverList.js";
@@ -1662,16 +1662,18 @@ export function createRemoteApp(
): void => {
if (!(entry.type === "stdio" || entry.type === undefined)) return;
const stdio = entry as StdioServerConfig & StoredMCPServer;
+ // Shared with the web client's connect-time application of the same
+ // mapping (#2096), so the environment a save persists and the one the
+ // child process is spawned with cannot be derived differently.
+ const { env, cwd } = stdioConfigFieldsFromSettings(settings);
if (provided.env) {
- const env = envPairsToRecord(settings.env);
- if (Object.keys(env).length > 0) {
+ if (env) {
stdio.env = env;
} else {
delete stdio.env;
}
}
if (provided.cwd) {
- const cwd = settings.cwd?.trim();
if (cwd) {
stdio.cwd = cwd;
} else {
diff --git a/core/mcp/serverList.ts b/core/mcp/serverList.ts
index 00da60b9..d7937da9 100644
--- a/core/mcp/serverList.ts
+++ b/core/mcp/serverList.ts
@@ -197,6 +197,62 @@ export function envPairsToRecord(
return out;
}
+/**
+ * The stdio `env` / `cwd` **config** fields implied by a settings mirror.
+ * `undefined` means "the field is not set" — i.e. remove it — which is how a
+ * user clears a value through the Server Settings modal (empty env list, blank
+ * cwd).
+ *
+ * `env` and `cwd` are the two fields the modal edits as *settings* while the
+ * transport reads them off *config* (see `storedFieldsToInspectorSettings`), so
+ * every consumer that has settings and needs a config has to perform this same
+ * mapping. It lives here, once, because the two consumers are on opposite sides
+ * of the wire — the `/api/servers` PUT write-through persists it, and the web
+ * client applies it when constructing an `InspectorClient` (#2096) — and a
+ * disagreement between them is invisible until a spawned child process gets an
+ * environment the file does not describe.
+ */
+export function stdioConfigFieldsFromSettings(
+ settings: InspectorServerSettings,
+): { env: Record | undefined; cwd: string | undefined } {
+ const env = envPairsToRecord(settings.env);
+ const cwd = settings.cwd?.trim();
+ return {
+ env: Object.keys(env).length > 0 ? env : undefined,
+ cwd: cwd ? cwd : undefined,
+ };
+}
+
+/**
+ * A copy of `config` with the stdio `env` / `cwd` taken from `settings`.
+ *
+ * Used where a caller holds a settings value it trusts more than the config it
+ * was read alongside — the web client's connect path, which resolves settings
+ * through the last-persisted-write tracker while `config` still comes off a
+ * `servers` entry that stops advancing once a list read fails (#2089). Without
+ * this the two disagree and the child process is spawned with the *pre-save*
+ * environment even though the save reached disk (#2096).
+ *
+ * Non-stdio configs are returned untouched — they carry neither field, matching
+ * the modal's stdio-only UI. Absent settings likewise: a server with no settings
+ * node has nothing to apply, and treating that as an empty mirror would clear a
+ * config `env` the file does hold.
+ */
+export function applyStdioSettingsToConfig(
+ config: MCPServerConfig,
+ settings: InspectorServerSettings | undefined,
+): MCPServerConfig {
+ if (!settings) return config;
+ if (!(config.type === "stdio" || config.type === undefined)) return config;
+ const { env, cwd } = stdioConfigFieldsFromSettings(settings);
+ const next: StdioServerConfig = { ...(config as StdioServerConfig) };
+ if (env) next.env = env;
+ else delete next.env;
+ if (cwd) next.cwd = cwd;
+ else delete next.cwd;
+ return next;
+}
+
/**
* Read a server's `metadata` off disk into the in-memory `RequestMetadata`
* object, accepting both the current object shape and the pre-#1910