diff --git a/server/src/copilot.ts b/server/src/copilot.ts index 766ce4f2..8582a85d 100644 --- a/server/src/copilot.ts +++ b/server/src/copilot.ts @@ -8,6 +8,7 @@ import { import { createCopilotHonoHandler } from "@copilotkit/runtime/v2/hono"; import { z } from "zod"; import { COMPUTER_GUIDANCE } from "../../shared/bot-prompt"; +import { grantedToolGuidance } from "./plugins/tools"; import type { AgentActor } from "./agents/profile-types"; import type { StallGuard } from "./channels/stall-guard"; import type { DeploymentConfig } from "./config"; @@ -200,9 +201,18 @@ export function builtInAgentConfiguration( return { model: `${model.provider}/${model.defaultModel}`, - prompt: computerGuidance - ? `${agent.systemPrompt}\n\n${computerGuidance}` - : agent.systemPrompt, + /* + * The package's role, then what this Bot actually holds, then the computer. + * + * The grants go BEFORE the computer prose on purpose. That prose is long and emphatic about the + * browser and mentions connectors nowhere, so a Bot that read it last reached for the browser + * even when it held a tool for the exact system being asked about. + */ + prompt: [ + agent.systemPrompt, + ...(grantedToolGuidance(tools) ? [grantedToolGuidance(tools)] : []), + ...(computerGuidance ? [computerGuidance] : []), + ].join("\n\n"), apiKey, /* * A run stops after one step unless told otherwise, which for a Bot with tools means it calls @@ -327,13 +337,37 @@ function remoteAgentWithStandingRole( ? { fetch: stallGuard.watch({ id: agent.id, name: agent.name }) } : {}), }); + /* + * What this Bot holds, as a second standing message. + * + * Beside the role rather than inside it, because the role comes from the package and this comes + * from the grants: they change for different reasons and at different times. Sent on every run for + * the same reason the tools are, so switching a connector on reaches the next run. + * + * The remote path needs this more than the built-in one, not less. A framework Bot is handed the + * tools as an offer and decides for itself what to call, with `COMPUTER_GUIDANCE` as its whole + * prompt — a page about the browser that mentions connectors nowhere. That is the Bot that browsed + * to drive.google.com holding four Drive tools. + */ + const holdings = grantedToolGuidance(tools); + const holdingsMessage = holdings + ? { + id: `granted-tools:${agent.id}`, + role: "system" as const, + content: holdings, + } + : null; + remote.use((input, next) => next.run({ ...input, messages: [ agent.standingMessage, + ...(holdingsMessage ? [holdingsMessage] : []), ...input.messages.filter( - (message) => message.id !== agent.standingMessage.id, + (message) => + message.id !== agent.standingMessage.id && + message.id !== holdingsMessage?.id, ), ], /* diff --git a/server/src/plugins/tools.ts b/server/src/plugins/tools.ts index 0bc35518..35aa4753 100644 --- a/server/src/plugins/tools.ts +++ b/server/src/plugins/tools.ts @@ -48,6 +48,48 @@ export function parametersFor(inputSchema: Record): z.ZodType { return z.object({}).catchall(z.unknown()); } +/** + * What this Bot holds, said in its instructions rather than left to be inferred from a tool list. + * + * A tool array tells a model a tool exists. It does not tell it that the tool is the right way to + * reach that system, and it competes with a page of prose about the browser that every Bot is given + * whether or not it has any connectors at all. The browser prose wins: it is emphatic, it is about + * capability, and it says "never claim you cannot browse". + * + * So a Bot holding four Google Drive tools browsed to drive.google.com, met a sign-in page its + * container could never satisfy, and asked its person to sign in to a vendor that person had already + * connected. The tools were there the whole time. + * + * Generated from the grants rather than written down, because the point is that it tracks them. An + * administrator switching a connector on, or granting one more of its tools, changes what the Bot is + * told on its next run with nothing else to remember and nothing to keep in step. + * + * Empty when the Bot holds nothing, so a deployment with no connectors says nothing about them. + */ +export function grantedToolGuidance(tools: GrantedTool[]): string { + if (tools.length === 0) return ""; + + const bySystem = new Map(); + for (const tool of tools) { + // `mcp__server__tool`, which is the shape the model is offered. + const parts = tool.name.replace(/^mcp__/, "").split("__"); + const system = parts.length > 1 ? (parts[0] as string) : "this deployment"; + const rest = parts.length > 1 ? parts.slice(1).join("__") : tool.name; + bySystem.set(system, [...(bySystem.get(system) ?? []), rest]); + } + + return [ + "You can reach these systems directly, as the person asking, with their own access:", + ...[...bySystem.entries()].map( + ([system, names]) => `- ${system}: ${names.join(", ")}`, + ), + "Use them for anything about those systems. Do NOT browse to one of their websites instead: your", + "browser is signed in as nobody, so it sees less than these tools do and will meet a sign-in wall", + "that connecting an account has already solved. If one of these systems is involved but no tool", + "above covers the part you need, say which part is missing rather than going around it.", + ].join("\n"); +} + /** * Every MCP tool granted to one Bot, ready to hand to the runtime. * diff --git a/server/tests/copilot.test.ts b/server/tests/copilot.test.ts index 5bd14d47..ee623640 100644 --- a/server/tests/copilot.test.ts +++ b/server/tests/copilot.test.ts @@ -9,6 +9,7 @@ import { resolveRuntimeAgents, standingRoleMessage, } from "../src/copilot"; +import { grantedToolGuidance } from "../src/plugins/tools"; // Every agent row now joins its profile, so the row a coworker is built from always names it. const assistantRow = { @@ -502,3 +503,63 @@ function fakeAgUiEndpoint() { [Symbol.asyncDispose]: () => server.stop(true), }; } + +/** + * A Bot is told what it holds, not only handed it. + * + * A tool array tells a model a tool exists. It does not say the tool is the right way to reach that + * system, and it competes with `COMPUTER_GUIDANCE`: a page of emphatic prose about the browser that + * every Bot gets whether or not it has a single connector, and that mentions connectors nowhere. + * + * The browser prose won. A Bot holding four Google Drive tools browsed to drive.google.com, met a + * sign-in page its container could never satisfy, and asked its person to sign in to a vendor that + * person had already connected. Asked a question with no tool for it, another went reading a + * government website and looped on its 404 page. + * + * Both kinds are asserted because they are built by different functions, and the remote one is the + * one that failed in the product. + */ +describe("what a Bot is told it holds", () => { + const drive = [ + { name: "mcp__google-drive__search_files" }, + { name: "mcp__google-drive__read_file_content" }, + ] as never[]; + + test("names the system and its tools", () => { + const guidance = grantedToolGuidance(drive); + expect(guidance).toContain("google-drive"); + expect(guidance).toContain("search_files"); + expect(guidance).toContain("read_file_content"); + }); + + test("says not to browse to a vendor it has a tool for", () => { + // The whole point. Without this line the tool list is inert beside the browser prose. + expect(grantedToolGuidance(drive).toLowerCase()).toContain("do not browse"); + }); + + test("says nothing at all when the Bot holds nothing", () => { + // A deployment with no connectors must not be told about connectors it does not have. + expect(grantedToolGuidance([])).toBe(""); + }); + + test("a built-in Bot is told before it is told about the browser", () => { + const prompt = builtInAgentConfiguration( + { + id: "risk-analyst", + name: "Risk Analyst", + type: "built_in", + systemPrompt: "Investigate policies.", + }, + { provider: "openai", defaultModel: "gpt-4.1" }, + "openai-secret", + drive, + "BROWSER GUIDANCE HERE", + ).prompt as string; + + // Order is the fix, not merely presence: the grants have to land before the browser prose. + expect(prompt.indexOf("google-drive")).toBeGreaterThan(-1); + expect(prompt.indexOf("google-drive")).toBeLessThan( + prompt.indexOf("BROWSER GUIDANCE HERE"), + ); + }); +});