From 9cb559f860eebb8463c239841a953de802e9d329 Mon Sep 17 00:00:00 2001 From: David McKay Date: Thu, 12 Nov 2026 16:34:36 -0800 Subject: [PATCH 1/3] Fix five bugs the computer and approvals journeys found - Activity names a declined or handed-over step's control from the snapshot its ref came from, not the ref ("Declined Place order"). - Every Activity row has its time: a step's answer says when it finished (stamped by the server for the steps it runs after an approval, and by the browser for its own), and a step the person answered is timed by their answer. - The Bot's screen fills its frame after a tab closes and after Hand back: the window is fitted to the page the live screen starts showing, so Chromium's --no-sandbox warning bar no longer cuts 56 px off the tab left in front. - The Reset and Update computer dialogs keep their row while they close, so the title no longer falls back to "Reset the computer?". - The masked secret box asks for the field by its ref: the instructions no longer tell the Bot to click the field first, which handed the step to the person. Each fix has a journey assertion that fails on main, plus unit tests where the rule is cheap to pin. New computer-secret journey, recorded against the real model because the Bot's instructions changed. --- agent-computer/src/bot-browser-host.ts | 2 + agent-computer/src/index.ts | 12 +- agent-computer/src/profiles.ts | 9 + agent-computer/src/recorder-in-browser.ts | 47 +-- .../components/approvals/inline-approval.tsx | 4 +- app/src/components/computer/activity-log.tsx | 3 +- app/src/components/manage/computers-page.tsx | 72 +++-- app/src/lib/computers/activity.ts | 151 ++++++++-- app/src/lib/copilot/approval-tools.tsx | 18 +- app/src/lib/copilot/computer-tools.tsx | 5 +- .../computer-activity-names-and-times.test.ts | 274 ++++++++++++++++++ app/tests/personal-settings.test.tsx | 24 +- e2e/README.md | 7 +- ...nai-2026-11-13T00-04-55-037Z-51ce2bd9.json | 31 ++ ...nai-2026-11-13T00-04-57-413Z-a8059788.json | 76 +++++ ...nai-2026-11-13T00-05-03-056Z-b4352ecf.json | 39 +++ ...nai-2026-11-13T00-05-07-952Z-dc12df55.json | 59 ++++ ...nai-2026-11-13T00-05-15-061Z-7bd22944.json | 54 ++++ ...nai-2026-11-13T00-05-16-803Z-50396085.json | 34 +++ ...nai-2026-11-13T00-05-25-959Z-12d29145.json | 47 +++ ...nai-2026-11-13T00-05-38-758Z-ccc2f2d7.json | 37 +++ ...nai-2026-11-13T00-05-48-192Z-c2cbbfd5.json | 42 +++ e2e/journeys/approvals-allow-once.journey.ts | 20 ++ e2e/journeys/computer-secret.journey.ts | 94 ++++++ e2e/journeys/computer-sign-in.journey.ts | 9 + e2e/journeys/computer-steps.ts | 58 ++++ e2e/journeys/computer-take-control.journey.ts | 23 ++ server/src/computer/headless-tools.ts | 9 +- .../computer-secret-without-a-click.test.ts | 73 +++++ server/tests/headless-tools.test.ts | 28 ++ shared/bot-prompt.ts | 5 +- shared/computer-control.ts | 33 +++ 32 files changed, 1315 insertions(+), 84 deletions(-) create mode 100644 app/tests/computer-activity-names-and-times.test.ts create mode 100644 e2e/fixtures/computer-secret/openai-2026-11-13T00-04-55-037Z-51ce2bd9.json create mode 100644 e2e/fixtures/computer-secret/openai-2026-11-13T00-04-57-413Z-a8059788.json create mode 100644 e2e/fixtures/computer-secret/openai-2026-11-13T00-05-03-056Z-b4352ecf.json create mode 100644 e2e/fixtures/computer-secret/openai-2026-11-13T00-05-07-952Z-dc12df55.json create mode 100644 e2e/fixtures/computer-secret/openai-2026-11-13T00-05-15-061Z-7bd22944.json create mode 100644 e2e/fixtures/computer-secret/openai-2026-11-13T00-05-16-803Z-50396085.json create mode 100644 e2e/fixtures/computer-secret/openai-2026-11-13T00-05-25-959Z-12d29145.json create mode 100644 e2e/fixtures/computer-secret/openai-2026-11-13T00-05-38-758Z-ccc2f2d7.json create mode 100644 e2e/fixtures/computer-secret/openai-2026-11-13T00-05-48-192Z-c2cbbfd5.json create mode 100644 e2e/journeys/computer-secret.journey.ts create mode 100644 server/tests/computer-secret-without-a-click.test.ts diff --git a/agent-computer/src/bot-browser-host.ts b/agent-computer/src/bot-browser-host.ts index 7917020b9c..3aad2c842e 100644 --- a/agent-computer/src/bot-browser-host.ts +++ b/agent-computer/src/bot-browser-host.ts @@ -458,6 +458,8 @@ export function createBotBrowserHost(options: BotBrowserHostOptions) { ? await bar.navigate(browser, navigate.value) : ({ ok: true } as const); if (!done.ok) return json({ error: done.error }, done.status); + // The tab now in front, fitted at once, so the screen never shows it short (see the live screen's `attach`). + if (tabs?.ok) await view.fitWindow(view.active()); const after = profiles.bot(botId); return json(after ? await stateOf(botId, after) : closedState); } diff --git a/agent-computer/src/index.ts b/agent-computer/src/index.ts index bc1b8321f6..c8e95d3450 100644 --- a/agent-computer/src/index.ts +++ b/agent-computer/src/index.ts @@ -1024,6 +1024,15 @@ serve({ const attach = async () => { const target = await currentPage(ws.data.botId); if (target === casting) return; + /* + * THE WINDOW FITTED TO THE PAGE THE SCREEN IS ABOUT TO SHOW, every time it starts showing one. A tab laid out + * again (in front again after a person closed the one before it) takes what its window has then, and a window + * fitted for another tab can have less: Chromium's warning bar for `--no-sandbox` sits in a Bot's first tab and + * is only made room for when that tab is laid out again, so it came back 56 pixels short and the screen showed + * a black band under the page, after Hand back too. Fitted here, from the tab as it is now laid out, it holds + * the whole page. A Bot's browser only: a person's is fitted as its tabs open, and local Chrome is theirs. + */ + await profiles.bot(ws.data.botId)?.fitWindow(target); const cast = await startScreencast(target, send); // The claim stops a cast it refuses, so a launch that lost the screen leaks nothing. if (!(await claim.install(cast))) return; @@ -1307,7 +1316,8 @@ serve({ return json(state); } - // The Bot asking for one value it must not be told. It has already focused the field. + // The Bot asking for one value it must not be told, naming the field by its ref. It does not click it first: + // the value is put into that field below, when the person sends it. if (url.pathname === "/control/secret" && request.method === "POST") { const body = (await request.json().catch(() => null)) as { label?: unknown; diff --git a/agent-computer/src/profiles.ts b/agent-computer/src/profiles.ts index 35c38c2e6b..7ca15a06c9 100644 --- a/agent-computer/src/profiles.ts +++ b/agent-computer/src/profiles.ts @@ -83,6 +83,7 @@ import { seatIdsIn } from "./profile-listing"; import { findRecorder, isRecorderPage, type Recorder } from "./recorder"; import { fitWindowsToViewport, + fitWindowToViewport, holdFilterCredentials, keepRecorderCurrent, reachRecorder, @@ -489,6 +490,8 @@ export type OpenBotBrowser = { * `fitWindowsToViewport`). Not at launch: a Bot's browser nobody works stays exactly as it launches. */ fitWindows(): Promise; + /** Fit the window to this tab now, as it is laid out (see `fitWindowToViewport`). */ + fitWindow(page: Page): Promise; }; /** A person's open browser, for the browser routes. */ @@ -556,6 +559,7 @@ export function createProfiles( pin(page: Page): void; session(): Promise; fitWindows(): Promise; + fitWindow(page: Page): Promise; }; }; @@ -1251,6 +1255,10 @@ export function createProfiles( fitted ??= fitWindowsToViewport(context, VIEWPORT, botId); return fitted; }, + fitWindow: (shown: Page) => + local + ? Promise.resolve() + : fitWindowToViewport(shown, VIEWPORT, botId), }, }; touchTab(page); @@ -1532,6 +1540,7 @@ export function createProfiles( .pages() .filter((page) => !page.isClosed() && bot.own(page)), fitWindows: bot.fitWindows, + fitWindow: bot.fitWindow, }; }, diff --git a/agent-computer/src/recorder-in-browser.ts b/agent-computer/src/recorder-in-browser.ts index 30743a7559..ae1ea134f4 100644 --- a/agent-computer/src/recorder-in-browser.ts +++ b/agent-computer/src/recorder-in-browser.ts @@ -121,30 +121,41 @@ export async function fitWindowsToViewport( viewport: { width: number; height: number }, botId: string, ): Promise { - const fit = async (page: Page) => { - const session = await context.newCDPSession(page); + context.on("page", (page) => void fitWindowToViewport(page, viewport, botId)); + await Promise.all( + context.pages().map((page) => fitWindowToViewport(page, viewport, botId)), + ); +} + +/** + * Make the window hold exactly this tab's page, now. Fitting is measured from the tab as it is laid + * out at that moment, so fitting again corrects a window fitted for a tab laid out differently. + */ +export async function fitWindowToViewport( + page: Page, + viewport: { width: number; height: number }, + botId: string, +): Promise { + try { + const session = await page.context().newCDPSession(page); try { const { windowId } = await session.send("Browser.getWindowForTarget"); await session.send("Browser.setContentsSize", { windowId, ...viewport }); } finally { await session.detach().catch(() => undefined); } - }; - const fitting = (page: Page) => - fit(page).catch((error: unknown) => { - // A tab that closed while it was being fitted needs nothing; anything else is said, because - // the live screen then shows less than the whole page. - if (page.isClosed()) return; - console.info( - JSON.stringify({ - type: "computer-window-not-fitted", - botId, - error: error instanceof Error ? error.message : String(error), - }), - ); - }); - context.on("page", (page) => void fitting(page)); - await Promise.all(context.pages().map(fitting)); + } catch (error) { + // A tab that closed while it was being fitted needs nothing; anything else is said, because + // the live screen then shows less than the whole page. + if (page.isClosed()) return; + console.info( + JSON.stringify({ + type: "computer-window-not-fitted", + botId, + error: error instanceof Error ? error.message : String(error), + }), + ); + } } /** diff --git a/app/src/components/approvals/inline-approval.tsx b/app/src/components/approvals/inline-approval.tsx index 09aa5fd215..aa52504aec 100644 --- a/app/src/components/approvals/inline-approval.tsx +++ b/app/src/components/approvals/inline-approval.tsx @@ -10,7 +10,7 @@ import { approvalInboxOptions, decideApproval, } from "@/lib/approvals"; -import { noteWatched } from "@/lib/computers/activity"; +import { noteAnswered } from "@/lib/computers/activity"; import { EASE_OUT } from "@/lib/motion"; /** @@ -80,7 +80,7 @@ export function InlineApproval({ onSuccess: (_, choice) => { setAnswered(choice); // Answered here, so the step's entry in Activity says when, even on a page opened after it was asked. - if (toolCallId) noteWatched(toolCallId); + if (toolCallId) noteAnswered(toolCallId); return cache.invalidateQueries({ queryKey: ["approvals"] }); }, }); diff --git a/app/src/components/computer/activity-log.tsx b/app/src/components/computer/activity-log.tsx index fcfeecbca5..45ddb0388a 100644 --- a/app/src/components/computer/activity-log.tsx +++ b/app/src/components/computer/activity-log.tsx @@ -9,8 +9,7 @@ * * Read from the conversation, so it is the same whichever way a call ran: in the browser, at the * server's gate, or after a person allowed it. A conversation opened again, after a reload too, - * shows what was already done in it. What this page watched happen says when; what was done before - * the page opened it does not, because when it ran is not something the conversation says. The + * shows what was already done in it, and when: each step's answer says when it finished. The * record is still the audit trail on the server; this is the window beside the screen. */ import { ownToolLabel } from "@openbot/shared/tool-name"; diff --git a/app/src/components/manage/computers-page.tsx b/app/src/components/manage/computers-page.tsx index 9b0015074f..828dbc8f77 100644 --- a/app/src/components/manage/computers-page.tsx +++ b/app/src/components/manage/computers-page.tsx @@ -68,6 +68,29 @@ function isOwnBrowser(computer: ComputerProfile): boolean { return computer.botId !== undefined && isPersonBrowserId(computer.botId); } +/** + * What a confirmation is about, kept while its dialog closes. + * + * The dialog fades out after Cancel or Reset it, and its words are drawn on every frame of that. + * Cleared on close, the row went with it, and the title fell back to its words for no row: "Reset the + * computer?" flashed up as the dialog left after "Reset Fernhill Guide's browser?" was answered. So + * closing only closes, and what the dialog names is replaced the next time it asks. + */ +function useConfirmation() { + const [asking, setAsking] = useState<{ + about: ComputerProfile; + open: boolean; + } | null>(null); + return { + /** The row asked about, still named while the dialog closes. */ + about: asking?.about ?? null, + open: asking?.open === true, + ask: (about: ComputerProfile) => setAsking({ about, open: true }), + close: () => + setAsking((current) => (current ? { ...current, open: false } : null)), + }; +} + /** * `children` are sections only the team edition's Admin adds below the page's own (the admin's * controls for these). The personal edition's Settings renders this page with none. @@ -97,10 +120,11 @@ export function ComputersPage({ /** The row currently running a stop/reset request. */ const [busy, setBusy] = useState(null); /** Reset deletes the browser profiles, so it requires confirmation. */ - const [confirming, setConfirming] = useState(null); + const reset = useConfirmation(); + const confirming = reset.about; /** Updating restarts a running computer mid-task, so it is confirmed too. */ - const [confirmingUpdate, setConfirmingUpdate] = - useState(null); + const update = useConfirmation(); + const confirmingUpdate = update.about; const queryClient = useQueryClient(); const nameFor = useBotNames(); /* @@ -156,8 +180,8 @@ export function ComputersPage({ action: "stop" | "reset" | "update", ) => { setBusy(rowId(computer)); - setConfirming(null); - setConfirmingUpdate(null); + reset.close(); + update.close(); setState.mutate( { action, @@ -347,7 +371,7 @@ export function ComputersPage({ {computer.updateAvailable ? ( @@ -456,9 +478,9 @@ export function ComputersPage({ { - if (!open) setConfirmingUpdate(null); + if (!open) update.close(); }} - open={confirmingUpdate !== null} + open={update.open} > @@ -472,23 +494,23 @@ export function ComputersPage({ - diff --git a/app/src/lib/computers/activity.ts b/app/src/lib/computers/activity.ts index ac28202722..57634422cf 100644 --- a/app/src/lib/computers/activity.ts +++ b/app/src/lib/computers/activity.ts @@ -48,9 +48,12 @@ export type ComputerActivity = { /** The tool call it was. A call read again is this same entry, never a second one. */ id: string; /** - * When it finished, for the timestamp beside it. Only for a call this page watched: one it started - * running, or one the person answered here. One already done when its conversation was opened says - * what was done, not when: the time the page read it would be a guess drawn as a fact. + * When it finished, for the timestamp beside it. From the step's own answer, which says when it + * finished (`finishedAt`, see `withFinishedAt` in `shared/computer-control.ts`), whether the + * browser ran it or the server did after an approval. Otherwise only for a call this page watched: + * one the person answered here (when they answered), or one it started running. A step whose + * answer is a sentence, or from before answers said when, opened later says what was done, not + * when: the time the page read it would be a guess drawn as a fact. */ at?: number; kind: "command" | "read_file" | "write_file" | "list_files" | "browser"; @@ -84,8 +87,15 @@ export type ComputerActivity = { /** What a call did, without the bookkeeping the pane adds to it. */ export type ActivityDetails = Omit; -/** One computer call in a conversation: what it did, or null while it waits for its answer. */ -export type ConversationCall = { id: string; did: ActivityDetails | null }; +/** + * One computer call in a conversation: what it did, or null while it waits for its answer, and when + * it finished when its answer says so. + */ +export type ConversationCall = { + id: string; + did: ActivityDetails | null; + finishedAt?: number; +}; /** * The most recent entries per computer. @@ -140,6 +150,12 @@ const WIPED_KEPT = 1000; * was stamped with the time the page was loaded, a different one on each reload. */ const asked = new Set(); +/** + * When the person answered a step held for them here: when a step they declined or did themselves + * happened. Its answer reaches this page later, once the turn the server carried on with is over, and + * timed by that arrival it read as after the steps the server took following it. + */ +const answeredHere = new Map(); const listeners = new Set<() => void>(); /** A stable empty array, so `useSyncExternalStore` does not see a new value on every render. */ @@ -302,13 +318,15 @@ export function recordActivity( if (!call?.did) continue; const existing = present.get(call.id); if (existing) { - if (!sameDetails(existing, call.did)) { + // The server's answer can arrive after the browser's placeholder, with when it finished. + const at = existing.at ?? call.finishedAt; + if (!sameDetails(existing, call.did) || at !== existing.at) { next ??= [...current]; const position = next.findIndex((entry) => entry.id === call.id); next[position] = { ...call.did, id: call.id, - ...(existing.at === undefined ? {} : { at: existing.at }), + ...(at === undefined ? {} : { at }), }; } after = call.id; @@ -319,7 +337,12 @@ export function recordActivity( // A conversation that cannot place the wipe: only what this page watched is from after it. if (since === null && !asked.has(call.id)) continue; next ??= [...current]; - const at = asked.delete(call.id) ? Date.now() : undefined; + const watched = asked.delete(call.id); + const answeredAt = answeredHere.get(call.id); + answeredHere.delete(call.id); + // When the step says it finished, when the person answered it here, or when this page saw its answer arrive. + const at = + call.finishedAt ?? answeredAt ?? (watched ? Date.now() : undefined); const entry: ComputerActivity = { ...call.did, id: call.id, @@ -353,6 +376,12 @@ export function noteWatched(toolCallId: string): void { asked.add(toolCallId); } +/** A step held for the person, answered by them here, now. Watched, and timed by the answer. */ +export function noteAnswered(toolCallId: string): void { + asked.add(toolCallId); + answeredHere.set(toolCallId, Date.now()); +} + export function activityFor(computerId: string): ComputerActivity[] { return byComputer.get(computerId) ?? NONE; } @@ -416,15 +445,22 @@ function textOf(value: unknown): string | undefined { /** * What one answered browser step did, as the pane draws it. * - * The subject is what it acted on: the page's address, the element's label (its ref when the computer - * named none), the key. NEVER WHAT WAS TYPED. The computer declines to echo a field's value back for - * that reason, since a form is where a password or a card number goes, and the arguments that carry - * it are not read here. + * The subject is what it acted on: the page's address, the element's label, the key. NEVER WHAT WAS + * TYPED. The computer declines to echo a field's value back for that reason, since a form is where a + * password or a card number goes, and the arguments that carry it are not read here. + * + * THE LABEL, EVEN FOR A STEP THAT DID NOT RUN. The computer names the element in its answer, and a + * step the person declined or did themselves has no such answer, only a sentence. It drew the ref + * instead ("Declined f2e13", "Clicked on page e14"), a handle only the Bot can read. The snapshot the + * Bot took the ref from names it too (`label`, see `snapshotLabelsOf`), so that name is used, and the + * ref only when neither says. */ export function browserDetails( step: BrowserStepName, args: Record, result: ToolOutcome, + /** What the snapshot the ref came from calls the element, when the step's own answer does not. */ + label?: string, ): ActivityDetails { const notTheBot = { ...(result.refused === true ? { refused: true } : {}), @@ -440,7 +476,7 @@ export function browserDetails( step === "computer_navigate" ? (page ?? textOf(args.url) ?? "") : step === "computer_click" || step === "computer_type" - ? (elementName(result) ?? textOf(args.ref) ?? "") + ? (elementName(result) ?? label ?? textOf(args.ref) ?? "") : step === "computer_key" ? (textOf(args.key) ?? "") : step === "computer_scroll" @@ -539,7 +575,13 @@ const answers = new Map< >(); const done = new Map< string, - { args: string; answer: ToolOutcome; did: ActivityDetails } + { + args: string; + answer: ToolOutcome; + did: ActivityDetails; + /** A browser step's arguments, read, and the label its snapshot gives its ref. */ + browser?: { args: Record; label: string | undefined }; + } >(); function answerOf(message: { @@ -577,21 +619,79 @@ function browserDidOf( step: BrowserStepName, call: { id: string; function: { arguments: string } }, answer: ToolOutcome, + labels: SnapshotLabels, ): ActivityDetails { const known = done.get(call.id); + const sameArgs = known?.args === call.function.arguments; + const args = + sameArgs && known?.browser + ? known.browser.args + : argumentsOf(call.function.arguments); + const label = labelFrom(labels, args); if ( known && + sameArgs && known.answer === answer && - known.args === call.function.arguments + known.browser?.label === label ) return known.did; return remember(done, call.id, { args: call.function.arguments, answer, - did: browserDetails(step, argumentsOf(call.function.arguments), answer), + browser: { args, label }, + did: browserDetails(step, args, answer, label), }).did; } +/** + * The label each snapshot gave each ref, by the snapshot's id: `{ snapshotId, elements: [{ ref, + * name }] }` is what `computer_snapshot` answers (`SnapshotResult` in `server/src/computer/schema.ts`). + */ +type SnapshotLabels = Map>; + +/** Each snapshot's labels, read once per answer: the conversation is read on every change. */ +const labelled = new WeakMap< + ToolOutcome, + { snapshotId: number; labels: Map } | null +>(); + +function snapshotLabelsOf( + answer: ToolOutcome, +): { snapshotId: number; labels: Map } | null { + const known = labelled.get(answer); + if (known !== undefined) return known; + let read: { snapshotId: number; labels: Map } | null = null; + if (typeof answer.snapshotId === "number" && Array.isArray(answer.elements)) { + const labels = new Map(); + for (const element of answer.elements) { + if (!element || typeof element !== "object") continue; + const { ref, name } = element as { ref?: unknown; name?: unknown }; + const label = textOf(name); + if (typeof ref === "string" && label) labels.set(ref, label); + } + read = { snapshotId: answer.snapshotId, labels }; + } + labelled.set(answer, read); + return read; +} + +/** What the snapshot a step's ref came from calls that element. */ +function labelFrom( + labels: SnapshotLabels, + args: Record, +): string | undefined { + if (typeof args.ref !== "string" || typeof args.snapshotId !== "number") + return undefined; + return labels.get(args.snapshotId)?.get(args.ref); +} + +/** When the server finished a step it ran, from its answer (`finishedAt`), as a time. */ +function finishedAtOf(answer: ToolOutcome | undefined): number | undefined { + if (typeof answer?.finishedAt !== "string") return undefined; + const at = Date.parse(answer.finishedAt); + return Number.isFinite(at) ? at : undefined; +} + function argumentsOf(text: string): Record { try { const parsed: unknown = JSON.parse(text); @@ -770,13 +870,16 @@ export function conversationActivity( ): ConversationCall[] { const computerCalls = new Set(); const browserCalls = new Set(); + const snapshots = new Set(); const askingAbout = new Map(); for (const message of messages) { if (message.role !== "assistant") continue; for (const call of message.toolCalls ?? []) { if (KINDS[call.function.name]) computerCalls.add(call.id); - else if (isBrowserStep(call.function.name)) browserCalls.add(call.id); - else if (call.function.name === ASK_APPROVAL) { + else if (isBrowserStep(call.function.name)) { + browserCalls.add(call.id); + if (call.function.name === "computer_snapshot") snapshots.add(call.id); + } else if (call.function.name === ASK_APPROVAL) { const command = commandAskedAbout(call); if (command) askingAbout.set(call.id, command); } @@ -785,6 +888,7 @@ export function conversationActivity( // Only the computer's answers, and the person's on those cards: another tool's can be long, and nothing here reads it. const answered = new Map(); const decided = new Map(); + const labels: SnapshotLabels = new Map(); for (const message of messages) { if (message.role !== "tool") continue; if ( @@ -793,6 +897,11 @@ export function conversationActivity( ) { const answer = answerOf(message); if (answer) answered.set(message.toolCallId, answer); + const named = + answer && snapshots.has(message.toolCallId) + ? snapshotLabelsOf(answer) + : null; + if (named) labels.set(named.snapshotId, named.labels); } else if (askingAbout.has(message.toolCallId)) { const decision = decisionIn(message.content); if (decision) decided.set(message.toolCallId, decision); @@ -816,18 +925,22 @@ export function conversationActivity( if (isBrowserStep(name)) { const answer = answered.get(call.id); if (answer && awaitingFreshLook(answer)) continue; + const finishedAt = finishedAtOf(answer); calls.push({ id: call.id, - did: answer ? browserDidOf(name, call, answer) : null, + did: answer ? browserDidOf(name, call, answer, labels) : null, + ...(finishedAt === undefined ? {} : { finishedAt }), }); continue; } const kind = KINDS[name]; if (!kind) continue; const answer = answered.get(call.id); + const finishedAt = finishedAtOf(answer); calls.push({ id: call.id, did: answer ? didOf(kind, call, answer) : null, + ...(finishedAt === undefined ? {} : { finishedAt }), }); } } diff --git a/app/src/lib/copilot/approval-tools.tsx b/app/src/lib/copilot/approval-tools.tsx index 8ca8472c1c..5e667b52ab 100644 --- a/app/src/lib/copilot/approval-tools.tsx +++ b/app/src/lib/copilot/approval-tools.tsx @@ -3,6 +3,7 @@ import { useCopilotKit, useFrontendTool as useSDKFrontendTool, } from "@copilotkit/react-core/v2"; +import { withFinishedAt } from "@openbot/shared/computer-control"; import { type QueryClient, useQueryClient } from "@tanstack/react-query"; import type { ReactNode } from "react"; import { InlineApproval } from "@/components/approvals/inline-approval"; @@ -119,13 +120,16 @@ export function useFrontendTool>( * to the computer. */ if (!tool.name.startsWith("computer_")) return gated(args, context); - return inStepOrder( - { - computerId: botOf(context.agent), - messages: context.agent?.messages, - toolCallId: context.toolCall?.id, - }, - () => gated(args, context), + // When it finished, kept in the answer for Activity (`withFinishedAt`), whichever way it ran. + return withFinishedAt( + await inStepOrder( + { + computerId: botOf(context.agent), + messages: context.agent?.messages, + toolCallId: context.toolCall?.id, + }, + () => gated(args, context), + ), ); } : undefined, diff --git a/app/src/lib/copilot/computer-tools.tsx b/app/src/lib/copilot/computer-tools.tsx index 40d8411555..f9ef672c32 100644 --- a/app/src/lib/copilot/computer-tools.tsx +++ b/app/src/lib/copilot/computer-tools.tsx @@ -1,4 +1,5 @@ import { outcomeForPerson } from "@openbot/shared/approval-outcomes"; +import { SECRET_FIELD_BY_REF } from "@openbot/shared/computer-control"; import { z } from "zod"; import { ToolLine } from "@/components/channels/tool-line"; import { @@ -570,8 +571,8 @@ export function ComputerTools() { name: "computer_request_secret", description: "Ask the person for a single value you must not be told: a password, a one-time code, a card number. " + - "Focus the field first with computer_click, then call this with the ref of that field and a " + - "short label for what you need. They type it into a masked box that goes straight to the page. " + + `${SECRET_FIELD_BY_REF} Add a short label for what you need. ` + + "They type it into a masked box that goes straight to the page. " + "You will never see the value, and you must not ask for it any other way. Prefer this over a " + "full takeover when you only need one field filled in. The value is only TYPED into the field: " + "if the form needs submitting, do that yourself afterwards with computer_click.", diff --git a/app/tests/computer-activity-names-and-times.test.ts b/app/tests/computer-activity-names-and-times.test.ts new file mode 100644 index 0000000000..6be9963233 --- /dev/null +++ b/app/tests/computer-activity-names-and-times.test.ts @@ -0,0 +1,274 @@ +import { expect, test } from "bun:test"; +import type { Message } from "@ag-ui/core"; +import { + DECLINED_BY_PERSON, + HANDLED_BY_PERSON, +} from "@openbot/shared/approval-outcomes"; +import { withFinishedAt } from "@openbot/shared/computer-control"; +import { + activityFor, + conversationActivity, + noteAnswered, + recordActivity, +} from "../src/lib/computers/activity"; + +/** + * What Activity says about a step that went through an approval, as the end-to-end journeys found it. + * + * A click the person declined, or one handed to them that they did themselves, came back as a sentence + * rather than the computer's answer, so nothing named the button and the row showed the Bot's ref: + * "Declined f2e13", "Clicked on page e14". And the steps the server took after the person answered + * never passed through the browser, so their rows had no time beside them. The conversations here are + * the ones those journeys made: a checkout's Place order denied, a sign-in's password handed over. + */ + +let computers = 0; +function computer(): string { + computers += 1; + return `names-and-times-${computers}`; +} + +/** One assistant message with one call, and its answer. */ +function step( + id: string, + name: string, + args: Record, + answer: string | Record, +): Message[] { + return [ + { + id: `assistant-${id}`, + role: "assistant", + toolCalls: [ + { + id, + type: "function", + function: { name, arguments: JSON.stringify(args) }, + }, + ], + }, + { + id: `tool-${id}`, + role: "tool", + toolCallId: id, + content: typeof answer === "string" ? answer : JSON.stringify(answer), + }, + ]; +} + +/** The checkout's snapshot, as the computer answered it: the Place order button is `f2e13` in snapshot 4. */ +const checkout = step( + "snapshot", + "computer_snapshot", + {}, + { + ok: true, + snapshotId: 4, + url: "http://host.docker.internal:46600/shop/basil", + title: "Checkout: Genovese basil seeds", + truncated: false, + elements: [ + { ref: "f2e5", role: "link", name: "Home" }, + { ref: "f2e13", role: "button", name: "Place order" }, + ], + }, +); + +/** The members' sign-in, where the password field is `e14` in snapshot 2. */ +const signIn = step( + "members-snapshot", + "computer_snapshot", + {}, + { + ok: true, + snapshotId: 2, + url: "http://host.docker.internal:46600/members", + title: "Members sign in", + truncated: false, + elements: [ + { ref: "e12", role: "textbox", name: "Username" }, + { ref: "e14", role: "textbox", name: "Password" }, + ], + }, +); + +function didOf(messages: Message[], id: string) { + return conversationActivity(messages).find((call) => call.id === id)?.did; +} + +test("a declined click names the button it would have pressed, as a click that ran does", () => { + const declined = didOf( + [ + ...checkout, + ...step( + "place-order", + "computer_click", + { ref: "f2e13", snapshotId: 4 }, + DECLINED_BY_PERSON, + ), + ], + "place-order", + ); + expect(declined).toMatchObject({ + step: "computer_click", + subject: "Place order", + declined: true, + output: "You declined it.", + }); +}); + +test("a click handed to the person and done by them names the field", () => { + const handed = didOf( + [ + ...signIn, + ...step( + "password", + "computer_click", + { ref: "e14", snapshotId: 2 }, + HANDLED_BY_PERSON, + ), + ], + "password", + ); + expect(handed).toMatchObject({ + step: "computer_click", + subject: "Password", + doneByPerson: true, + }); +}); + +test("the label comes from the snapshot the ref came from, and the ref only when none names it", () => { + // Snapshot 4 has no e14: a ref from another snapshot is not named after whatever holds it here. + const elsewhere = didOf( + [ + ...checkout, + ...step( + "stale", + "computer_click", + { ref: "e14", snapshotId: 4 }, + DECLINED_BY_PERSON, + ), + ], + "stale", + ); + expect(elsewhere?.subject).toBe("e14"); + // The computer's own name for what it clicked still wins. + const ran = didOf( + [ + ...checkout, + ...step( + "ran", + "computer_click", + { ref: "f2e13", snapshotId: 4 }, + { + ok: true, + action: "click", + element: { role: "button", name: "Place order now" }, + url: "http://host.docker.internal:46600/shop/basil", + elapsedMs: 40, + }, + ), + ], + "ran", + ); + expect(ran?.subject).toBe("Place order now"); +}); + +test("a step the server took after an approval has the time it finished, after a reload too", () => { + const id = computer(); + const finishedAt = "2026-11-12T22:05:35.000Z"; + // The page never ran it: the server did, after the person allowed the order, and said when. + recordActivity( + id, + conversationActivity( + step( + "read-after-approval", + "computer_read", + {}, + { + ok: true, + url: "http://host.docker.internal:46600/shop/tomato", + title: "Order confirmed", + text: "Your order FH-2041 is placed.", + finishedAt, + }, + ), + ), + ); + expect(activityFor(id)).toEqual([ + expect.objectContaining({ + id: "read-after-approval", + at: Date.parse(finishedAt), + }), + ]); +}); + +test("a step the person answered here is timed by their answer, not by when its answer arrived", () => { + const id = computer(); + const realNow = Date.now; + try { + // "I did it myself" on the password, at 1 s. + Date.now = () => 1_000; + noteAnswered("password-answered"); + // Its answer reaches the page at 9 s, once the turn the server carried on with is over. + Date.now = () => 9_000; + recordActivity( + id, + conversationActivity([ + ...signIn, + ...step( + "password-answered", + "computer_click", + { ref: "e14", snapshotId: 2 }, + HANDLED_BY_PERSON, + ), + ]), + ); + } finally { + Date.now = realNow; + } + expect( + activityFor(id).find((entry) => entry.id === "password-answered")?.at, + ).toBe(1_000); +}); + +test("an answer that does not say when, from before answers did, still says nothing about when", () => { + const id = computer(); + recordActivity( + id, + conversationActivity( + step( + "opened-before", + "computer_navigate", + { url: "http://host.docker.internal:46600/" }, + { + ok: true, + url: "http://host.docker.internal:46600/", + title: "Fernhill Garden Supply", + }, + ), + ), + ); + const entries = activityFor(id); + expect(entries).toHaveLength(1); + expect(Object.hasOwn(entries[0] as object, "at")).toBe(false); +}); + +test("a step's answer is stamped with when it finished, once, and a step waiting on a person is not", () => { + const before = Date.now(); + const stamped = withFinishedAt({ + ok: true, + url: "http://host.docker.internal:46600/", + }) as { + finishedAt: string; + }; + expect(Date.parse(stamped.finishedAt)).toBeGreaterThanOrEqual(before); + // Stamped already, by the server: kept as it said. + const kept = { ok: true, finishedAt: "2026-11-12T22:05:35.000Z" }; + expect(withFinishedAt(kept)).toBe(kept); + // The placeholder a step waiting on a person leaves is no answer, and has no time. + const waiting = { waiting: true }; + expect(withFinishedAt(waiting)).toBe(waiting); + // A sentence (declined, done by the person) is not an object to stamp. + expect(withFinishedAt(DECLINED_BY_PERSON)).toBe(DECLINED_BY_PERSON); +}); diff --git a/app/tests/personal-settings.test.tsx b/app/tests/personal-settings.test.tsx index 793225917f..bc19871c1c 100644 --- a/app/tests/personal-settings.test.tsx +++ b/app/tests/personal-settings.test.tsx @@ -15,7 +15,7 @@ import { isRedirect, RouterProvider, } from "@tanstack/react-router"; -import { cleanup, render, within } from "@testing-library/react"; +import { cleanup, render, waitFor, within } from "@testing-library/react"; import { ComputersPage } from "@/components/manage/computers-page"; import { PluginToolPage } from "@/components/manage/plugin-tool-page"; import { settingsSidebarItems } from "@/components/settings/settings-sidebar"; @@ -379,6 +379,28 @@ describe("the Computer page", () => { expect(dialog.textContent).not.toContain("The Bot is signed out"); }); + // FOUND BY THE END-TO-END SUITE: answered with Reset it, the dialog fades out, and as it did its title fell back to + // "Reset the computer?", the words for no row at all. It names the Bot's browser until it has gone. + test("the Reset dialog keeps naming the Bot's browser while it closes", async () => { + const view = await renderPage("personal"); + within(browserRow(view, "Personal Assistant")) + .getByRole("button", { name: "Reset" }) + .click(); + const dialog = await view.findByRole("dialog"); + const title = () => + dialog.querySelector('[data-slot="dialog-title"]')?.textContent; + expect(title()).toBe("Reset Personal Assistant's browser?"); + const titles = new Set([title()]); + new MutationObserver(() => titles.add(title())).observe(dialog, { + subtree: true, + childList: true, + characterData: true, + }); + within(dialog).getByRole("button", { name: "Reset it" }).click(); + await waitFor(() => expect(view.queryByRole("dialog")).toBeNull()); + expect([...titles]).toEqual(["Reset Personal Assistant's browser?"]); + }); + /** The section the desktop's folder picker is reached from. */ function foldersSection(view: Awaited>) { return view.getByRole("heading", { name: "Folders on this computer" }) diff --git a/e2e/README.md b/e2e/README.md index f62e82bf29..c547dbe88d 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -167,11 +167,12 @@ The `computer-*` and `approvals-*` journeys: | Journey | What a person does | | --- | --- | | `computer-browse` | Asks a Bot to read a page. The Computer opens on its live screen; the answer comes from the page; Activity lists the page with its time, and still does after a reload; the conversation folds the browsing into one line that opens to its steps. | -| `computer-take-control` | Opens the Bot's screen full size, takes control, goes somewhere from the address bar, back, forward, reload, opens a tab and closes it, is asked before clearing the browser, and hands it back. Stops and resets the Bot's browser in Settings → Computer. | +| `computer-take-control` | Opens the Bot's screen full size, takes control, goes somewhere from the address bar, back, forward, reload, opens a tab and closes it (the screen still fills its frame, with no band under the page, then and after hand back), is asked before clearing the browser, and hands it back. Stops and resets the Bot's browser in Settings → Computer, and the Reset dialog names the Bot's browser until it has gone. | | `computer-terminal` | Asks a Bot to run a command: Activity shows it with what it printed. Takes control of the Terminal, types a command, hands it back. | | `computer-form` | Asks a Bot to fill in and send a form from one message, its fields and checkbox in one step: every action runs, the form arrives filled in, and Activity lists each with its time. | -| `computer-sign-in` | A Bot reaches a password field and hands it over (no standing answer offered). The person takes control, types the password into the Bot's screen, hands back and answers "I did it myself"; the Bot carries on from the signed-in page. | -| `approvals-allow-once` | A Bot paying asks first. Allow once places that order and the reply arrives whole; the next order asks again, and waits on the Bots page under Needs you too; Deny there places nothing and the Bot says so. | +| `computer-sign-in` | A Bot reaches a password field and hands it over (no standing answer offered). The person takes control, types the password into the Bot's screen, hands back and answers "I did it myself"; the Bot carries on from the signed-in page. Activity names the field handed over, and every row has its time. | +| `computer-secret` | A Bot asks for a password with the masked box, naming the field by its ref, without clicking it into a hand-over. The person types it into the box beside the Bot's screen; the Bot signs in and answers, and no step was handed over. | +| `approvals-allow-once` | A Bot paying asks first. Allow once places that order and the reply arrives whole, and every Activity row has its time, the steps the server took after the approval too; the next order asks again, and waits on the Bots page under Needs you too; Deny there places nothing, the Bot says so, and Activity names the button declined. | | `approvals-this-conversation` | Allow for this conversation: the next order there goes through with no card, the banner says what is allowed, Turn off makes the next order ask again. | | `approvals-always` | Always allow here: the Bot pays without asking in its next conversation; the rule is on the Bot's Approvals page, where Remove takes it away. | | `approvals-settings` | Settings → Approvals: Extra vigilant, Auto-review and commands on this computer, kept across a reload; a rule for every Bot added, changed, removed, and cancelled. | diff --git a/e2e/fixtures/computer-secret/openai-2026-11-13T00-04-55-037Z-51ce2bd9.json b/e2e/fixtures/computer-secret/openai-2026-11-13T00-04-55-037Z-51ce2bd9.json new file mode 100644 index 0000000000..d9cb3a21c1 --- /dev/null +++ b/e2e/fixtures/computer-secret/openai-2026-11-13T00-04-55-037Z-51ce2bd9.json @@ -0,0 +1,31 @@ +{ + "fixtures": [ + { + "match": { + "userMessage": "You choose which capabilities to load for the message below. You are not answering it.\n\nCapabilities available:\n- research-public-web: Research the public web. Search current public sources and write a cited brief.\n- find-a-document: Find a document. Search the connected document sources for a file and read what it says.\n- find-a-notion-page: Find a Notion page. Search Notion for a page and read what it says.\n- skill-creator: Write a skill. Interview me about a skill I want, then write it and save it as a new skill.\n- bot-creator: Make a Bot. Work out what a new Bot should be, then put it in front of you to create.\n\nMessage:\nOpen http://host.docker.internal:46600/members and fill in my username robin.okafor. Ask me for my password with the masked box, so it goes straight into the page and you never see it. Then sign in and tell me my seed allowance in one plain sentence.\n\nReply with only JSON: {\"skills\": [\"\", ...]}.\nChoose every capability that might be needed, including ones you are only somewhat sure about.\nChoosing one that turns out to be unnecessary costs almost nothing. Failing to choose one that\nwas needed means the work cannot be done at all, because its tools will not be loaded. When in\ndoubt, include it. Use an empty list only when the message plainly needs none of them.", + "model": "gpt-5.6-terra", + "turnIndex": 0, + "hasToolResult": false + }, + "response": { + "content": "{\"skills\":[]}", + "usage": { + "prompt_tokens": 304, + "completion_tokens": 43, + "total_tokens": 347, + "prompt_tokens_details": { + "cached_tokens": 0, + "cache_write_tokens": 0, + "audio_tokens": 0 + }, + "completion_tokens_details": { + "reasoning_tokens": 27, + "audio_tokens": 0, + "accepted_prediction_tokens": 0, + "rejected_prediction_tokens": 0 + } + } + } + } + ] +} \ No newline at end of file diff --git a/e2e/fixtures/computer-secret/openai-2026-11-13T00-04-57-413Z-a8059788.json b/e2e/fixtures/computer-secret/openai-2026-11-13T00-04-57-413Z-a8059788.json new file mode 100644 index 0000000000..d557a9a932 --- /dev/null +++ b/e2e/fixtures/computer-secret/openai-2026-11-13T00-04-57-413Z-a8059788.json @@ -0,0 +1,76 @@ +{ + "fixtures": [ + { + "match": { + "userMessage": "Open http://host.docker.internal:46600/members and fill in my username robin.okafor. Ask me for my password with the masked box, so it goes straight into the page and you never see it. Then sign in and tell me my seed allowance in one plain sentence.", + "model": "gpt-5.6-terra", + "turnIndex": 0, + "hasToolResult": false + }, + "response": { + "content": "I’m opening the Fernhill members page to sign in and check your seed allowance.", + "toolCalls": [ + { + "name": "computer_navigate", + "arguments": "{\"url\":\"http://host.docker.internal:46600/members\"}", + "id": "call_3linqC0o4R2T4TAsFxdYJoFw" + } + ] + }, + "recordedTimings": { + "ttftMs": 935, + "interChunkDelaysMs": [ + 10, + 435, + 208, + 0, + 4, + 0, + 0, + 24, + 4, + 2, + 12, + 1, + 0, + 0, + 8, + 15, + 5, + 10, + 1, + 6, + 1, + 0, + 66, + 4, + 8, + 0, + 9, + 8, + 0, + 18, + 0, + 0, + 4, + 13, + 40, + 55, + 45, + 13, + 0, + 0, + 0, + 0, + 0, + 199 + ], + "totalDurationMs": 2163 + }, + "metadata": { + "systemHash": "d1bff223", + "toolsHash": "5dcc471c" + } + } + ] +} \ No newline at end of file diff --git a/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-03-056Z-b4352ecf.json b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-03-056Z-b4352ecf.json new file mode 100644 index 0000000000..89bdc7c03f --- /dev/null +++ b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-03-056Z-b4352ecf.json @@ -0,0 +1,39 @@ +{ + "fixtures": [ + { + "match": { + "userMessage": "Open http://host.docker.internal:46600/members and fill in my username robin.okafor. Ask me for my password with the masked box, so it goes straight into the page and you never see it. Then sign in and tell me my seed allowance in one plain sentence.", + "model": "gpt-5.6-terra", + "turnIndex": 2, + "hasToolResult": true + }, + "response": { + "toolCalls": [ + { + "name": "computer_snapshot", + "arguments": "{}", + "id": "call_R6ExYMnknnmlMWls37Ez6aMS" + } + ] + }, + "recordedTimings": { + "ttftMs": 796, + "interChunkDelaysMs": [ + 11, + 275, + 143, + 24, + 30, + 764, + 13, + 172 + ], + "totalDurationMs": 2228 + }, + "metadata": { + "systemHash": "d1bff223", + "toolsHash": "5dcc471c" + } + } + ] +} \ No newline at end of file diff --git a/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-07-952Z-dc12df55.json b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-07-952Z-dc12df55.json new file mode 100644 index 0000000000..c37e69744e --- /dev/null +++ b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-07-952Z-dc12df55.json @@ -0,0 +1,59 @@ +{ + "fixtures": [ + { + "match": { + "userMessage": "Open http://host.docker.internal:46600/members and fill in my username robin.okafor. Ask me for my password with the masked box, so it goes straight into the page and you never see it. Then sign in and tell me my seed allowance in one plain sentence.", + "model": "gpt-5.6-terra", + "turnIndex": 3, + "hasToolResult": true + }, + "response": { + "toolCalls": [ + { + "name": "computer_type", + "arguments": "{\"ref\":\"e12\",\"snapshotId\":2,\"text\":\"robin.okafor\",\"submit\":false}", + "id": "call_j7dHNTf9hqcoMvh48PUxfXW1" + } + ] + }, + "recordedTimings": { + "ttftMs": 1214, + "interChunkDelaysMs": [ + 20, + 461, + 4, + 5, + 0, + 0, + 0, + 19, + 5, + 3, + 2, + 2, + 13, + 0, + 0, + 15, + 19, + 0, + 0, + 0, + 0, + 0, + 0, + 29, + 0, + 106, + 13, + 166 + ], + "totalDurationMs": 2096 + }, + "metadata": { + "systemHash": "d1bff223", + "toolsHash": "5dcc471c" + } + } + ] +} \ No newline at end of file diff --git a/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-15-061Z-7bd22944.json b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-15-061Z-7bd22944.json new file mode 100644 index 0000000000..2dedb654e0 --- /dev/null +++ b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-15-061Z-7bd22944.json @@ -0,0 +1,54 @@ +{ + "fixtures": [ + { + "match": { + "userMessage": "Open http://host.docker.internal:46600/members and fill in my username robin.okafor. Ask me for my password with the masked box, so it goes straight into the page and you never see it. Then sign in and tell me my seed allowance in one plain sentence.", + "model": "gpt-5.6-terra", + "turnIndex": 4, + "hasToolResult": true + }, + "response": { + "toolCalls": [ + { + "name": "computer_request_secret", + "arguments": "{\"label\":\"your Fernhill password\",\"ref\":\"e14\",\"snapshotId\":2}", + "id": "call_J6z2I4nJ6nL1K7RlyptqeaUa" + } + ] + }, + "recordedTimings": { + "ttftMs": 1144, + "interChunkDelaysMs": [ + 10, + 411, + 0, + 5, + 0, + 33, + 46, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 31, + 0, + 0, + 0, + 20, + 33, + 129, + 0, + 175 + ], + "totalDurationMs": 2037 + }, + "metadata": { + "systemHash": "d1bff223", + "toolsHash": "5dcc471c" + } + } + ] +} \ No newline at end of file diff --git a/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-16-803Z-50396085.json b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-16-803Z-50396085.json new file mode 100644 index 0000000000..15cd2be938 --- /dev/null +++ b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-16-803Z-50396085.json @@ -0,0 +1,34 @@ +{ + "fixtures": [ + { + "match": { + "userMessage": "Asked: Open http://host.docker.internal:46600/members and fill in my username robin.okafor. Ask me for my password with the masked box, so it goes straight into the page and you never see it. Then sign in and tell me my seed allowance in one plain sentence.", + "model": "gpt-5.6-terra", + "turnIndex": 0, + "hasToolResult": false + }, + "response": { + "content": "Fernhill Seed Allowance Sign-In", + "usage": { + "prompt_tokens": 181, + "completion_tokens": 20, + "total_tokens": 201, + "prompt_tokens_details": { + "cached_tokens": 0, + "cache_write_tokens": 0, + "audio_tokens": 0 + }, + "completion_tokens_details": { + "reasoning_tokens": 4, + "audio_tokens": 0, + "accepted_prediction_tokens": 0, + "rejected_prediction_tokens": 0 + } + } + }, + "metadata": { + "systemHash": "dedd9ec9" + } + } + ] +} \ No newline at end of file diff --git a/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-25-959Z-12d29145.json b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-25-959Z-12d29145.json new file mode 100644 index 0000000000..3528d11189 --- /dev/null +++ b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-25-959Z-12d29145.json @@ -0,0 +1,47 @@ +{ + "fixtures": [ + { + "match": { + "userMessage": "Open http://host.docker.internal:46600/members and fill in my username robin.okafor. Ask me for my password with the masked box, so it goes straight into the page and you never see it. Then sign in and tell me my seed allowance in one plain sentence.", + "model": "gpt-5.6-terra", + "turnIndex": 5, + "hasToolResult": true + }, + "response": { + "toolCalls": [ + { + "name": "computer_click", + "arguments": "{\"ref\":\"e15\",\"snapshotId\":2}", + "id": "call_10LxxrI7Etke2PfJcfAvpNo6" + } + ] + }, + "recordedTimings": { + "ttftMs": 1259, + "interChunkDelaysMs": [ + 9, + 395, + 0, + 13, + 4, + 3, + 3, + 4, + 2, + 1, + 0, + 25, + 7, + 85, + 1, + 178 + ], + "totalDurationMs": 1989 + }, + "metadata": { + "systemHash": "d1bff223", + "toolsHash": "5dcc471c" + } + } + ] +} \ No newline at end of file diff --git a/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-38-758Z-ccc2f2d7.json b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-38-758Z-ccc2f2d7.json new file mode 100644 index 0000000000..0e6d17c128 --- /dev/null +++ b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-38-758Z-ccc2f2d7.json @@ -0,0 +1,37 @@ +{ + "fixtures": [ + { + "match": { + "userMessage": "Open http://host.docker.internal:46600/members and fill in my username robin.okafor. Ask me for my password with the masked box, so it goes straight into the page and you never see it. Then sign in and tell me my seed allowance in one plain sentence.", + "model": "gpt-5.6-terra", + "turnIndex": 6, + "hasToolResult": true + }, + "response": { + "toolCalls": [ + { + "name": "computer_read", + "arguments": "{}", + "id": "call_w1Qxav0P5BazudgaWvQTPDKW" + } + ] + }, + "recordedTimings": { + "ttftMs": 912, + "interChunkDelaysMs": [ + 33, + 320, + 0, + 107, + 0, + 255 + ], + "totalDurationMs": 1627 + }, + "metadata": { + "systemHash": "d1bff223", + "toolsHash": "5dcc471c" + } + } + ] +} \ No newline at end of file diff --git a/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-48-192Z-c2cbbfd5.json b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-48-192Z-c2cbbfd5.json new file mode 100644 index 0000000000..65b85383bd --- /dev/null +++ b/e2e/fixtures/computer-secret/openai-2026-11-13T00-05-48-192Z-c2cbbfd5.json @@ -0,0 +1,42 @@ +{ + "fixtures": [ + { + "match": { + "userMessage": "Open http://host.docker.internal:46600/members and fill in my username robin.okafor. Ask me for my password with the masked box, so it goes straight into the page and you never see it. Then sign in and tell me my seed allowance in one plain sentence.", + "model": "gpt-5.6-terra", + "turnIndex": 7, + "hasToolResult": true + }, + "response": { + "content": "Your seed allowance is 12 packets this season." + }, + "recordedTimings": { + "ttftMs": 1005, + "interChunkDelaysMs": [ + 13, + 350, + 4, + 1, + 5, + 0, + 0, + 4, + 0, + 20, + 6, + 5, + 5, + 108, + 5, + 0, + 159 + ], + "totalDurationMs": 1690 + }, + "metadata": { + "systemHash": "d1bff223", + "toolsHash": "5dcc471c" + } + } + ] +} \ No newline at end of file diff --git a/e2e/journeys/approvals-allow-once.journey.ts b/e2e/journeys/approvals-allow-once.journey.ts index 500d93eb1a..ed7a9daad1 100644 --- a/e2e/journeys/approvals-allow-once.journey.ts +++ b/e2e/journeys/approvals-allow-once.journey.ts @@ -1,10 +1,13 @@ import { ITEMS, SITE, SITE_HOST } from "../site/site"; import { answer, approvalCard, expectAsksToPay, PAYS } from "./approval-steps"; import { + activityRows, expect, + expectEveryRowTimed, expectWholeReply, named, newConversation, + openComputer, replyAfter, runsSoFar, send, @@ -55,6 +58,17 @@ test("Allow once places one order and the next one asks again; Deny buys nothing await expectWholeReply(page, reply); await named(page, channelId); expect(ordered()).toEqual(["tomato"]); + // The order placed is named, and the steps the server took after the approval have their time too. + await openComputer(page); + await expect( + activityRows(page) + .filter({ hasText: "Clicked on page" }) + .filter({ hasText: "Place order" }), + ).toHaveCount(1); + await expect( + activityRows(page).filter({ hasText: "Read page" }), + ).toHaveCount(1); + await expectEveryRowTimed(page); await shot("allowed-once"); }); @@ -100,6 +114,12 @@ test("Allow once places one order and the next one asks again; Deny buys nothing await expectWholeReply(page, reply); expect(reply).not.toContain(ITEMS.basil.order); expect(ordered(), "a denied order is not placed").toEqual(["tomato"]); + // Activity names the button that was declined, as it names one that was clicked, never its ref. + await openComputer(page); + const declined = activityRows(page).filter({ hasText: "Declined" }); + await expect(declined).toHaveCount(1); + await expect(declined).toContainText("Place order"); + await expect(declined).toContainText("You declined it."); await shot("denied"); }); }); diff --git a/e2e/journeys/computer-secret.journey.ts b/e2e/journeys/computer-secret.journey.ts new file mode 100644 index 0000000000..f3c427c01b --- /dev/null +++ b/e2e/journeys/computer-secret.journey.ts @@ -0,0 +1,94 @@ +import { MEMBER, SITE } from "../site/site"; +import { approvalCard } from "./approval-steps"; +import { + activityRows, + computerPanel, + expect, + expectEveryRowTimed, + expectWholeReply, + named, + newConversation, + openComputer, + replyAfter, + send, + serveSite, + test, + turnOver, +} from "./computer-steps"; + +/** + * THE BOT'S COMPUTER, A PASSWORD THROUGH THE MASKED BOX. A Bot that needs one value it must not be + * told asks for it with `computer_request_secret`, naming the field by its ref. The person types it + * into a masked box beside the Bot's screen, it goes straight into the field, and the Bot signs in + * and carries on. Nothing is handed over: the Bot never clicks the password field, which the gate + * hands to the person (server/src/approvals/policy.ts), and the password never reaches the Bot. + */ + +const site = serveSite(); + +test("a Bot asks for a password in the masked box and signs in without handing the browser over", async ({ + page, + shot, + makeBot, +}) => { + test.setTimeout(400_000); + const botId = await makeBot( + "Fernhill Keeper", + "Look after my Fernhill Garden Supply members' account. Keep replies short.", + ); + const before = site().signIns.length; + let channelId = ""; + const masked = computerPanel(page).getByLabel(/^The Bot needs/); + + await test.step("the Bot fills in the username and asks for the password in the masked box", async () => { + await newConversation(page, botId); + channelId = await send( + page, + `Open ${SITE}/members and fill in my username ${MEMBER.username}. Ask me for my password with the masked box, so it goes straight into the page and you never see it. Then sign in and tell me my seed allowance in one plain sentence.`, + ); + await openComputer(page); + await expect(masked).toBeVisible({ timeout: 120_000 }); + // Asked for, not handed over: no card waits on the person, and nothing is signed in yet. + await expect(approvalCard(page)).toHaveCount(0); + await expect( + activityRows(page) + .filter({ hasText: "Filled in field" }) + .filter({ hasText: "Username" }), + ).toHaveCount(1); + expect(site().signIns.slice(before)).toEqual([]); + await shot("asks-for-the-password"); + }); + + await test.step("the person types it into the masked box, and the Bot signs in and answers", async () => { + await masked.fill(MEMBER.password); + await computerPanel(page) + .getByRole("button", { name: "Send to the page" }) + .click(); + await expect(masked).toBeHidden({ timeout: 60_000 }); + const reply = await replyAfter(page, channelId, 0, /12 packets/); + await turnOver(page); + await expectWholeReply(page, reply); + await named(page, channelId); + expect(site().signIns.slice(before)).toEqual([ + { username: MEMBER.username, ok: true }, + ]); + // The password went from the person's keyboard to the page, never into the conversation. + expect(reply).not.toContain(MEMBER.password); + await shot("signed-in"); + }); + + await test.step("no step was handed over, and Activity lists every one with its time", async () => { + await expect(approvalCard(page)).toHaveCount(0); + const rows = activityRows(page); + await expect(rows.filter({ hasText: "You did it yourself." })).toHaveCount( + 0, + ); + await expect( + rows + .filter({ hasText: "Clicked on page" }) + .filter({ hasText: "Password" }), + ).toHaveCount(0); + await expectEveryRowTimed(page); + await shot("activity"); + }); +}); diff --git a/e2e/journeys/computer-sign-in.journey.ts b/e2e/journeys/computer-sign-in.journey.ts index 9e0e0dd1ff..a74de68ee8 100644 --- a/e2e/journeys/computer-sign-in.journey.ts +++ b/e2e/journeys/computer-sign-in.journey.ts @@ -3,6 +3,7 @@ import { answer, approvalCard, HANDED_OVER } from "./approval-steps"; import { activityRows, expect, + expectEveryRowTimed, expectWholeReply, named, newConversation, @@ -103,6 +104,14 @@ test("a password is handed to the person, who signs in and hands the browser bac await named(page, channelId); // The Bot read the page the person signed in to; it did not sign in again. expect(site().signIns.slice(before)).toHaveLength(1); + // The step handed over is named for its field, not its ref, and every step has its time, the + // ones the Bot took after the hand-over too. + await openComputer(page); + const handed = activityRows(page).filter({ hasText: "Clicked on page" }); + await expect(handed).toHaveCount(1); + await expect(handed).toContainText("Password"); + await expect(handed).toContainText("You did it yourself."); + await expectEveryRowTimed(page); await shot("carried-on"); }); }); diff --git a/e2e/journeys/computer-steps.ts b/e2e/journeys/computer-steps.ts index e73c92422c..b3ad0978c8 100644 --- a/e2e/journeys/computer-steps.ts +++ b/e2e/journeys/computer-steps.ts @@ -260,6 +260,64 @@ export function serveSite(): () => Site { /** A time as Activity prints one beside a step: "12:57:38 PM", or "12:57:38". */ export const CLOCK = /\d{1,2}:\d{2}:\d{2}/; +/** + * Every row of Activity has the time its step finished: the steps the Bot took itself, the one a + * person answered, and the ones the server took after an approval, which once had none. And the + * times run newest first, as the rows do: the step a person answered once read as later than the + * steps the server took after it, timed by when its answer reached the page. + */ +export async function expectEveryRowTimed(page: Page) { + const rows = activityRows(page); + const count = await rows.count(); + expect(count).toBeGreaterThan(0); + const times: number[] = []; + for (let index = 0; index < count; index += 1) { + const time = rows.nth(index).locator("time"); + await expect(time).toHaveText(CLOCK); + times.push(Date.parse((await time.getAttribute("datetime")) ?? "")); + } + expect(times, "Activity's times run newest first").toEqual( + [...times].sort((a, b) => b - a), + ); +} + +/** + * The Bot's screen fills the frame kept for it: the page is drawn as tall as the frame, with none of + * the black behind it showing as a band underneath. Read as a person sees it, from where the screen + * and its frame are laid out, once a frame of the page has been drawn (a canvas starts 300 wide). + * + * HELD, NOT SEEN ONCE. A tab left in front after another closed came back short about a second + * later, so a screen that fills its frame at the first look has to go on filling it: it passes once + * it has read as filled every time for two seconds. + */ +export async function expectScreenFillsFrame(screen: Locator) { + const HELD_MS = 2_000; + let filledSince: number | null = null; + await expect + .poll( + async () => { + const gap = await screen.evaluate((canvas: HTMLCanvasElement) => { + const frame = canvas.parentElement?.getBoundingClientRect(); + const drawn = canvas.getBoundingClientRect(); + if (!frame || canvas.width === 300) return "no frame drawn yet"; + return Math.round(frame.bottom - drawn.bottom); + }); + if (gap !== 0) { + filledSince = null; + return gap; + } + filledSince ??= Date.now(); + return Date.now() - filledSince >= HELD_MS ? 0 : "filled, not yet held"; + }, + { + message: "the Bot's screen fills its frame, with no band under it", + intervals: [250], + timeout: 15_000, + }, + ) + .toBe(0); +} + /** * The conversation has its generated name. OpenBot names a conversation on a loop after its first * answer; waited for here so the naming call is made inside the journey that caused it, and a diff --git a/e2e/journeys/computer-take-control.journey.ts b/e2e/journeys/computer-take-control.journey.ts index a71cc0bbff..22984f9ea8 100644 --- a/e2e/journeys/computer-take-control.journey.ts +++ b/e2e/journeys/computer-take-control.journey.ts @@ -4,6 +4,7 @@ import { activityRows, computerPanel, expect, + expectScreenFillsFrame, expectWholeReply, named, newConversation, @@ -130,6 +131,8 @@ test("the person takes the Bot's browser over, browses, and hands it back", asyn "Fernhill Garden Supply", ); await expect(address).toHaveValue(`${SITE}/`); + // The tab left in front fills the screen, with no black band under the page. + await expectScreenFillsFrame(view.locator("canvas")); await shot("tab-closed"); }); @@ -153,6 +156,7 @@ test("the person takes the Bot's browser over, browses, and hands it back", asyn view.getByRole("button", { name, exact: true }), ).toBeDisabled(); } + await expectScreenFillsFrame(view.locator("canvas")); await shot("handed-back"); await view.getByRole("button", { name: "Close the Bot's screen" }).click(); await expect(view).toBeHidden(); @@ -183,8 +187,27 @@ test("the person takes the Bot's browser over, browses, and hands it back", asyn }); await expect(confirm).toContainText("You can't undo this."); await shot("reset-asks"); + // Every title the dialog shows from here until it has gone, as it fades out. + await confirm.evaluate((dialog) => { + const titles = new Set(); + const heading = () => + dialog.querySelector('[data-slot="dialog-title"]')?.textContent ?? ""; + titles.add(heading()); + new MutationObserver(() => titles.add(heading())).observe(dialog, { + subtree: true, + childList: true, + characterData: true, + }); + (window as unknown as { resetTitles: Set }).resetTitles = titles; + }); await confirm.getByRole("button", { name: "Reset it" }).click(); await expect(page.getByRole("dialog")).toHaveCount(0); + // It says whose browser it resets until it has gone: it never falls back to another title as it closes. + expect( + await page.evaluate(() => [ + ...(window as unknown as { resetTitles: Set }).resetTitles, + ]), + ).toEqual(["Reset Fernhill Guide's browser?"]); // Reset deletes the Bot's browser: it leaves the list until the Bot next opens a page. await expect(row).toHaveCount(0); await shot("reset-done"); diff --git a/server/src/computer/headless-tools.ts b/server/src/computer/headless-tools.ts index 6b00e4d892..1970575c7f 100644 --- a/server/src/computer/headless-tools.ts +++ b/server/src/computer/headless-tools.ts @@ -2,6 +2,10 @@ import { type HeadlessTool, HeadlessToolSuspension, } from "@openbot/toolkit/headless-tools"; +import { + SECRET_FIELD_BY_REF, + withFinishedAt, +} from "@openbot/shared/computer-control"; import { z } from "zod"; import { currentApprovalContext } from "../approvals/types"; import { type AuditStore, recordAuditEvent } from "../audit"; @@ -70,7 +74,8 @@ export function createHeadlessComputerTools(options: { if (context.signal.aborted) throw new Error("The computer action was stopped."); try { - return await execute(input, context); + // When it finished, for Activity: the one a person allowed and every step after it run here. + return withFinishedAt(await execute(input, context)); } catch (error) { /* * A cold start that outlasted the wait is answered, not thrown. @@ -258,7 +263,7 @@ export function createHeadlessComputerTools(options: { ), tool( "computer_request_secret", - "Ask the person to enter one secret directly into the focused field. Click the field first. You must never receive the secret; this unattended turn pauses for the person.", + `Ask the person to enter one secret directly into a field. ${SECRET_FIELD_BY_REF} You must never receive the secret; this unattended turn pauses for the person.`, z.strictObject({ label: text, ref, snapshotId }), async (input) => waiting( diff --git a/server/tests/computer-secret-without-a-click.test.ts b/server/tests/computer-secret-without-a-click.test.ts new file mode 100644 index 0000000000..2f353762a0 --- /dev/null +++ b/server/tests/computer-secret-without-a-click.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, test } from "bun:test"; +import { computerGuidance } from "@openbot/shared/bot-prompt"; +import { SECRET_FIELD_BY_REF } from "@openbot/shared/computer-control"; +import { safetyRequirement } from "../src/approvals/policy"; +import type { ApprovalCandidate } from "../src/approvals/types"; +import { botSeat } from "../src/computer/computer-key"; +import type { ComputerGateway } from "../src/computer/gateway"; +import { createHeadlessComputerTools } from "../src/computer/headless-tools"; + +/** + * A PASSWORD THE PERSON GIVES THROUGH THE MASKED BOX, WITHOUT A HAND-OVER. + * + * A Bot that needs one value it must not be told asks for it with `computer_request_secret`, and the + * person types it into a masked box that puts it straight into the field. The Bot was told to click + * the field first, and a Bot's click on a password field is a credential typed on a website, which + * the gate hands to the person (`safetyRequirement`). Doing as it was told, a Bot handed the whole + * browser over on the click and the masked box was never reached. The rule is right: a Bot's own + * click or typing there is the person's to do. So the words changed: the field is named by its ref, + * which the computer fills itself, and nothing tells a Bot to click it first. + */ + +/** A Bot's click on the members' page's password field, as the gate sees it. */ +const clickPassword: ApprovalCandidate = { + actorId: "owner", + botId: "keeper", + toolRef: "computer_click", + effect: "write", + scope: "host.docker.internal", + args: { ref: "e14", snapshotId: 2 }, + target: { + pageUrl: "http://host.docker.internal:46600/members", + element: { ref: "e14", role: "textbox", name: "Password" }, + }, +}; + +/** What every Bot with a computer is told about it, as built-in and brought Bots alike are told. */ +const prompt = computerGuidance({ search: false, personal: true }); + +/** The server's own `computer_request_secret`, for a turn no browser runs. */ +const serverTool = createHeadlessComputerTools({ + gateway: {} as ComputerGateway, + seat: botSeat("keeper"), + actor: { id: "owner", userId: "owner" }, +}).find((tool) => tool.definition.name === "computer_request_secret"); + +/** What tells a Bot to click or focus the field before asking. */ +const CLICK_FIRST = /(click|focus)(es)? the field first(?!:)|focused field/i; + +describe("a secret goes into its field by its ref, with no click the gate hands over", () => { + test("a Bot's own click on a password field is still the person's to do", () => { + expect(safetyRequirement(clickPassword)).toMatchObject({ + id: "credentials", + behaviour: "hand_off", + }); + }); + + test("the standing instructions ask for a secret by the field's ref, and never to click it first", () => { + const said = prompt.replace(/\s+/g, " "); + expect(said).toContain( + "Call computer_request_secret with that field's ref and a short label, without clicking the field first", + ); + expect(said).not.toMatch(/Click the field first, then call/); + }); + + test("the server's tool says it in the sentence the browser's tool is built from", () => { + const description = serverTool?.definition.description ?? ""; + expect(description).toContain(SECRET_FIELD_BY_REF); + expect(description).not.toMatch(CLICK_FIRST); + expect(SECRET_FIELD_BY_REF).toContain( + "Do not click or type into the field first", + ); + }); +}); diff --git a/server/tests/headless-tools.test.ts b/server/tests/headless-tools.test.ts index 0844c9867d..616b14ddb8 100644 --- a/server/tests/headless-tools.test.ts +++ b/server/tests/headless-tools.test.ts @@ -375,6 +375,34 @@ function computerHarness(deny: string[] = []) { return { gateway, audit, received }; } +/** + * A step the server takes says when it finished, in the answer the conversation keeps. + * + * After a person allows a step, the server takes it and carries the turn on with steps of its own, + * none of which pass through the browser. Activity beside the Bot's screen read the time of each step + * from the browser watching it, so every one of those rows had no time (found by the approvals + * journeys). The answer now carries it, read by `conversationActivity` in the app. + */ +test("a computer step the server runs answers with the time it finished", async () => { + const { gateway } = computerHarness(); + const tools = headless.createHeadlessComputerTools({ + gateway, + seat: botSeat("bot"), + actor: { id: "owner", userId: "owner" }, + }); + const click = tools.find((tool) => tool.definition.name === "computer_click"); + const before = Date.now(); + const answer = (await click?.execute( + { ref: "e1", snapshotId: 7 }, + { toolCallId: "place-order", signal: new AbortController().signal }, + )) as { ok?: boolean; finishedAt?: unknown }; + expect(answer.ok).toBe(true); + expect(typeof answer.finishedAt).toBe("string"); + const at = Date.parse(String(answer.finishedAt)); + expect(at).toBeGreaterThanOrEqual(before); + expect(at).toBeLessThanOrEqual(Date.now()); +}); + test("scheduled computer actions carry the owner and routine into both policy and audit", async () => { const { gateway, received, audit } = computerHarness([ 'initiator.kind == "routine" && intent == "run_command"', diff --git a/shared/bot-prompt.ts b/shared/bot-prompt.ts index 4470478e7c..3bfd407a2e 100644 --- a/shared/bot-prompt.ts +++ b/shared/bot-prompt.ts @@ -117,8 +117,9 @@ const ACTING_LINES = [ "them to enter a username, a password or a code, neither into this conversation nor 'on the sign-in", "page': you do not need it, should not hold it, and they cannot reach the page anyway.", "When you need a single value, a password, a code, a card number, do not hand over the whole", - "browser. Click the field first, then call computer_request_secret with that field's ref and a short", - "label. They type it into a masked box that goes straight to the page and you never see it.", + "browser. Call computer_request_secret with that field's ref and a short label, without clicking", + "the field first: clicking a password field hands that step to the person instead. They type the", + "value into a masked box that goes straight into the field, and you never see it.", "Use a full takeover for anything more involved than one field.", "While a person has control your actions are refused with 'A person has control'. That is not an", "error: wait, say you are waiting, and continue when it is handed back.", diff --git a/shared/computer-control.ts b/shared/computer-control.ts index 9d8937fa6b..bdf844c62b 100644 --- a/shared/computer-control.ts +++ b/shared/computer-control.ts @@ -73,3 +73,36 @@ export type BrowserChallenge = { reason: string; requestId: string; }; + +/** + * How a Bot asks for a value it must not be told (`computer_request_secret`), said the same way by the + * browser's tool, the server's and the standing instructions (`bot-prompt.ts`). + * + * WITHOUT A CLICK. All three used to say to click the field first. A click on a password field is a + * credential typed on a website to the approval gate (`safetyRequirement` in + * `server/src/approvals/policy.ts`), which hands that step to the person: a Bot that did as it was told + * handed the whole browser over on the click and never reached the masked box. The box needs no click. + * The value goes into the field the ref names, which the computer clicks and fills itself when the + * person sends it (`/human/secret` in `agent-computer/src/index.ts`). The gate is right to hand a + * Bot's own click there to the person, so the words changed, not the rule. + */ +export const SECRET_FIELD_BY_REF = + "Give the ref of the field from your most recent snapshot and the snapshotId it came from. Do not click or type into the field first: the value goes into the field that ref names, and clicking a password field hands that step to the person instead."; + +/** + * A computer step's answer, with when it finished (`finishedAt`), for the time beside it in Activity. + * + * Activity used to time a step by the page watching it run, and the conversation it reads said what a + * step did and not when. The steps the server takes after a person allows one never pass through the + * page, so their rows had no time; nor did any row once the conversation was opened again. Said in + * the answer the conversation keeps, every step has it: stamped by the server for a step it runs + * (`headless-tools.ts`) and by the browser for one it runs (`approval-tools.tsx`). An answer that + * already says, and the placeholder a step waiting on a person leaves, are left as they are. + */ +export function withFinishedAt(answer: unknown): unknown { + if (!answer || typeof answer !== "object" || Array.isArray(answer)) + return answer; + if ("finishedAt" in answer || (answer as { waiting?: unknown }).waiting) + return answer; + return { ...answer, finishedAt: new Date().toISOString() }; +} From 10dcc0dca692e311738fb8248993275387571591 Mon Sep 17 00:00:00 2001 From: David McKay Date: Thu, 12 Nov 2026 17:42:11 -0800 Subject: [PATCH 2/3] Say how a step was allowed in Activity, and keep every time across a reload - A step the person allowed on its card has a line saying how ("Allowed for this conversation."), as a declined one says "You declined it." The decision comes from the approvals inbox the conversation already reads. - A step declined or done by the person keeps the time it was answered when the conversation is opened again: its answer is a sentence for the Bot, so the time comes from the same decision. --- app/src/components/computer/activity-log.tsx | 21 ++++ app/src/lib/approvals.ts | 2 + app/src/lib/computers/activity.ts | 99 ++++++++++++++++--- app/tests/activity-allowed-line.test.tsx | 50 ++++++++++ .../computer-activity-names-and-times.test.ts | 91 +++++++++++++++++ e2e/README.md | 4 +- e2e/journeys/approvals-allow-once.journey.ts | 2 + .../approvals-this-conversation.journey.ts | 24 +++++ e2e/journeys/computer-browse.journey.ts | 3 + 9 files changed, 278 insertions(+), 18 deletions(-) create mode 100644 app/tests/activity-allowed-line.test.tsx diff --git a/app/src/components/computer/activity-log.tsx b/app/src/components/computer/activity-log.tsx index 45ddb0388a..20c219c4d3 100644 --- a/app/src/components/computer/activity-log.tsx +++ b/app/src/components/computer/activity-log.tsx @@ -27,6 +27,7 @@ import { } from "@tabler/icons-react"; import { useSyncExternalStore } from "react"; import { + type AllowedDecision, activityFor, type BrowserStepName, type ComputerActivity, @@ -61,6 +62,23 @@ const BROWSER_ICONS: Record = { computer_scroll: IconArrowsVertical, }; +/** How the person allowed a step on its card, as the card says it once answered. */ +const ALLOWED: Record = { + allow_once: "Allowed once.", + allow_conversation: "Allowed for this conversation.", + allow_always: "Always allowed here.", +}; + +/** The line under a step the person allowed on its card, as one they declined has its own. */ +function AllowedLine({ allowed }: { allowed: AllowedDecision | undefined }) { + if (!allowed) return null; + return ( +

+ {ALLOWED[allowed]} +

+ ); +} + function timeOf(at: number): string { return new Date(at).toLocaleTimeString(undefined, { hour: "2-digit", @@ -107,6 +125,7 @@ function CommandEntry({ })} truncated={entry.truncated === true} /> + ); } @@ -160,6 +179,7 @@ function BrowserEntry({ entry }: { entry: ComputerActivity }) { : "It did not work."}

) : null} + ); } @@ -221,6 +241,7 @@ function Entry({ entry }: { entry: ComputerActivity }) { /> )} + ); } diff --git a/app/src/lib/approvals.ts b/app/src/lib/approvals.ts index 095200bd08..7750d1a5d1 100644 --- a/app/src/lib/approvals.ts +++ b/app/src/lib/approvals.ts @@ -65,6 +65,8 @@ export type ApprovalInboxData = { createdAt: string; /** When the person (or a rule) decided it; null while it waits. */ decidedAt?: string | null; + /** What the person answered; null while it waits. */ + decision?: ApprovalDecision | null; action: { botId: string; toolRef: string; diff --git a/app/src/lib/computers/activity.ts b/app/src/lib/computers/activity.ts index 57634422cf..2477feb40d 100644 --- a/app/src/lib/computers/activity.ts +++ b/app/src/lib/computers/activity.ts @@ -7,7 +7,13 @@ import { outcomeForPerson, } from "@openbot/shared/approval-outcomes"; import type { DecisionToolName } from "@openbot/shared/decision-tools"; +import { useQuery } from "@tanstack/react-query"; import { useEffect } from "react"; +import { + type ApprovalDecision, + type ApprovalInboxData, + approvalInboxOptions, +} from "@/lib/approvals"; import type { ToolOutcome } from "./call"; import { followView, latestView } from "./panel-view"; @@ -74,6 +80,8 @@ export type ComputerActivity = { refused?: boolean; /** Refused because the person declined it, so the line says declined rather than what it would have done. */ declined?: boolean; + /** The person allowed it on its card, and how: the line says so, as it says a step was declined. */ + allowed?: AllowedDecision; /** * It was handed to the person and they did it themselves. Done, so not drawn as refused, and * nothing ran on the computer for it, so not drawn as a command that printed nothing either. @@ -84,6 +92,12 @@ export type ComputerActivity = { timedOut?: boolean; }; +/** How a person allowed a step on its card. */ +export type AllowedDecision = Extract< + ApprovalDecision, + "allow_once" | "allow_conversation" | "allow_always" +>; + /** What a call did, without the bookkeeping the pane adds to it. */ export type ActivityDetails = Omit; @@ -95,8 +109,49 @@ export type ConversationCall = { id: string; did: ActivityDetails | null; finishedAt?: number; + /** When the person answered it on its card, for a step whose answer is a sentence that says no time. */ + decidedAt?: number; }; +/** What the person answered on a step's card, and when, by the step's call. */ +export type StepDecision = { decision: ApprovalDecision; at?: number }; +export type StepDecisions = ReadonlyMap; +const NO_DECISIONS: StepDecisions = new Map(); + +/** + * The person's answers on the cards of this owner's recent steps, by call, from the approvals inbox + * the conversation already reads. The card says what was answered while it shows; once answered it + * folds away, and the step's own answer is the computer's, or a sentence for the Bot, neither of + * which says how the person answered or, for a sentence, when. So "Allow for this conversation" left + * no line in Activity, and a step declined there had no time once the conversation was opened again. + */ +export function decisionsIn( + inbox: ApprovalInboxData | undefined, +): StepDecisions { + const decisions = new Map(); + for (const request of inbox?.requests ?? []) { + const id = request.action.toolCallId; + if (!id || !request.decision) continue; + const at = request.decidedAt ? Date.parse(request.decidedAt) : Number.NaN; + decisions.set(id, { + decision: request.decision, + ...(Number.isFinite(at) ? { at } : {}), + }); + } + return decisions; +} + +function allowedBy( + decision: StepDecision | undefined, +): AllowedDecision | undefined { + const answered = decision?.decision; + return answered === "allow_once" || + answered === "allow_conversation" || + answered === "allow_always" + ? answered + : undefined; +} + /** * The most recent entries per computer. * @@ -175,6 +230,7 @@ function sameDetails(entry: ComputerActivity, details: ActivityDetails) { entry.exitCode === details.exitCode && entry.refused === details.refused && entry.declined === details.declined && + entry.allowed === details.allowed && entry.doneByPerson === details.doneByPerson && entry.truncated === details.truncated && entry.timedOut === details.timedOut @@ -319,7 +375,7 @@ export function recordActivity( const existing = present.get(call.id); if (existing) { // The server's answer can arrive after the browser's placeholder, with when it finished. - const at = existing.at ?? call.finishedAt; + const at = existing.at ?? call.finishedAt ?? call.decidedAt; if (!sameDetails(existing, call.did) || at !== existing.at) { next ??= [...current]; const position = next.findIndex((entry) => entry.id === call.id); @@ -342,7 +398,10 @@ export function recordActivity( answeredHere.delete(call.id); // When the step says it finished, when the person answered it here, or when this page saw its answer arrive. const at = - call.finishedAt ?? answeredAt ?? (watched ? Date.now() : undefined); + call.finishedAt ?? + call.decidedAt ?? + answeredAt ?? + (watched ? Date.now() : undefined); const entry: ComputerActivity = { ...call.did, id: call.id, @@ -867,6 +926,7 @@ function declinedCommand(command: string): ActivityDetails { */ export function conversationActivity( messages: readonly Message[], + decisions: StepDecisions = NO_DECISIONS, ): ConversationCall[] { const computerCalls = new Set(); const browserCalls = new Set(); @@ -922,25 +982,24 @@ export function conversationActivity( continue; } const name = call.function.name; - if (isBrowserStep(name)) { - const answer = answered.get(call.id); - if (answer && awaitingFreshLook(answer)) continue; - const finishedAt = finishedAtOf(answer); - calls.push({ - id: call.id, - did: answer ? browserDidOf(name, call, answer, labels) : null, - ...(finishedAt === undefined ? {} : { finishedAt }), - }); - continue; - } + const browser = isBrowserStep(name); const kind = KINDS[name]; - if (!kind) continue; + if (!browser && !kind) continue; const answer = answered.get(call.id); + if (browser && answer && awaitingFreshLook(answer)) continue; + const did = !answer + ? null + : browser + ? browserDidOf(name, call, answer, labels) + : didOf(kind as ComputerActivity["kind"], call, answer); + const decision = decisions.get(call.id); + const allowed = allowedBy(decision); const finishedAt = finishedAtOf(answer); calls.push({ id: call.id, - did: answer ? didOf(kind, call, answer) : null, + did: did && allowed ? { ...did, allowed } : did, ...(finishedAt === undefined ? {} : { finishedAt }), + ...(decision?.at === undefined ? {} : { decidedAt: decision.at }), }); } } @@ -967,6 +1026,11 @@ export function useConversationActivity( messages: readonly Message[], ): void { const { copilotkit } = useCopilotKit(); + // The person's answers on the steps' cards: the same inbox the conversation's cards read. + const decisions = useQuery({ + ...approvalInboxOptions(), + select: decisionsIn, + }).data; useEffect( () => copilotkit.subscribe({ @@ -983,7 +1047,10 @@ export function useConversationActivity( [copilotkit], ); useEffect(() => { - recordActivity(computerId, conversationActivity(messages)); + recordActivity( + computerId, + conversationActivity(messages, decisions ?? NO_DECISIONS), + ); // And which of the panel's views the Bot is working in: see panel-view.ts. followView(computerId, latestView(messages)); }); diff --git a/app/tests/activity-allowed-line.test.tsx b/app/tests/activity-allowed-line.test.tsx new file mode 100644 index 0000000000..ded3cb6290 --- /dev/null +++ b/app/tests/activity-allowed-line.test.tsx @@ -0,0 +1,50 @@ +import { afterAll, afterEach, beforeAll, expect, test } from "bun:test"; +import { GlobalRegistrator } from "@happy-dom/global-registrator"; +import { cleanup, render } from "@testing-library/react"; +import { ActivityLog } from "@/components/computer/activity-log"; +import { recordActivity } from "@/lib/computers/activity"; +import { settleReactWork } from "./settle-react-work"; + +/** + * A step the person allowed on its card says how in Activity, as one they declined says "You + * declined it.": after "Allow for this conversation" no row said so, though the banner above the + * composer did. + */ + +beforeAll(() => GlobalRegistrator.register()); +afterEach(cleanup); +afterAll(async () => { + await settleReactWork(); + GlobalRegistrator.unregister(); +}); + +test("a step allowed on its card has a line that says how", () => { + recordActivity("allowed-line", [ + { + id: "sunflower", + did: { + kind: "browser", + step: "computer_click", + subject: "Place order", + output: "", + allowed: "allow_conversation", + }, + finishedAt: Date.parse("2026-11-12T22:05:33.000Z"), + }, + { + id: "chamomile", + did: { + kind: "browser", + step: "computer_click", + subject: "Place order", + output: "", + allowed: "allow_once", + }, + finishedAt: Date.parse("2026-11-12T22:06:10.000Z"), + }, + ]); + const view = render(); + const rows = view.getAllByRole("listitem"); + expect(rows[0]?.textContent).toContain("Allowed once."); + expect(rows[1]?.textContent).toContain("Allowed for this conversation."); +}); diff --git a/app/tests/computer-activity-names-and-times.test.ts b/app/tests/computer-activity-names-and-times.test.ts index 6be9963233..1a8d3acff9 100644 --- a/app/tests/computer-activity-names-and-times.test.ts +++ b/app/tests/computer-activity-names-and-times.test.ts @@ -5,9 +5,11 @@ import { HANDLED_BY_PERSON, } from "@openbot/shared/approval-outcomes"; import { withFinishedAt } from "@openbot/shared/computer-control"; +import type { ApprovalInboxData } from "../src/lib/approvals"; import { activityFor, conversationActivity, + decisionsIn, noteAnswered, recordActivity, } from "../src/lib/computers/activity"; @@ -272,3 +274,92 @@ test("a step's answer is stamped with when it finished, once, and a step waiting // A sentence (declined, done by the person) is not an object to stamp. expect(withFinishedAt(DECLINED_BY_PERSON)).toBe(DECLINED_BY_PERSON); }); + +/** The approvals inbox as the server answers it, with one decided request for this call. */ +function inboxWith( + toolCallId: string, + decision: "allow_conversation" | "deny", + decidedAt: string, +): ApprovalInboxData { + return { + enabled: true, + rules: [], + teamRules: [], + questions: [], + requests: [ + { + id: `request-${toolCallId}`, + status: "completed", + createdAt: "2026-11-12T22:05:00.000Z", + decidedAt, + decision, + action: { + botId: "fernhill-shopper", + toolRef: "computer_click", + effect: "write", + scope: "host.docker.internal", + threadId: "thread-1", + toolCallId, + args: { ref: "f2e13", snapshotId: 4 }, + target: {}, + }, + }, + ], + }; +} + +test("a step allowed for the conversation on its card says so", () => { + const decisions = decisionsIn( + inboxWith("sunflower", "allow_conversation", "2026-11-12T22:05:31.000Z"), + ); + const did = conversationActivity( + [ + ...checkout, + ...step( + "sunflower", + "computer_click", + { ref: "f2e13", snapshotId: 4 }, + { + ok: true, + action: "click", + element: { role: "button", name: "Place order" }, + url: "http://host.docker.internal:46600/shop/sunflower", + elapsedMs: 40, + finishedAt: "2026-11-12T22:05:33.000Z", + }, + ), + ], + decisions, + ).find((call) => call.id === "sunflower")?.did; + expect(did).toMatchObject({ + subject: "Place order", + allowed: "allow_conversation", + }); +}); + +test("a step declined on its card keeps the time it was declined when the conversation is opened again", () => { + const id = computer(); + const decidedAt = "2026-11-12T22:05:42.000Z"; + // Opened again: nothing here watched it, and its answer is a sentence that says no time. + recordActivity( + id, + conversationActivity( + [ + ...checkout, + ...step( + "basil", + "computer_click", + { ref: "f2e13", snapshotId: 4 }, + DECLINED_BY_PERSON, + ), + ], + decisionsIn(inboxWith("basil", "deny", decidedAt)), + ), + ); + const declined = activityFor(id).find((entry) => entry.id === "basil"); + expect(declined).toMatchObject({ + declined: true, + subject: "Place order", + at: Date.parse(decidedAt), + }); +}); diff --git a/e2e/README.md b/e2e/README.md index c547dbe88d..b313c36a53 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -166,14 +166,14 @@ The `computer-*` and `approvals-*` journeys: | Journey | What a person does | | --- | --- | -| `computer-browse` | Asks a Bot to read a page. The Computer opens on its live screen; the answer comes from the page; Activity lists the page with its time, and still does after a reload; the conversation folds the browsing into one line that opens to its steps. | +| `computer-browse` | Asks a Bot to read a page. The Computer opens on its live screen; the answer comes from the page; Activity lists the page with its time, and still does after a reload, every row with its time; the conversation folds the browsing into one line that opens to its steps. | | `computer-take-control` | Opens the Bot's screen full size, takes control, goes somewhere from the address bar, back, forward, reload, opens a tab and closes it (the screen still fills its frame, with no band under the page, then and after hand back), is asked before clearing the browser, and hands it back. Stops and resets the Bot's browser in Settings → Computer, and the Reset dialog names the Bot's browser until it has gone. | | `computer-terminal` | Asks a Bot to run a command: Activity shows it with what it printed. Takes control of the Terminal, types a command, hands it back. | | `computer-form` | Asks a Bot to fill in and send a form from one message, its fields and checkbox in one step: every action runs, the form arrives filled in, and Activity lists each with its time. | | `computer-sign-in` | A Bot reaches a password field and hands it over (no standing answer offered). The person takes control, types the password into the Bot's screen, hands back and answers "I did it myself"; the Bot carries on from the signed-in page. Activity names the field handed over, and every row has its time. | | `computer-secret` | A Bot asks for a password with the masked box, naming the field by its ref, without clicking it into a hand-over. The person types it into the box beside the Bot's screen; the Bot signs in and answers, and no step was handed over. | | `approvals-allow-once` | A Bot paying asks first. Allow once places that order and the reply arrives whole, and every Activity row has its time, the steps the server took after the approval too; the next order asks again, and waits on the Bots page under Needs you too; Deny there places nothing, the Bot says so, and Activity names the button declined. | -| `approvals-this-conversation` | Allow for this conversation: the next order there goes through with no card, the banner says what is allowed, Turn off makes the next order ask again. | +| `approvals-this-conversation` | Allow for this conversation: Activity says the order was allowed for this conversation, the next order there goes through with no card, the banner says what is allowed, Turn off makes the next order ask again. After a reload Activity still says how each order was allowed, every row with its time. | | `approvals-always` | Always allow here: the Bot pays without asking in its next conversation; the rule is on the Bot's Approvals page, where Remove takes it away. | | `approvals-settings` | Settings → Approvals: Extra vigilant, Auto-review and commands on this computer, kept across a reload; a rule for every Bot added, changed, removed, and cancelled. | diff --git a/e2e/journeys/approvals-allow-once.journey.ts b/e2e/journeys/approvals-allow-once.journey.ts index ed7a9daad1..b253d91519 100644 --- a/e2e/journeys/approvals-allow-once.journey.ts +++ b/e2e/journeys/approvals-allow-once.journey.ts @@ -120,6 +120,8 @@ test("Allow once places one order and the next one asks again; Deny buys nothing await expect(declined).toHaveCount(1); await expect(declined).toContainText("Place order"); await expect(declined).toContainText("You declined it."); + // This page was opened again after the Deny, and every row kept its time, the declined one too. + await expectEveryRowTimed(page); await shot("denied"); }); }); diff --git a/e2e/journeys/approvals-this-conversation.journey.ts b/e2e/journeys/approvals-this-conversation.journey.ts index 76ffba3de5..deb1954782 100644 --- a/e2e/journeys/approvals-this-conversation.journey.ts +++ b/e2e/journeys/approvals-this-conversation.journey.ts @@ -6,10 +6,13 @@ import { expectAsksToPay, } from "./approval-steps"; import { + activityRows, expect, + expectEveryRowTimed, expectWholeReply, named, newConversation, + openComputer, replyAfter, runsSoFar, send, @@ -54,6 +57,14 @@ test("Allow for this conversation lets the next order through, until it is turne await expectWholeReply(page, reply); await named(page, channelId); expect(ordered()).toEqual(["sunflower"]); + // Activity says how the order was allowed, as it says when one is declined. + await openComputer(page); + await expect( + activityRows(page) + .filter({ hasText: "Place order" }) + .filter({ hasText: "Allowed for this conversation." }), + ).toHaveCount(1); + await expectEveryRowTimed(page); }); const allowed = allowances(page); @@ -102,4 +113,17 @@ test("Allow for this conversation lets the next order through, until it is turne await expect(allowed).toHaveCount(0); await shot("allowed-once-after-turn-off"); }); + + await test.step("after a reload, Activity still says how each order was allowed, and when", async () => { + await page.reload(); + await openComputer(page); + const orders = activityRows(page).filter({ hasText: "Place order" }); + await expect(orders).toHaveCount(3); + await expect( + orders.filter({ hasText: "Allowed for this conversation." }), + ).toHaveCount(1); + await expect(orders.filter({ hasText: "Allowed once." })).toHaveCount(1); + await expectEveryRowTimed(page); + await shot("activity-after-reload"); + }); }); diff --git a/e2e/journeys/computer-browse.journey.ts b/e2e/journeys/computer-browse.journey.ts index 4d6e7ddd69..e4bc25adfb 100644 --- a/e2e/journeys/computer-browse.journey.ts +++ b/e2e/journeys/computer-browse.journey.ts @@ -4,6 +4,7 @@ import { CLOCK, computerPanel, expect, + expectEveryRowTimed, expectWholeReply, named, newConversation, @@ -98,6 +99,8 @@ test("a Bot opens a page, reads it and answers, live on its computer", async ({ await expect(opened).toHaveCount(1); await expect(opened).toContainText(`${SITE}/`); await expect(panel.getByText(/^Nothing yet/)).toHaveCount(0); + // Every step keeps its time across the reload: its answer says when it finished. + await expectEveryRowTimed(page); await shot("activity-after-reload"); }); }); From c7b41bcc0f52bed2130d1daf3fdd3fe63253c6d6 Mon Sep 17 00:00:00 2001 From: David McKay Date: Thu, 12 Nov 2026 18:05:01 -0800 Subject: [PATCH 3/3] Expect the finish time on the answers the approval wrapper gives the Bot The browser now stamps each computer step's answer with when it finished, so the wrapper's tests expect it beside the result they pin. --- app/tests/approval-tools.test.tsx | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/app/tests/approval-tools.test.tsx b/app/tests/approval-tools.test.tsx index e68614862d..77dd9ecc29 100644 --- a/app/tests/approval-tools.test.tsx +++ b/app/tests/approval-tools.test.tsx @@ -254,6 +254,8 @@ test("a change the computer could not make comes back to the Bot as a result, no reason: "Element e7 is not on the page any more. Take a fresh snapshot and use the refs from it.", staleRefs: true, + // Every answer says when it finished, for the time beside it in Activity (`withFinishedAt`). + finishedAt: expect.any(String), }); }); @@ -269,6 +271,7 @@ test("a refused change comes back as a refusal the Bot can explain", async () => reason: "Your team does not allow this.", refused: true, rule: "team-rule-1", + finishedAt: expect.any(String), }); }); @@ -280,10 +283,15 @@ test("a gate that cannot be reached, or answers with no result, is still a resul ).toEqual({ ok: false, reason: "The assistant's computer could not be reached.", + finishedAt: expect.any(String), }); expect( await clickAgainstGate(async () => Response.json({}, { status: 200 })), - ).toEqual({ ok: false, reason: "The computer action returned no result." }); + ).toEqual({ + ok: false, + reason: "The computer action returned no result.", + finishedAt: expect.any(String), + }); }); /* @@ -436,7 +444,11 @@ test("a look nothing asks about goes straight to the computer and nowhere else", expect(looked).toHaveLength(1); // Asked once about the conversation, and the answer kept: it never changes. expect(paths).toEqual(["/api/approvals/threads/thread"]); - expect(result).toEqual({ ok: true, title: "Example Domain" }); + expect(result).toEqual({ + ok: true, + title: "Example Domain", + finishedAt: expect.any(String), + }); }); test("in a task's own conversation, opened with no conversation list loaded, a look goes to the gate first", async () => { @@ -474,7 +486,11 @@ test("a look whose conversation couldn't be checked goes through the gate, where "/api/approvals/threads/thread", "/api/approvals/computer/default", ]); - expect(result).toEqual({ ok: true, title: "Example Domain" }); + expect(result).toEqual({ + ok: true, + title: "Example Domain", + finishedAt: expect.any(String), + }); }); test("in a task's own conversation, a look with no conversation to ask in is not taken at all", async () => { @@ -492,6 +508,7 @@ test("in a task's own conversation, a look with no conversation to ask in is not ok: false, reason: "Not done: this conversation asks before every step, and this step couldn't be asked about here.", + finishedAt: expect.any(String), }); });