Stop mistaking a filled-in form for a payment, and stop refusing a browser step while it saves

CHANGELOG.md

@@ -35,6 +35,25 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
   request for an administrator to switch on a work app, grant it to a Bot, or connect a Shared app's
   team account is kept, with who answered it and when. It starts empty and nothing is backfilled.
 
+### Filling in a form is no longer mistaken for paying
+
+The built-in safety rules (paying or moving money, credentials, security settings) read the words a
+Bot typed as well as where it typed them, so a Bot entering a CRM note about a customer's purchase
+orders, or a press release's source such as Business Wire, was stopped with "Paying, purchasing or
+moving money is always done by the person" and the step handed back to you. Typing is now judged by
+the field, the page and the button: a card number field, a checkout page or a new password field
+still goes to you, whatever is typed. A news wire (business, news or press wire) on a link or button
+no longer counts as wiring money; wiring money still does.
+
+### A Bot's browser step is no longer refused while it is being saved
+
+Before your browser runs a click or a typed entry for a Bot, the server checks it against the copy
+saved in the conversation. That copy is saved a moment behind, a piece at a time, and a check that
+read it half saved refused the step with "That unanswered tool call is not saved in this
+conversation" or "Supply a valid request", so a form the Bot was filling in could stop partway or ask
+for your approval twice. The check now waits, as it already did for a step not saved at all, until
+the step is saved whole.
+
 ### A Bot can choose from a dropdown
 
 A Bot filling in a form in its browser could not set a dropdown (a native `<select>`, such as a CRM's

server/src/approvals/policy.ts

@@ -103,7 +103,9 @@ const SAFETY_REQUIREMENTS: readonly {
     id: "payments",
     reason: "Paying, purchasing or moving money is always done by the person.",
     pattern:
-      /payment|\bpay(ing)?\b|pay[-_ ]?now|checkout|purchase|place[-_ ]?order|buy[-_ ]?now|\bwire\b|bank[-_ ]?transfer|transfer[-_ ]?(money|funds)|send[-_ ]?money|credit[-_ ]?card|card[-_ ]?number|\bcvv\b|\biban\b|routing[-_ ]?number|billing/i,
+      // A wire of money, not a news wire: "Business Wire" is where press releases go out, so a Bot
+      // filling in where its research came from would otherwise hand that typing to the person.
+      /payment|\bpay(ing)?\b|pay[-_ ]?now|checkout|purchase|place[-_ ]?order|buy[-_ ]?now|(?<!\b(business|news|press)[-_ ]?)\bwire\b|bank[-_ ]?transfer|transfer[-_ ]?(money|funds)|send[-_ ]?money|credit[-_ ]?card|card[-_ ]?number|\bcvv\b|\biban\b|routing[-_ ]?number|billing/i,
   },
 ];
 
