From 6b105994c82d9e49316080a5a968e34716d0a6f5 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 25 Mar 2026 07:51:34 -0400 Subject: [PATCH] feat(site): persist MCP server selection in localStorage (#23572) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Previously the user's MCP server toggles were ephemeral — every page reload or navigation to a new chat reset them to the admin-configured defaults (`force_on` + `default_on`). This was frustrating for users who routinely disabled a default-on server or enabled a default-off one. This PR persists the MCP server picker selection to `localStorage` under the key `agents.selected-mcp-server-ids`. ## Changes ### `MCPServerPicker.tsx` - **`mcpSelectionStorageKey`** — exported constant for the localStorage key. - **`getSavedMCPSelection(servers)`** — reads from localStorage, filters out stale/disabled IDs, always includes `force_on` servers. - **`saveMCPSelection(ids)`** — writes the current selection to localStorage. ### `AgentCreateForm.tsx` - Initialises `userMCPServerIds` from `getSavedMCPSelection` instead of `null`. - Calls `saveMCPSelection` on every toggle. ### `AgentDetail.tsx` - Adds localStorage as a fallback tier in `effectiveMCPServerIds`: user override → chat record → **saved selection** → defaults. - Calls `saveMCPSelection` on every toggle. ### `MCPServerPicker.test.ts` (new) - 13 unit tests covering save, restore, stale-ID filtering, force_on merging, invalid JSON handling, and disabled server filtering. ## Fallback priority | Priority | Source | When | |----------|--------|------| | 1 | In-memory state | User toggled during this session | | 2 | Chat record | Existing conversation with `mcp_server_ids` | | 3 | localStorage | User has a saved selection from a prior session | | 4 | Server defaults | `force_on` + `default_on` servers | --- site/src/pages/AgentsPage/AgentDetail.tsx | 12 +- .../AgentsPage/components/AgentCreateForm.tsx | 23 ++- .../components/MCPServerPicker.test.ts | 135 ++++++++++++++++++ .../AgentsPage/components/MCPServerPicker.tsx | 56 ++++++++ 4 files changed, 221 insertions(+), 5 deletions(-) create mode 100644 site/src/pages/AgentsPage/components/MCPServerPicker.test.ts diff --git a/site/src/pages/AgentsPage/AgentDetail.tsx b/site/src/pages/AgentsPage/AgentDetail.tsx index c2a379e656..3af474aaff 100644 --- a/site/src/pages/AgentsPage/AgentDetail.tsx +++ b/site/src/pages/AgentsPage/AgentDetail.tsx @@ -51,7 +51,11 @@ import { AgentDetailNotFoundView, AgentDetailView, } from "./components/AgentDetailView"; -import { getDefaultMCPSelection } from "./components/MCPServerPicker"; +import { + getDefaultMCPSelection, + getSavedMCPSelection, + saveMCPSelection, +} from "./components/MCPServerPicker"; import { useGitWatcher } from "./hooks/useGitWatcher"; import { buildModelConfigIDByModelID, @@ -323,6 +327,7 @@ const AgentDetail: FC = () => { const handleMCPSelectionChange = (ids: string[]) => { setSelectedMCPServerIds(ids); + saveMCPSelection(ids); }; const handleMCPAuthComplete = (_serverId: string) => { @@ -411,6 +416,11 @@ const AgentDetail: FC = () => { if (chatRecord?.mcp_server_ids) { return chatRecord.mcp_server_ids; } + // Check for a previously saved selection in localStorage. + const saved = getSavedMCPSelection(mcpServers); + if (saved !== null) { + return saved; + } // Otherwise, compute defaults from server availability. return getDefaultMCPSelection(mcpServers); })(); diff --git a/site/src/pages/AgentsPage/components/AgentCreateForm.tsx b/site/src/pages/AgentsPage/components/AgentCreateForm.tsx index e7050c611b..8b547541f6 100644 --- a/site/src/pages/AgentsPage/components/AgentCreateForm.tsx +++ b/site/src/pages/AgentsPage/components/AgentCreateForm.tsx @@ -37,7 +37,11 @@ import { isUsageLimitData, } from "../utils/usageLimitMessage"; import { AgentChatInput } from "./AgentChatInput"; -import { getDefaultMCPSelection } from "./MCPServerPicker"; +import { + getDefaultMCPSelection, + getSavedMCPSelection, + saveMCPSelection, +} from "./MCPServerPicker"; /** @internal Exported for testing. */ export const emptyInputStorageKey = "agents.empty-input"; @@ -241,8 +245,16 @@ export const AgentCreateForm: FC = ({ const [userMCPServerIds, setUserMCPServerIds] = useState( null, ); - const effectiveMCPServerIds = - userMCPServerIds ?? getDefaultMCPSelection(mcpServers ?? []); + const effectiveMCPServerIds = (() => { + if (userMCPServerIds !== null) { + return userMCPServerIds; + } + const saved = getSavedMCPSelection(mcpServers ?? []); + if (saved !== null) { + return saved; + } + return getDefaultMCPSelection(mcpServers ?? []); + })(); const selectedMCPServerIdsRef = useRef(effectiveMCPServerIds); useEffect(() => { selectedWorkspaceIdRef.current = selectedWorkspaceId; @@ -374,7 +386,10 @@ export const AgentCreateForm: FC = ({ textContents={textContents} mcpServers={mcpServers} selectedMCPServerIds={effectiveMCPServerIds} - onMCPSelectionChange={setUserMCPServerIds} + onMCPSelectionChange={(ids) => { + setUserMCPServerIds(ids); + saveMCPSelection(ids); + }} onMCPAuthComplete={onMCPAuthComplete} leftActions={ & { id: string }, +): MCPServerConfig => + ({ + id: overrides.id, + display_name: overrides.display_name ?? overrides.id, + enabled: overrides.enabled ?? true, + availability: overrides.availability ?? "default_on", + auth_type: overrides.auth_type ?? "none", + auth_connected: overrides.auth_connected ?? false, + icon_url: overrides.icon_url ?? "", + description: overrides.description ?? "", + url: overrides.url ?? "", + transport: overrides.transport ?? "sse", + }) as MCPServerConfig; + +describe("MCP selection persistence", () => { + beforeEach(() => { + localStorage.clear(); + }); + + describe("saveMCPSelection", () => { + it("writes a JSON array to localStorage", () => { + saveMCPSelection(["a", "b"]); + expect(localStorage.getItem(mcpSelectionStorageKey)).toBe( + JSON.stringify(["a", "b"]), + ); + }); + + it("writes an empty array when no servers are selected", () => { + saveMCPSelection([]); + expect(localStorage.getItem(mcpSelectionStorageKey)).toBe("[]"); + }); + }); + + describe("getSavedMCPSelection", () => { + const servers = [ + makeServer({ id: "s1", availability: "force_on" }), + makeServer({ id: "s2", availability: "default_on" }), + makeServer({ id: "s3", availability: "default_off" }), + ]; + + it("returns null when nothing is stored", () => { + expect(getSavedMCPSelection(servers)).toBeNull(); + }); + + it("returns null when the server list is empty", () => { + saveMCPSelection(["s1", "s2"]); + expect(getSavedMCPSelection([])).toBeNull(); + }); + + it("returns null for invalid JSON", () => { + localStorage.setItem(mcpSelectionStorageKey, "not-json"); + expect(getSavedMCPSelection(servers)).toBeNull(); + }); + + it("returns null when stored value is not an array", () => { + localStorage.setItem(mcpSelectionStorageKey, '"a string"'); + expect(getSavedMCPSelection(servers)).toBeNull(); + }); + + it("restores saved IDs that still exist as enabled servers", () => { + saveMCPSelection(["s2", "s3"]); + const result = getSavedMCPSelection(servers); + expect(result).toContain("s2"); + expect(result).toContain("s3"); + }); + + it("filters out IDs for servers that no longer exist", () => { + saveMCPSelection(["s2", "deleted-server"]); + const result = getSavedMCPSelection(servers); + expect(result).toContain("s2"); + expect(result).not.toContain("deleted-server"); + }); + + it("filters out IDs for disabled servers", () => { + const withDisabled = [ + ...servers, + makeServer({ id: "s4", enabled: false }), + ]; + saveMCPSelection(["s2", "s4"]); + const result = getSavedMCPSelection(withDisabled); + expect(result).toContain("s2"); + expect(result).not.toContain("s4"); + }); + + it("always includes force_on servers even if not in saved list", () => { + saveMCPSelection(["s3"]); + const result = getSavedMCPSelection(servers); + expect(result).toContain("s1"); + expect(result).toContain("s3"); + }); + + it("does not duplicate force_on servers already in saved list", () => { + saveMCPSelection(["s1", "s3"]); + const result = getSavedMCPSelection(servers)!; + const s1Count = result.filter((id) => id === "s1").length; + expect(s1Count).toBe(1); + }); + + it("returns an empty selection (plus force_on) when user opted out", () => { + saveMCPSelection([]); + const result = getSavedMCPSelection(servers); + // Only force_on should be present. + expect(result).toEqual(["s1"]); + }); + }); + + describe("getDefaultMCPSelection", () => { + it("includes force_on and default_on, excludes default_off", () => { + const servers = [ + makeServer({ id: "a", availability: "force_on" }), + makeServer({ id: "b", availability: "default_on" }), + makeServer({ id: "c", availability: "default_off" }), + ]; + expect(getDefaultMCPSelection(servers)).toEqual(["a", "b"]); + }); + + it("excludes disabled servers", () => { + const servers = [ + makeServer({ id: "a", availability: "default_on", enabled: false }), + ]; + expect(getDefaultMCPSelection(servers)).toEqual([]); + }); + }); +}); diff --git a/site/src/pages/AgentsPage/components/MCPServerPicker.tsx b/site/src/pages/AgentsPage/components/MCPServerPicker.tsx index 2a48057f0a..51b456ee90 100644 --- a/site/src/pages/AgentsPage/components/MCPServerPicker.tsx +++ b/site/src/pages/AgentsPage/components/MCPServerPicker.tsx @@ -87,6 +87,62 @@ export const getDefaultMCPSelection = ( .map((s) => s.id); }; +/** localStorage key for persisting the user's MCP server selection. */ +export const mcpSelectionStorageKey = "agents.selected-mcp-server-ids"; + +/** + * Read the persisted MCP selection from localStorage, filtered to only + * include IDs that still exist in the current server list. + * Returns `null` when nothing is stored (caller should fall back to defaults). + */ +export const getSavedMCPSelection = ( + servers: readonly TypesGen.MCPServerConfig[], +): string[] | null => { + const raw = localStorage.getItem(mcpSelectionStorageKey); + if (raw === null) { + return null; + } + // If the server list is empty (e.g. the query hasn't loaded yet), + // we can't validate any IDs so signal "unknown" rather than + // returning an empty array that would be mistaken for "user + // deliberately deselected everything". + if (servers.length === 0) { + return null; + } + try { + const parsed: unknown = JSON.parse(raw); + if (!Array.isArray(parsed)) { + return null; + } + const enabledIds = new Set( + servers.filter((s) => s.enabled).map((s) => s.id), + ); + // Always include force_on servers even if the user didn't save them. + const forceOnIds = servers + .filter((s) => s.enabled && s.availability === "force_on") + .map((s) => s.id); + const restored = parsed.filter( + (id): id is string => typeof id === "string" && enabledIds.has(id), + ); + // Merge force_on servers that might not be in the saved list. + for (const id of forceOnIds) { + if (!restored.includes(id)) { + restored.push(id); + } + } + return restored; + } catch { + return null; + } +}; + +/** + * Persist the current MCP selection to localStorage. + */ +export const saveMCPSelection = (ids: readonly string[]): void => { + localStorage.setItem(mcpSelectionStorageKey, JSON.stringify(ids)); +}; + // ── Overlapping icon stack for the trigger ───────────────────── const ICON_STACK_MAX = 3;