diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8c488b2f25..301973e270 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -139,7 +139,7 @@ jobs: - run: >- bun test tests/sign-in.test.ts tests/session-cookies.test.ts tests/follows-popup.test.ts tests/demonstration-live.test.ts tests/webrtc-proxy.test.ts tests/web-bot-auth-browser.test.ts - tests/handoff-page-restore.test.ts + tests/handoff-page-restore.test.ts tests/enter-text.test.ts working-directory: agent-computer env: OPENBOT_COMPUTER_BROWSER: '1' diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cd51b8e0c..3f9bc54a70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,23 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. request for an administrator to switch on a work app, grant it to a Bot, or connect a Shared app's team account is kept, with who answered it and when. It starts empty and nothing is backfilled. +### A Bot can choose from a dropdown + +A Bot filling in a form in its browser could not set a dropdown (a native ``: its value in the YAML is the list of its options, not text. The option marked + // `[selected]` is what the field shows, which is the value a person reads off it. + if (descriptor.role === "combobox" || descriptor.role === "listbox") { + const list = optionsOf(value); + if (list) { + element.options = list.options; + if (list.selected !== undefined && element.value === undefined) + element.value = list.selected; + } + } + // Playwright emits `[checked]` only when something is checked, so absence is ambiguous on its own: a // Bot cannot tell an unticked box from a control that does not tick. Reported as false for the roles // that can be checked, and left off entirely for the ones that cannot. @@ -218,6 +237,35 @@ function toElement( return element; } +/** + * The options listed under a dropdown, and which one is selected. + * + * Only options with no ref of their own: one with a ref is a custom listbox's option, already an + * element the Bot clicks, and listing it twice would offer two ways to do one thing. + */ +function optionsOf( + value: unknown, +): { options: string[]; selected?: string } | null { + if (!Array.isArray(value)) return null; + const options: string[] = []; + let selected: string | undefined; + for (const child of value) { + const key = + typeof child === "string" + ? child + : child && typeof child === "object" + ? Object.keys(child)[0] + : undefined; + const descriptor = key ? parseDescriptor(key) : null; + if (descriptor?.role !== "option" || descriptor.flags.has("ref")) continue; + const name = cutAtCodeUnits(descriptor.name, 200); + if (descriptor.flags.has("selected") && selected === undefined) + selected = name; + if (options.length < OPTION_LIMIT) options.push(name); + } + return options.length ? { options, selected } : null; +} + /** Turn an aria snapshot into the flat element list the tool contract publishes. */ export function parseAriaSnapshot(yaml: string): { elements: SnapshotElement[]; diff --git a/agent-computer/src/enter-text.ts b/agent-computer/src/enter-text.ts new file mode 100644 index 0000000000..5db55ffaf7 --- /dev/null +++ b/agent-computer/src/enter-text.ts @@ -0,0 +1,62 @@ +import type { Locator } from "playwright"; + +/** What every Playwright call here is given: the action timeout, and Stop when there is one. */ +type Acting = { timeout: number; signal?: AbortSignal }; + +/** + * The text a Bot sent cannot go in this field: a list with no option by that name. + * + * The caller's to fix, like a stale ref, so the route answers 400 rather than 502. The message names + * neither the text (a typed value is never echoed) nor the options (they are page content, and page + * content reaches the model only inside the untrusted envelope a snapshot puts it in). + */ +export class FieldValueError extends Error { + constructor(message: string) { + super(message); + this.name = "FieldValueError"; + } +} + +/** + * Put `text` in a field the way a person would. + * + * A text box is filled: `fill` clears it first, which is what "put this value in this box" means. + * A native ``s + * and a custom listbox. A native select's options carry no refs, so they used to be dropped and a Bot + * saw `combobox "Industry"` with nothing it could choose and no way to tell what it was set to. + */ +const DROPDOWNS = `- generic [active] [ref=e1]: + - generic [ref=e2]: + - text: Industry + - combobox "Industry" [ref=e3]: + - option "Select an industry" [selected] + - option "Logistics" + - option "Retail" + - generic [ref=e4]: + - text: Stage + - combobox "Stage" [ref=e5]: + - option "Prospect" [selected] + - option "Qualified" + - listbox "Owner" [ref=e6]: + - option "Alex" [ref=e7]`; + +describe("parseAriaSnapshot, dropdowns", () => { + test("a native select reports its options and the one it shows", () => { + const { elements } = parseAriaSnapshot(DROPDOWNS); + expect(elements.find((element) => element.ref === "e3")).toEqual({ + ref: "e3", + role: "combobox", + name: "Industry", + options: ["Select an industry", "Logistics", "Retail"], + value: "Select an industry", + }); + expect(elements.find((element) => element.ref === "e5")?.value).toBe( + "Prospect", + ); + }); + + test("a custom listbox's options stay elements of their own, listed once", () => { + const { elements } = parseAriaSnapshot(DROPDOWNS); + expect(elements.find((element) => element.ref === "e6")).toEqual({ + ref: "e6", + role: "listbox", + name: "Owner", + }); + expect(elements.find((element) => element.ref === "e7")).toEqual({ + ref: "e7", + role: "option", + name: "Alex", + }); + }); + + test("a very long list is cut to fifty choices", () => { + const many = Array.from( + { length: 120 }, + (_, index) => ` - option "Country ${index}"`, + ).join("\n"); + const { elements } = parseAriaSnapshot( + `- combobox "Country" [ref=e9]:\n${many}`, + ); + expect(elements[0]?.options).toHaveLength(50); + }); +}); diff --git a/agent-computer/tests/enter-text.test.ts b/agent-computer/tests/enter-text.test.ts new file mode 100644 index 0000000000..ac9510cfb0 --- /dev/null +++ b/agent-computer/tests/enter-text.test.ts @@ -0,0 +1,136 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import type { Browser, Page } from "playwright"; +import { parseAriaSnapshot } from "../src/aria-snapshot"; +import { enterText, FieldValueError } from "../src/enter-text"; + +/** + * Typing into a form the way a Bot does: an AI snapshot, then the field by its `aria-ref`. + * + * A CRM's New account form has text boxes and dropdowns side by side. Typing "Logistics" into the + * Industry dropdown used `fill`, which waits on a ` + + +0 +`; + +let browser: Browser; + +beforeAll(async () => { + if (!asked) return; + const { chromium } = await import("playwright"); + browser = await chromium.launch(); +}); + +afterAll(async () => { + await browser?.close(); +}); + +/** The form, and the refs its AI snapshot gives each field, as the computer reads them. */ +async function openForm(): Promise<{ + tab: Page; + refOf: (name: string) => string; + industry: { options?: string[]; value?: string } | undefined; +}> { + const tab = await browser.newPage(); + await tab.setContent(FORM); + const snapshot = await tab.ariaSnapshot({ mode: "ai" }); + const { elements } = parseAriaSnapshot(snapshot); + return { + tab, + industry: elements.find((element) => element.name === "Industry"), + refOf: (name) => { + const found = elements.find((element) => element.name === name); + if (!found) throw new Error(`No ${name} in the snapshot.`); + return found.ref; + }, + }; +} + +describe.skipIf(!asked)("entering text in a form", () => { + test("a dropdown is chosen from by the option's label, and the page sees a change", async () => { + const { tab, refOf } = await openForm(); + await enterText( + tab.locator(`aria-ref=${refOf("Industry")}`), + " logistics ", + ACTING, + ); + expect(await tab.locator("select").inputValue()).toBe("logistics"); + expect(await tab.locator("#changes").textContent()).toBe("1"); + await tab.close(); + }); + + test("a dropdown falls back to the option's value", async () => { + const { tab, refOf } = await openForm(); + await enterText( + tab.locator(`aria-ref=${refOf("Industry")}`), + "fin", + ACTING, + ); + expect(await tab.locator("select").inputValue()).toBe("fin"); + await tab.close(); + }); + + test("text that is not an option, or names a disabled one, is refused and changes nothing", async () => { + const { tab, refOf } = await openForm(); + const industry = tab.locator(`aria-ref=${refOf("Industry")}`); + for (const text of ["Aerospace", "Retired"]) { + const refused = await enterText(industry, text, ACTING).catch( + (error: unknown) => error, + ); + expect(refused).toBeInstanceOf(FieldValueError); + // Never the text it was sent, never the page's own option labels. + expect((refused as Error).message).not.toContain(text); + expect((refused as Error).message).not.toContain("Logistics"); + } + expect(await tab.locator("select").inputValue()).toBe(""); + expect(await tab.locator("#changes").textContent()).toBe("0"); + await tab.close(); + }); + + test("the snapshot the refusal points to lists the dropdown's options and its choice", async () => { + const { tab, industry } = await openForm(); + expect(industry?.options).toEqual([ + "Select an industry", + "Financial services", + "Logistics", + "Retired", + ]); + expect(industry?.value).toBe("Select an industry"); + await tab.close(); + }); + + test("a text box is still filled, replacing what was there", async () => { + const { tab, refOf } = await openForm(); + await enterText( + tab.locator(`aria-ref=${refOf("Account name")}`), + "Navia Freight", + ACTING, + ); + expect(await tab.locator("input[name=name]").inputValue()).toBe( + "Navia Freight", + ); + await tab.close(); + }); +}); diff --git a/app/src/lib/computers/call.ts b/app/src/lib/computers/call.ts index 44ce4f1bca..38357e5696 100644 --- a/app/src/lib/computers/call.ts +++ b/app/src/lib/computers/call.ts @@ -76,26 +76,39 @@ export async function callComputer( */ if (response.status === 403 && run !== null) withdrawComputerActivity(botId, run); - return { - ok: false, - reason: (body?.error as string) ?? "That did not work.", - // Preserve refusal/stale-ref/control distinctions for the model's next step. - ...(response.status === 403 - ? { refused: true, rule: body?.rule ?? null } - : {}), - ...(response.status === 409 - ? body?.humanHasControl === true - ? { - humanHasControl: true, - requestId: body?.requestId, - handoff: body?.handoff, - } - : body?.snapshotRequired === true - ? { staleRefs: true, snapshotRequired: true } - : { staleRefs: true } - : {}), - }; + return failedComputerCall(response.status, body); } return { ok: true, ...(body ?? {}) }; } + +/** + * What a Bot is told when a computer call came back refused or failed. + * + * A result, never a thrown error. The SDK starts no follow-up run after a frontend tool throws, so a + * throw ends the Bot's turn on the spot with nothing said; a result reaches the model, which can take + * a fresh snapshot, pick another field, or tell the person what stopped it. Shared with the approval + * gate's wrapper, which reaches the same computer by another route and must answer the same way. + */ +export function failedComputerCall( + status: number, + body: Record | null, +): ToolOutcome { + return { + ok: false, + reason: typeof body?.error === "string" ? body.error : "That did not work.", + // Preserve refusal/stale-ref/control distinctions for the model's next step. + ...(status === 403 ? { refused: true, rule: body?.rule ?? null } : {}), + ...(status === 409 + ? body?.humanHasControl === true + ? { + humanHasControl: true, + requestId: body?.requestId, + handoff: body?.handoff, + } + : body?.snapshotRequired === true + ? { staleRefs: true, snapshotRequired: true } + : { staleRefs: true } + : {}), + }; +} diff --git a/app/src/lib/copilot/approval-tools.tsx b/app/src/lib/copilot/approval-tools.tsx index c6eeb111e8..ca7014b84e 100644 --- a/app/src/lib/copilot/approval-tools.tsx +++ b/app/src/lib/copilot/approval-tools.tsx @@ -7,6 +7,7 @@ import { useQueryClient } from "@tanstack/react-query"; import type { ReactNode } from "react"; import { InlineApproval } from "@/components/approvals/inline-approval"; import { tryClient } from "@/lib/client"; +import { failedComputerCall } from "@/lib/computers/call"; import { useActiveBotHolder } from "./active-bot"; import { frontendApprovalSnapshot } from "./approval-context"; @@ -69,11 +70,22 @@ export function useFrontendTool>( // run it when nothing asks, and says why when something does. return tool.handler?.(args, context); } - const response = await tryClient( - `/api/approvals/computer/${encodeURIComponent(bot.current)}`, - { method: "POST", body: snapshot, signal: context.signal }, - ); - const payload: unknown = await response.json(); + let response: Response; + try { + response = await tryClient( + `/api/approvals/computer/${encodeURIComponent(bot.current)}`, + { method: "POST", body: snapshot, signal: context.signal }, + ); + } catch (error) { + // The same two answers callComputer gives when the request itself does not complete. + if (error instanceof DOMException && error.name === "AbortError") + return { ok: false, reason: "Stopped.", stopped: true }; + return { + ok: false, + reason: "The assistant's computer could not be reached.", + }; + } + const payload: unknown = await response.json().catch(() => null); if (response.status === 202) { if (!context.agent) throw new Error("The interrupted conversation is unavailable."); @@ -84,17 +96,19 @@ export function useFrontendTool>( void queryClient.invalidateQueries({ queryKey: ["approvals"] }); return { waiting: true }; } + // Answered, not thrown: a thrown handler ends the Bot's turn with nothing said (see + // failedComputerCall), where a failed result lets it snapshot again or try another way. if (!response.ok) - throw new Error( - payload && - typeof payload === "object" && - "error" in payload && - typeof payload.error === "string" - ? payload.error - : "The computer action could not be approved.", + return failedComputerCall( + response.status, + payload && typeof payload === "object" + ? (payload as Record) + : null, ); if (!payload || typeof payload !== "object" || !("result" in payload)) - throw new Error("The computer action returned no result."); + return failedComputerCall(response.status, { + error: "The computer action returned no result.", + }); return payload.result; } : undefined, diff --git a/app/tests/approval-tools.test.tsx b/app/tests/approval-tools.test.tsx index 8945763f53..77c881efbc 100644 --- a/app/tests/approval-tools.test.tsx +++ b/app/tests/approval-tools.test.tsx @@ -155,3 +155,121 @@ test("a change goes to the server's gate even when nothing about approvals has l global.fetch = originalFetch; } }); + +/** + * Runs the kept computer_click handler against a gate that answers with `answer`. + * + * The SDK starts no follow-up run after a frontend tool throws (core's executeSpecificTool returns + * false on a handler error), so a handler that throws ends the Bot's turn on the spot with nothing + * said. A form field the computer could not fill did exactly that: the Bot opened a CRM form, failed + * on one dropdown, and the conversation simply stopped. + */ +async function clickAgainstGate( + answer: () => Promise, +): Promise { + const originalFetch = global.fetch; + global.fetch = Object.assign(async () => answer(), { + preconnect: originalFetch.preconnect, + }); + try { + render( + + + , + ); + const tool = registered.find((entry) => entry.name === "computer_click"); + const toolCall = { + id: "click-call", + type: "function" as const, + function: { + name: "computer_click", + arguments: '{"ref":"e7","snapshotId":3}', + }, + }; + const subscribers: { + onRunInitialized?: (input: { input: unknown }) => void; + }[] = []; + const agent = { + threadId: "thread", + state: {}, + messages: [ + { id: "u1", role: "user", content: "Submit the form" }, + { 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 { + return await tool?.handler?.({ ref: "e7", snapshotId: 3 }, { + agent, + toolCall, + } as never); + } finally { + stop(); + } + } finally { + global.fetch = originalFetch; + } +} + +test("a change the computer could not make comes back to the Bot as a result, not a throw", async () => { + const result = await clickAgainstGate(async () => + Response.json( + { + error: + "Element e7 is not on the page any more. Take a fresh snapshot and use the refs from it.", + }, + { status: 409 }, + ), + ); + expect(result).toEqual({ + ok: false, + reason: + "Element e7 is not on the page any more. Take a fresh snapshot and use the refs from it.", + staleRefs: true, + }); +}); + +test("a refused change comes back as a refusal the Bot can explain", async () => { + const result = await clickAgainstGate(async () => + Response.json( + { error: "Your team does not allow this.", rule: "team-rule-1" }, + { status: 403 }, + ), + ); + expect(result).toEqual({ + ok: false, + reason: "Your team does not allow this.", + refused: true, + rule: "team-rule-1", + }); +}); + +test("a gate that cannot be reached, or answers with no result, is still a result", async () => { + expect( + await clickAgainstGate(async () => { + throw new TypeError("fetch failed"); + }), + ).toEqual({ + ok: false, + reason: "The assistant's computer could not be reached.", + }); + expect( + await clickAgainstGate(async () => Response.json({}, { status: 200 })), + ).toEqual({ ok: false, reason: "The computer action returned no result." }); +}); diff --git a/server/src/computer/schema.ts b/server/src/computer/schema.ts index 4836eaed94..62da7af707 100644 --- a/server/src/computer/schema.ts +++ b/server/src/computer/schema.ts @@ -133,6 +133,8 @@ export type SnapshotElement = { type?: string; disabled?: boolean; checked?: boolean; + /** A native dropdown's choices, by label. A Bot chooses one by typing it into the dropdown. */ + options?: string[]; }; export type SnapshotResult = {