@@ -134,10 +136,20 @@ export function safetyRequirement(
 ): { id: string; reason: string } | undefined {
   if (candidate.effect === "read") return undefined;
   if (NOT_ACCOUNT_ACTIONS.test(candidate.toolRef)) return undefined;
+  // What a Bot types is content, as reading a checkout page is not paying: a CRM note about a
+  // customer's purchase orders is not a purchase. The field it types into and the page it is on
+  // say what the action is ("Card number", ".../checkout", "New password"), so they are read.
+  const args =
+    candidate.toolRef === "computer_type" &&
+    candidate.args !== null &&
+    typeof candidate.args === "object" &&
+    !Array.isArray(candidate.args)
+      ? { ...(candidate.args as Record<string, unknown>), text: undefined }
+      : candidate.args;
   const words = [
     candidate.toolRef,
     candidate.scope,
-    flatten(candidate.args),
+    flatten(args),
     flatten(candidate.target),
   ].join(" ");
   const hit = SAFETY_REQUIREMENTS.find((entry) => entry.pattern.test(words));

server/src/approvals/wiring.ts

@@ -13,6 +13,52 @@ import { createApprovalQuestions, type PersonQuestion } from "./questions";
 import { createApprovalService } from "./service";
 import { ApprovalRefusedError, type ApprovalStore } from "./types";
 
+type ThreadHistory = Awaited<
+  ReturnType<Pick<IntelligenceLike, "getThreadMessages">["getThreadMessages"]>
+>;
+
+/**
+ * The saved copy of the call the browser is about to run, read back from the conversation.
+ *
+ * Intelligence saves a streaming run's messages a moment behind the stream, and the browser runs
+ * the tool as soon as the call arrives. It saves the call's arguments delta by delta too, so a read
+ * can find the call with half its arguments: that is not a mismatch yet, any more than a call not
+ * saved yet is. Either is waited for briefly; one that never appears whole does not match, and the
+ * caller refuses it.
+ */
+export async function savedToolCall(
+  read: () => Promise<ThreadHistory>,
+  snapshot: { toolCallId: string; toolName: string; args: unknown },
+  wait: { attempts?: number; sleep?: (ms: number) => Promise<void> } = {},
+): Promise<{ matches: boolean; answered: boolean }> {
+  const attempts = wait.attempts ?? 10;
+  const sleep = wait.sleep ?? ((ms: number) => Bun.sleep(ms));
+  let history = await read();
+  const matches = () => {
+    const call = history.messages
+      .flatMap((message) => message.toolCalls ?? [])
+      .find((saved) => saved.id === snapshot.toolCallId);
+    if (!call || call.name !== snapshot.toolName) return false;
+    try {
+      return isDeepStrictEqual(JSON.parse(call.args), snapshot.args);
+    } catch {
+      // Arguments still being saved.
+      return false;
+    }
+  };
+  for (let attempt = 0; !matches() && attempt < attempts; attempt++) {
+    await sleep(500);
+    history = await read();
+  }
+  return {
+    matches: matches(),
+    answered: history.messages.some(
+      (message) =>
+        message.role === "tool" && message.toolCallId === snapshot.toolCallId,
+    ),
+  };
+}
+
 /**
  * The deployment's approval service: the stored approvals, told the person's channel when one
  * opens; the question store; and a computer tool resumed only when its call is really saved in the
@@ -120,35 +166,15 @@ export function createServerApprovalService(deps: {
         throw new ApprovalRefusedError(
           "That conversation is no longer available to this Bot and person.",
         );
-      // Intelligence saves a streaming run's messages a moment behind the stream, and the browser runs
-      // the tool as soon as the call arrives. A call that is not saved yet is waited for briefly; one
-      // that never appears is refused below.
-      let history = await routineIntelligence.getThreadMessages({
-        threadId: snapshot.threadId,
-        userId: ownerUserId,
-      });
-      const findCall = () =>
-        history.messages
-          .flatMap((message) => message.toolCalls ?? [])
-          .find((call) => call.id === snapshot.toolCallId);
-      for (let attempt = 0; !findCall() && attempt < 10; attempt++) {
-        await Bun.sleep(500);
-        history = await routineIntelligence.getThreadMessages({
-          threadId: snapshot.threadId,
-          userId: ownerUserId,
-        });
-      }
-      const canonicalCall = findCall();
-      if (
-        !canonicalCall ||
-        canonicalCall.name !== snapshot.toolName ||
-        !isDeepStrictEqual(JSON.parse(canonicalCall.args), snapshot.args) ||
-        history.messages.some(
-          (message) =>
-            message.role === "tool" &&
-            message.toolCallId === snapshot.toolCallId,
-        )
-      )
+      const saved = await savedToolCall(
+        () =>
+          routineIntelligence.getThreadMessages({
+            threadId: snapshot.threadId,
+            userId: ownerUserId,
+          }),
+        snapshot,
+      );
+      if (!saved.matches || saved.answered)
         throw new ApprovalRefusedError(
           "That unanswered tool call is not saved in this conversation.",
         );

server/tests/approvals-policy.test.ts

@@ -224,6 +224,70 @@ describe("built-in safety requirements", () => {
       ),
     ).toBeUndefined();
   });
+
+  test("what a Bot types into a field is content: the field and the page say what the action is", () => {
+    // As the computer describes a typing action: the field it types into and the page it is on.
+    const typing = (text: string, field: string, pageUrl: string) =>
+      safetyRequirement(
+        candidate({
+          toolRef: "computer_type",
+          scope: new URL(pageUrl).host,
+          args: { ref: "f1e145", text, submit: false, snapshotId: 3 },
+          target: {
+            element: { ref: "f1e145", name: field, role: "textbox" },
+            pageUrl,
+            snapshotId: 3,
+          },
+        }),
+      )?.id;
+    const crm = "http://host.docker.internal:4900/accounts/new";
+    // A new CRM account, filled in from research: its notes and source name purchase orders,
+    // a payments company, a news wire. None of it is paying.
+    expect(
+      typing(
+        "Expanded its AI agents for purchase orders, detention and dock workflows.",
+        "Notes",
+        crm,
+      ),
+    ).toBeUndefined();
+    expect(typing("Business Wire; LinkedIn", "Source", crm)).toBeUndefined();
+    expect(
+      typing("Payments platform for freight billing", "Notes", crm),
+    ).toBeUndefined();
+    // A payment field or a checkout page still goes to the person, whatever is typed.
+    expect(
+      typing(
+        "4111 1111 1111 1111",
+        "Card number",
+        "https://shop.example/account",
+      ),
+    ).toBe("payments");
+    expect(typing("1", "Quantity", "https://shop.example/checkout")).toBe(
+      "payments",
+    );
+    expect(
+      typing("hunter2", "New password", "https://mail.example/settings"),
+    ).toBe("credentials");
+  });
+
+  test("a news wire is not a wire of money", () => {
+    const clicking = (name: string) =>
+      safetyRequirement(
+        candidate({
+          toolRef: "computer_click",
+          scope: "news.example",
+          args: { ref: "e7" },
+          target: { element: { ref: "e7", name, role: "link" } },
+        }),
+      )?.id;
+    // Press releases go out on Business Wire; following one is not paying.
+    expect(clicking("Read the release on Business Wire")).toBeUndefined();
+    expect(clicking("news wire")).toBeUndefined();
+    // Moving money by wire still goes to the person.
+    expect(clicking("Wire the deposit to the vendor")).toBe("payments");
+    expect(clicking("Send a wire")).toBe("payments");
+    expect(clicking("Start a wire transfer")).toBe("payments");
+  });
 });
 
 describe("the reviewer's safety classification", () => {

server/tests/approvals-saved-call.test.ts

@@ -0,0 +1,81 @@
+import { describe, expect, test } from "bun:test";
+import { savedToolCall } from "../src/approvals/wiring";
+
+// What the browser asks to run: the Bot's click, as the live stream delivered it whole.
+const snapshot = {
+  toolCallId: "call_trzjXH0dFVyoivzeFqY2kn4D",
+  toolName: "computer_click",
+  args: { ref: "f2e94", snapshotId: 7 },
+};
+
+/** The conversation as Intelligence returns it, holding the click with these saved arguments. */
+const conversation = (args: string | null, answered = false) =>
+  ({
+    messages: [
+      ...(args === null
+        ? []
+        : [
+            {
+              id: "1941c748-b06c-4cf6-89f0-a8904e89b22c",
+              role: "assistant",
+              toolCalls: [
+                { id: snapshot.toolCallId, name: "computer_click", args },
+              ],
+            },
+          ]),
+      ...(answered
+        ? [{ id: "result", role: "tool", toolCallId: snapshot.toolCallId }]
+        : []),
+    ],
+  }) as never;
+
+/** Each read returns the next saved state, then keeps returning the last. */
+const reads = (...states: never[]) => {
+  let index = 0;
+  return async () => states[Math.min(index++, states.length - 1)]!;
+};
+const noWait = { sleep: async () => {} };
+
+describe("the saved copy of a call the browser is about to run", () => {
+  test("a call read while its arguments are still being saved is waited for, not refused", async () => {
+    // Intelligence saves a call's arguments delta by delta, behind the stream: a read can land
+    // after the call starts and before its arguments end. The first reads here are what run 112
+    // of the Kite demo held part-way through: half the JSON, then all of it but the last key.
+    const saved = await savedToolCall(
+      reads(
+        conversation('{"ref":"f2e'),
+        conversation('{"ref":"f2e94"}'),
+        conversation('{"ref":"f2e94","snapshotId":7}'),
+      ),
+      snapshot,
+      noWait,
+    );
+    expect(saved).toEqual({ matches: true, answered: false });
+  });
+
+  test("a call that never appears, or never matches, is still refused once the wait runs out", async () => {
+    expect(
+      await savedToolCall(reads(conversation(null)), snapshot, noWait),
+    ).toMatchObject({ matches: false });
+    expect(
+      await savedToolCall(
+        reads(conversation('{"ref":"e12","snapshotId":7}')),
+        snapshot,
+        noWait,
+      ),
+    ).toMatchObject({ matches: false });
+    expect(
+      await savedToolCall(reads(conversation('{"ref":"f2e')), snapshot, noWait),
+    ).toMatchObject({ matches: false });
+  });
+
+  test("a call that already has a result is reported as answered", async () => {
+    expect(
+      await savedToolCall(
+        reads(conversation('{"ref":"f2e94","snapshotId":7}', true)),
+        snapshot,
+        noWait,
+      ),
+    ).toEqual({ matches: true, answered: true });
+  });
+});

server/tests/assertion-shape.test.ts

@@ -639,7 +639,7 @@ const ALLOW: Allowed[] = [
   },
   {
     file: "server/tests/approvals-policy.test.ts",
-    line: 715,
+    line: 779,
     shape: "b",
     finding: "NEW (round 2 did not find this)",
     why: "expect(continued[0]?.error).toBeUndefined();",