From 8e7998ab4ee26ccfbe0c5dbdc9a236f9f015ec63 Mon Sep 17 00:00:00 2001 From: Jerel John Velarde Date: Wed, 19 Aug 2026 13:32:46 -0700 Subject: [PATCH 1/2] Count a Bot repeating itself, and hand the count to the boundary A model that cannot get something to work retries. It clicks the same button, reloads the same page, writes the same file, and nothing counted it. Every one of those attempts is a real action on somebody's live website and a real charge against somebody's model credit, and the audit trail recorded them one row at a time with no way to see that they were the same row thirty times over. The gateway is the one place this can be counted, because it is the one place every governed action already goes through and already writes a row for. `computer/repeat.ts` keys a call on the tool plus the argument that says which thing it acted on, over a sliding three-minute window, and the gateway asks it before it asks the policy. The count lands in `PolicyContext` as `repeat.count`, so a deployment can write `repeat.count >= 10` in its deny list and stop a Bot going in circles with the boundary it already has, on the page it already uses. The detector does not refuse anything, on purpose. Blocking here would be a second boundary with rules of its own, invisible on the Boundaries page, unanswerable to dry-run, and impossible to relax for the one Bot whose job really is to poll something. It observes; the policy decides. Crossing 3, 10, or 25 writes one `computer.action_repeated` row each, carrying the tool, a readable fingerprint and the count. It is deliberately not filed as a refusal: nothing was forbidden and nothing was stopped, and a trail that files an observation as a refusal teaches a reader to skim past the refusals that are real. The audit page gives it its own filter and its own words for the same reason. The window costs something and the preset says so. It is time-based, so a Bot slow enough to spread its attempts wider than the window never trips it, and one that varies a single argument each time round is ten different calls. COMPUTER_REPEAT_WINDOW_MS widens it for a deployment whose provider is slow enough to need that, and refuses to start rather than falling back, because a rule about repetition that never fires looks exactly like a Bot behaving itself. --- .env.example | 17 +- app/src/routes/_authed/admin/audit.tsx | 18 ++ app/src/routes/_authed/admin/boundaries.tsx | 6 + docs/architecture.md | 7 + server/src/audit.ts | 16 ++ server/src/computer/gateway.ts | 90 +++++++ server/src/computer/policy.ts | 19 ++ server/src/computer/repeat.ts | 245 +++++++++++++++++ server/src/config.ts | 34 +++ server/src/index.ts | 10 + server/src/plugins/store.ts | 6 + server/tests/computer-gateway.test.ts | 186 ++++++++++++- server/tests/computer-policy.test.ts | 48 ++++ server/tests/computer-repeat.test.ts | 279 ++++++++++++++++++++ server/tests/config.test.ts | 33 +++ 15 files changed, 1012 insertions(+), 2 deletions(-) create mode 100644 server/src/computer/repeat.ts create mode 100644 server/tests/computer-repeat.test.ts diff --git a/.env.example b/.env.example index 89d8a3999..68cbff74a 100644 --- a/.env.example +++ b/.env.example @@ -90,7 +90,12 @@ AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true # which Bot took it. A rule can still restrict a single Bot with `bot.id`. # # Attributes: tool.name, bot.id, actor.id, page.url, page.host, element.ref/role/name/type, -# key, file.path, file.name, file.extension. +# key, file.path, file.name, file.extension, repeat.count. +# +# repeat.count is how many times this Bot has just made this exact call, counting the one being +# decided. A stuck model retries, and each retry is a real action on somebody's live website that is +# perfectly reasonable on its own terms; only the count tells the thirtieth click apart from the +# first. `repeat.count >= 10` in `deny` stops a Bot going in circles. # # Name every route to the same effect. A form submits from a keypress in any of its fields, so a rule # that only blocks a Submit button does not block Enter from another field. The example below refuses @@ -101,6 +106,16 @@ AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true # # AGENT_COMPUTER_POLICY={"mode":"enforce","deny":["(intent == \"activate\" && contains(element.name, \"submit\")) || (tool.name == \"computer_key\" && key == \"Enter\")"],"allow":["true"]} +# How long two identical calls count as the same repetition, in ms. Three minutes unset, which +# assumes a retry loop is a model round trip apart: call the tool, read the failure, try again. +# Widen it for a deployment whose provider is slow or heavily queued, where genuine retries arrive +# minutes apart and every attempt would otherwise be counted as the first one. Widen it too far and +# honest work starts to accumulate: a Bot told to watch a dashboard all morning reloads the same page +# and is not stuck. Anything that is not a positive whole number stops the server rather than falling +# back to the default, because a rule about repetition that never fires looks exactly like a Bot +# behaving itself. +# COMPUTER_REPEAT_WINDOW_MS=180000 + # How long one action waits for its element, in ms. Read by agent-computer, not the server. # ACTION_TIMEOUT_MS=10000 diff --git a/app/src/routes/_authed/admin/audit.tsx b/app/src/routes/_authed/admin/audit.tsx index d6f44e585..182ec683e 100644 --- a/app/src/routes/_authed/admin/audit.tsx +++ b/app/src/routes/_authed/admin/audit.tsx @@ -39,6 +39,12 @@ const FILTERS = [ "?eventType=computer.action_refused,mcp.call_rejected,component.refused,component.function_refused", }, { label: "Did not happen", search: "?eventType=computer.action_failed" }, + { + // Its own filter rather than a place in "Blocked". A Bot repeating itself has not been stopped by + // anything, and putting it beside the refusals would make the refusals look less real. + label: "Going in circles", + search: "?eventType=computer.action_repeated", + }, ] as const; function AuditPage() { @@ -163,6 +169,10 @@ function Row({ ) : null} + ) : typeof payload.fingerprint === "string" ? ( + // A repeat row has no element and no file of its own: what it is about is the call, which + // the fingerprint names in full. + {payload.fingerprint} ) : typeof payload.file === "string" ? ( {payload.file} ) : typeof element === "object" && element?.name ? ( @@ -223,6 +233,12 @@ function Row({ , reported by the Bot itself ) : null} + {event.eventType === "computer.action_repeated" && + typeof payload.count === "number" ? ( +
+ {payload.count} times within a few minutes +
+ ) : null} {failed && typeof payload.failure === "string" ? (
{payload.failure} @@ -268,6 +284,8 @@ const DECISIONS: Record = { "computer.secret_supplied": "A person supplied a secret", "computer.reset": "The computer was reset", "computer.stopped": "A person pressed stop", + // Not "Blocked". Nothing refused this; the Bot did the same thing again and the trail is saying so. + "computer.action_repeated": "The Bot repeated itself", "component.granted": "Granted to this Bot", "component.revoked": "Taken away from this Bot", diff --git a/app/src/routes/_authed/admin/boundaries.tsx b/app/src/routes/_authed/admin/boundaries.tsx index dd920c2d5..0af0ea0af 100644 --- a/app/src/routes/_authed/admin/boundaries.tsx +++ b/app/src/routes/_authed/admin/boundaries.tsx @@ -32,6 +32,12 @@ const PRESETS: { label: string; rule: string; cost?: string }[] = [ rule: 'intent == "type" && contains(element.name, "password")', cost: "A password box the page labels something else is not covered, the rule matches the label.", }, + { + label: "Stop a Bot repeating itself", + // The count includes the attempt being decided, so this refuses the tenth, not the eleventh. + rule: "repeat.count >= 10", + cost: "Counts identical calls within a few minutes, so a Bot slow enough to spread its attempts wider is never caught, and one that changes a single argument each time is ten different calls.", + }, { label: "Stay off social media", rule: 'intent == "navigate" && (contains(page.host, "facebook.com") || contains(page.host, "x.com"))', diff --git a/docs/architecture.md b/docs/architecture.md index 90b52c14d..6fe04e602 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -56,6 +56,13 @@ Policy rules can inspect: - `key` - `file.path`, `file.name`, `file.extension` - `mcp.server`, `mcp.tool`, `mcp.effect` +- `repeat.count` + +`repeat.count` is how many times that Bot has just made that exact call, counting the one being +decided. The gateway keys it on the tool plus the ref, key, file path, or target URL, over a sliding +window that defaults to three minutes and is set by `COMPUTER_REPEAT_WINDOW_MS`. Crossing 3, 10, or +25 writes one `computer.action_repeated` row each; the detector itself never refuses anything, so +`repeat.count >= 10` in `deny` is what stops a Bot going in circles. Rules use CEL expressions plus case-insensitive `contains()` and `matches()`. Deny rules are evaluated before allow rules. The policy engine fails closed: a diff --git a/server/src/audit.ts b/server/src/audit.ts index 0ff57945d..405ea43fa 100644 --- a/server/src/audit.ts +++ b/server/src/audit.ts @@ -50,6 +50,22 @@ export const auditEventTypes = [ // Permitted by policy, attempted, and did not succeed. Its own type because "allowed" reads as // "happened", and a trail that cannot tell those apart misleads exactly when it matters most. "computer.action_failed", + /** + * The same call, again, and again. + * + * The rows above record actions one at a time, which is the only way to record them and the reason + * a Bot stuck in a retry loop is invisible here: thirty identical rows look like thirty rows. This + * one says the thing the sequence cannot, that these are the same call, and how many times. + * + * It is not a refusal. Nothing was forbidden and nothing was stopped; a Bot did the same thing + * again, which is often merely a retry that is about to work. Filing it as a refusal would teach a + * reader to skim past the refusals that are real, so it is its own type and the audit page gives it + * its own words. + * + * Written when a count crosses a threshold rather than on every repeat, because a row per attempt + * would bury the attempts themselves under the observation that they kept happening. + */ + "computer.action_repeated", // A person taking the wheel and giving it back. Recorded as a period rather than as keystrokes: the // useful fact for an investigator is that a human drove this browser between these two times, and // logging every click a person made would bury it while telling nobody anything. diff --git a/server/src/computer/gateway.ts b/server/src/computer/gateway.ts index b2337be96..4d6bd11b2 100644 --- a/server/src/computer/gateway.ts +++ b/server/src/computer/gateway.ts @@ -25,6 +25,7 @@ import { type PolicyContext, type PolicyDecision, } from "./policy"; +import { createRepeatDetector, type RepeatDetector } from "./repeat"; import type { ClickInput, KeyInput, @@ -76,6 +77,15 @@ export type ComputerGatewayOptions = { auditStore: AuditStore; /** Absent denies everything. See evaluateActionPolicy. */ policy: () => ActionPolicy | undefined; + /** + * Counts a Bot repeating itself, so that the policy can be told how many times. + * + * Absent, the gateway makes its own, which is what almost every deployment gets. Passed in only to + * widen the window for a slow provider, or to hand a test a clock it can move, because otherwise + * proving that a window expires means a test that waits three minutes, and a test that waits three + * minutes is a test somebody eventually deletes. + */ + repeat?: RepeatDetector; }; /** @@ -95,6 +105,7 @@ type CachedSnapshot = { export function createComputerGateway(options: ComputerGatewayOptions) { const { client, auditStore, supervisor } = options; const snapshots = new Map(); + const repeat = options.repeat ?? createRepeatDetector(); /** * The computer, addressed as the Bot that is asking. @@ -169,11 +180,29 @@ export function createComputerGateway(options: ComputerGatewayOptions) { const intent = intentOf(toolName, subject.key); + /* + * Counted before the policy is asked, so that a rule written against the count decides the very + * attempt that crossed the line rather than the one after it. Off by one here would mean a + * deployment forbidding a tenth identical click allows the tenth and refuses the eleventh, which + * is the kind of thing nobody notices until they are counting rows in an incident. + * + * Reading a page never reaches this function, so nothing counts a Bot looking at the same screen + * over and over. That is the cheapest thing it does and the one nobody minds. + */ + const repetition = repeat.observe(botId, { + tool: toolName, + ref, + key: subject.key, + filePath, + targetUrl: subject.targetUrl, + }); + const context: PolicyContext = { tool: { name: toolName }, bot: { id: botId }, actor: { id: actor.id }, page: { url: pageUrl, host: hostOf(pageUrl) }, + repeat: { count: repetition.count }, ...(intent ? { intent } : {}), ...(subject.key ? { key: subject.key } : {}), ...(element @@ -189,6 +218,24 @@ export function createComputerGateway(options: ComputerGatewayOptions) { ...(filePath ? { file: describeFile(filePath) } : {}), }; + if (repetition.threshold !== null && repetition.fingerprint) { + /* + * Ahead of the decision row, so the trail reads in the order the thing happened: this was the + * tenth identical attempt, and this is what the policy did about it. Filed the other way round + * a reader has to deduce the cause from a row written after its effect. + */ + await writeRepeat(auditStore, { + toolName, + botId, + actor, + computerId, + pageUrl, + filePath, + fingerprint: repetition.fingerprint, + count: repetition.count, + }); + } + const decision = evaluateActionPolicy(options.policy(), context); await write(auditStore, { toolName, @@ -732,6 +779,49 @@ async function write( }); } +/** + * One row for a Bot going round in circles. + * + * Separate from `write` because there is no policy decision to record. This row is an observation + * about the call that is about to be decided, not the decision, and giving it a `decision` block + * would mean inventing an answer the policy was never asked for. It is also why it is not a refusal: + * nothing was forbidden here. + * + * The fingerprint goes in as written, which is why `repeat.ts` builds a readable one. A reader + * arriving at "the same call, 25 times" needs to be told which call in the row itself. + */ +async function writeRepeat( + auditStore: AuditStore, + entry: { + toolName: string; + botId: string; + actor: ActionActor; + computerId: string; + pageUrl: string; + filePath: string | undefined; + fingerprint: string; + count: number; + }, +) { + await recordAuditEvent(auditStore, { + eventType: "computer.action_repeated", + targetType: "computer", + targetId: entry.computerId, + ...(entry.actor.userId ? { actorUserId: entry.actor.userId } : {}), + payload: { + action: entry.toolName, + bot: entry.botId, + actor: entry.actor.id, + // The page, for a browser action only. A file call has nothing to do with whatever the browser + // happens to be showing, and naming a host on that row sends a reader somewhere irrelevant, the + // same trap `describeRefusal` avoids. + ...(entry.filePath ? {} : { page: entry.pageUrl }), + fingerprint: entry.fingerprint, + count: entry.count, + }, + }); +} + /** * The host a rule can match on, or empty. * diff --git a/server/src/computer/policy.ts b/server/src/computer/policy.ts index 7b18ceeb8..1b33d714b 100644 --- a/server/src/computer/policy.ts +++ b/server/src/computer/policy.ts @@ -46,6 +46,25 @@ export type PolicyContext = { bot: { id: string }; page: { url: string; host: string }; actor: { id: string }; + /** + * How many times this Bot has just made this exact call, counting the one being decided. + * + * A stuck model retries, and every retry is a real action on somebody's live website. Each one is + * permitted on its own terms, because each one is: the rule that would refuse the thirtieth click + * on a button would refuse the first, and refusing the first is refusing the product. Only the + * count separates them, so the count is here, and a deployment that wants to stop a Bot going in + * circles writes `repeat.count >= 10` and nothing else changes. + * + * Always present, at one on a call the Bot has not made before, so that a rule mentioning it is + * evaluable on every action. An absent field would throw inside CEL, and a deny rule that throws + * denies, so an optional `repeat` would turn one rule about repetition into a deployment that + * refuses everything. + * + * The cost is that the window is time-based: a Bot slow enough to spread its attempts wider than + * the window never trips this, and one that varies a single argument each time round is thirty + * different calls. It is a backstop against the loop that actually happens, not a guarantee. + */ + repeat: { count: number }; element?: { ref: string; role: string; diff --git a/server/src/computer/repeat.ts b/server/src/computer/repeat.ts new file mode 100644 index 000000000..81b40b42c --- /dev/null +++ b/server/src/computer/repeat.ts @@ -0,0 +1,245 @@ +/** + * How many times a Bot has just made this exact call. + * + * A model that cannot get something to work retries. It clicks the same button, reloads the same + * page, writes the same file, and every one of those attempts is a real action on somebody's live + * website and a real charge against somebody's model credit. The trail records all thirty of them, + * one row at a time, with nothing to say that they are the same row thirty times over, so nobody + * notices until the other side's rate limiter does. + * + * This counts, and it does nothing else with the answer. The count goes to the policy, which is the + * one thing in this codebase allowed to refuse an action. A detector that blocked on its own would + * be a second boundary with rules of its own, invisible on the Boundaries page, unanswerable to + * `dry-run`, and impossible to switch off for the one Bot whose job really is to poll something. + * One boundary, given better information. + * + * In memory, and per process, for the same reason the gateway's snapshot cache is: it describes what + * a Bot did in the last few minutes, and after a restart that question has no useful answer. The + * loop is over, because whatever was driving it is gone too. + */ + +/** + * How long a call stays counted. + * + * Time rather than "this turn", because the gateway has no idea where a turn begins or ends. It is + * handed one action at a time by whichever route is serving the model, and nothing in that path + * carries a turn boundary; inventing one would mean threading a conversation id through every acting + * call to get a worse answer than a clock gives. + * + * Three minutes. A retry loop is a model round trip each time round — call the tool, read the + * failure, decide to try again — which is seconds, not minutes, so ten identical attempts fit inside + * this comfortably. Much longer and honest repetition starts to accumulate: a Bot told to watch a + * dashboard reloads the same page all morning and is not stuck. Much shorter and a model that thinks + * for twenty seconds between attempts never trips anything. + */ +export const DEFAULT_REPEAT_WINDOW_MS = 3 * 60_000; + +/** + * The counts worth writing a row about. + * + * Three is "this is now a pattern rather than a retry", ten is "nobody is going to fix this by + * trying again", twenty-five is "somebody should look". Each fires once, so the trail gains three + * rows for a Bot stuck all afternoon rather than a row per attempt, which would bury the actions it + * was taking under the observation that it kept taking them. + */ +export const DEFAULT_REPEAT_THRESHOLDS: readonly number[] = [3, 10, 25]; + +/** + * How many distinct calls are remembered per Bot. + * + * A Bot doing genuinely varied work never repeats anything, so every call it makes is a new key, and + * without a cap the map grows for exactly the Bots this feature has nothing to say about. The + * least recently seen key is dropped, which is the one furthest from tripping anything. + */ +export const DEFAULT_REPEAT_KEYS_PER_BOT = 64; + +/** + * One governed call, in the terms the detector cares about. + * + * The same fields the gateway already assembles for the policy and the audit row. Nothing is added + * to a call site to support this. + */ +export type RepeatedCall = { + tool: string; + ref?: string | undefined; + key?: string | undefined; + filePath?: string | undefined; + targetUrl?: string | undefined; +}; + +export type RepeatObservation = { + /** Including the call being observed, so the first one counts as one. */ + count: number; + /** + * The call's identity, in a form a person can read. + * + * Null when nothing distinguished it. See `fingerprintOf`: a bare tool name is not a call worth + * counting, and the honest count for one of those is a single occurrence. + */ + fingerprint: string | null; + /** + * The threshold this call has just reached, or null on the overwhelming majority of calls. + * + * Reported once per run of repetition rather than on every call past it. A count that falls back + * out of the window and climbs again is a new run and reports again, because it is a Bot that got + * stuck twice. + */ + threshold: number | null; +}; + +export type RepeatDetector = { + /** Records the call and answers with what it now knows. Never throws, never blocks. */ + observe: (botId: string, call: RepeatedCall) => RepeatObservation; +}; + +export type RepeatDetectorOptions = { + windowMs?: number; + thresholds?: readonly number[]; + maxKeysPerBot?: number; + /** Injected so a test can move time without waiting for it. */ + now?: () => number; +}; + +/** One key's history, held only while it is still inside the window. */ +type Occurrences = { + /** When each counted call happened, oldest first. */ + at: number[]; + /** Which thresholds this run of repetition has already reported. */ + reported: Set; + /** Used to choose what to drop when a Bot is at its key limit. */ + lastSeen: number; +}; + +export function createRepeatDetector( + options: RepeatDetectorOptions = {}, +): RepeatDetector { + const windowMs = options.windowMs ?? DEFAULT_REPEAT_WINDOW_MS; + const maxKeysPerBot = options.maxKeysPerBot ?? DEFAULT_REPEAT_KEYS_PER_BOT; + // Ascending, so the loop below ends on the highest threshold a call crossed rather than on + // whichever one the caller happened to list last. + const thresholds = [ + ...(options.thresholds ?? DEFAULT_REPEAT_THRESHOLDS), + ].sort((a, b) => a - b); + const clock = options.now ?? Date.now; + + /** + * Per Bot, then per call. + * + * Nested rather than keyed on a combined string so that one Bot's varied work cannot push another + * Bot's history out: the cap is per Bot, and a shared map would make it a race between them. The + * outer map is bounded by how many Bots a deployment has, which is a list somebody wrote down. + */ + const perBot = new Map>(); + + return { + observe(botId, call) { + const fingerprint = fingerprintOf(call); + if (!fingerprint) { + return { count: 1, fingerprint: null, threshold: null }; + } + + const now = clock(); + let calls = perBot.get(botId); + if (!calls) { + calls = new Map(); + perBot.set(botId, calls); + } + + let entry = calls.get(fingerprint); + if (!entry) { + if (calls.size >= maxKeysPerBot) { + evictLeastRecent(calls); + } + entry = { at: [], reported: new Set(), lastSeen: now }; + calls.set(fingerprint, entry); + } + + const cutoff = now - windowMs; + entry.at = entry.at.filter((time) => time > cutoff); + if (entry.at.length === 0) { + // Nothing survived the window, so whatever run of repetition this key was in has ended and + // its thresholds are free to report again. Without this a Bot that got stuck, recovered and + // got stuck again an hour later would leave one row for two incidents. + entry.reported.clear(); + } + entry.at.push(now); + entry.lastSeen = now; + + const count = entry.at.length; + let threshold: number | null = null; + for (const candidate of thresholds) { + if (count >= candidate && !entry.reported.has(candidate)) { + entry.reported.add(candidate); + threshold = candidate; + } + } + + return { count, fingerprint, threshold }; + }, + }; +} + +/** + * What makes two calls the same call. + * + * The tool name is not enough, and a detector keyed on it alone would be worse than none: five + * clicks may be five different buttons, so a Bot working steadily down a form would look exactly + * like one stuck on its first field, and the first person to see a rule fire on that would turn the + * feature off. So the key is the tool plus the argument saying WHICH thing it acted on — the ref, + * the key pressed, the file path, the address being opened — and a call carrying none of those is + * not counted at all. + * + * Deliberately absent: the text being typed. A Bot that fills the same field thirty times is worth + * catching, but what it filled it with is a password as often as it is anything else, and from here + * the fingerprint travels into an audit row. The cost is that thirty different values into one field + * read as thirty repeats, which is the direction to be wrong in. + * + * A ref only means anything against the snapshot it came from. A page that re-renders and hands back + * a different ref for the same button reads as a different call, so a Bot looping around a reload is + * undercounted. Keying on the element's label instead would follow the button across snapshots and + * merge two buttons that share a label, and a count that is sometimes low is easier to live with + * than one that is sometimes about the wrong thing. + * + * Readable, because it goes on the audit row as-is. An investigator reading "the same action 25 + * times" needs to be told which action without going and decoding a hash. + */ +export function fingerprintOf(call: RepeatedCall): string | null { + const parts: string[] = []; + const add = (label: string, value: string | undefined) => { + const normalized = normalize(value); + if (normalized) parts.push(`${label}=${normalized}`); + }; + + add("ref", call.ref); + add("key", call.key); + add("file", call.filePath); + add("url", call.targetUrl); + + if (parts.length === 0) return null; + return [normalize(call.tool) || call.tool, ...parts].join(" "); +} + +/** + * Whitespace collapsed and trimmed. + * + * A model reproducing an argument from its own earlier output does not always reproduce the spacing, + * and a Bot writing to `reports/q3.md` and to `reports/q3.md ` is doing one thing twice. Treating + * those as two calls would let a stuck Bot slip the count without changing anything about what it + * was actually doing. + */ +function normalize(value: string | undefined): string { + return (value ?? "").replaceAll(/\s+/g, " ").trim(); +} + +/** The key a Bot touched longest ago, which is the one least likely to be part of a live loop. */ +function evictLeastRecent(calls: Map) { + let oldestKey: string | null = null; + let oldestSeen = Number.POSITIVE_INFINITY; + for (const [key, entry] of calls) { + if (entry.lastSeen < oldestSeen) { + oldestSeen = entry.lastSeen; + oldestKey = key; + } + } + if (oldestKey !== null) calls.delete(oldestKey); +} diff --git a/server/src/config.ts b/server/src/config.ts index 7d2c1a752..d9f8bd9fd 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -73,6 +73,15 @@ export type DeploymentConfig = { * precedence, which is the only subtle thing about them, impossible to see. */ policy?: ActionPolicy; + /** + * How long two identical calls count as the same repetition, in milliseconds. + * + * Absent uses the built-in window, which assumes a retry loop is a model round trip apart. It is + * here because that assumption is about someone else's model: a deployment on a slow or heavily + * queued provider can have genuine retries minutes apart, and there the built-in window counts + * every attempt as the first one and a rule about repetition never fires at all. + */ + repeatWindowMs?: number; }; }; @@ -269,11 +278,13 @@ function computerConfig( const computerToken = optional(environment, "COMPUTER_TOKEN"); const supervisorUrl = url(environment, "COMPUTER_SUPERVISOR_URL"); const supervisorToken = optional(environment, "SUPERVISOR_TOKEN"); + const repeatWindowMs = milliseconds(environment, "COMPUTER_REPEAT_WINDOW_MS"); return { baseUrl, allowPrivateHosts: optional(environment, "AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS") === "true", ...(policy ? { policy } : {}), + ...(repeatWindowMs ? { repeatWindowMs } : {}), ...(computerToken ? { token: computerToken } : {}), ...(supervisorUrl ? { @@ -286,6 +297,29 @@ function computerConfig( }; } +/** + * A duration in milliseconds, or a refusal to start. + * + * Refused rather than quietly defaulted, for the same reason a malformed policy is. An operator who + * widened a window and typed `3m` would otherwise get a running deployment on the built-in value, + * and the only evidence would be a rule that never fires, which reads exactly like a Bot behaving + * itself. + */ +function milliseconds( + environment: Environment, + name: string, +): number | undefined { + const raw = optional(environment, name); + if (!raw) { + return undefined; + } + const value = Number(raw); + if (!Number.isSafeInteger(value) || value <= 0) { + throw new Error(`${name} must be a positive whole number of milliseconds`); + } + return value; +} + /** * The action policy, as JSON in one variable. * diff --git a/server/src/index.ts b/server/src/index.ts index 2d72bca9f..cb9f7f0f2 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -22,6 +22,7 @@ import { createPolicyStore, DEFAULT_ACTION_POLICY, } from "./computer/policy-store"; +import { createRepeatDetector } from "./computer/repeat"; import { createSupervisorClient } from "./computer/supervisor"; import { loadConfig } from "./config"; import { createConnectorAdminService } from "./connectors"; @@ -329,6 +330,15 @@ const app = createApp( policy: () => policyStore.get(), // Stop, reset and the listing act on containers when there are containers to act on. ...(supervisor ? { supervisor } : {}), + // Only when a deployment has said its Bots retry on a slower rhythm than the built-in window + // assumes. Otherwise the gateway makes its own and nobody has to know it exists. + ...(config.computer?.repeatWindowMs + ? { + repeat: createRepeatDetector({ + windowMs: config.computer.repeatWindowMs, + }), + } + : {}), }) : undefined, policyStore, diff --git a/server/src/plugins/store.ts b/server/src/plugins/store.ts index b4caf3950..66fc4b46c 100644 --- a/server/src/plugins/store.ts +++ b/server/src/plugins/store.ts @@ -799,6 +799,12 @@ export function createPluginStore(options: PluginStoreOptions) { bot: { id: input.botId }, actor: { id: input.actorId }, page: { url: "", host: "" }, + // One, for the same reason as the empty strings, and with a cost worth naming: repetition is + // counted by the computer gateway, and nothing counts a Bot calling the same MCP tool over + // and over. A rule about repetition is therefore false here rather than unevaluable, which + // keeps a browser rule from refusing every tool call, and leaves a Bot looping through + // somebody else's server as a gap this deployment cannot yet see. + repeat: { count: 1 }, element: { ref: "", role: "", name: "", type: "" }, key: "", file: { path: "", name: "", extension: "" }, diff --git a/server/tests/computer-gateway.test.ts b/server/tests/computer-gateway.test.ts index eff68c07c..d29182c58 100644 --- a/server/tests/computer-gateway.test.ts +++ b/server/tests/computer-gateway.test.ts @@ -6,6 +6,10 @@ import { createComputerGateway, } from "../src/computer/gateway"; import type { ActionPolicy } from "../src/computer/policy"; +import { + createRepeatDetector, + type RepeatDetector, +} from "../src/computer/repeat"; import type { SnapshotResult } from "../src/computer/schema"; /** @@ -103,13 +107,18 @@ function fakeAudit() { const ACTOR = { id: "dev-local-user" }; const PERMISSIVE: ActionPolicy = { mode: "enforce", deny: [], allow: ["true"] }; -async function gatewayWith(policy: ActionPolicy | undefined) { +async function gatewayWith( + policy: ActionPolicy | undefined, + /** Only the repetition tests supply one; everything else gets the gateway's own. */ + repeat?: RepeatDetector, +) { const { client, calls } = fakeClient(); const { store, rows } = fakeAudit(); const gateway = createComputerGateway({ client, auditStore: store, policy: () => policy, + ...(repeat ? { repeat } : {}), }); // Every test acts on refs, so the server must hold a snapshot first, exactly as the real flow does. await gateway.snapshot("default"); @@ -397,3 +406,178 @@ describe("the computer gateway", () => { expect(rows[0]?.payload.element).toBe("not in the current snapshot"); }); }); + +/** + * The gateway is the only place that can count this, which is why it does. + * + * Every governed action already passes through one function that already writes a row for it, so + * counting is very nearly free here and impossible anywhere else. The part that matters is where the + * count goes: into the policy context, before the decision, so a deployment can act on it rather than + * read about it a week later. + */ +describe("a Bot going in circles", () => { + const click = { ref: "e9", snapshotId: 7 }; + + test("the count reaches the policy, and the rule refuses the attempt that crosses the line", async () => { + const { gateway, calls, rows } = await gatewayWith({ + ...PERMISSIVE, + deny: ["repeat.count >= 3"], + }); + + await gateway.click("default", "bot-1", ACTOR, click); + await gateway.click("default", "bot-1", ACTOR, click); + // The first two are the same action on the same button and nothing about them is objectionable. + expect(calls).toEqual(["click", "click"]); + + await expect( + gateway.click("default", "bot-1", ACTOR, click), + ).rejects.toThrow(ActionRefusedError); + // Counted before the decision, so the third attempt is the one refused rather than the fourth. + expect(calls).toEqual(["click", "click"]); + expect(rows.at(-1)?.eventType).toBe("computer.action_refused"); + }); + + test("a rule about repetition leaves a Bot doing varied work alone", async () => { + // The failure that would take this feature back out again: a Bot working steadily down a form + // refused for doing its job. + const { gateway, calls } = await gatewayWith({ + ...PERMISSIVE, + deny: ["repeat.count >= 3"], + }); + + await gateway.click("default", "bot-1", ACTOR, { + ref: "e1", + snapshotId: 7, + }); + await gateway.click("default", "bot-1", ACTOR, { + ref: "e9", + snapshotId: 7, + }); + await gateway.type("default", "bot-1", ACTOR, { + ref: "e1", + snapshotId: 7, + text: "Grace Hopper", + }); + + expect(calls).toEqual(["click", "click", "type"]); + }); + + test("crossing a threshold writes its own row, ahead of the decision it explains", async () => { + const { gateway, rows } = await gatewayWith( + PERMISSIVE, + createRepeatDetector({ thresholds: [3] }), + ); + + await gateway.click("default", "bot-1", ACTOR, click); + await gateway.click("default", "bot-1", ACTOR, click); + await gateway.click("default", "bot-1", ACTOR, click); + + // Two allowed actions, then the observation, then the third allowed action. Filed the other way + // round a reader has to deduce the cause from a row written after its effect. + expect(rows.map((row) => row.eventType)).toEqual([ + "computer.action_allowed", + "computer.action_allowed", + "computer.action_repeated", + "computer.action_allowed", + ]); + }); + + test("the row says which call, and how many times", async () => { + const { gateway, rows } = await gatewayWith( + PERMISSIVE, + createRepeatDetector({ thresholds: [2] }), + ); + + await gateway.click("default", "bot-1", ACTOR, click); + await gateway.click("default", "bot-1", ACTOR, click); + + const repeated = rows.find( + (row) => row.eventType === "computer.action_repeated", + ); + expect(repeated?.payload).toMatchObject({ + action: "computer_click", + bot: "bot-1", + fingerprint: "computer_click ref=e9", + count: 2, + }); + // Not a refusal, and it must not carry the furniture of one. A row with a `decision` block would + // read as the policy having answered a question nobody asked it. + expect(repeated?.payload.decision).toBeUndefined(); + }); + + test("a refused action is still counted, because the Bot still tried", async () => { + // A Bot hammering a button the policy forbids is going in circles as surely as one hammering a + // button that works, and it is the case an operator most wants to see. + const { gateway, rows } = await gatewayWith( + { ...PERMISSIVE, deny: ['contains(element.name, "submit")'] }, + createRepeatDetector({ thresholds: [2] }), + ); + + await gateway.click("default", "bot-1", ACTOR, click).catch(() => {}); + await gateway.click("default", "bot-1", ACTOR, click).catch(() => {}); + + expect(rows.map((row) => row.eventType)).toEqual([ + "computer.action_refused", + "computer.action_repeated", + "computer.action_refused", + ]); + }); + + test("two Bots on one gateway do not pool a count", async () => { + const { gateway, rows } = await gatewayWith( + PERMISSIVE, + createRepeatDetector({ thresholds: [2] }), + ); + + await gateway.click("default", "sales-bot", ACTOR, click); + await gateway.click("default", "research-bot", ACTOR, click); + + // One click each. A pooled count would file this against whichever Bot happened to go second. + expect( + rows.some((row) => row.eventType === "computer.action_repeated"), + ).toBe(false); + }); + + test("a call with nothing to distinguish it is never reported as a repeat", async () => { + // Scrolling names no element. Counting it by tool name alone would report a Bot reaching the + // bottom of a long page as one going in circles. + const { gateway, rows } = await gatewayWith( + PERMISSIVE, + createRepeatDetector({ thresholds: [2] }), + ); + + await gateway.scroll("default", "bot-1", ACTOR, { direction: "down" }); + await gateway.scroll("default", "bot-1", ACTOR, { direction: "down" }); + await gateway.scroll("default", "bot-1", ACTOR, { direction: "down" }); + + expect( + rows.every((row) => row.eventType === "computer.action_allowed"), + ).toBe(true); + }); + + test("a repeated file write names the path and not the browser's page", async () => { + // The workspace has nothing to do with whatever the browser happens to be showing, and naming a + // host on that row sends a reader somewhere irrelevant. + const { gateway, rows } = await gatewayWith( + PERMISSIVE, + createRepeatDetector({ thresholds: [2] }), + ); + + await gateway.writeFile("default", "bot-1", ACTOR, { + path: "notes.md", + contents: "again", + }); + await gateway.writeFile("default", "bot-1", ACTOR, { + path: "notes.md", + contents: "again", + }); + + const repeated = rows.find( + (row) => row.eventType === "computer.action_repeated", + ); + expect(repeated?.payload.fingerprint).toBe( + "computer_write_file file=notes.md", + ); + expect(repeated?.payload.page).toBeUndefined(); + }); +}); diff --git a/server/tests/computer-policy.test.ts b/server/tests/computer-policy.test.ts index 5191793cb..c2c8efe50 100644 --- a/server/tests/computer-policy.test.ts +++ b/server/tests/computer-policy.test.ts @@ -21,6 +21,10 @@ function context(overrides: Partial = {}): PolicyContext { bot: { id: "risk-analyst" }, actor: { id: "dev-local-user" }, page: { url: "https://example.com/order", host: "example.com" }, + // The first time this Bot has made this call, which is what a context with nothing to say about + // repetition means. Always present: an absent field throws inside CEL, and a throwing deny rule + // denies, so a rule about repetition would otherwise refuse everything. + repeat: { count: 1 }, element: { ref: "e13", role: "button", name: "Submit order" }, ...overrides, }; @@ -191,6 +195,7 @@ describe("the second door", () => { bot: { id: "sales" }, actor: { id: "someone" }, page: { url: "https://example.com/order", host: "example.com" }, + repeat: { count: 1 }, key: "Enter", }); expect(refused.allowed).toBe(false); @@ -201,6 +206,7 @@ describe("the second door", () => { bot: { id: "sales" }, actor: { id: "someone" }, page: { url: "https://example.com/order", host: "example.com" }, + repeat: { count: 1 }, key: "a", }); expect(allowed.allowed).toBe(true); @@ -217,6 +223,7 @@ describe("a rule written about what an action does", () => { bot: { id: "b" }, actor: { id: "a" }, page: { url: "https://example.com/", host: "example.com" }, + repeat: { count: 1 }, intent: "activate", ...extra, }); @@ -307,6 +314,7 @@ describe("a rule that names an identifier only some actions carry", () => { bot: { id: "b" }, actor: { id: "a" }, page: { url: "https://httpbin.org/forms/post", host: "httpbin.org" }, + repeat: { count: 1 }, intent: "navigate", }; @@ -350,3 +358,43 @@ describe("a rule that names an identifier only some actions carry", () => { expect(decision.allowed).toBe(false); }); }); + +/** + * The one attribute that separates the thirtieth click on a button from the first. + * + * Both are the same action on the same element, and any rule able to refuse the thirtieth by its + * shape would refuse the first as well. So the count is the whole of it, and these check that a rule + * written against it actually evaluates: `repeat` is a nested field like `page` and `element`, and a + * rule the engine cannot evaluate denies, which would turn one restriction into a Bot that can do + * nothing at all. + */ +describe("a rule about a Bot repeating itself", () => { + const repeating: ActionPolicy = { + mode: "enforce", + deny: ["repeat.count >= 10"], + allow: ["true"], + }; + + test("leaves the attempts below the line alone", () => { + expect( + evaluateActionPolicy(repeating, context({ repeat: { count: 9 } })) + .allowed, + ).toBe(true); + }); + + test("refuses the attempt that crosses it, not the one after", () => { + const decision = evaluateActionPolicy( + repeating, + context({ repeat: { count: 10 } }), + ); + expect(decision.allowed).toBe(false); + expect(decision.matched).toBe("repeat.count >= 10"); + }); + + test("goes on refusing past it", () => { + expect( + evaluateActionPolicy(repeating, context({ repeat: { count: 40 } })) + .allowed, + ).toBe(false); + }); +}); diff --git a/server/tests/computer-repeat.test.ts b/server/tests/computer-repeat.test.ts new file mode 100644 index 000000000..b84938ea1 --- /dev/null +++ b/server/tests/computer-repeat.test.ts @@ -0,0 +1,279 @@ +import { describe, expect, test } from "bun:test"; +import { + createRepeatDetector, + DEFAULT_REPEAT_WINDOW_MS, + fingerprintOf, +} from "../src/computer/repeat"; + +/** + * What the detector must get right, and every one of them is a way of being wrong that looks fine. + * + * A detector that overcounts is worse than none: the first person to see a boundary refuse a Bot + * working steadily down a form turns the boundary off, and takes the real refusals with it. A + * detector that fires its thresholds again on every call past them buries the actions a Bot took + * under the observation that it kept taking them. And a detector two Bots share reports one Bot's + * loop against another Bot's name, which is the one thing an audit trail may never do. + * + * Time is injected. A test that waits three minutes to prove a window expires is a test somebody + * eventually deletes. + */ + +/** A clock a test drives by hand, so a window can expire in a microsecond. */ +function clock(start = 1_000_000) { + let time = start; + return { + now: () => time, + advance(ms: number) { + time += ms; + }, + }; +} + +describe("counting a Bot repeating itself", () => { + test("identical calls count up", () => { + const detector = createRepeatDetector({ now: clock().now }); + const call = { tool: "computer_click", ref: "e9" }; + + expect(detector.observe("sales-bot", call).count).toBe(1); + expect(detector.observe("sales-bot", call).count).toBe(2); + expect(detector.observe("sales-bot", call).count).toBe(3); + }); + + test("the count includes the call being observed", () => { + // The gateway counts before it asks the policy, so a rule saying `repeat.count >= 10` has to + // refuse the tenth attempt. Starting at zero would make it refuse the eleventh, and nobody would + // notice until they were counting rows in an incident. + const detector = createRepeatDetector({ now: clock().now }); + expect( + detector.observe("sales-bot", { tool: "computer_click", ref: "e1" }), + ).toMatchObject({ count: 1 }); + }); + + test("a different argument is a different call", () => { + const detector = createRepeatDetector({ now: clock().now }); + + detector.observe("sales-bot", { tool: "computer_click", ref: "e1" }); + detector.observe("sales-bot", { tool: "computer_click", ref: "e2" }); + const third = detector.observe("sales-bot", { + tool: "computer_click", + ref: "e3", + }); + + // Three clicks, three buttons. A Bot working down a form must not look like one stuck on its + // first field, or the feature gets switched off the first day somebody uses it. + expect(third.count).toBe(1); + }); + + test("a different tool on the same argument is a different call", () => { + const detector = createRepeatDetector({ now: clock().now }); + + detector.observe("sales-bot", { + tool: "computer_read_file", + filePath: "a", + }); + const written = detector.observe("sales-bot", { + tool: "computer_write_file", + filePath: "a", + }); + + expect(written.count).toBe(1); + }); + + test("whitespace around an argument does not buy a Bot a fresh count", () => { + // A model reproducing a path from its own earlier output does not always reproduce the spacing, + // and it is doing the same thing either way. + const detector = createRepeatDetector({ now: clock().now }); + + detector.observe("bot", { tool: "computer_write_file", filePath: "q3.md" }); + const spaced = detector.observe("bot", { + tool: "computer_write_file", + filePath: " q3.md ", + }); + + expect(spaced.count).toBe(2); + }); + + test("a call with nothing to distinguish it is not counted", () => { + // Scrolling is the only governed call that names nothing. Counting it by tool name alone would + // mean a Bot reading a long page trips a rule about repetition just by reaching the bottom. + const detector = createRepeatDetector({ now: clock().now }); + + for (let attempt = 0; attempt < 30; attempt++) { + const seen = detector.observe("bot", { tool: "computer_scroll" }); + expect(seen.count).toBe(1); + expect(seen.fingerprint).toBeNull(); + expect(seen.threshold).toBeNull(); + } + }); + + test("the window expires, and the count starts again", () => { + const time = clock(); + const detector = createRepeatDetector({ + now: time.now, + windowMs: 60_000, + }); + const call = { + tool: "computer_navigate", + targetUrl: "https://example.com", + }; + + detector.observe("bot", call); + time.advance(30_000); + expect(detector.observe("bot", call).count).toBe(2); + + // Far enough that the first two fall out of the window entirely. + time.advance(61_000); + expect(detector.observe("bot", call).count).toBe(1); + }); + + test("the window slides rather than resetting on a fixed tick", () => { + // A Bot pacing itself just inside the window still accumulates, which is the point: the window + // is about how close together attempts are, not about which minute they landed in. + const time = clock(); + const detector = createRepeatDetector({ now: time.now, windowMs: 60_000 }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("bot", call); + time.advance(50_000); + detector.observe("bot", call); + time.advance(50_000); + + // The first has aged out, the second has not. + expect(detector.observe("bot", call).count).toBe(2); + }); + + test("each threshold fires exactly once", () => { + const detector = createRepeatDetector({ + now: clock().now, + thresholds: [3, 10], + }); + const call = { tool: "computer_click", ref: "e9" }; + + const fired: number[] = []; + for (let attempt = 0; attempt < 15; attempt++) { + const seen = detector.observe("bot", call); + if (seen.threshold !== null) fired.push(seen.threshold); + } + + // A row per attempt past the line would bury the attempts themselves under the observation that + // they kept happening. + expect(fired).toEqual([3, 10]); + }); + + test("a threshold fires on the attempt that reaches it", () => { + const detector = createRepeatDetector({ + now: clock().now, + thresholds: [3], + }); + const call = { tool: "computer_click", ref: "e9" }; + + expect(detector.observe("bot", call).threshold).toBeNull(); + expect(detector.observe("bot", call).threshold).toBeNull(); + expect(detector.observe("bot", call)).toMatchObject({ + count: 3, + threshold: 3, + }); + }); + + test("a Bot that gets stuck twice is reported twice", () => { + // Once the window has emptied the run of repetition is over, so the next one is a new incident + // and deserves its own row. Holding the thresholds for the life of the process would leave one + // row for an afternoon of separate failures. + const time = clock(); + const detector = createRepeatDetector({ + now: time.now, + windowMs: 60_000, + thresholds: [3], + }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("bot", call); + detector.observe("bot", call); + expect(detector.observe("bot", call).threshold).toBe(3); + + time.advance(120_000); + detector.observe("bot", call); + detector.observe("bot", call); + expect(detector.observe("bot", call).threshold).toBe(3); + }); + + test("two Bots do not share a count", () => { + // The audit row names a Bot. A count pooled across Bots would report one Bot's loop against + // another Bot's name, which is the one thing a trail may never do. + const detector = createRepeatDetector({ now: clock().now }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("sales-bot", call); + detector.observe("sales-bot", call); + detector.observe("sales-bot", call); + + expect(detector.observe("research-bot", call).count).toBe(1); + expect(detector.observe("sales-bot", call).count).toBe(4); + }); + + test("the number of calls held per Bot is capped", () => { + // A Bot doing genuinely varied work never repeats anything, so every call it makes is a new key. + // Without a cap the map grows for exactly the Bots this has nothing to say about. + const detector = createRepeatDetector({ + now: clock().now, + maxKeysPerBot: 3, + }); + + detector.observe("bot", { tool: "computer_click", ref: "e1" }); + detector.observe("bot", { tool: "computer_click", ref: "e2" }); + detector.observe("bot", { tool: "computer_click", ref: "e3" }); + detector.observe("bot", { tool: "computer_click", ref: "e4" }); + + // The oldest went, so its history is gone and it counts as new. + expect( + detector.observe("bot", { tool: "computer_click", ref: "e1" }).count, + ).toBe(1); + // The newest is still held, which is the half of the cap that matters: a live loop must survive + // a Bot doing other things in between. + expect( + detector.observe("bot", { tool: "computer_click", ref: "e4" }).count, + ).toBe(2); + }); + + test("one Bot's varied work does not evict another Bot's loop", () => { + const detector = createRepeatDetector({ + now: clock().now, + maxKeysPerBot: 2, + }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("stuck-bot", call); + for (let ref = 0; ref < 20; ref++) { + detector.observe("busy-bot", { tool: "computer_click", ref: `e${ref}` }); + } + + expect(detector.observe("stuck-bot", call).count).toBe(2); + }); + + test("the default window is a few minutes, not a few seconds", () => { + // A retry loop is a model round trip each time round. A window of seconds would count nothing at + // all, and the feature would look like it worked because it never fired. + expect(DEFAULT_REPEAT_WINDOW_MS).toBeGreaterThanOrEqual(60_000); + expect(DEFAULT_REPEAT_WINDOW_MS).toBeLessThanOrEqual(10 * 60_000); + }); +}); + +describe("the fingerprint an audit row carries", () => { + test("names the tool and the argument, in words", () => { + // It goes onto the row as written. An investigator reading "the same call, 25 times" has to be + // told which call without going and decoding anything. + expect(fingerprintOf({ tool: "computer_click", ref: "e9" })).toBe( + "computer_click ref=e9", + ); + expect( + fingerprintOf({ tool: "computer_write_file", filePath: "notes.md" }), + ).toBe("computer_write_file file=notes.md"); + expect( + fingerprintOf({ tool: "computer_key", ref: "e1", key: "Enter" }), + ).toBe("computer_key ref=e1 key=Enter"); + }); + + test("is null when the call named nothing", () => { + expect(fingerprintOf({ tool: "computer_scroll" })).toBeNull(); + }); +}); diff --git a/server/tests/config.test.ts b/server/tests/config.test.ts index 722a5c2f1..9e7b31261 100644 --- a/server/tests/config.test.ts +++ b/server/tests/config.test.ts @@ -153,4 +153,37 @@ describe("deployment configuration", () => { }), ).toThrow("Google authentication requires BETTER_AUTH_SECRET"); }); + + test("takes a widened repetition window, and leaves it absent when nobody set one", () => { + expect( + loadConfig({ + ...baseEnvironment, + AGENT_COMPUTER_URL: "http://localhost:4100", + COMPUTER_REPEAT_WINDOW_MS: "600000", + }).computer?.repeatWindowMs, + ).toBe(600_000); + + expect( + loadConfig({ + ...baseEnvironment, + AGENT_COMPUTER_URL: "http://localhost:4100", + }).computer?.repeatWindowMs, + ).toBeUndefined(); + }); + + // Refused rather than quietly defaulted, like a malformed policy. An operator who typed `3m` would + // otherwise get a deployment running the built-in window, and the only evidence would be a rule + // about repetition that never fires, which reads exactly like a Bot behaving itself. + test.each(["3m", "0", "-1", "180000.5"])( + "refuses to start on a repetition window of %p", + (value) => { + expect(() => + loadConfig({ + ...baseEnvironment, + AGENT_COMPUTER_URL: "http://localhost:4100", + COMPUTER_REPEAT_WINDOW_MS: value, + }), + ).toThrow("COMPUTER_REPEAT_WINDOW_MS"); + }, + ); }); From a856d0cbff083f1e24707b01e5d8e558ce8ccebc Mon Sep 17 00:00:00 2001 From: Jerel John Velarde Date: Wed, 19 Aug 2026 14:04:26 -0700 Subject: [PATCH 2/2] Bound what the repeat detector remembers, and stop it dropping the key it is about to need Three things the detector claimed and did not do. The outer map was described as bounded by how many Bots a deployment has. It is not: the id it counts against is the `:botId` in the request path, checked against a session and against nothing else, because no acting route resolves it to a row in `bots` first. A signed-in caller looping over invented ids bought a map, an inner map and an occurrence record each time round, before the policy was consulted and whether or not any such Bot existed, and none of it was ever given back. Both levels are capped now and neither drops anything that is still inside the window, so what is held is the recent past and nothing else. Eviction was least recently seen, described as the key furthest from tripping anything. For the one behaviour this feature exists to catch it is the nearest: a Bot going round a loop of more distinct calls than the cap holds lost each key exactly one step before it came back, so twenty rounds of a sixty-five step circle reported every call as a first attempt and no rule about repetition could fire. Nothing live is evicted at all now. A place is freed when a call ages out of the window, and while every place is held by something still inside it, a call the Bot has not made before is not counted rather than something live being thrown out for it. The cost is a Bot whose first sixty-four distinct calls are honest work and which only then gets stuck: its loop is invisible until one of those falls out of the window, at most one window away. A blind spot that clears itself is worth more than an eviction rule that can be wrong for as long as the loop lasts, and it buys the other half: a call already being counted can no longer be pushed out by a Bot doing other things in between. The observational audit row blocked. `recordAuditEvent` rethrows, and that row is written ahead of the policy, so a moment's trouble at the audit store refused every third, tenth and twenty-fifth identical call, on an action nothing objected to, from the one part of this that is not allowed to refuse anything. It is swallowed and logged now. The invariant is untouched: the decision row goes to the same store a few lines below, and a store that is really down stops the action there. Also honest about the costs where an operator sees them. The Boundaries preset named only the two ways the rule under-fires and never the way it over-fires, which is the failure that costs somebody their Bot: a Bot typing ten different searches into one box is ten repeats, and the rule refuses the tenth. That admission, the fact that the count lives in one process and so splits across replicas, and the fact that MCP tool calls always report one, now reach the preset, the `PolicyContext` docblock, `.env.example` and the architecture note. The threshold docblock said a count that falls out of the window and climbs again reports again; it is the window emptying completely that ends a run, which is what the code has always done and what the type now says. --- .env.example | 7 +- app/src/routes/_authed/admin/boundaries.tsx | 2 +- docs/architecture.md | 4 +- server/src/computer/gateway.ts | 40 ++++-- server/src/computer/policy.ts | 15 +- server/src/computer/repeat.ts | 151 ++++++++++++++++---- server/tests/computer-gateway.test.ts | 34 +++++ server/tests/computer-repeat.test.ts | 124 +++++++++++++++- 8 files changed, 326 insertions(+), 51 deletions(-) diff --git a/.env.example b/.env.example index 68cbff74a..b47f26cc4 100644 --- a/.env.example +++ b/.env.example @@ -95,7 +95,12 @@ AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true # repeat.count is how many times this Bot has just made this exact call, counting the one being # decided. A stuck model retries, and each retry is a real action on somebody's live website that is # perfectly reasonable on its own terms; only the count tells the thirtieth click apart from the -# first. `repeat.count >= 10` in `deny` stops a Bot going in circles. +# first. `repeat.count >= 10` in `deny` stops a Bot going in circles. Two calls are the same call +# when the thing acted on is the same, whatever was typed into it, so ten searches typed into one box +# are ten repeats and a rule about repetition refuses the tenth: try one in `dry-run` first. The +# count is held in memory by the process that served the call, so a deployment running two API +# replicas splits every count and a rule about ten attempts fires at twenty or never, and calls to +# another server's tools over MCP are not counted at all. # # Name every route to the same effect. A form submits from a keypress in any of its fields, so a rule # that only blocks a Submit button does not block Enter from another field. The example below refuses diff --git a/app/src/routes/_authed/admin/boundaries.tsx b/app/src/routes/_authed/admin/boundaries.tsx index 0af0ea0af..e8e76297d 100644 --- a/app/src/routes/_authed/admin/boundaries.tsx +++ b/app/src/routes/_authed/admin/boundaries.tsx @@ -36,7 +36,7 @@ const PRESETS: { label: string; rule: string; cost?: string }[] = [ label: "Stop a Bot repeating itself", // The count includes the attempt being decided, so this refuses the tenth, not the eleventh. rule: "repeat.count >= 10", - cost: "Counts identical calls within a few minutes, so a Bot slow enough to spread its attempts wider is never caught, and one that changes a single argument each time is ten different calls.", + cost: "Two calls count as the same call when the thing acted on is the same, whatever was typed into it, so a Bot running ten searches from one box, or reading one file ten times, is refused on the tenth. It misses the other way too: a Bot slow enough to spread its attempts wider than a few minutes is never caught, one that changes a single argument each time is ten different calls, and calls to another server's tools are not counted at all. Worth adding while a match is recorded and allowed, before it starts refusing anybody's work.", }, { label: "Stay off social media", diff --git a/docs/architecture.md b/docs/architecture.md index 6fe04e602..9e5f14010 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -62,7 +62,9 @@ Policy rules can inspect: decided. The gateway keys it on the tool plus the ref, key, file path, or target URL, over a sliding window that defaults to three minutes and is set by `COMPUTER_REPEAT_WINDOW_MS`. Crossing 3, 10, or 25 writes one `computer.action_repeated` row each; the detector itself never refuses anything, so -`repeat.count >= 10` in `deny` is what stops a Bot going in circles. +`repeat.count >= 10` in `deny` is what stops a Bot going in circles. The count is held in memory by +the process that served the call, so two API replicas split it, and it covers the browser and the +workspace only: a call to another server's tools over MCP always reports one. Rules use CEL expressions plus case-insensitive `contains()` and `matches()`. Deny rules are evaluated before allow rules. The policy engine fails closed: a diff --git a/server/src/computer/gateway.ts b/server/src/computer/gateway.ts index 4d6bd11b2..c435d7805 100644 --- a/server/src/computer/gateway.ts +++ b/server/src/computer/gateway.ts @@ -223,17 +223,37 @@ export function createComputerGateway(options: ComputerGatewayOptions) { * Ahead of the decision row, so the trail reads in the order the thing happened: this was the * tenth identical attempt, and this is what the policy did about it. Filed the other way round * a reader has to deduce the cause from a row written after its effect. + * + * Its failure is swallowed, which nothing else in this file does. This row is an observation, + * and an observation is not allowed to refuse anything: letting a lost insert throw from here + * would stop every third, tenth and twenty-fifth identical call before the policy had even + * been asked, so a deployment that permits an action would lose it to a moment's trouble at the + * audit store. Nothing is weakened by that. An action that was not recorded still does not + * happen, because the decision row goes to the same store a few lines below, and a store that + * is genuinely down refuses the action there. */ - await writeRepeat(auditStore, { - toolName, - botId, - actor, - computerId, - pageUrl, - filePath, - fingerprint: repetition.fingerprint, - count: repetition.count, - }); + try { + await writeRepeat(auditStore, { + toolName, + botId, + actor, + computerId, + pageUrl, + filePath, + fingerprint: repetition.fingerprint, + count: repetition.count, + }); + } catch (error) { + console.error( + JSON.stringify({ + type: "computer-repeat-row-lost", + bot: botId, + fingerprint: repetition.fingerprint, + count: repetition.count, + error: String(error), + }), + ); + } } const decision = evaluateActionPolicy(options.policy(), context); diff --git a/server/src/computer/policy.ts b/server/src/computer/policy.ts index 1b33d714b..06ac3a97e 100644 --- a/server/src/computer/policy.ts +++ b/server/src/computer/policy.ts @@ -60,9 +60,18 @@ export type PolicyContext = { * denies, so an optional `repeat` would turn one rule about repetition into a deployment that * refuses everything. * - * The cost is that the window is time-based: a Bot slow enough to spread its attempts wider than - * the window never trips this, and one that varies a single argument each time round is thirty - * different calls. It is a backstop against the loop that actually happens, not a guarantee. + * It is wrong in both directions, and a rule written against it has to be worth both. Under, three + * ways: the window is time-based, so a Bot slow enough to spread its attempts wider than the window + * never trips this, and one that varies a single argument each time round is thirty calls; the + * count is held by the process that served the call, so a deployment behind two API replicas + * splits every count and a rule about ten attempts fires at twenty or never; and a call to another + * server's tools over MCP is not counted at all, because only the computer gateway counts. + * + * Over, once, and that one costs somebody their Bot rather than their evidence. Two calls are the + * same call when the thing acted on is the same, whatever was typed into it, so ten searches typed + * into one box and one file read ten times while a Bot works through it are both ten repeats, and + * `repeat.count >= 10` refuses the tenth. It is a backstop against the loop that actually happens, + * not a guarantee, which is the argument for trying a rule about it in `dry-run` first. */ repeat: { count: number }; element?: { diff --git a/server/src/computer/repeat.ts b/server/src/computer/repeat.ts index 81b40b42c..297a5235a 100644 --- a/server/src/computer/repeat.ts +++ b/server/src/computer/repeat.ts @@ -48,11 +48,40 @@ export const DEFAULT_REPEAT_THRESHOLDS: readonly number[] = [3, 10, 25]; * How many distinct calls are remembered per Bot. * * A Bot doing genuinely varied work never repeats anything, so every call it makes is a new key, and - * without a cap the map grows for exactly the Bots this feature has nothing to say about. The - * least recently seen key is dropped, which is the one furthest from tripping anything. + * without a cap the map grows for exactly the Bots this feature has nothing to say about. + * + * Full means full. A call the Bot has not made before is not counted, and nothing still inside the + * window is dropped to make room for it, because dropping something is a guess at which key will not + * come round again and the obvious guess is the one that fails hardest. Least recently seen is + * exactly the key a Bot going round a long loop is about to make next, so a loop of more than this + * many distinct calls would lose each key one step before it returned, and a Bot in a tight circle + * would be reported as making every one of its calls for the first time. + * + * The cost is the Bot whose first sixty-four distinct calls are honest work and which only then gets + * stuck: its loop is invisible until one of those sixty-four falls out of the window, which is at + * most one window away. A blind spot that clears itself is worth more than an eviction rule that can + * be wrong for as long as the loop lasts, and it buys the other half of the bargain, that a call + * already being counted can never be pushed out by a Bot doing other things in between. */ export const DEFAULT_REPEAT_KEYS_PER_BOT = 64; +/** + * How many Bots are remembered at once. + * + * The id counted against is the one in the request path. It is checked against a session and against + * nothing else, because a Bot's computer answers to whatever it is addressed as and no acting route + * resolves the id to a row in `bots` first. So the number of ids this map can be asked to hold is + * not the number of Bots somebody wrote down, it is however many a signed-in caller cares to type, + * and an uncapped map of maps would grow for the life of the process on nothing more than a loop of + * requests naming a fresh id each time. + * + * Reclaimed the way the per-Bot cap is: a Bot that has gone quiet for a whole window gives its place + * up, and while every place is held by a Bot that is still working, one more Bot is not counted. + * Two hundred and fifty-six, which is far more Bots than a deployment has acting inside any three + * minutes, and small enough that the worst case is a few megabytes rather than the whole heap. + */ +export const DEFAULT_REPEAT_BOTS = 256; + /** * One governed call, in the terms the detector cares about. * @@ -68,7 +97,14 @@ export type RepeatedCall = { }; export type RepeatObservation = { - /** Including the call being observed, so the first one counts as one. */ + /** + * Including the call being observed, so the first one counts as one. + * + * One is also the answer for a call the detector had no room to remember, and for one carrying + * nothing to identify it. It is the only number that cannot make anything happen: a rule about + * repetition reads it as a first attempt and stands aside. A count nobody can substantiate must + * never be the reason a Bot is refused. + */ count: number; /** * The call's identity, in a form a person can read. @@ -80,9 +116,11 @@ export type RepeatObservation = { /** * The threshold this call has just reached, or null on the overwhelming majority of calls. * - * Reported once per run of repetition rather than on every call past it. A count that falls back - * out of the window and climbs again is a new run and reports again, because it is a Bot that got - * stuck twice. + * Reported once per run of repetition rather than on every call past it. A run ends when the + * window empties completely, and a key that fills it again afterwards reports again, because that + * is a Bot that got stuck twice. A count that merely dips and climbs is the same run still going + * and says nothing further: one incident, one row per threshold, or a row per wobble would be a + * row per attempt under another name. */ threshold: number | null; }; @@ -96,6 +134,7 @@ export type RepeatDetectorOptions = { windowMs?: number; thresholds?: readonly number[]; maxKeysPerBot?: number; + maxBots?: number; /** Injected so a test can move time without waiting for it. */ now?: () => number; }; @@ -106,7 +145,16 @@ type Occurrences = { at: number[]; /** Which thresholds this run of repetition has already reported. */ reported: Set; - /** Used to choose what to drop when a Bot is at its key limit. */ +}; + +/** + * One Bot's history, and when it was last heard from. + * + * The time is kept here as well as inside the keys because a Bot that has stopped acting has to give + * its place up without anybody walking its whole map to work out that it has. + */ +type BotHistory = { + calls: Map; lastSeen: number; }; @@ -115,6 +163,7 @@ export function createRepeatDetector( ): RepeatDetector { const windowMs = options.windowMs ?? DEFAULT_REPEAT_WINDOW_MS; const maxKeysPerBot = options.maxKeysPerBot ?? DEFAULT_REPEAT_KEYS_PER_BOT; + const maxBots = options.maxBots ?? DEFAULT_REPEAT_BOTS; // Ascending, so the loop below ends on the highest threshold a call crossed rather than on // whichever one the caller happened to list last. const thresholds = [ @@ -126,10 +175,14 @@ export function createRepeatDetector( * Per Bot, then per call. * * Nested rather than keyed on a combined string so that one Bot's varied work cannot push another - * Bot's history out: the cap is per Bot, and a shared map would make it a race between them. The - * outer map is bounded by how many Bots a deployment has, which is a list somebody wrote down. + * Bot's history out: the cap is per Bot, and a shared map would make it a race between them. + * + * Both levels are capped, and neither ever drops something that is still inside the window. What + * is in here is therefore the recent past and nothing else, which is the only claim about memory + * this module can honestly make: an id it has never usefully counted cannot be made to sit here + * for the life of the process. */ - const perBot = new Map>(); + const perBot = new Map(); return { observe(botId, call) { @@ -139,23 +192,29 @@ export function createRepeatDetector( } const now = clock(); - let calls = perBot.get(botId); - if (!calls) { - calls = new Map(); - perBot.set(botId, calls); + const cutoff = now - windowMs; + + let history = perBot.get(botId); + if (!history) { + if (perBot.size >= maxBots) forgetQuietBots(perBot, cutoff); + if (perBot.size >= maxBots) return untracked(fingerprint); + history = { calls: new Map(), lastSeen: now }; + perBot.set(botId, history); } + // Whether or not the call itself is counted. A Bot making calls is a Bot at work, and one that + // has filled its keys with a long loop must not lose the loop by looking idle. + history.lastSeen = now; + const calls = history.calls; let entry = calls.get(fingerprint); if (!entry) { - if (calls.size >= maxKeysPerBot) { - evictLeastRecent(calls); - } - entry = { at: [], reported: new Set(), lastSeen: now }; + if (calls.size >= maxKeysPerBot) forgetExpiredCalls(calls, cutoff); + if (calls.size >= maxKeysPerBot) return untracked(fingerprint); + entry = { at: [], reported: new Set() }; calls.set(fingerprint, entry); } - const cutoff = now - windowMs; - entry.at = entry.at.filter((time) => time > cutoff); + trimToWindow(entry.at, cutoff); if (entry.at.length === 0) { // Nothing survived the window, so whatever run of repetition this key was in has ended and // its thresholds are free to report again. Without this a Bot that got stuck, recovered and @@ -163,7 +222,6 @@ export function createRepeatDetector( entry.reported.clear(); } entry.at.push(now); - entry.lastSeen = now; const count = entry.at.length; let threshold: number | null = null; @@ -231,15 +289,48 @@ function normalize(value: string | undefined): string { return (value ?? "").replaceAll(/\s+/g, " ").trim(); } -/** The key a Bot touched longest ago, which is the one least likely to be part of a live loop. */ -function evictLeastRecent(calls: Map) { - let oldestKey: string | null = null; - let oldestSeen = Number.POSITIVE_INFINITY; +/** + * What a call is worth when there was no room to remember it. + * + * The fingerprint is still returned, because it is a fact about the call and costs nothing to say. + * The count is one and the threshold is null, so nothing downstream acts on a number this module + * could not stand behind. + */ +function untracked(fingerprint: string): RepeatObservation { + return { count: 1, fingerprint, threshold: null }; +} + +/** + * Timestamps outside the window, dropped in place. + * + * Oldest first, so the scan stops at the first survivor and costs what it actually removes rather + * than the length of the list. Rebuilding the list instead would make a Bot hammering one call pay + * for its own history on every attempt, which is the Bot this module exists to describe. + */ +function trimToWindow(at: number[], cutoff: number) { + const surviving = at.findIndex((time) => time > cutoff); + if (surviving === -1) { + at.length = 0; + return; + } + if (surviving > 0) at.splice(0, surviving); +} + +/** + * Keys whose last call has aged out, which are the only ones safe to forget. + * + * They carry no count any more: the next call on one of them would start from one whether it was + * held or not, so dropping it loses nothing and frees the place for a call that might. + */ +function forgetExpiredCalls(calls: Map, cutoff: number) { for (const [key, entry] of calls) { - if (entry.lastSeen < oldestSeen) { - oldestSeen = entry.lastSeen; - oldestKey = key; - } + if ((entry.at.at(-1) ?? 0) <= cutoff) calls.delete(key); + } +} + +/** The same rule one level up: a Bot that has not acted for a whole window has nothing left to say. */ +function forgetQuietBots(perBot: Map, cutoff: number) { + for (const [botId, history] of perBot) { + if (history.lastSeen <= cutoff) perBot.delete(botId); } - if (oldestKey !== null) calls.delete(oldestKey); } diff --git a/server/tests/computer-gateway.test.ts b/server/tests/computer-gateway.test.ts index d29182c58..db7576b1a 100644 --- a/server/tests/computer-gateway.test.ts +++ b/server/tests/computer-gateway.test.ts @@ -555,6 +555,40 @@ describe("a Bot going in circles", () => { ).toBe(true); }); + test("a lost observation row does not refuse an action the policy allows", async () => { + // The detector observes and the policy decides, and that has to survive the audit store having a + // bad moment. Letting the observation row throw would refuse every third, tenth and twenty-fifth + // identical call, before the policy had been asked, on an action nothing objected to. + const { client, calls } = fakeClient(); + const rows: AuditEventInput[] = []; + const store: AuditStore = { + insert: async (event) => { + if (event.eventType === "computer.action_repeated") { + throw new Error("the audit store is unreachable"); + } + rows.push(event); + }, + }; + const gateway = createComputerGateway({ + client, + auditStore: store, + policy: () => PERMISSIVE, + repeat: createRepeatDetector({ thresholds: [2] }), + }); + await gateway.snapshot("default"); + + await gateway.click("default", "bot-1", ACTOR, click); + await gateway.click("default", "bot-1", ACTOR, click); + + // Both clicks happened and both decisions are on the record. What was lost is the note saying + // they were the same click twice, which is the only thing that may be lost here. + expect(calls).toEqual(["click", "click"]); + expect(rows.map((row) => row.eventType)).toEqual([ + "computer.action_allowed", + "computer.action_allowed", + ]); + }); + test("a repeated file write names the path and not the browser's page", async () => { // The workspace has nothing to do with whatever the browser happens to be showing, and naming a // host on that row sends a reader somewhere irrelevant. diff --git a/server/tests/computer-repeat.test.ts b/server/tests/computer-repeat.test.ts index b84938ea1..f45b5459b 100644 --- a/server/tests/computer-repeat.test.ts +++ b/server/tests/computer-repeat.test.ts @@ -197,6 +197,31 @@ describe("counting a Bot repeating itself", () => { expect(detector.observe("bot", call).threshold).toBe(3); }); + test("a run that dips without emptying is still the same run", () => { + // The run ends when the window is empty, not when the count falls back under a threshold. A row + // every time a stuck Bot's count wobbles past the line would be a row per attempt under another + // name, which is the thing the thresholds exist to avoid. + const time = clock(); + const detector = createRepeatDetector({ + now: time.now, + windowMs: 60_000, + thresholds: [2], + }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("bot", call); + time.advance(10_000); + expect(detector.observe("bot", call).threshold).toBe(2); + + // The first attempt ages out and the second does not, so the count falls to one and climbs + // straight back. The key never went quiet, so this is the same incident, already reported. + time.advance(55_000); + expect(detector.observe("bot", call)).toMatchObject({ + count: 2, + threshold: null, + }); + }); + test("two Bots do not share a count", () => { // The audit row names a Bot. A count pooled across Bots would report one Bot's loop against // another Bot's name, which is the one thing a trail may never do. @@ -222,19 +247,108 @@ describe("counting a Bot repeating itself", () => { detector.observe("bot", { tool: "computer_click", ref: "e1" }); detector.observe("bot", { tool: "computer_click", ref: "e2" }); detector.observe("bot", { tool: "computer_click", ref: "e3" }); - detector.observe("bot", { tool: "computer_click", ref: "e4" }); - // The oldest went, so its history is gone and it counts as new. + // Full, so the fourth call is not counted. Nothing still inside the window is thrown out to make + // room for it, and a call nobody could count reports one, which no rule acts on. expect( - detector.observe("bot", { tool: "computer_click", ref: "e1" }).count, + detector.observe("bot", { tool: "computer_click", ref: "e4" }).count, ).toBe(1); - // The newest is still held, which is the half of the cap that matters: a live loop must survive - // a Bot doing other things in between. expect( detector.observe("bot", { tool: "computer_click", ref: "e4" }).count, + ).toBe(1); + // The half of the cap that matters: a live loop survives a Bot doing other things in between, + // however much of it there is. + expect( + detector.observe("bot", { tool: "computer_click", ref: "e1" }).count, + ).toBe(2); + }); + + test("a Bot going round a loop wider than the cap is still counted", () => { + // The failure that would make the whole feature ornamental. Dropping the least recently seen key + // to make room drops each key exactly one step before it comes round again, so a Bot circling + // all afternoon would report every call as its first and no rule about repetition would ever + // fire on the one thing this exists to catch. + const detector = createRepeatDetector({ + now: clock().now, + maxKeysPerBot: 3, + thresholds: [3], + }); + + const counts: number[] = []; + const fired: number[] = []; + for (let round = 0; round < 3; round++) { + for (const ref of ["e1", "e2", "e3", "e4"]) { + const seen = detector.observe("bot", { tool: "computer_click", ref }); + counts.push(seen.count); + if (seen.threshold !== null) fired.push(seen.threshold); + } + } + + // Three of the four keys fitted, and three times round is what the trail is told about. + expect(Math.max(...counts)).toBe(3); + expect(fired).toEqual([3, 3, 3]); + }); + + test("a call the cap turned away is counted once the window drains", () => { + // The cost of not evicting is a blind spot, and this is the thing that makes it bearable: it + // ends by itself within a window rather than lasting as long as the Bot does. + const time = clock(); + const detector = createRepeatDetector({ + now: time.now, + windowMs: 60_000, + maxKeysPerBot: 2, + }); + + detector.observe("bot", { tool: "computer_click", ref: "e1" }); + detector.observe("bot", { tool: "computer_click", ref: "e2" }); + expect( + detector.observe("bot", { tool: "computer_click", ref: "e3" }).count, + ).toBe(1); + + time.advance(61_000); + detector.observe("bot", { tool: "computer_click", ref: "e3" }); + expect( + detector.observe("bot", { tool: "computer_click", ref: "e3" }).count, ).toBe(2); }); + test("the number of Bots held is capped", () => { + // The id counted against is the one in the request path, checked against a session and against + // nothing else. A caller naming a fresh Bot on every request would otherwise buy a map of its + // own each time, for the life of the process. + const detector = createRepeatDetector({ now: clock().now, maxBots: 2 }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("sales-bot", call); + detector.observe("research-bot", call); + for (let invented = 0; invented < 100; invented++) { + detector.observe(`bot-${invented}`, call); + } + + // The two that were really working keep their counts, and the hundred invented ones bought + // nothing at all. + expect(detector.observe("sales-bot", call).count).toBe(2); + expect(detector.observe("research-bot", call).count).toBe(2); + }); + + test("a Bot that has gone quiet gives its place up", () => { + const time = clock(); + const detector = createRepeatDetector({ + now: time.now, + windowMs: 60_000, + maxBots: 1, + }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("morning-bot", call); + // Still working, so it keeps its place and the second Bot is not counted. + expect(detector.observe("afternoon-bot", call).count).toBe(1); + + time.advance(61_000); + detector.observe("afternoon-bot", call); + expect(detector.observe("afternoon-bot", call).count).toBe(2); + }); + test("one Bot's varied work does not evict another Bot's loop", () => { const detector = createRepeatDetector({ now: clock().now,