diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fa48b0d..0a5d4315 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -223,6 +223,52 @@ Sessions survive and nobody signs in again. next migration.** Repair it with `update drizzle.__drizzle_migrations set created_at = 1787359000000 where created_at = 1787444747113;` or start from a fresh database, where migrations all run in one pass and ordering cannot bite. +- **Every Bot ran a model two generations old, and it was costing tool calls.** The example package + shipped `gpt-4.1` as the default for every built-in Bot. Asked to open a page behind a sign-in, + those Bots answered "would you like me to prompt you to sign in?" and called nothing, three times + out of three, while the prompt forbids that sentence in as many words. On `gpt-5.6-terra` the same + question produces the tool call first try. The default is now `gpt-5.6-terra` across the package, + the compose services and both example Bots, and the Responses API is inferred from the model rather + than left to a separate switch, because `gpt-5.6-*` rejects function tools on chat completions and + a deployment that set the model without knowing that got a Bot which started, looked healthy, and + failed on its first tool call. It is a default, not a commitment: `BOT_MODEL` and the package's + `model.yaml` still decide. `agent-bot` stays on `gpt-5.5` on purpose, since the only ways to 5.6 on + the endpoint it writes by hand are a streaming rewrite or turning reasoning off, and it is the Bot + whose job includes deciding when to ask a person for help. +- **A Bot browsed to a vendor this deployment already connects to.** A Bot holding no grants was told + nothing about connectors at all, so it treated a connected vendor as an ordinary website: asked + about Google Drive it opened `drive.google.com`, met a sign-in page, and asked the person to sign + in to an account the deployment had already connected. Every Bot is now told which vendors exist + here, held or not, and says plainly which one it has not been granted rather than reaching for the + browser. +- **A conversation was destroyed by a declined take-the-wheel.** A Bot that asks for help with a + sign-in and never gets it left a tool call nothing ever answered, and every later turn in that + thread failed at the provider. This was fixed once for the framework Bot and not for the Bot in the + box, which is the one behind the Browser Bot, so it went on happening where most people would meet + it. Both now answer their own unanswered calls with the truth rather than a fake success. +- **A Bot refused because a person had the wheel was told its refs were stale.** The computer flags a + takeover, the surface branches on that flag, and the flag did not survive the server, so a Bot was + sent back round the same action against the person who had just taken the browser. Reported and + fixed by @beardthelion. +- **A person could not take the wheel unless the Bot offered it.** The button appeared only after a + Bot called for help, so the control a person needs depended on the Bot getting one instruction + right, and when it did not there was nothing to press. It is there whenever the Bot is driving now. + The Bot asking for help is still its own row, with its reason. +- **The first message of a new channel could be lost.** A new channel's thread does not exist until + its first run, so the join that restores history had nothing to settle against; the message was sent + anyway after a deadline, while that join was still in flight, and the join finishing replaced it + with the thread's messages, which were none. The deadline now ends the join and waits for it, so + nothing is left in flight to overwrite anything. The transcript also says it is loading rather than + showing an empty conversation, and the thinking line is visible for the first time: a CSS rule + blanked the colour a gradient was built from, so the glyphs were painted with nothing. Reported and + fixed by @zopeVaibhav. +- **The in-memory snapshot store disagreed with the table.** The database only ever moves a snapshot + forward; the in-memory one, which is what a test reaches for when it has no database, took whatever + arrived last. A test could therefore prove a boundary property that is false in a deployment. + Reported by @beardthelion, fixed by @NathanTarbert. +- **A computer that had opened nothing still reserved a browser-sized frame.** That put a placeholder + the height of a browser window into the middle of a conversation, above an answer that never + involved the browser. - **A Bot named after a deployment route was served without its guard.** The computer router steps aside for `/policy` and `/fleet`, which are its own paths and not about a Bot, because Hono matches `/*` against zero segments and a single-segment path arrives as a Bot id. It stepped aside on the diff --git a/app/src/components/computer/computer-view.tsx b/app/src/components/computer/computer-view.tsx index 49352d37..9ba91baa 100644 --- a/app/src/components/computer/computer-view.tsx +++ b/app/src/components/computer/computer-view.tsx @@ -171,11 +171,22 @@ export function ComputerView({ return () => window.removeEventListener("keydown", onKey); }, [expanded]); - // Sized from the ratio, never from the payload, so the frame is identical in all three states. - const frameStyle = { aspectRatio, minWidth, minHeight }; - // Always render the card frame; help/secret controls live below the conditional picture. const blankBrowser = shot ? isBlankBrowser(shot) : false; + + /* + * Sized from the ratio, never from the payload, so the frame is identical while a screen is + * loading and once it arrives. + * + * A browser that has opened nothing is the exception. Reserving a screen-sized frame for it put a + * placeholder the height of a browser window into the middle of a conversation, above an answer + * that never involved the browser at all: a Bot asked about Google Drive rendered a full-size + * empty panel saying it had not opened a page. Nothing is loading there and nothing is coming, so + * there is no layout jump to protect against and no reason to take the room. + */ + const frameStyle = blankBrowser + ? { minWidth } + : { aspectRatio, minWidth, minHeight }; /** Blank browser placeholders should not be opened as readable screens. */ const showScreen = shot !== null && !blankBrowser; @@ -206,6 +217,8 @@ export function ComputerView({ {blankBrowser ? ( ) : null} + {/* The blank state is a line of text, so it needs its own height rather than the frame's. */} + {blankBrowser ? : null} {showScreen ? null : ( Promise = async () => [], ): Promise> { + const vendors = await loadVendors().catch(() => [] as readonly string[]); return Object.fromEntries( await Promise.all( agents.map(async (agent) => [ @@ -284,6 +299,7 @@ export async function buildAgents( loadTools, signRun, computerGuidance, + vendors, ), ]), ), @@ -298,6 +314,7 @@ async function buildAgent( loadTools: LoadToolsForBot, signRun?: SignRun, computerGuidance?: string, + connectedVendors: readonly string[] = [], ): Promise { if (agent.type === "built_in") { return new BuiltInAgent( @@ -307,6 +324,7 @@ async function buildAgent( apiKey, await loadTools(agent.id), computerGuidance, + connectedVendors, ), ); } @@ -318,6 +336,7 @@ async function buildAgent( stallGuard, await loadTools(agent.id), signRun, + connectedVendors, ); } @@ -345,6 +364,8 @@ function remoteAgentWithStandingRole( */ tools: GrantedTool[] = [], signRun?: SignRun, + /** As for the built-in path: what this deployment connects to, held or not. */ + connectedVendors: readonly string[] = [], ) { const remote = new HttpAgent({ url: agent.endpoint, @@ -368,7 +389,7 @@ function remoteAgentWithStandingRole( * 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 holdings = grantedToolGuidance(tools, connectedVendors); const holdingsMessage = holdings ? { id: `granted-tools:${agent.id}`, @@ -467,6 +488,7 @@ export async function resolveRuntimeAgents( loadTools?: LoadToolsForBot, signRun?: SignRun, computerGuidance?: string, + loadVendors?: () => Promise, ): Promise> { const registered = await loadAgents(); if (registered.length === 0) { @@ -486,6 +508,7 @@ export async function resolveRuntimeAgents( loadTools, signRun, computerGuidance, + loadVendors, ); } @@ -533,6 +556,8 @@ export function createRequestAgents( signRunForActor?: (actorId: string) => SignRun, /** What every built-in Bot is told about the computer. Absent means this deployment has none. */ computerGuidance?: string, + /** Which vendors this deployment connects to, held by a Bot or not. Absent means none. */ + loadVendors?: () => Promise, ) { return async ({ request }: { request: Request }) => { const actor = await identifyActor(request); @@ -544,6 +569,7 @@ export function createRequestAgents( loadToolsForActor?.(actor.id), signRunForActor?.(actor.id), computerGuidance, + loadVendors, ); }; } @@ -571,6 +597,7 @@ export function mountCopilotRuntime( loadToolsForActor?: (actorId: string) => LoadToolsForBot, signRunForActor?: (actorId: string) => SignRun, basePath = "/api/copilotkit", + loadVendors?: () => Promise, ) { const { intelligence } = config.runtime; @@ -609,6 +636,7 @@ export function mountCopilotRuntime( * as impossible. Absent computer, absent guidance: a Bot is not told about hands it has not got. */ config.computer ? COMPUTER_GUIDANCE : undefined, + loadVendors, ) as never, }); diff --git a/server/src/index.ts b/server/src/index.ts index 423c366b..e848e385 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -1,5 +1,3 @@ -import { createIntentRouter } from "./routing/classify"; -import { createModelCompleter } from "./routing/model"; import { serve } from "bun"; import { mintRunAssertion } from "./agents/callback-token"; import { createAgentProfileStore } from "./agents/profile-store"; @@ -48,6 +46,8 @@ import { createDatabase } from "./db/client"; import { createPeopleStore } from "./people/store"; import { createPluginStore } from "./plugins/store"; import { grantedTools } from "./plugins/tools"; +import { createIntentRouter } from "./routing/classify"; +import { createModelCompleter } from "./routing/model"; import { createPackageStatusReader, loadTenantPackage, @@ -416,6 +416,25 @@ const app = createApp( */ (actorId) => (botId, runId) => mintRunAssertion({ botId, actorId, runId }, config.keyEncryptionKey), + undefined, + /* + * Which vendors this deployment connects to, held by a Bot or not. + * + * A Bot holding no grants used to be told nothing about connectors at all, so it treated a + * connected vendor as an ordinary website and browsed to it: a Bot with no Drive grant opened + * Google's sign-in page and asked a person to sign in to an account the deployment had already + * connected. Naming them lets it say which one it has not been granted instead. + * + * Read per request rather than held, because a connector added a minute ago has to count, and + * failing is the same as having none: a Bot that cannot be told loses a sentence, not a run. + */ + async () => { + try { + return (await pluginStore.listServers()).map((server) => server.id); + } catch { + return []; + } + }, ), // The only path to an acting call. computerGateway, diff --git a/server/src/plugins/tools.ts b/server/src/plugins/tools.ts index 9036b0e3..297cf87c 100644 --- a/server/src/plugins/tools.ts +++ b/server/src/plugins/tools.ts @@ -66,8 +66,19 @@ export function parametersFor(inputSchema: Record): z.ZodType { * * 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 ""; +export function grantedToolGuidance( + tools: GrantedTool[], + /** + * Systems this deployment connects to that this Bot holds nothing for. + * + * Without these a Bot holding no grants is told nothing at all, so it treats a connected vendor as + * an ordinary website and browses to it. That is how a Bot with no Drive grant ended up on Google's + * sign-in page asking a person to sign in to an account the deployment had already connected: the + * connector existed, the Bot simply was not on it, and nothing said so. + */ + connectedButNotHeld: readonly string[] = [], +): string { + if (tools.length === 0 && connectedButNotHeld.length === 0) return ""; const bySystem = new Map(); for (const tool of tools) { @@ -78,19 +89,51 @@ export function grantedToolGuidance(tools: GrantedTool[]): string { bySystem.set(system, [...(bySystem.get(system) ?? []), rest]); } + const held = [...bySystem.keys()]; + const missing = connectedButNotHeld.filter( + (system) => !held.includes(system), + ); + return [ - "You can reach these systems directly, as the person asking, with their own access:", + ...(tools.length > 0 + ? [ + "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 and no tool above covers the part you need, that is a", - "missing grant and not something to work around. Say so plainly, name the capability you would", - "need, and say an administrator can grant it on that connector. Do not reach for the browser, do", - "not ask the person to sign in, and do not ask them to fetch it for you: they already have the", - "access, and the thing that is missing is yours, not theirs.", + ...(tools.length > 0 + ? [ + "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 and no tool above covers the part you need, that is a", + "missing grant and not something to work around. Say so plainly, name the capability you would", + "need, and say an administrator can grant it on that connector. Do not reach for the browser, do", + "not ask the person to sign in, and do not ask them to fetch it for you: they already have the", + "access, and the thing that is missing is yours, not theirs.", + ] + : []), + /* + * The vendors this deployment connects to and this Bot does not hold. + * + * Named so the Bot can say which one, because "I have not been granted it" is only actionable if + * the person is told what "it" is. The browser is refused for these by the same reasoning as + * above and for a sharper reason: a connector exists precisely so the vendor is reached as the + * person asking, and the container's browser is signed in as nobody, so browsing there abandons + * the per-person path and lands on a login wall by construction. + */ + ...(missing.length > 0 + ? [ + ...(tools.length > 0 ? [""] : []), + `This deployment also connects to: ${missing.join(", ")}. You hold none of their tools.`, + "If a question needs one of them, say plainly that you have not been granted it and that an", + "administrator can grant it on that connector. Do NOT browse to its website: that is not the", + "same thing, your browser is signed in as nobody, and it will meet a sign-in wall that the", + "connector exists to avoid. Do not ask the person to sign in there either.", + ] + : []), ].join("\n"); } diff --git a/server/tests/copilot.test.ts b/server/tests/copilot.test.ts index 41d6d4b2..7f72ff28 100644 --- a/server/tests/copilot.test.ts +++ b/server/tests/copilot.test.ts @@ -1,6 +1,7 @@ import { describe, expect, spyOn, test } from "bun:test"; import { HttpAgent } from "@ag-ui/client"; import { BuiltInAgent } from "@copilotkit/runtime/v2"; +import { PROVENANCE_GUIDANCE } from "../../shared/bot-prompt"; import { buildAgents, builtInAgentConfiguration, @@ -9,7 +10,6 @@ import { resolveRuntimeAgents, standingRoleMessage, } from "../src/copilot"; -import { PROVENANCE_GUIDANCE } from "../../shared/bot-prompt"; import { grantedToolGuidance } from "../src/plugins/tools"; // Every agent row now joins its profile, so the row a coworker is built from always names it. @@ -571,9 +571,40 @@ describe("what a Bot is told it holds", () => { expect(guidance).toContain("do not ask the person to sign in"); }); + test("names a connected vendor it holds nothing for, so it can say which", () => { + /* + * The case a Bot holding no grants used to be told nothing about. + * + * The deployment had Google Drive connected and this Bot was not on it, so the guidance was + * empty and the Bot treated the vendor as an ordinary website: it opened Google's sign-in page + * and asked a person to sign in to an account the deployment had already connected. The + * connector existed; nothing said the Bot simply was not on it. + */ + const guidance = grantedToolGuidance([], ["google-drive"]) + .toLowerCase() + .replace(/\s+/g, " "); + + expect(guidance).toContain("google-drive"); + expect(guidance).toContain("you hold none of their tools"); + expect(guidance).toContain("have not been granted it"); + expect(guidance).toContain("do not browse to its website"); + }); + + test("does not name a vendor it does hold as one it does not", () => { + // The list is the deployment's, so it includes what this Bot has. Saying "you hold none of + // their tools" about a system it is holding four tools for would be worse than saying nothing. + const guidance = grantedToolGuidance(drive, ["google-drive"]); + + expect(guidance).toContain("search_files"); + expect(guidance.toLowerCase()).not.toContain( + "you hold none of their tools", + ); + }); + 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(""); + expect(grantedToolGuidance([], [])).toBe(""); }); test("a built-in Bot is told before it is told about the browser", () => {