diff --git a/frontend/src/lib/toolGroups.test.ts b/frontend/src/lib/toolGroups.test.ts index e1b2af3..62bf4ba 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 c3758f3..6fa5c7d 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 e0b8450..5fb8a6d 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 365194b..50f12ac 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,19 @@ 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 }); + 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"] }); } + } + return Api.updateMcpServer(server!.name, body); }, onSuccess: onSaved, }, editing ? "MCP server settings saved" : "MCP server added"); @@ -446,8 +459,34 @@ 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. - +