From d85c728e347eb061a258b7fca3546cb88b2a6032 Mon Sep 17 00:00:00 2001 From: bymarvin Date: Mon, 26 Oct 2026 22:29:06 +0100 Subject: [PATCH 1/2] Grant an MCP server to several bots from its edit dialog (#239) The edit dialog lists the bots with a checkbox each: ticked when a bot has all of the server's tools, partly when it has some. Saving grants or clears the server's current tools for the bots that changed, the same way the bot editor's server checkbox does. --- frontend/src/lib/toolGroups.test.ts | 9 +++- frontend/src/lib/toolGroups.ts | 11 +++-- frontend/src/pages/SettingsPage.test.tsx | 53 +++++++++++++++++++++++- frontend/src/pages/SettingsPage.tsx | 44 ++++++++++++++++++-- 4 files changed, 107 insertions(+), 10 deletions(-) diff --git a/frontend/src/lib/toolGroups.test.ts b/frontend/src/lib/toolGroups.test.ts index e1b2af31..62bf4ba8 100644 --- a/frontend/src/lib/toolGroups.test.ts +++ b/frontend/src/lib/toolGroups.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { ToolInfo } from "../api/types"; -import { groupState, groupTools, toggleGroup, toolLabel, unavailableGrants } from "./toolGroups"; +import { groupState, groupTools, setGroup, toggleGroup, toolLabel, unavailableGrants } from "./toolGroups"; const tool = (name: string, source: string): ToolInfo => ({ name, description: name, source, args_schema: {} }); const tools = [ @@ -36,6 +36,13 @@ describe("toggleGroup", () => { expect(toggleGroup({ tool_names: ["run_shell", ...names], approval_tools: ["Linear__create_issue", "run_shell"] }, names)) .toEqual({ tool_names: ["run_shell"], approval_tools: ["run_shell"] }); }); + + it("setGroup grants or clears the server's tools whatever the current state", () => { + const names = ["Linear__create_issue", "Linear__list_issues"]; + const some = { tool_names: ["run_shell", "Linear__create_issue"], approval_tools: ["Linear__create_issue"] }; + expect(setGroup(some, names, true)).toEqual({ tool_names: ["run_shell", ...names], approval_tools: ["Linear__create_issue"] }); + expect(setGroup(some, names, false)).toEqual({ tool_names: ["run_shell"], approval_tools: [] }); + }); }); describe("unavailableGrants and toolLabel", () => { diff --git a/frontend/src/lib/toolGroups.ts b/frontend/src/lib/toolGroups.ts index c3758f3d..6fa5c7de 100644 --- a/frontend/src/lib/toolGroups.ts +++ b/frontend/src/lib/toolGroups.ts @@ -36,11 +36,14 @@ export function groupState(names: string[], selected: string[]): GroupState { interface Selection { tool_names: string[]; approval_tools: string[] } -/** Group checkbox: select every tool of the server unless all already are, in which case clear them. - * Approvals for cleared tools are dropped, since approval_tools must stay a subset of tool_names. */ -export function toggleGroup(sel: Selection, names: string[]): Selection { - if (groupState(names, sel.tool_names) === "all") { +/** Grant all of `names`, or clear them. Approvals for cleared tools are dropped, since approval_tools must + * stay a subset of tool_names. */ +export function setGroup(sel: Selection, names: string[], on: boolean): Selection { + if (!on) { return { tool_names: sel.tool_names.filter((n) => !names.includes(n)), approval_tools: sel.approval_tools.filter((n) => !names.includes(n)) }; } return { tool_names: [...sel.tool_names, ...names.filter((n) => !sel.tool_names.includes(n))], approval_tools: sel.approval_tools }; } + +/** Group checkbox: select every tool of the server unless all already are, in which case clear them. */ +export const toggleGroup = (sel: Selection, names: string[]): Selection => setGroup(sel, names, groupState(names, sel.tool_names) !== "all"); diff --git a/frontend/src/pages/SettingsPage.test.tsx b/frontend/src/pages/SettingsPage.test.tsx index e0b8450f..5fb8a6d9 100644 --- a/frontend/src/pages/SettingsPage.test.tsx +++ b/frontend/src/pages/SettingsPage.test.tsx @@ -5,7 +5,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { MemoryRouter } from "react-router-dom"; import { afterEach, beforeEach, expect, it, vi } from "vitest"; import { Api } from "../api/client"; -import type { AppSetting } from "../api/types"; +import type { AppSetting, Bot, McpServer } from "../api/types"; import SettingsPage from "./SettingsPage"; (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; @@ -64,3 +64,54 @@ it("renders the default bot model as an OpenRouter catalog picker and saves the await until(() => patch.mock.calls.length > 0); expect(patch.mock.calls[0][0]).toEqual({ bot_model: "z-ai/glm-5.3-flash" }); }); + +const mcpServer = (tools: string[]): McpServer => ({ name: "lin", transport: "http", status: tools.length ? "connected" : "disconnected", enabled: true, oauth: false, + url: "https://mcp.example.com/mcp", error: null, tools, source: "db", authorization_url: null, command: null, args: [], cwd: null, env: {}, headers: {} }); +const bot = (id: string, tool_names: string[], approval_tools: string[] = []): Bot => ({ id, handle: id, name: `Bot ${id}`, description: "", icon: "bot", enabled: true, active: false, + instructions: "", provider: "auto", model: "", model_settings: {}, tool_names, approval_tools, skill_names: [], memory_enabled: true, created_at: "", updated_at: "" }); +const grantBox = (handle: string) => document.body.querySelector(`input[aria-label="Grant lin to @${handle}"]`); +const dialogButton = (label: string) => [...document.getElementById("mcp-dialog-title")!.closest("form")!.querySelectorAll("button")].find((b) => b.textContent === label)!; +async function editServer() { + await act(async () => root.render()); + await until(() => [...el.querySelectorAll("button")].some((b) => b.textContent === "Edit")); + await act(async () => { [...el.querySelectorAll("button")].find((b) => b.textContent === "Edit")!.click(); }); +} + +it("grants or clears a server's tools for several bots from its edit dialog", async () => { + const tools = ["lin__create_issue", "lin__list_issues"]; + vi.mocked(Api.listMcpServers).mockResolvedValue([mcpServer(tools)]); + vi.spyOn(Api, "listBots").mockResolvedValue([ + bot("all", ["run_shell", ...tools], ["lin__create_issue"]), bot("some", ["lin__list_issues"]), bot("none", ["run_shell"]), + ]); + const updateBot = vi.spyOn(Api, "updateBot").mockImplementation(async (id) => bot(id, [])); + const updateServer = vi.spyOn(Api, "updateMcpServer").mockResolvedValue(mcpServer(tools)); + await editServer(); + await until(() => grantBox("none") !== null); + expect([grantBox("all")!.checked, grantBox("some")!.checked, grantBox("some")!.indeterminate, grantBox("none")!.checked]).toEqual([true, false, true, false]); + expect(document.body.textContent).toContain("1 of 2"); + + await act(async () => { grantBox("all")!.click(); }); + await act(async () => { grantBox("some")!.click(); }); + expect([grantBox("all")!.checked, grantBox("some")!.checked]).toEqual([false, true]); + await act(async () => { dialogButton("Save").click(); }); + await until(() => updateServer.mock.calls.length > 0); + expect(updateBot.mock.calls).toEqual([ + ["all", { tool_names: ["run_shell"], approval_tools: [] }], + ["some", { tool_names: ["lin__list_issues", "lin__create_issue"], approval_tools: [] }], + ]); +}); + +it("asks to connect a server without tools before granting it to bots, and lists no bots when adding one", async () => { + vi.mocked(Api.listMcpServers).mockResolvedValue([mcpServer([])]); + const listBots = vi.spyOn(Api, "listBots").mockResolvedValue([bot("b1", [])]); + await editServer(); + await until(() => document.body.textContent!.includes("Connect the server to grant its tools to bots here.")); + expect(grantBox("b1")).toBeNull(); + + listBots.mockClear(); + await act(async () => { dialogButton("Cancel").click(); }); + await act(async () => { [...el.querySelectorAll("button")].find((b) => b.textContent === "Add server")!.click(); }); + await until(() => document.body.textContent!.includes("Add MCP server")); + expect(document.body.textContent).not.toContain("Bots with this server's tools"); + expect(listBots).not.toHaveBeenCalled(); +}); diff --git a/frontend/src/pages/SettingsPage.tsx b/frontend/src/pages/SettingsPage.tsx index 365194ba..de935c1b 100644 --- a/frontend/src/pages/SettingsPage.tsx +++ b/frontend/src/pages/SettingsPage.tsx @@ -8,7 +8,7 @@ import { dismissSaveNotification, notifySave, useSaveMutation } from "../lib/sav import { formatSettingValue, groupSettings, parseSettingInput, type SettingGroup } from "../lib/appSettings"; import { formatArgs, formatKeyValues, mcpActions, mcpInFlight, mcpStatusBadge, parseArgs, parseKeyValues, validateNewMcpServer, type McpTransport } from "../lib/mcpServers"; import { ChevronDownIcon, ChevronRightIcon } from "../components/icons"; -import { groupTools, toolLabel } from "../lib/toolGroups"; +import { groupState, groupTools, setGroup, toolLabel } from "../lib/toolGroups"; const emptyExt = { handle: "", name: "", webhook_url: "", webhook_secret: "" }; const Code = ({ children }: { children: React.ReactNode }) => {children}; @@ -297,7 +297,7 @@ function McpServersCard() { MCP servers - Tools from Model Context Protocol servers, remote (HTTPS, OAuth when the server asks) or local (a command run over stdio). Tools appear in the tool list as server__tool; grant them per bot in the bot editor, by server or one at a time. + Tools from Model Context Protocol servers, remote (HTTPS, OAuth when the server asks) or local (a command run over stdio). Tools appear in the tool list as server__tool; grant them per bot in the bot editor, by server or one at a time, or to several bots at once under Edit.
Browse MCP catalog
{dialog && setDialog(null)} onSaved={() => { setDialog(null); refresh(); }} />} @@ -381,6 +381,11 @@ function McpCatalog({ installed, onInstalled }: { installed: McpServer[]; onInst function McpServerDialog({ server, onClose, onSaved }: { server?: McpServer; onClose: () => void; onSaved: () => void }) { const editing = !!server; + const qc = useQueryClient(); + const bots = useQuery({ queryKey: ["bots"], queryFn: Api.listBots, enabled: editing }); + const serverTools = server?.tools ?? []; + // Bots ticked or unticked here: true grants all of the server's tools, false clears them. + const [grants, setGrants] = useState>({}); const [transport, setTransport] = useState(server?.transport ?? "http"); const [name, setName] = useState(server?.name ?? ""); const [url, setUrl] = useState(server?.url ?? ""); @@ -393,11 +398,16 @@ function McpServerDialog({ server, onClose, onSaved }: { server?: McpServer; onC const errors = validateNewMcpServer(name, url, transport, command); const kv = parseKeyValues(transport === "http" ? headers : env); const save = useSaveMutation({ - mutationFn: () => { + mutationFn: async () => { const body: McpServerInput = transport === "http" ? { url: url.trim(), headers: kv.values } : { command: command.trim(), args: parseArgs(args), env: kv.values, cwd: cwd.trim() || null }; - return editing ? Api.updateMcpServer(server!.name, body) : Api.addMcpServer({ name: name.trim(), ...body }); + if (!editing) return Api.addMcpServer({ name: name.trim(), ...body }); + for (const b of bots.data ?? []) { + if (b.id in grants) await Api.updateBot(b.id, setGroup(b, serverTools, grants[b.id])); + } + if (Object.keys(grants).length) { qc.invalidateQueries({ queryKey: ["bots"] }); qc.invalidateQueries({ queryKey: ["bot"] }); } + return Api.updateMcpServer(server!.name, body); }, onSuccess: onSaved, }, editing ? "MCP server settings saved" : "MCP server added"); @@ -446,6 +456,32 @@ function McpServerDialog({ server, onClose, onSaved }: { server?: McpServer; onC setCwd(e.target.value)} /> )} + {editing && ( +
+
Bots with this server's tools
+ {serverTools.length === 0 ? ( + Connect the server to grant its tools to bots here. + ) : ( + <> +
+ {bots.data?.map((b) => { + const state = b.id in grants ? (grants[b.id] ? "all" : "none") : groupState(serverTools, b.tool_names); + return ( + + ); + })} +
+ Grants every tool the server has now. Tools it adds later, and single tools, are granted in the bot editor. + + )} +
+ )} Only add servers from developers you trust. OpenBot does not control which tools a server exposes or what they do, and every bot you grant those tools acts with whatever access you provide. Mark tools that write as needing approval in the bot editor.
From e1a34ffcd2db768e68c64fce484ac2b4c2eac249 Mon Sep 17 00:00:00 2001 From: bymarvin Date: Mon, 26 Oct 2026 22:31:29 +0100 Subject: [PATCH 2/2] Refresh bots after a partial grant failure; show bot list errors --- frontend/src/pages/SettingsPage.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/frontend/src/pages/SettingsPage.tsx b/frontend/src/pages/SettingsPage.tsx index de935c1b..50f12ac1 100644 --- a/frontend/src/pages/SettingsPage.tsx +++ b/frontend/src/pages/SettingsPage.tsx @@ -403,10 +403,13 @@ function McpServerDialog({ server, onClose, onSaved }: { server?: McpServer; onC ? { url: url.trim(), headers: kv.values } : { command: command.trim(), args: parseArgs(args), env: kv.values, cwd: cwd.trim() || null }; if (!editing) return Api.addMcpServer({ name: name.trim(), ...body }); - for (const b of bots.data ?? []) { - if (b.id in grants) await Api.updateBot(b.id, setGroup(b, serverTools, grants[b.id])); + try { + for (const b of bots.data ?? []) { + if (b.id in grants) await Api.updateBot(b.id, setGroup(b, serverTools, grants[b.id])); + } + } finally { + if (Object.keys(grants).length) { qc.invalidateQueries({ queryKey: ["bots"] }); qc.invalidateQueries({ queryKey: ["bot"] }); } } - if (Object.keys(grants).length) { qc.invalidateQueries({ queryKey: ["bots"] }); qc.invalidateQueries({ queryKey: ["bot"] }); } return Api.updateMcpServer(server!.name, body); }, onSuccess: onSaved, @@ -483,7 +486,7 @@ function McpServerDialog({ server, onClose, onSaved }: { server?: McpServer; onC
)} Only add servers from developers you trust. OpenBot does not control which tools a server exposes or what they do, and every bot you grant those tools acts with whatever access you provide. Mark tools that write as needing approval in the bot editor. - +