mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat(site): persist MCP server selection in localStorage (#23572)
## 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 |
This commit is contained in:
@@ -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);
|
||||
})();
|
||||
|
||||
@@ -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<AgentCreateFormProps> = ({
|
||||
const [userMCPServerIds, setUserMCPServerIds] = useState<string[] | null>(
|
||||
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<AgentCreateFormProps> = ({
|
||||
textContents={textContents}
|
||||
mcpServers={mcpServers}
|
||||
selectedMCPServerIds={effectiveMCPServerIds}
|
||||
onMCPSelectionChange={setUserMCPServerIds}
|
||||
onMCPSelectionChange={(ids) => {
|
||||
setUserMCPServerIds(ids);
|
||||
saveMCPSelection(ids);
|
||||
}}
|
||||
onMCPAuthComplete={onMCPAuthComplete}
|
||||
leftActions={
|
||||
<Popover
|
||||
|
||||
@@ -0,0 +1,135 @@
|
||||
import type { MCPServerConfig } from "api/typesGenerated";
|
||||
import { beforeEach, describe, expect, it } from "vitest";
|
||||
import {
|
||||
getDefaultMCPSelection,
|
||||
getSavedMCPSelection,
|
||||
mcpSelectionStorageKey,
|
||||
saveMCPSelection,
|
||||
} from "./MCPServerPicker";
|
||||
|
||||
const makeServer = (
|
||||
overrides: Partial<MCPServerConfig> & { 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([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user