From 9417adeb2534def0671979a6d14505b1a29c6d9c Mon Sep 17 00:00:00 2001 From: David McKay Date: Tue, 27 Oct 2026 13:09:40 -0700 Subject: [PATCH 1/3] Ask about a page visit a rule covers instead of refusing it The browser sends a Bot's looks at a page (navigate, snapshot, scroll) straight to the computer; only changes travel through the approval route that carries the conversation. A rule that asks about a look, such as "ask before opening pages", left the gate nowhere to ask, so the route answered a bare 500 and the Bot told the person the action needed an approval flow that wasn't available. The gate now marks that case (ApprovalNeedsConversationError), the computer routes answer it as 409 needsConversation with nothing done, and the browser sends the same call again through the approval route, where it waits on the inline card like any change. Looks nothing asks about still go straight to the computer. --- app/src/lib/computers/call.ts | 16 +++ app/src/lib/copilot/approval-tools.tsx | 25 +++- app/tests/approval-tools.test.tsx | 136 ++++++++++++++++++ server/src/approvals/types.ts | 23 ++- server/src/computer/routes.ts | 12 +- .../tests/approval-needs-conversation.test.ts | 93 ++++++++++++ 6 files changed, 296 insertions(+), 9 deletions(-) create mode 100644 server/tests/approval-needs-conversation.test.ts diff --git a/app/src/lib/computers/call.ts b/app/src/lib/computers/call.ts index 52a333ec41..01183616da 100644 --- a/app/src/lib/computers/call.ts +++ b/app/src/lib/computers/call.ts @@ -111,6 +111,22 @@ export function failedComputerCall( : "The computer is still starting. This call was not performed. Try it again in a moment.", }; } + /* + * A rule wants to ask about this call, and it went straight to the computer without its + * conversation. Nothing was done. `useFrontendTool` in approval-tools sends it again through the + * approval route, so this result only reaches the Bot from a surface with no conversation at all. + */ + if (body?.needsConversation === true) { + return { + ok: false, + needsConversation: true, + notPerformed: true, + reason: + typeof body.error === "string" + ? body.error + : "This action needs your approval in a chat with this Bot.", + }; + } return { ok: false, reason: typeof body?.error === "string" ? body.error : "That did not work.", diff --git a/app/src/lib/copilot/approval-tools.tsx b/app/src/lib/copilot/approval-tools.tsx index ca7014b84e..ca3eeef760 100644 --- a/app/src/lib/copilot/approval-tools.tsx +++ b/app/src/lib/copilot/approval-tools.tsx @@ -13,6 +13,11 @@ import { frontendApprovalSnapshot } from "./approval-context"; type RenderProps = { toolCallId: string; [key: string]: unknown }; +const needsConversation = (outcome: unknown) => + !!outcome && + typeof outcome === "object" && + (outcome as { needsConversation?: unknown }).needsConversation === true; + const writes = new Set([ "computer_click", "computer_type", @@ -32,6 +37,13 @@ const writes = new Set([ * handler kept the switch as it was on first render, usually still loading. Every change then went * straight to the computer outside any run, and the gate could only answer "this action needs an * authenticated run". Nothing in this handler may depend on render-time state for that reason. + * + * A LOOK GOES STRAIGHT TO THE COMPUTER, AND COMES BACK THROUGH THE GATE WHEN A RULE ASKS ABOUT IT. A + * navigation or a snapshot is usually allowed, and sending each one with the whole conversation would + * be paid on every step of every page. A rule can still ask about one ("ask before opening pages"), + * and that call arrives with no conversation to ask in, so the computer answers `needsConversation` + * having done nothing. It is sent again here through the gate, where the person is asked; before + * this, the Bot was handed the refusal and could only apologise for a card nobody was shown. */ export function useFrontendTool>( tool: ReactFrontendTool, @@ -43,7 +55,8 @@ export function useFrontendTool>( const render = tool.render; useSDKFrontendTool({ ...tool, - ...(writes.has(tool.name) + // Every computer tool, not only changes: a look a rule asks about waits on the same card. + ...(tool.name.startsWith("computer_") ? { // A component, so the approval card can use hooks; the SDK keeps this function forever. render: (props: RenderProps) => { @@ -61,14 +74,18 @@ export function useFrontendTool>( : {}), handler: tool.handler ? async (args, context) => { - if (!writes.has(tool.name)) return tool.handler?.(args, context); + let direct: unknown; + if (!writes.has(tool.name)) { + direct = await tool.handler?.(args, context); + if (!needsConversation(direct)) return direct; + } let snapshot: ReturnType; try { snapshot = frontendApprovalSnapshot(context); } catch { // A surface with no observed conversation. The server still gates the action; it can - // run it when nothing asks, and says why when something does. - return tool.handler?.(args, context); + // run it when nothing asks, and says why when something does. A look already went. + return direct ?? tool.handler?.(args, context); } let response: Response; try { diff --git a/app/tests/approval-tools.test.tsx b/app/tests/approval-tools.test.tsx index 77c881efbc..35ceb82d63 100644 --- a/app/tests/approval-tools.test.tsx +++ b/app/tests/approval-tools.test.tsx @@ -3,6 +3,7 @@ import * as ReactCoreV2 from "@copilotkit/react-core/v2"; import { GlobalRegistrator } from "@happy-dom/global-registrator"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { cleanup, render } from "@testing-library/react"; +import { failedComputerCall } from "@/lib/computers/call"; import { observeApprovalAgent } from "@/lib/copilot/approval-context"; import { useFrontendTool } from "@/lib/copilot/approval-tools"; @@ -273,3 +274,138 @@ test("a gate that cannot be reached, or answers with no result, is still a resul await clickAgainstGate(async () => Response.json({}, { status: 200 })), ).toEqual({ ok: false, reason: "The computer action returned no result." }); }); + +/* + * A look at a page a rule asks about. + * + * A navigation goes straight to the computer, without the conversation the approval card is drawn + * in, so a rule like "ask before opening pages" used to leave the gate nowhere to ask. The Bot was + * handed the refusal and told the person "this browser action requires an approval flow that isn't + * available in this chat". The computer now answers `needsConversation`, and the call goes again + * through the gate, where it waits on the card like any change. + */ +let lookOutcome: unknown; +const looked: unknown[] = []; +function LookTools() { + useFrontendTool({ + name: "computer_navigate", + description: "Open a page", + handler: async (args: Record) => { + looked.push(args); + return lookOutcome; + }, + }); + return null; +} + +async function navigateWith( + answer: (path: string) => Promise, +): Promise<{ result: unknown; paths: string[] }> { + const paths: string[] = []; + const originalFetch = global.fetch; + global.fetch = Object.assign( + async (path: Parameters[0]) => { + paths.push(String(path)); + return answer(String(path)); + }, + { preconnect: originalFetch.preconnect }, + ); + try { + render( + + + , + ); + const tool = registered.find((entry) => entry.name === "computer_navigate"); + const toolCall = { + id: "navigate-call", + type: "function" as const, + function: { + name: "computer_navigate", + arguments: '{"url":"https://example.com"}', + }, + }; + const subscribers: { + onRunInitialized?: (input: { input: unknown }) => void; + }[] = []; + const agent = { + threadId: "thread", + state: {}, + messages: [ + { id: "u1", role: "user", content: "Open example.com" }, + { id: "a1", role: "assistant", content: "", toolCalls: [toolCall] }, + ], + subscribe: (subscriber: (typeof subscribers)[number]) => { + subscribers.push(subscriber); + return { unsubscribe: () => undefined }; + }, + }; + const stop = observeApprovalAgent(agent as never); + for (const subscriber of subscribers) + subscriber.onRunInitialized?.({ + input: { + runId: "run-1", + threadId: "thread", + messages: agent.messages, + tools: [], + context: [], + state: {}, + forwardedProps: {}, + }, + }); + try { + const result = await tool?.handler?.({ url: "https://example.com" }, { + agent, + toolCall, + } as never); + return { result, paths }; + } finally { + stop(); + } + } finally { + global.fetch = originalFetch; + } +} + +test("a look a rule asks about goes again through the gate, with its conversation", async () => { + looked.length = 0; + stopped.length = 0; + lookOutcome = failedComputerCall(409, { + error: "This action needs your approval.", + needsConversation: true, + notPerformed: true, + }); + const { result, paths } = await navigateWith(async () => + Response.json({ waiting: true, approvalId: "approval-2" }, { status: 202 }), + ); + expect(looked).toHaveLength(1); + expect(paths).toEqual(["/api/approvals/computer/default"]); + expect(result).toEqual({ waiting: true }); + expect(stopped).toHaveLength(1); +}); + +test("a look nothing asks about goes straight to the computer and nowhere else", async () => { + looked.length = 0; + lookOutcome = { ok: true, title: "Example Domain" }; + const { result, paths } = await navigateWith(async () => { + throw new Error("The gate should not be called."); + }); + expect(looked).toHaveLength(1); + expect(paths).toEqual([]); + expect(result).toEqual({ ok: true, title: "Example Domain" }); +}); + +test("needing a conversation is not read as stale refs", () => { + expect( + failedComputerCall(409, { + error: "This action needs your approval.", + needsConversation: true, + notPerformed: true, + }), + ).toEqual({ + ok: false, + needsConversation: true, + notPerformed: true, + reason: "This action needs your approval.", + }); +}); diff --git a/server/src/approvals/types.ts b/server/src/approvals/types.ts index 048c17e47d..1a2b69ba4a 100644 --- a/server/src/approvals/types.ts +++ b/server/src/approvals/types.ts @@ -1,11 +1,11 @@ import { createHash } from "node:crypto"; import { RunAgentInputSchema } from "@ag-ui/core/schemas"; -import type { AuditInitiator } from "../audit"; import { type ApprovalContinuation, currentApprovalContext, withApprovalContext, } from "@openbot/toolkit/continuation"; +import type { AuditInitiator } from "../audit"; /** `handled` answers a hand-off: the person did it themselves, so it is never executed for them. */ export type ApprovalDecision = @@ -223,6 +223,23 @@ export class ApprovalRefusedError extends Error { this.name = "ApprovalRefusedError"; } } +/** + * The policy wants to ask, and the call arrived with no conversation to ask in. + * + * A browser sends a Bot's look at a page (a navigation, a snapshot) straight to the computer; only its + * changes travel through the approval route that carries the conversation. A rule can still ask about + * a look. Marked apart from other refusals so the computer routes can say so, and the browser can send + * the same call again through that route, where the person is asked, instead of handing the Bot a + * refusal it can only apologise for. + */ +export class ApprovalNeedsConversationError extends ApprovalRefusedError { + constructor() { + super( + "This action needs your approval, but it was not started from a conversation that can ask you. Nothing was done. Ask for it in a chat with this Bot, or change your rules in Settings → Approvals.", + ); + this.name = "ApprovalNeedsConversationError"; + } +} export class ApprovalNotFoundError extends Error { constructor() { super("This approval could not be found."); @@ -304,9 +321,7 @@ export function approvalAction(candidate: ApprovalCandidate): ApprovalAction { !continuation.threadId || !continuation.toolCallId ) - throw new ApprovalRefusedError( - "This action needs your approval, but it was not started from a conversation that can ask you. Nothing was done. Ask for it in a chat with this Bot, or change your rules in Settings → Approvals.", - ); + throw new ApprovalNeedsConversationError(); if ( ![ candidate.actorId, diff --git a/server/src/computer/routes.ts b/server/src/computer/routes.ts index 512e3ec8ec..55df55ddc0 100644 --- a/server/src/computer/routes.ts +++ b/server/src/computer/routes.ts @@ -1,7 +1,8 @@ +import { downloadHeaders } from "@openbot/shared/file-download"; import type { Context, MiddlewareHandler } from "hono"; import { Hono } from "hono"; -import { downloadHeaders } from "@openbot/shared/file-download"; import type { BotAccessCheck } from "../agents/profile-policy"; +import { ApprovalNeedsConversationError } from "../approvals/types"; import type { AuditReader } from "../audit"; import type { AppVariables } from "../auth/guards"; import { requireAdmin } from "../auth/guards"; @@ -1030,6 +1031,13 @@ function errorBody(error: unknown): Record { ...(error instanceof ComputerStillStartingError ? { starting: true, notPerformed: true } : {}), + /* + * A rule asks about this call and it came without the conversation the asking happens in. Marked + * so the browser sends the same call again through the approval route, which carries it. + */ + ...(error instanceof ApprovalNeedsConversationError + ? { needsConversation: true, notPerformed: true } + : {}), }; } @@ -1053,6 +1061,8 @@ function statusFor(error: unknown): 400 | 403 | 404 | 409 | 413 | 500 | 503 { // another snapshot. Not 503, which says the computer is unavailable and sends an operator hunting a // container that is running perfectly. if (error instanceof ElementNotFoundError) return 409; + // Nothing is broken here either: the call has to come again with its conversation first. + if (error instanceof ApprovalNeedsConversationError) return 409; // A person holding the wheel, or a person driving before taking it. Nothing is broken; the caller // has to wait or take control first, and 409 is how both of those are already reported. if ( diff --git a/server/tests/approval-needs-conversation.test.ts b/server/tests/approval-needs-conversation.test.ts new file mode 100644 index 0000000000..72dcdea197 --- /dev/null +++ b/server/tests/approval-needs-conversation.test.ts @@ -0,0 +1,93 @@ +import { afterEach, expect, test } from "bun:test"; +import type { MiddlewareHandler } from "hono"; +import { approvalAction } from "../src/approvals/types"; +import type { AuditEventInput, AuditStore } from "../src/audit"; +import type { AppVariables, AuthenticatedActor } from "../src/auth/guards"; +import { createComputerGateway } from "../src/computer/gateway"; +import type { PolicyStore } from "../src/computer/policy-store"; +import type { ComputerProvider } from "../src/computer/provider"; +import { createComputerRoutes } from "../src/computer/routes"; + +/** + * A look at a page that a rule asks about, sent without its conversation. + * + * The browser sends a navigation straight to the computer; only changes travel with the conversation + * the approval card is drawn in. With a rule like "ask before opening pages", the gate wanted to ask + * and had nowhere to, so the route answered a bare 500 with the refusal text, and the Bot told the + * person "this browser action requires an approval flow that isn't available in this chat". The route + * now says what the browser has to do, so it can send the same call again with the conversation. + * + * The gate here asks the way the approval service does, through `approvalAction`, and the computer is + * a real listener so the test can show nothing reached it. + */ + +const servers: Array<{ stop: (force?: boolean) => void }> = []; + +afterEach(() => { + for (const server of servers.splice(0)) server.stop(true); +}); + +test("a navigation a rule asks about comes back as needing its conversation, not done", async () => { + let computerCalls = 0; + const computer = Bun.serve({ + port: 0, + fetch: async () => { + computerCalls++; + return Response.json({ url: "https://example.com/", title: "Example" }); + }, + }); + servers.push(computer); + const provider: ComputerProvider = { + name: "test", + isolation: "per-bot", + locate: async () => `http://127.0.0.1:${computer.port}`, + status: async (botId) => ({ botId, state: "ready" }), + stop: async () => ({ wasRunning: true }), + reset: async () => ({ cleared: true }), + list: async () => [], + }; + const gateway = createComputerGateway({ + provider, + auditStore: { + insert: async (_event: AuditEventInput) => undefined, + } as unknown as AuditStore, + policy: () => ({ mode: "enforce", deny: [], allow: ["true"] }), + token: "computer-token-for-this-test", + // A rule that asks: the approval service turns the candidate into an action to wait on. + approvalGate: async (candidate) => { + approvalAction(candidate); + return undefined; + }, + }); + const actor: AuthenticatedActor = { + id: "user-1", + email: "member@openbot.test", + role: "user", + }; + const asActor: MiddlewareHandler<{ Variables: AppVariables }> = async ( + context, + next, + ) => { + context.set("actor", actor); + await next(); + }; + const app = createComputerRoutes( + gateway, + {} as PolicyStore, + asActor, + async () => true, + ); + + const response = await app.request("http://openbot.test/bot-1/navigate", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ url: "https://example.com/" }), + }); + + expect(response.status).toBe(409); + expect(await response.json()).toMatchObject({ + needsConversation: true, + notPerformed: true, + }); + expect(computerCalls).toBe(0); +}); From 484073acf1c5aa75d4e955b90cdee3a6ab8b1464 Mon Sep 17 00:00:00 2001 From: David McKay Date: Tue, 27 Oct 2026 13:20:21 -0700 Subject: [PATCH 2/3] Note the page-visit approval fix in the changelog --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 500cb41634..a75021151e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,6 +162,14 @@ the field, the page and the button: a card number field, a checkout page or a ne still goes to you, whatever is typed. A news wire (business, news or press wire) on a link or button no longer counts as wiring money; wiring money still does. +### A page visit an approval rule covers now asks you + +With an approval rule that asks before a Bot opens, reads or scrolls a page, the Bot was told the +action "was not started from a conversation that can ask you", did nothing, and said it could not get +an approval in this chat. The visit now waits on the approval card in the conversation, like a click +or a command does: Allow once opens the page and the Bot carries on, Deny tells the Bot it was +declined. Page visits no rule asks about still go straight to the computer. + ### A Bot's browser step is no longer refused while it is being saved Before your browser runs a click or a typed entry for a Bot, the server checks it against the copy From e7af38aeb64f8bd2d94d0446fa1a6f4db90537c8 Mon Sep 17 00:00:00 2001 From: David McKay Date: Tue, 27 Oct 2026 14:53:29 -0700 Subject: [PATCH 3/3] Read the Anthropic key from either header in the AG2 test Anthropic's SDK 2.3.0, which CI now installs for the AG2 harness, sends the key as Authorization: Bearer instead of x-api-key. The agent's key still reaches Anthropic; the test only looked for the old header, so it failed on every pull request. It now accepts either, and passes with Anthropic's SDK 1.17.0 and 2.3.0 alike. (cherry picked from commit a1391e7526807b51dde669b4a295f0d56e0ab2d9) --- agent-ag2/tests/test_main.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/agent-ag2/tests/test_main.py b/agent-ag2/tests/test_main.py index 93cf26a8c3..e3824d90d0 100644 --- a/agent-ag2/tests/test_main.py +++ b/agent-ag2/tests/test_main.py @@ -190,7 +190,11 @@ def test_an_anthropic_key_uses_the_official_endpoint_when_compose_sets_a_blank_u async def respond(transport, request): body = json.loads(request.content) - seen.append((str(request.url), request.headers.get("x-api-key"), body["model"])) + # Anthropic's SDK sends the key as `x-api-key` up to 1.x and as `Authorization: Bearer` from + # 2.x; either is the key reaching Anthropic, which is what this test is about. + bearer = request.headers.get("authorization") or "" + key = request.headers.get("x-api-key") or bearer.removeprefix("Bearer ") or None + seen.append((str(request.url), key, body["model"])) return httpx2.Response( 200, request=request,