diff --git a/.changeset/doom-loop-fanout.md b/.changeset/doom-loop-fanout.md new file mode 100644 index 0000000..90ae330 --- /dev/null +++ b/.changeset/doom-loop-fanout.md @@ -0,0 +1,145 @@ +--- +'@openrouter/agent': minor +--- + +Fix doom-loop detection missing a repeated same-tool fan-out. + +Streaks compared a tool's *last* fingerprint, so `read(a), read(b), read(c)` +reissued verbatim had a different last call every round and each round's first +call reset the streak to 1. Eight identical rounds of a three-call fan-out +produced zero detections, while single-call rounds tripped at round 2 — and +distinct-argument fan-out is the dominant shape in parallel-tool-calling agents. + +A round's identity for one tool is now the *set* of fingerprints it was called +with, compared across rounds. The engine declares a round's complete set before +any of its calls is scored, so ordering within the round does not matter, a +changed member resets the streak, and neither a strict subset nor a superset is +a repeat — a round that adds new work is progress, not repetition. Every call in +a repeating round reports that round's streak, so at the block rung a repeating +fan-out stops spending rather than only its last call being refused. + +**Per-call streaks** accumulate alongside the round-set streak, and the +stronger evidence decides. Each `(tool, arguments)` identity counts its own +consecutive rounds, whatever its round-mates did — so a call repeating inside +varying company (`[a,b]`, `[a,c]`, `[a,d]`: `a` is a 3-peat) is flagged even +though every round's set differs, a repeat keeps counting when a paused HITL +member drops from the resumed round, and undeclared paths (server-tool records, +direct callers) get order-independent per-call detection without a declaration. +When the per-call count alone crosses a rung, only that call is refused and its +verdict quotes its own identity; genuinely new round-mates run free. For an +exactly-repeating round both counts are equal, so nothing double-fires. A +partial repeat (`[a,b,c]` then `[a,b]`) flags the re-issued calls at the +observe rung rather than being invisible; a superset round (`[a,b]`, `[a,b]`, +`[a,b,c]`) flags the repeated members while the new call always executes. + +A call that a round's declaration could not include (unhashable key material) +cannot inherit or move the round's counters; its own verbatim repetition still +accumulates per-call evidence like any other repeat. + +**Resumed runs**: a multi-call round's fingerprint set and per-call counts are +persisted alongside its streak (new optional `roundFingerprints` and +`callStreaks` on `DoomLoopStreak` — additive; pre-existing blobs restore with +their old single-call semantics). A repeating +fan-out therefore keeps its evidence across save/resume boundaries: approval +pauses no longer reset a fan-out sitting at the block rung, and per-turn-resume +topologies (one `callModel` per user turn, state persisted between) accumulate +across turns instead of re-baselining on every one. Because the streak travels +with the exact set that earned it, a resumed round containing only a subset of +that set is a different round and starts at 1 — a lesser call can never inherit +a fan-out's evidence. Single-call streaks behave exactly as before. + +**New API**: `DoomLoopMonitor.declareRound(round, calls)` — declares a round's +complete call set before any of it is scored. `DoomLoopMonitor` is exported, so +this is a new public method, additive only. Callers using `callModel` need not +touch it (the engine calls it); direct `DoomLoopMonitor` users and SDK ports +should, so a repeating fan-out is flagged as one unit (shared verdict, shared +steer message) rather than only via each member's individual per-call count. + +Single-call round timing, in-round duplicate collapsing, verdict payloads, and +the number of times a tool's `loopKey` is invoked (once per checked call) are +unchanged. The persisted shape gains two optional fields (`roundFingerprints` +and `callStreaks`, both above); everything existing is untouched and old blobs +restore cleanly with their old semantics. + +**Newly reachable false positive.** The detector compares arguments, not +results, so repetition shapes that were previously invisible now accumulate and +are refused at the default `block` rung from round 3. Two variants: + +- A stable *set* of parallel arguments every round — an agent re-reading the + same context files each turn, or a fixed fan-out of pollers — blocks with one + synthesized error per call in the round. +- A single call re-issued verbatim while its round-mates CHANGE — re-reading an + anchor file (README, config, schema) while exploring new files each turn + (`[a]`, `[a,b]`, `[a,b,c]`: `a` blocks from round 3 even though every round + adds work). The per-call detector counts the call's own consecutive rounds, + so the round being "progress" does not exempt a member that itself repeats: + a file already read is in context, and re-reading it is spend without + progress. + +Exempt such tools with `loopKey: false` (or a `loopKey` returning `null` for +the call). These classes were invisible to the detector before, so no existing +exemption covered them; the graduated ladder gives every shape a free round and +an `observe` warning before anything is refused. + +For `callModel` users, nothing to change — `doomLoop` is configured exactly as +before, and the engine declares each round for you. What changed is when it +fires: + +```ts +import { callModel } from '@openrouter/agent'; + +const result = callModel(client, { + model: 'z-ai/glm-5.2', + input: 'Summarize these files.', + tools: [readTool], + // Unchanged config; the ladder default is observe@2, block@3, stop@6. + doomLoop: true, +}); + +// Say the model reissues the SAME three-call fan-out every round: +// round 1: read(a), read(b), read(c) +// round 2: read(a), read(b), read(c) <- identical set +// +// was: no detection, ever. Each round's first call reset the streak, so +// a fan-out could spin indefinitely while single calls tripped at +// round 2. +// now: round 2 is streak 2 (observe), round 3 is streak 3 (block) — and +// EVERY call of the round is refused at the block rung, not just one, +// so the fan-out stops spending. +// +// A round that ADDS work resets the ROUND streak, but each repeated call +// keeps its own count — the model re-read a, b, c a third time: +// round 3: read(a), read(b), read(c), read(d) +// -> a, b, c blocked (3rd consecutive round each); d executes. +// +// `loopKey` still runs exactly once per checked call. Persisted state gains +// two optional fields so fan-out and per-call evidence survive save/resume; +// old state restores cleanly. +``` + +Driving `DoomLoopMonitor` directly (or porting it) is the case that needs the +new call — declare a round's whole batch before recording any of it. +`resolveDoomLoopOption` and `ResolvedDoomLoopConfig` are now exported too: +`DoomLoopMonitor` was previously exported without its config resolver, so it +could not actually be constructed from the public API. + +```ts +import { DoomLoopMonitor, resolveDoomLoopOption } from '@openrouter/agent'; + +const monitor = new DoomLoopMonitor(resolveDoomLoopOption(true)); + +for (const [round, batch] of batches.entries()) { + // NEW: declare the round's complete set BEFORE recording any of its calls, + // so a repeating fan-out is scored as one unit. (Per-call repetition is + // detected either way; the declaration adds whole-round identity.) + await monitor.declareRound( + round, + batch.map((call) => ({ toolName: call.name, keyMaterial: call.arguments })), + ); + + for (const call of batch) { + const { verdict } = await monitor.recordToolCall(call.name, call.arguments, round); + if (verdict?.action === 'block') refuse(call, verdict.message); + } +} +``` diff --git a/packages/agent/README.md b/packages/agent/README.md index ee5e536..c6fa4f3 100644 --- a/packages/agent/README.md +++ b/packages/agent/README.md @@ -285,11 +285,37 @@ if (verdict) console.warn(verdict.message); Detection is **deterministic** — a verdict is a pure function of the transcript, so the same sequence of calls/text always fires at the same -point. Identical calls in consecutive **rounds** build a per-tool streak: -interleaved calls to *other* tools don't reset it, and N identical calls -fanned out in parallel within ONE round count once (a streak measures the -model re-issuing a call *after seeing its result*, which requires a round -trip). The streak crosses a graduated ladder — strongest crossed rung wins: +point. Repeated **rounds** build a per-tool streak: interleaved calls to +*other* tools don't reset it, and N identical calls fanned out in parallel +within ONE round count once (a streak measures the model re-issuing a call +*after seeing its result*, which requires a round trip). + +Two kinds of evidence accumulate side by side, and the stronger one decides: + +- **Round-set streaks.** A round's identity for one tool is the **set** of + calls it made, so a fan-out of *distinct* arguments reissued verbatim + counts: `read(a), read(b), read(c)` every round accumulates. Ordering + within the round is irrelevant, and a round whose membership changes — in + either direction — resets this streak, since adding or dropping work is + progress for the round as a unit. +- **Per-call streaks.** Each `(tool, arguments)` identity also counts its own + consecutive rounds, whatever its round-mates did. A call repeating inside + varying company (`[a,b]`, `[a,c]`, `[a,d]` — `a` is a 3-peat) is flagged + even though every round's set differs, and a repeat spanning an approval + pause keeps counting when the paused member drops from the resumed round. + For an exactly-repeating round both counts are equal, so nothing + double-fires. + +When a repeating fan-out crosses a rung, every call in the round gets the +verdict (so `block` stops the whole fan-out, not just one member), and calls +carrying the SAME evidence share byte-identical text — the `steer` rung +dedupes on exact text, so one piece of evidence injects one correction. A +round can carry two pieces of evidence at once (`[a]`, `[a,b]`, `[a,b]`: by +round 3, `a` is a 3-peat call while `{a,b}` is a 2-peat set), in which case +each renders its own message — at most two per tool per round, each stating +a distinct fact. When the per-call count alone crosses a rung, only that +call is refused and genuinely new round-mates run free. The streak crosses +a graduated ladder — strongest crossed rung wins: | Action | Effect | |---|---| @@ -380,6 +406,18 @@ via `_meta['openrouter/loopKey']`. MCP-wrapped tools accept a `loopKey` via `markMcp(tool, { loopKey })` or the `loopKeys` map on `createMCPTools`. +> **Exempt tools that repeat by design — including repeating *fan-outs*.** +> The detector compares arguments, not results, so a call whose arguments are +> stable while its results change is indistinguishable from a loop. Since a +> round's identity is now the whole *set* of a tool's calls, this covers +> parallel shapes too: an agent that re-reads the same context files at the +> start of every turn, or fans out a fixed set of pollers, accumulates a streak +> and is refused at the default `block` rung from round 3 — and because every +> call in the round gets the verdict, that is N synthesized error outputs per +> round, not one. These shapes were invisible before this behavior existed, so +> `loopKey: false` (or a `loopKey` returning `null`) is the opt-out for any +> tool whose repetition is legitimate. + **Fingerprints are a cross-port contract**: key material is canonicalized per RFC 8785 (JCS) and hashed with SHA-256 over the UTF-8 bytes, so the Python/Go ports produce identical fingerprints — they MUST use an RFC 8785 @@ -415,6 +453,11 @@ block to observe for a known-chatty tool, or escalate straight to stop. - **Manual/client-executed calls** pause the loop for the caller and are not recorded (only executed, blocked, and parse-error calls are evidence). +- **Cross-tool round patterns.** Streaks are per tool: a loop alternating + BETWEEN tools with no per-tool repetition (`read(a)` one round, `grep(a)` + the next, forever) shows each tool a sparse pattern its own evidence + cannot condemn. Interleaved calls to other tools never *reset* a tool's + streak, so an every-other-round repeat still accumulates — slowly. ### Async Tools @@ -661,7 +704,7 @@ const result = callModel(client, { model, input, tools, hooks }); | `SessionStart` | Once per run, before the initial request. `config` summarizes the session (`hasTools`, `hasApproval`, `hasState`) | none (void) | | `SessionEnd` | Once per run, on every exit path — completion, approval pause, interruption, error, and the no-tools streaming paths. `reason` is `'complete' \| 'error' \| 'max_turns' \| 'user' \| 'doom_loop'`. When at least one model call completed, `totalUsage` aggregates tokens/cost across all of them (`modelCalls`, `inputTokens`, `outputTokens`, `totalTokens`, `cachedTokens`, `reasoningTokens`, and `cost` when the server reported it) | none (void) | | `PostModelCall` | Once per completed model response, on **every** request the loop makes — initial, each tool-round follow-up, the empty-final retry, the `allowFinalResponse` final turn, and approval-resume requests. Payload: `responseId` (the OpenRouter generation id), `model`, `durationMs` (dispatch → fully materialized response, including stream consumption), `turnType` (`'initial' \| 'resume' \| 'tool_round' \| 'final' \| 'retry'`), `turnNumber`, and `usage` (`inputTokens`, `outputTokens`, `totalTokens`, `cachedTokens`, `reasoningTokens`, `cost?`) when the server reported usage accounting. Purely observational — the telemetry primitive for tracing/benchmark consumers: one span per model call | none (void) | -| `DoomLoopDetected` | Every time doom-loop detection crosses a ladder rung, once per `(tool, fingerprint)` per round — parallel duplicates in one round share the event (requires the `doomLoop` option). Payload: `detector` (`'tool-fingerprint' \| 'server-tool-fingerprint' \| 'text-repetition' \| 'text-streak'`), the resolved `action` (`'observe' \| 'steer' \| 'escalate' \| 'block' \| 'stop'`), the `streak`, the `fingerprint`, `toolName`/`toolInput` for tool verdicts, and the explanatory `message` | `overrideAction` replaces the engine's resolved action for this event (last handler wins); `block` on a text or server-tool verdict downgrades to `observe`; `escalate` without an `escalation` config or remaining budget downgrades to `observe` | +| `DoomLoopDetected` | Every time doom-loop detection crosses a ladder rung, once per `(tool, fingerprint)` per round — *identical* parallel duplicates in one round share the event, but a repeating fan-out of DISTINCT arguments emits one event per member, since each is its own `(tool, fingerprint)` (requires the `doomLoop` option). Payload: `detector` (`'tool-fingerprint' \| 'server-tool-fingerprint' \| 'text-repetition' \| 'text-streak'`), the resolved `action` (`'observe' \| 'steer' \| 'escalate' \| 'block' \| 'stop'`), the `streak`, the `fingerprint`, `toolName`/`toolInput` for tool verdicts, and the explanatory `message` | `overrideAction` replaces the engine's resolved action for this event (last handler wins); `block` on a text or server-tool verdict downgrades to `observe`; `escalate` without an `escalation` config or remaining budget downgrades to `observe` | Notes on lifecycle pairing: `SessionEnd` only fires when a matching `SessionStart` succeeded, and at most once per run. Pending async hook work is diff --git a/packages/agent/src/index.ts b/packages/agent/src/index.ts index 3119732..3b61266 100644 --- a/packages/agent/src/index.ts +++ b/packages/agent/src/index.ts @@ -138,6 +138,8 @@ export type { DoomLoopTextOptions, DoomLoopVerdict, LoopKeyResolution, + ResolvedDoomLoopConfig, + ResolvedEscalationConfig, TextRepetitionResult, } from './lib/doom-loop.js'; export { @@ -149,6 +151,7 @@ export { fingerprintKeyMaterial, fingerprintToolCall, MAX_CANONICALIZE_DEPTH, + resolveDoomLoopOption, resolveLoopKeyMaterial, } from './lib/doom-loop.js'; // Lifecycle hooks system (PreToolUse, PostToolUse, Stop, SessionStart, ...). diff --git a/packages/agent/src/lib/doom-loop.ts b/packages/agent/src/lib/doom-loop.ts index 398a886..3005384 100644 --- a/packages/agent/src/lib/doom-loop.ts +++ b/packages/agent/src/lib/doom-loop.ts @@ -32,7 +32,14 @@ * - **Round-scoped streaks.** A streak measures the model *re-issuing a call * after seeing its result*, which requires a model round trip. N identical * calls fanned out in ONE round count once; the duplicates share that - * round's decision. + * round's decision. A round's identity for one tool is the *set* of + * fingerprints it was called with, so a fan-out of distinct arguments + * (`read(a), read(b), read(c)`) reissued verbatim accumulates, and every + * call in the round reports the round's streak. Ports must declare a + * round's complete set before scoring any of its calls (see + * `declareRound`): scoring a set as it accumulates makes a superset round + * transiently match the previous one, which both fires on real progress + * and makes the outcome depend on emission order. * - **Graduated response.** Detection feeds a configurable action ladder * (observe → steer → block → stop) rather than killing the run outright. * - **Never the cause of failure.** Detection may only affect a run through @@ -75,10 +82,12 @@ export type DoomLoopDetectorKind = * A doom-loop detection event. * * `streak` is the repetition count that crossed a ladder threshold: for - * fingerprint detectors the consecutive identical-fingerprint round count - * for that tool; for `text-repetition` the number of consecutive repeats of - * a token block within one response; for `text-streak` the number of - * consecutive steps with identical assistant text. + * fingerprint detectors the number of consecutive rounds in which that tool + * was called with the same fingerprint *set* (so a repeated fan-out counts, + * and every call in the round reports the round's count); for + * `text-repetition` the number of consecutive repeats of a token block within + * one response; for `text-streak` the number of consecutive steps with + * identical assistant text. */ export interface DoomLoopVerdict { detector: DoomLoopDetectorKind; @@ -186,6 +195,29 @@ export type DoomLoopOption = boolean | DoomLoopConfig; export interface DoomLoopStreak { fingerprint: string; streak: number; + /** + * The full fingerprint set of the tool's last round, present only when that + * round had more than one distinct call. `fingerprint`+`streak` alone cannot + * say WHICH set earned a count, so without this a fan-out streak either + * attached to one arbitrary member (refusing a lesser resumed call on first + * appearance) or had to be discarded at every save — losing block-level + * evidence across approval pauses and re-baselining on every per-turn + * resume. Absent for single-call rounds and in pre-existing blobs, where + * `fingerprint` fully describes the round. Never present on text streaks. + */ + roundFingerprints?: readonly string[]; + /** + * Per-call streaks for the tool's last round: fingerprint → number of + * consecutive rounds that exact call has been issued in. This is the + * evidence for the PER-CALL detector, which catches a call repeating inside + * rounds whose other members keep changing (`[a,b]`, `[a,c]`, `[a,d]` — the + * round set differs every time, but `a` is a 3-peat). Persisted so a repeat + * spanning a save/resume boundary (approval pause, per-turn resume) keeps + * counting. Bounded by the width of one round; absent in pre-existing + * blobs, where per-call evidence simply restarts. Never present on text + * streaks. + */ + callStreaks?: Record; } /** @@ -344,7 +376,16 @@ function warnOnLadderHazards(ladder: Required): void { /** * Normalize the `doomLoop` option. `undefined` / `false` → null (detection * off — the SDK's default posture: explicit control over implicit magic). + * + * Overloaded so the direct-construction pattern compiles without a null + * check: `true` or a config object always yields a resolved config, so + * `new DoomLoopMonitor(resolveDoomLoopOption(true))` is well-typed. Only the + * engine's pass-through of a caller's raw option can produce `null`. */ +export function resolveDoomLoopOption(option: DoomLoopConfig | true): ResolvedDoomLoopConfig; +export function resolveDoomLoopOption( + option: DoomLoopOption | undefined, +): ResolvedDoomLoopConfig | null; export function resolveDoomLoopOption( option: DoomLoopOption | undefined, ): ResolvedDoomLoopConfig | null { @@ -806,6 +847,108 @@ function isValidStreak(value: unknown): value is DoomLoopStreak { ); } +/** + * True when an entry's per-call counts carry no information beyond its round + * set and round streak, so getState() may omit them and restore() can rebuild + * them exactly. Holds in the steady state of a repeating fan-out — every + * member has been issued in the same consecutive rounds, so every count equals + * the round streak. Persisting both in that state stored each 64-char hash + * twice (once in the set, once as a count key): ~13 KB per width-100 round, + * copied on every save. The counts must also cover exactly the set members — + * a shrunk round (count > streak) or an extra recorded non-member is real + * evidence and is still persisted verbatim. + */ +function callStreaksReconstructible(entry: StreakEntry): boolean { + const counts = entry.callStreaks; + if (counts === undefined) { + return true; + } + const set = entry.roundFingerprints ?? [ + entry.fingerprint, + ]; + const keys = Object.keys(counts); + return ( + keys.length === set.length && + keys.every((key) => set.includes(key) && counts[key] === entry.streak) + ); +} + +/** + * Rebuild one tool's in-memory streak entry from its persisted shape. + * + * The persisted set (when the last round was multi-call) restores the round's + * identity exactly, so a resumed fan-out continues its streak and a resumed + * SUBSET cannot match it — the false-positive and false-negative failure + * modes of a single-fingerprint save. Older blobs (and single-call rounds) + * carry no set; the lone fingerprint fully describes those rounds. Malformed + * sets fall back the same way rather than dropping the entry. + * + * Per-call counts restore into the CURRENT-round slot: the first resumed + * record is a new round, so `recordToolCall` rolls them into its baseline + * (`priorCallStreaks`) exactly as a live round transition would. Validated + * entry-by-entry — the blob is caller-writable JSON, and one malformed count + * must not poison the rest. `Object.create(null)` + direct assignment so a + * persisted "__proto__" key is inert data, mirroring the Map rationale at the + * `tools` field. `round` is intentionally absent from the result: the first + * resumed record is always a new round, whatever the numbering. + */ +function restoreStreakEntry(entry: DoomLoopStreak): StreakEntry { + const persistedSet = + Array.isArray(entry.roundFingerprints) && + entry.roundFingerprints.length > 1 && + entry.roundFingerprints.every((value) => typeof value === 'string') + ? [ + ...entry.roundFingerprints, + ].sort() + : [ + entry.fingerprint, + ]; + const persistedCallStreaks: Record = Object.create(null) as Record< + string, + number + >; + if (typeof entry.callStreaks === 'object' && entry.callStreaks !== null) { + for (const [callFingerprint, count] of Object.entries(entry.callStreaks)) { + if (typeof count === 'number' && Number.isFinite(count) && count >= 1) { + persistedCallStreaks[callFingerprint] = Math.floor(count); + } + } + } + /* + * The round streak gets the same range validation as the per-call counts — + * the blob is caller-writable JSON, and `isValidStreak` only checks "finite + * number". A negative count would hold the guardrail below every rung for + * that tool (thresholds compare with `>=`); a fractional or absurd one + * would feed the ladder garbage. Clamped to a sane count rather than + * rejected: a corrupt streak must degrade the entry, not drop the tool's + * evidence entirely (fail-open, like every other restore path). The cap is + * generous — real streaks stop the run at single digits — and exists only + * to bound arithmetic, not to change semantics. + */ + const streak = Math.min(Math.max(Math.floor(entry.streak), 1), 1_000_000); + return { + fingerprint: entry.fingerprint, + streak, + roundFingerprints: persistedSet, + /* + * Absent per-call counts mean getState() proved them reconstructible: + * every member of the round set carried exactly the round streak (the + * steady state of a repeating fan-out — see callStreaksReconstructible). + * Rebuild them for the WHOLE set, not just the last-recorded member, or a + * width-W fan-out would resume with W−1 members' evidence reset. + */ + callStreaks: + Object.keys(persistedCallStreaks).length > 0 + ? persistedCallStreaks + : Object.fromEntries( + persistedSet.map((member) => [ + member, + streak, + ]), + ), + }; +} + /** In-memory streak entry: serialized shape + the round of the last record. */ interface StreakEntry extends DoomLoopStreak { /** @@ -814,6 +957,51 @@ interface StreakEntry extends DoomLoopStreak { * always increment on its first record, whatever its round numbering. */ round?: number; + /** + * The fingerprint set this tool was called with during {@link round}, in + * sorted order. A round's identity is the whole set, not its last call, so a + * fan-out of *distinct* arguments (`read(a), read(b), read(c)`) reissued + * verbatim is a repeat. Comparing only the last call let each round's first + * call reset the streak, so a repeating fan-out never accumulated evidence. + * + * This is the round's COMPLETE set, known before its first call is scored — + * see {@link DoomLoopMonitor.declareRound}. Accumulating it incrementally + * instead made a growing round transiently equal the previous round's set, + * so a superset round (`[a,b]`, `[a,b]`, `[a,b,c]`) scored a verdict on the + * call that completed the prefix and blocked real progress — and did so + * order-dependently, since a different emission order never formed the + * matching prefix. Never serialized: meaningful only within one round. + */ + roundFingerprints?: readonly string[]; + /** + * Fingerprints of {@link round} already recorded, used to collapse in-round + * duplicates (one decision per `(tool, fingerprint)` per round). Distinct + * from {@link roundFingerprints}, which is the round's declared whole. + * Never serialized: meaningful only within one round — which is why it can + * be a Set: only serialized state needs a plain-array shape. + */ + seenThisRound?: ReadonlySet; + /** + * The set the previous round was called with, and the streak it earned. + * Carried unchanged for the length of {@link round} so that every call in + * an UNDECLARED round is measured against the round before it rather than + * against a set this round is still assembling. Unused once a round is + * declared (those calls share the round's streak directly). + * Never serialized: meaningful only within one run. + */ + priorRoundFingerprints?: readonly string[]; + priorStreak?: number; + /** + * The PREVIOUS round's per-call streaks (fingerprint → consecutive-round + * count), fixed at the round transition like {@link priorRoundFingerprints} + * so arrival order within the current round cannot affect a per-call score. + * A call present here repeated from last round and extends its own count; a + * call absent here starts at 1. The CURRENT round's counts live in the + * inherited {@link DoomLoopStreak.callStreaks}, accumulate as calls arrive + * (only calls actually recorded — a declared-but-skipped call must not + * carry per-call evidence), and become this field at the next transition. + */ + priorCallStreaks?: Record; } /** @@ -837,6 +1025,14 @@ export class DoomLoopMonitor { // Escalation recoveries consumed by this conversation. Persisted (see // getState/restore) so a resumed run cannot reset its budget. private escalationsUsed = 0; + // The current round's declared per-tool fingerprint sets — see + // `declareRound`. Run-local and never serialized. + private declaredRound: + | { + round: number; + fingerprints: Map; + } + | undefined; constructor(config: ResolvedDoomLoopConfig, initialState?: unknown) { this.config = config; @@ -845,6 +1041,120 @@ export class DoomLoopMonitor { } } + /** + * Declare the complete set of calls a round will make, before any of them + * is recorded. Each entry is one call's `(toolName, keyMaterial)`; the + * monitor groups them per tool into that tool's round set. + * + * A round's identity is the *set* of fingerprints a tool was called with, + * so that set has to be known up front. Accumulating it call by call meant + * a round that is a strict superset of the previous one transiently equaled + * it while filling — `[a,b]`, `[a,b]`, `[a,b,c]` scored a verdict on the `b` + * of the third round and refused a call that represented real progress. It + * was also emission-order dependent: `[c,a,b]` never formed the matching + * prefix and scored nothing. Declaring the whole set removes both. + * + * Idempotent per round, and safe to skip: PER-CALL detection needs no + * declaration — every repeated `(tool, arguments)` identity accumulates its + * own consecutive-round count regardless, so an undeclared repeating + * fan-out still flags each repeated member (server-tool records take that + * path). What the declaration adds is round-set evidence: the fan-out + * scored as one unit, with a shared verdict and one steer message, instead + * of member-by-member. Unhashable key material is skipped here with a + * warning; the caller's own fallback chain handles it at record time. + * Never serialized. + */ + async declareRound( + round: number, + calls: readonly { + toolName: string; + keyMaterial: unknown; + }[], + ): Promise { + /* + * Independent digests run concurrently — declaration gates dispatch of + * the whole batch, so serial awaits would add one hash latency per call + * to every round. Failures are per-call and fail open, as before. + */ + const hashed = await Promise.all( + calls.map(async (call) => { + try { + return { + call, + fingerprint: await this.fingerprintOnce(call.toolName, call.keyMaterial), + }; + } catch (error) { + // Unhashable: leave it out of the declared set — recordToolCall's + // fallback chain decides this call's identity on its own — but say + // so, like every other fail-open path: a direct consumer debugging + // an unexpectedly incomplete declaration needs the tool name and + // cause. (The engine warns separately on its own declaration path.) + console.warn( + `[DoomLoop] could not fingerprint a "${call.toolName}" call while declaring round ` + + `${round}; excluding it from the round set:`, + error, + ); + return null; + } + }), + ); + + /* Accumulate per tool, dedupe and sort ONCE per set — not per member. */ + const collected = new Map>(); + for (const entry of hashed) { + if (entry === null) { + continue; + } + const set = collected.get(entry.call.toolName) ?? new Set(); + set.add(entry.fingerprint); + collected.set(entry.call.toolName, set); + } + const fingerprints = new Map(); + for (const [toolName, set] of collected) { + fingerprints.set( + toolName, + [ + ...set, + ].sort(), + ); + } + this.declaredRound = { + round, + fingerprints, + }; + } + + /** + * Fingerprint with a per-object memo. The engine hands the SAME key-material + * object to `declareRound` and then to `recordToolCall` (the resolved + * `loopKey` output is cached per call id), so without this every checked + * call is canonicalized and SHA-256'd twice per round. WeakMap keeps the + * memo from retaining arguments beyond their natural lifetime; primitive + * key material (raw-string malformed args) skips the memo — it cannot key a + * WeakMap, and such calls are rare and small. Callers that mutate key + * material between declare and record get the declare-time identity, which + * is the identity the round was declared with — the consistent choice. + */ + private readonly fingerprintMemo = new WeakMap>(); + private async fingerprintOnce(toolName: string, keyMaterial: unknown): Promise { + const memoizable = typeof keyMaterial === 'object' && keyMaterial !== null; + if (memoizable) { + // Keyed by object AND tool name: the tool name participates in the + // hash, so one arguments object used by two tools must not collide. + const hit = this.fingerprintMemo.get(keyMaterial as object)?.get(toolName); + if (hit !== undefined) { + return hit; + } + } + const fingerprint = await fingerprintToolCall(toolName, keyMaterial); + if (memoizable) { + const perTool = this.fingerprintMemo.get(keyMaterial as object) ?? new Map(); + perTool.set(toolName, fingerprint); + this.fingerprintMemo.set(keyMaterial as object, perTool); + } + return fingerprint; + } + /** * True when the escalate rung can still fire: a mechanism is configured * and the budget is not exhausted. @@ -882,6 +1192,46 @@ export class DoomLoopMonitor { { fingerprint: entry.fingerprint, streak: entry.streak, + /* + * A multi-call round persists its full set, so a resumed run knows + * WHICH set earned the count. Without it, the streak either + * attached to one arbitrary member — a resumed round of just that + * call inherited a fan-out's whole evidence and could be refused + * on its first appearance — or had to be discarded at every save, + * which reset condemned fan-outs across approval pauses and made + * per-turn-resume topologies never accumulate. Single-call rounds + * omit it: their fingerprint fully describes the round. + * + * COPIED, not aliased: the in-memory array is shared with the live + * streak entry (and, for declared rounds, with `declaredRound` + * itself), and the snapshot is handed to the caller's + * StateAccessor. An in-place mutation of the saved blob must not + * corrupt the detector still running against it. + */ + ...((entry.roundFingerprints?.length ?? 0) > 1 + ? { + roundFingerprints: [ + ...(entry.roundFingerprints as readonly string[]), + ], + } + : {}), + /* + * Per-call counts persist so a repeat spanning a save/resume + * boundary keeps counting. Copied for the same isolation reason + * as the set above. `fromEntries` for the same "__proto__" safety + * as the outer record. Omitted only when `fingerprint`+`streak` + * already carry the identical information (a single-call round + * whose per-call count equals its round count — `restore` + * reconstructs exactly that). The count-mismatch check matters: + * when a round SHRINKS to one call, the round streak resets while + * the per-call count keeps climbing, and dropping it would hand + * the repeat a fresh grace window on resume. + */ + ...(entry.callStreaks !== undefined && !callStreaksReconstructible(entry) + ? { + callStreaks: Object.fromEntries(Object.entries(entry.callStreaks)), + } + : {}), }, ]), ), @@ -913,11 +1263,7 @@ export class DoomLoopMonitor { if (typeof candidate.tools === 'object' && candidate.tools !== null) { for (const [name, entry] of Object.entries(candidate.tools)) { if (isValidStreak(entry)) { - tools.set(name, { - fingerprint: entry.fingerprint, - streak: entry.streak, - // round intentionally absent: first resumed record increments. - }); + tools.set(name, restoreStreakEntry(entry)); } } } @@ -949,7 +1295,14 @@ export class DoomLoopMonitor { * re-issuing a call *after seeing its result*, which requires a round * trip; N parallel identical calls in one turn are one piece of * evidence, not N. - * - A different fingerprint for the same tool resets the streak to 1. + * - A round's identity is the *set* of fingerprints the tool was called + * with, not its last call, so a fan-out of distinct arguments reissued + * verbatim accumulates. A round whose set differs from the previous + * round's — in either direction, including a superset — resets to 1. + * Every call in a round reports that round's streak, so the ladder + * applies to the round as a unit. See {@link declareRound}: the set must + * be declared before the round's first call for this to hold for + * multi-call rounds. * * Blocked calls are recorded like any other — a model re-issuing a * blocked call in a later round is stronger loop evidence, not progress. @@ -969,52 +1322,166 @@ export class DoomLoopMonitor { detector?: Extract; }, ): Promise { - const fingerprint = await fingerprintToolCall(toolName, keyMaterial); + const fingerprint = await this.fingerprintOnce(toolName, keyMaterial); const previous = this.tools.get(toolName); - let streak: number; - let duplicateInRound = false; - if (previous && previous.fingerprint === fingerprint) { - if (previous.round !== undefined && previous.round === round) { - streak = previous.streak; - duplicateInRound = true; - } else { - streak = previous.streak + 1; - } - } else { - streak = 1; - } - this.tools.set(toolName, { + const isSameRound = previous?.round !== undefined && previous.round === round; + + /* + * The round's declared membership, when the engine announced it (see + * `declareRound`). A declaration only speaks for the calls it contains: a + * call dropped as unhashable still records here via the caller's fallback + * chain, but as a NON-member — it cannot inherit or move the ROUND's + * counters. Its own repetition still counts: the per-call detector below + * needs no declaration, so a fallback-identity call reissued verbatim + * every round accumulates like any other repeat. + */ + const declared = + this.declaredRound?.round === round + ? this.declaredRound.fingerprints.get(toolName) + : undefined; + const declaredMember = declared?.includes(fingerprint) === true; + + const seen = isSameRound ? (previous?.seenThisRound ?? EMPTY_SEEN) : EMPTY_SEEN; + const duplicateInRound = seen.has(fingerprint); + + /* + * The baseline every call of this round is measured against: the set the + * PREVIOUS round was called with and the streak it earned. Fixed at the + * round transition and carried unchanged for the round's length, so + * arrival order within a round can never affect a score. + */ + const priorSet = isSameRound + ? previous?.priorRoundFingerprints + : (previous?.roundFingerprints ?? + (previous + ? [ + previous.fingerprint, + ] + : undefined)); + const priorStreak = isSameRound ? (previous?.priorStreak ?? 0) : (previous?.streak ?? 0); + const score = (set: readonly string[]): number => + priorSet !== undefined && setsMatch(priorSet, set) ? priorStreak + 1 : 1; + + /* + * PER-CALL evidence, alongside the round-set streak: the number of + * consecutive rounds THIS exact fingerprint has been issued in, whatever + * its round-mates did. Round identity treats any membership change as + * progress, which is right for the fan-out as a unit but blind to one + * call repeating inside varying company (`[a,b]`, `[a,c]`, `[a,d]` — the + * set differs every round, yet `a` is a 3-peat) and to a paused HITL + * member changing the resumed round's identity. The per-call count sees + * exactly those. Baseline fixed at the round transition, like the set + * baseline, so arrival order cannot affect it. + */ + const priorCallStreaks = isSameRound + ? (previous?.priorCallStreaks ?? {}) + : (previous?.callStreaks ?? {}); + const callStreak = + (Object.hasOwn(priorCallStreaks, fingerprint) + ? (priorCallStreaks[fingerprint] as number) + : 0) + 1; + + /* + * Two identities, one scoring rule. + * + * What this CALL reports: the declared set when it is a member (every + * member of a repeating fan-out shares the round's streak, so the ladder + * applies to the round as a unit), its own singleton otherwise. On the + * undeclared path the singleton comparison is the pre-fan-out per-call + * semantics for single-call rounds; for undeclared MULTI-call rounds it + * accumulates only on whichever call lands last (see the README limit). + * + * What the ROUND stores: the declared set when there is one — a + * non-member must not overwrite the round's identity with its singleton, + * or the next round's members would compare against it and reset forever. + */ + const callSet = declaredMember + ? (declared as readonly string[]) + : [ + fingerprint, + ]; + const streak = score(callSet); + const roundSet = declared ?? [ fingerprint, - streak, + ]; + + this.tools.set(toolName, { + /* + * The identity that pairs with `streak` in persisted state: a non-member + * must not become it, or the saved count would attach to a call that + * never earned it (blocked on first appearance after a resume, while the + * real repeat lost its evidence). + */ + fingerprint: + declared !== undefined && !declaredMember && previous !== undefined + ? previous.fingerprint + : fingerprint, round, + /* + * Grown in place, O(1) per record: run-local, never serialized, and only + * the latest entry's set is ever read, so sharing the object between the + * previous and next entry is fine. The shared EMPTY_SEEN sentinel is the + * one instance that must never be mutated. + */ + seenThisRound: + seen === EMPTY_SEEN + ? new Set([ + fingerprint, + ]) + : (seen as Set).add(fingerprint), + roundFingerprints: roundSet, + streak: score(roundSet), + ...(priorSet !== undefined + ? { + priorRoundFingerprints: priorSet, + } + : {}), + priorStreak, + priorCallStreaks, + callStreaks: growCallStreaks(isSameRound ? previous : undefined, fingerprint, callStreak), }); + /* + * The stronger of the two detectors decides. The round streak covers the + * reissued fan-out as a unit; the per-call streak covers a repeat whose + * round-mates keep changing. For an exactly-repeating round both counts + * are equal, so nothing double-fires — the counts only diverge when one + * detector sees something the other cannot. + */ + const effectiveStreak = Math.max(streak, callStreak); const allowBlock = options?.allowBlock ?? true; - const action = resolveLadderAction(this.config.ladder, streak, { + const action = resolveLadderAction(this.config.ladder, effectiveStreak, { allowBlock, allowEscalate: this.canEscalate(), }); if (!action) { return { fingerprint, - streak, + streak: effectiveStreak, duplicateInRound, }; } return { fingerprint, - streak, + streak: effectiveStreak, duplicateInRound, verdict: { detector: options?.detector ?? 'tool-fingerprint', action, - streak, + streak: effectiveStreak, fingerprint, toolName, - message: - `Doom loop suspected: tool "${toolName}" was invoked in ${streak} consecutive rounds ` + - `with identical arguments (fingerprint ${fingerprint.slice(0, 16)}…). Repeating the call ` + - 'will not change the result. Take a different approach, or explain why repetition is required.', + message: buildToolVerdictMessage({ + toolName, + fingerprint, + callSet, + roundStreak: streak, + callStreak, + // An undeclared record's `callSet` is just this one call, which says + // nothing about the round's real width — so it may not be described + // as a set, nor have its fingerprint quoted as the round's identity. + roundDeclared: declaredMember, + }), }, }; } @@ -1125,3 +1592,117 @@ function strongerVerdict( } //#endregion + +/** + * This round's per-call accumulator, grown in place — O(1) per record instead + * of an O(W) spread per call of a W-wide round. Safe because only the latest + * entry's object is ever read mid-round, and `priorCallStreaks` aliases the + * PREVIOUS round's object (a fresh object is created at each round + * transition), so mutating this round's accumulator never disturbs the + * baseline. getState() copies before persisting either. + */ +function growCallStreaks( + previous: StreakEntry | undefined, + fingerprint: string, + callStreak: number, +): Record { + if (previous?.callStreaks !== undefined) { + previous.callStreaks[fingerprint] = callStreak; + return previous.callStreaks; + } + return { + [fingerprint]: callStreak, + }; +} + +/** + * The empty seen-set, shared: `recordToolCall` runs per call, and most rounds + * open with no prior members — allocating a fresh empty Set for each would be + * churn for nothing. Never mutated (the write path copies before adding). + */ +const EMPTY_SEEN: ReadonlySet = new Set(); + +/** + * Short, stable identity for a round's fingerprint set: the first members' + * prefixes, so every call of one round produces the same string (see the + * verdict message — steer dedupes on exact text). + */ +function summarizeRound(fingerprints: readonly string[]): string { + const shown = fingerprints.slice(0, 3).map((value) => value.slice(0, 8)); + return fingerprints.length > 3 + ? `${shown.join('+')}+${fingerprints.length - 3} more…` + : `${shown.join('+')}…`; +} + +/** Set equality over two sorted fingerprint lists. */ +function setsMatch(left: readonly string[], right: readonly string[]): boolean { + return left.length === right.length && left.every((value, index) => value === right[index]); +} + +/** + * Verdict text for a tool-fingerprint detection, naming what actually + * repeated. When the round streak decides (or ties), a multi-call round quotes + * the ROUND's identity — identical text for every call, deliberately, because + * the steer rung dedupes queued guidance by exact message text and one round + * of evidence must not queue N near-identical corrections. When the PER-CALL + * streak alone decides (its count exceeds the round's), the text carries + * neither the call's fingerprint nor its exact count: both vary between + * members of one round (an expanding fan-out holds counts 4, 3, 2 at once), + * and either would split one piece of guidance into per-member strings. The + * enforced bound is therefore at most TWO distinct messages per tool per + * round — one per-call text, one round-set text — each stating a distinct + * fact; exact counts live in the verdict payload's `streak`. + * + * `roundDeclared` says whether the detector was told the round's true + * membership. Only then does `callSet` describe the round, so only then may the + * text name a set or quote an argument fingerprint. On the UNDECLARED path + * (server-tool records, direct `DoomLoopMonitor` consumers, the SDK ports) each + * call is recorded alone, so `callSet` holds just that call however wide the + * round really was: the last-recorded call tied at `roundStreak == callStreak` + * and rendered the fingerprint-bearing single-call text while its round-mates + * rendered the per-call text — two strings for one round, which defeats the + * exact-text steer dedupe this shaping exists to preserve. + */ +function buildToolVerdictMessage(input: { + toolName: string; + fingerprint: string; + callSet: readonly string[]; + roundStreak: number; + callStreak: number; + roundDeclared: boolean; +}): string { + const { toolName, fingerprint, callSet, roundStreak, callStreak, roundDeclared } = input; + if (callStreak > roundStreak || !roundDeclared) { + /* + * Fingerprint-free AND count-free, so every per-call verdict of one tool + * renders byte-identical text and the steer dedupe collapses them to one + * correction. Reached when the per-call streak decides, and when the + * round's true membership is unknown (undeclared path). Both varying + * elements had to go for the dedupe to hold: quoting one call's hash + * diverged the members of a wide round, and quoting the exact count + * diverged members whose repeats started at different times (an expanding + * fan-out carries counts 4, 3, 2 in one round — three near-identical + * strings for one piece of guidance). The exact count still reaches + * consumers via the verdict payload's `streak`; the prose tells the model + * what to do about it, which does not depend on the number. + */ + return ( + `Doom loop suspected: this exact "${toolName}" call has been repeated across ` + + 'consecutive rounds. Repeating it will not change the result. ' + + 'Take a different approach, or explain why repetition is required.' + ); + } + if (callSet.length > 1) { + return ( + `Doom loop suspected: tool "${toolName}" was invoked in ${roundStreak} consecutive rounds ` + + `with the same set of ${callSet.length} parallel calls ` + + `(round identity ${summarizeRound(callSet)}). Reissuing the same fan-out ` + + 'will not change the results. Take a different approach, or explain why repetition is required.' + ); + } + return ( + `Doom loop suspected: tool "${toolName}" was invoked in ${roundStreak} consecutive rounds ` + + `with identical arguments (fingerprint ${fingerprint.slice(0, 16)}…). Repeating the call ` + + 'will not change the result. Take a different approach, or explain why repetition is required.' + ); +} diff --git a/packages/agent/src/lib/model-result.ts b/packages/agent/src/lib/model-result.ts index cc048a1..e11af64 100644 --- a/packages/agent/src/lib/model-result.ts +++ b/packages/agent/src/lib/model-result.ts @@ -38,6 +38,7 @@ import type { DoomLoopOption, DoomLoopSerializedState, DoomLoopVerdict, + LoopKeyResolution, ResolvedEscalationConfig, } from './doom-loop.js'; import { DoomLoopMonitor, resolveDoomLoopOption, resolveLoopKeyMaterial } from './doom-loop.js'; @@ -195,6 +196,16 @@ function extractServerToolIdentity(item: ServerToolResultItem): Record(); + // Loop-key resolutions computed while declaring the current round, keyed by + // tool-call id. A `loopKey` function is user code that may count, log, or + // return something different each time, so it must run at most once per + // call: the per-call checkpoint reuses what the declaration resolved instead + // of resolving again. Cleared with the round. An id seen twice in one batch + // maps to DUPLICATE_CALL_ID — see the write site. + private readonly doomLoopRoundKeyMaterial = new Map< + string, + LoopKeyResolution | typeof DUPLICATE_CALL_ID + >(); // Serialization chain for doom evaluations: parallel tool executions // append their evaluation here in call order (the .map() over a round's // calls runs synchronously to the first await), so streak recording @@ -1487,17 +1508,154 @@ export class ModelResult< } /** - * Advance the doom-loop round counter. Called at every execution-batch - * boundary (main tool round, auto-approve batch while pausing, approved- - * on-resume batch). Identical calls WITHIN one round are duplicates — - * one piece of loop evidence, one shared decision. + * Advance the doom-loop round counter and declare the round's calls. Called + * at every execution-batch boundary (main tool round, auto-approve batch + * while pausing, approved-on-resume batch). Identical calls WITHIN one round + * are duplicates — one piece of loop evidence, one shared decision. + * + * `batch` is every call the round will make. A round's identity for one tool + * is the *set* of fingerprints it was called with, and that set has to be + * complete before any of the round's calls is scored: accumulating it call + * by call made a round that is a superset of the previous one transiently + * match it, blocking calls that represented real progress, with the outcome + * depending on emission order. Declaration is best-effort — a call whose key + * material is exempt or unhashable is simply left out, and the per-call + * fallback chain in `enqueueDoomLoopEvaluation` still governs identity at + * record time. + * + * Only calls that the round will actually CHECK are declared. A `loopKey` + * function is user code that may count or log, so it must not run for a call + * the detector never evaluates: manual tool calls (no `execute`, no + * `onToolCalled`) are handed to the caller and never recorded, and every + * execution path skips them with the same `isAutoResolvableTool` predicate + * used here. Their absence from the declared set is also correct on its own + * terms — they are not evidence, so they are not part of the round. + * + * INVARIANT — every declared member must eventually be recorded. A call + * that is declared but never reaches the doom-loop checkpoint is a phantom + * member of the round's identity: the tool's streak silently resets the + * moment the phantom stops being emitted (pinned by the "declared-but- + * never-recorded member" test in doom-loop-fanout.test.ts). The filters + * below therefore MIRROR every path that skips recording — unknown/manual + * tools (`isAutoResolvableTool`, also gating executeAutoApproveTools and + * the approval-resume loop), `hookDeniedCalls` (consumed before the + * checkpoint in executeSingleToolCall), and `loopKey`-exempt calls (early + * return in checkDoomLoopBeforeExecution; the same resolution is cached + * here so both sides agree). If you add a new short-circuit between + * declaration and the checkpoint — a pre-execution gate, a batch filter — + * it MUST be reflected here, or fan-out detection degrades silently for + * that tool rather than failing loudly. + * + * KNOWN RESIDUAL (documented, not mirrored): a per-request timeout or + * abort can cancel a call AFTER declaration but BEFORE its checkpoint, + * leaving a declared phantom for that round. Bounded and fail-safe: the + * round-set streak for that tool resets when the phantom stops recurring, + * while per-call streaks are unaffected — detection degrades to per-call + * for the affected tool rather than being lost. Un-declaring mid-round + * would mutate the round's identity while it is being scored, which is the + * incremental-set order-dependence this design exists to prevent. */ - private beginDoomLoopRound(): void { - if (!this.doomLoopMonitor) { + private async beginDoomLoopRound(batch: readonly ParsedToolCall[] = []): Promise { + const monitor = this.doomLoopMonitor; + if (!monitor) { return; } this.doomLoopRound++; this.doomLoopRoundDecisions.clear(); + this.doomLoopRoundKeyMaterial.clear(); + + const declared: { + toolName: string; + keyMaterial: unknown; + }[] = []; + for (const toolCall of batch) { + const tool = this.options.tools?.find( + (t) => isClientTool(t) && t.function.name === toolCall.name, + ); + /* + * Never checked => never recorded => must not be declared. Declaring a + * call that never arrives inflates the round's identity: a sibling that + * IS recorded gets scored against a set containing a phantom member, and + * the streak resets spuriously once the phantom stops appearing (e.g. the + * model drops a malformed call while still repeating the valid one). + * Keeping `loopKey` from running for such a call is the same check. + * + * This gate deliberately precedes the malformed-arguments branch below: + * a raw-string call to an unknown or manual tool is still never recorded. + */ + if (tool === undefined || !isAutoResolvableTool(tool)) { + continue; + } + /* + * Same reasoning for a call the PermissionRequest hook denied without + * pausing: `hookDeniedCalls` is populated before the round begins, and + * `runToolWithHooks` synthesizes the rejection before reaching the + * doom-loop checkpoint, so the call is never recorded either. + */ + if (this.hookDeniedCalls.has(toolCall.id)) { + continue; + } + const rawArgs: unknown = toolCall.arguments; + if (typeof rawArgs === 'string') { + // Malformed call: its identity is the raw string (see runToolWithHooks). + declared.push({ + toolName: String(toolCall.name), + keyMaterial: rawArgs, + }); + continue; + } + /* + * `resolveLoopKeyMaterial` catches a throwing `loopKey`, but not every + * throw: the field-list form reads `args[field]`, so a getter on the + * arguments object throws out of it uncaught. Declaration runs once for + * the whole batch, so letting that escape would fail the entire round — + * and the run — over one odd call, breaking the invariant that detection + * never affects a run except through its ladder actions. Skip just that + * call; the per-call checkpoint hits the same throw inside its own + * try/catch and applies the documented fallback chain there. + */ + let resolution: LoopKeyResolution; + try { + resolution = resolveLoopKeyMaterial( + isClientTool(tool) ? tool.function.loopKey : undefined, + (toolCall.arguments ?? {}) as Record, + ); + } catch (error) { + console.warn( + `[DoomLoop] could not resolve loop identity for "${toolCall.name}" while declaring the round; ` + + 'excluding it from the round set:', + error, + ); + continue; + } + /* + * Cache so the per-call checkpoint does not invoke `loopKey` a second + * time; keyed by call id. Ids are MODEL-emitted strings and nothing + * upstream enforces uniqueness, so a duplicate id must not alias one + * call's identity onto another: overwriting here meant the first call's + * checkpoint read the second call's key material, its true fingerprint + * was never recorded, and a model emitting `(id=X, read a), (id=X, + * read b_i)` each round evaded the per-call detector for `a` entirely + * (measured: zero detections across four such rounds). On a duplicate + * id the cache poisons that id instead: both calls fall through to + * per-call resolution at the checkpoint, costing at most a duplicate + * `loopKey` invocation for the colliding calls — correctness over the + * single-invocation economy, for protocol-malformed input only. + */ + if (this.doomLoopRoundKeyMaterial.has(toolCall.id)) { + this.doomLoopRoundKeyMaterial.set(toolCall.id, DUPLICATE_CALL_ID); + } else { + this.doomLoopRoundKeyMaterial.set(toolCall.id, resolution); + } + if (resolution.kind === 'exempt') { + continue; + } + declared.push({ + toolName: String(toolCall.name), + keyMaterial: resolution.keyMaterial, + }); + } + await monitor.declareRound(this.doomLoopRound, declared); } /** @@ -1661,8 +1819,41 @@ export class ModelResult< // arguments, so what remains is the parsed record (or null/undefined for // no-args calls — coerced to {} the same way the PreToolUse payload is). const callArguments = (toolCall.arguments ?? {}) as Record; - const loopKey = isClientTool(tool) ? tool.function.loopKey : undefined; - const resolution = resolveLoopKeyMaterial(loopKey, callArguments); + /* + * Reuse what `beginDoomLoopRound` resolved for this call when it declared + * the round. `loopKey` is user code — it may count, log, or return a fresh + * value each time — so it must run at most once per call. Resolving here + * as well would double-invoke it and, for a non-repeatable callback, make + * the declared identity and the recorded identity disagree. + */ + const cached = this.doomLoopRoundKeyMaterial.get(toolCall.id); + let resolution: LoopKeyResolution; + if (cached !== undefined && cached !== DUPLICATE_CALL_ID) { + resolution = cached; + } else { + /* + * Not declared (undeclared round, or the declaration skipped this call). + * `resolveLoopKeyMaterial` can throw despite catching `loopKey` itself — + * the field-list form reads `args[field]`, so a getter on the arguments + * throws out of it. Detection must never reject a call or fail a run + * except through a ladder action, so skip detection for this one call. + */ + try { + resolution = resolveLoopKeyMaterial( + isClientTool(tool) ? tool.function.loopKey : undefined, + callArguments, + ); + } catch (error) { + console.warn( + `[DoomLoop] could not resolve loop identity for "${toolCall.name}"; ` + + 'skipping detection for this call:', + error, + ); + return { + blocked: false, + }; + } + } if (resolution.kind === 'exempt') { return { blocked: false, @@ -2451,7 +2642,7 @@ export class ModelResult< ): Promise[]> { // Auto-approved batch = one doom-loop round: identical parallel calls // count once (see beginDoomLoopRound). - this.beginDoomLoopRound(); + await this.beginDoomLoopRound(toolCalls as ParsedToolCall[]); const toolCallPromises = toolCalls.map(async (tc) => { const tool = this.options.tools?.find((t) => isClientTool(t) && t.function.name === tc.name); if (!tool || !isAutoResolvableTool(tool)) { @@ -3222,7 +3413,7 @@ export class ModelResult< }> { // One executed batch = one doom-loop round: identical parallel calls in // this batch count as ONE piece of loop evidence and share a decision. - this.beginDoomLoopRound(); + await this.beginDoomLoopRound(toolCalls); const toolCallPromises = toolCalls.map((toolCall) => this.executeSingleToolCall(toolCall, turnContext), ); @@ -5095,7 +5286,15 @@ export class ModelResult< // The approved batch is one doom-loop round: N approved duplicates of // the same call count once (the sequential loop below still evaluates // in order; restored streaks from the persisted state carry forward). - this.beginDoomLoopRound(); + // Declared from the approved calls only — a pending call the user did not + // approve is not part of this round. + await this.beginDoomLoopRound( + [ + ...this.approvedToolCalls, + ] + .map((callId) => pendingCalls.find((tc) => tc.id === callId)) + .filter((tc): tc is ParsedToolCall => tc !== undefined), + ); // Process approvals - execute the approved tools. Route through // runToolWithHooks so PreToolUse/PostToolUse fire even on this path. diff --git a/packages/agent/tests/unit/doom-loop-fanout.test.ts b/packages/agent/tests/unit/doom-loop-fanout.test.ts new file mode 100644 index 0000000..a1810d9 --- /dev/null +++ b/packages/agent/tests/unit/doom-loop-fanout.test.ts @@ -0,0 +1,1571 @@ +/** + * Regression suite for the same-tool fan-out gap. + * + * A streak keyed on a tool's *last* fingerprint cannot see a repeating + * fan-out: `read(a), read(b), read(c)` reissued verbatim has a different last + * call every round, so each round's first call reset the streak to 1 and the + * run never accumulated evidence. Measured before the fix: 8 identical rounds + * of a 3-call fan-out produced zero detections, while single-call rounds + * tripped at round 2. + * + * A round's identity for one tool is therefore the *set* of fingerprints it + * was called with. The engine declares that set before scoring any of the + * round's calls, so every call in a round reports the round's streak and the + * comparison is between whole rounds. + * + * Scoring the set as it accumulated instead — the first attempt at this fix — + * made a round that is a strict *superset* of the previous one transiently + * equal it while filling, so `[a,b]`, `[a,b]`, `[a,b,c]` blocked the `b` call + * of a round that had added new work, and did so only for that emission order. + * The superset, order-permutation, and expanding-fan-out cases below guard + * that; the subset case alone did not catch it. + */ +import { describe, expect, it } from 'vitest'; +import type { DoomLoopCallRecord } from '../../src/lib/doom-loop.js'; +import { DoomLoopMonitor, resolveDoomLoopOption } from '../../src/lib/doom-loop.js'; + +type RecordedAction = string; + +const monitor = (): DoomLoopMonitor => new DoomLoopMonitor(resolveDoomLoopOption(true)); + +/** + * Plays each fan-out as one round, declaring the round's calls first — the + * engine does the same at every execution-batch boundary, so the round's + * fingerprint set is complete before any of its calls is scored. + */ +async function playRounds(fanouts: readonly (readonly string[])[]): Promise { + const detector = monitor(); + const actions: RecordedAction[][] = []; + for (const [round, paths] of fanouts.entries()) { + await detector.declareRound( + round, + paths.map((path) => ({ + toolName: 'read', + keyMaterial: { + path, + }, + })), + ); + const roundActions: RecordedAction[] = []; + for (const path of paths) { + const record = await detector.recordToolCall( + 'read', + { + path, + }, + round, + ); + roundActions.push(record.verdict?.action ?? 'none'); + } + actions.push(roundActions); + } + return actions; +} + +describe('same-tool fan-out streaks', () => { + it('accumulates across rounds that repeat a distinct-argument fan-out', async () => { + const actions = await playRounds([ + [ + 'a', + 'b', + 'c', + ], + [ + 'a', + 'b', + 'c', + ], + [ + 'a', + 'b', + 'c', + ], + ]); + + /* + * Round 0 is the baseline. Every call in a repeating round reports that + * round's streak — the round is the unit of evidence, so the ladder + * applies to the whole fan-out rather than only to whichever call + * happened to complete the match. At the block rung that means the + * repeating fan-out stops spending, not just its last call. + */ + expect(actions[0]).toEqual([ + 'none', + 'none', + 'none', + ]); + expect(actions[1]).toEqual([ + 'observe', + 'observe', + 'observe', + ]); + expect(actions[2]).toEqual([ + 'block', + 'block', + 'block', + ]); + }); + + it('is order-insensitive within the round', async () => { + const actions = await playRounds([ + [ + 'a', + 'b', + 'c', + ], + [ + 'c', + 'a', + 'b', + ], + [ + 'b', + 'c', + 'a', + ], + ]); + + /* Whole round, not just its last call: the set is what matched. */ + expect(actions[1]).toEqual([ + 'observe', + 'observe', + 'observe', + ]); + expect(actions[2]).toEqual([ + 'block', + 'block', + 'block', + ]); + }); + + it('scores each call individually when the fan-out membership changes', async () => { + const actions = await playRounds([ + [ + 'a', + 'b', + ], + [ + 'a', + 'b', + ], + [ + 'a', + 'z', + ], + [ + 'a', + 'z', + ], + ]); + + expect(actions[1]).toEqual([ + 'observe', + 'observe', + ]); + /* + * Round 3 changes a member: the ROUND identity resets (progress), but `a` + * itself is on its third consecutive round — the per-call detector flags + * it while the genuinely new `z` runs free. Swapping one argument while + * re-issuing the rest is not a loop escape. + */ + expect(actions[2]).toEqual([ + 'block', + 'none', + ]); + expect(actions[3]).toEqual([ + 'block', + 'observe', + ]); + }); + + it('flags the calls of a partial repeat that actually repeated', async () => { + const actions = await playRounds([ + [ + 'a', + 'b', + 'c', + ], + [ + 'a', + 'b', + ], + ]); + + /* + * A strict subset is a different ROUND, but `a` and `b` are each on their + * second consecutive round: doing strictly less work does not make the + * re-issued calls progress. Observe only — dropping work never escalates + * faster than repeating it. + */ + expect(actions[1]).toEqual([ + 'observe', + 'observe', + ]); + }); + + it('flags the repeated members of a superset round, never the new one', async () => { + const actions = await playRounds([ + [ + 'a', + 'b', + ], + [ + 'a', + 'b', + ], + [ + 'a', + 'b', + 'c', + ], + ]); + + /* + * Round 3 adds `c`: the ROUND is progress, and `c` must run — the original + * superset bug blocked it via a transient identity match, order- + * dependently. Under per-call scoring `a` and `b` are flagged because each + * genuinely IS on its third consecutive round (the model re-read both + * while adding one file), while `c` executes untouched. Unlike the old + * bug, this is order-independent — see the permutation test below. + */ + expect(actions[1]).toEqual([ + 'observe', + 'observe', + ]); + expect(actions[2]).toEqual([ + 'block', + 'block', + 'none', + ]); + }); + + it('scores a superset round the same whatever order it is emitted in', async () => { + const inOrder = await playRounds([ + [ + 'a', + 'b', + ], + [ + 'a', + 'b', + ], + [ + 'a', + 'b', + 'c', + ], + ]); + const permuted = await playRounds([ + [ + 'a', + 'b', + ], + [ + 'a', + 'b', + ], + [ + 'c', + 'a', + 'b', + ], + ]); + + /* + * Emission order must not decide any call's outcome. While the set + * accumulated, `[a,b,c]` blocked its `b` call and `[c,a,b]` fired nothing + * — same calls, same history, different outcome. Per-call verdicts follow + * each call's own identity, so a permutation reorders the verdicts with + * the calls but never changes what any call receives. + */ + expect(inOrder[2]).toEqual([ + 'block', + 'block', + 'none', + ]); + expect(permuted[2]).toEqual([ + 'none', + 'block', + 'block', + ]); + }); + + it('scores an expanding fan-out per call: repeats climb, each new call runs free', async () => { + const actions = await playRounds([ + [ + 'a', + ], + [ + 'a', + 'b', + ], + [ + 'a', + 'b', + 'c', + ], + [ + 'a', + 'b', + 'c', + 'd', + ], + ]); + + /* + * Every round adds work, so no ROUND repeats its predecessor — but `a` is + * re-issued in all four rounds and `b` in three. Per-call evidence tracks + * each: the newest call is always clean, the oldest climbs the ladder. + * (Genuine incremental exploration re-reads nothing and stays silent; a + * tool that legitimately re-reads its anchors opts out via `loopKey`.) + */ + expect(actions).toEqual([ + [ + 'none', + ], + [ + 'observe', + 'none', + ], + [ + 'block', + 'observe', + 'none', + ], + [ + 'block', + 'block', + 'observe', + 'none', + ], + ]); + }); + + it('leaves single-call rounds behaving exactly as before', async () => { + const actions = await playRounds([ + [ + 'a', + ], + [ + 'a', + ], + [ + 'a', + ], + [ + 'a', + ], + ]); + + expect(actions.flat()).toEqual([ + 'none', + 'observe', + 'block', + 'block', + ]); + }); + + it('still counts identical duplicates within one round only once', async () => { + const detector = monitor(); + const first = await detector.recordToolCall( + 'read', + { + path: 'a', + }, + 0, + ); + const second = await detector.recordToolCall( + 'read', + { + path: 'a', + }, + 0, + ); + + expect(first.duplicateInRound).toBe(false); + expect(second.duplicateInRound).toBe(true); + expect(second.streak).toBe(first.streak); + }); + + it('keeps a resumed single-call streak incrementing after restore', async () => { + const detector = monitor(); + await detector.recordToolCall( + 'read', + { + path: 'a', + }, + 0, + ); + await detector.recordToolCall( + 'read', + { + path: 'a', + }, + 1, + ); + + const resumed = new DoomLoopMonitor(resolveDoomLoopOption(true), detector.getState()); + const record = await resumed.recordToolCall( + 'read', + { + path: 'a', + }, + 0, + ); + + expect(record.streak).toBe(3); + }); + + it('falls back to per-call scoring when a round is never declared', async () => { + /* + * The engine declares every executed batch, but server-tool records go + * through `checkDoomLoopForResponse` undeclared, as do direct callers and + * ports. A round's membership is unknowable there, so each call is scored + * on its own identity rather than as part of a set. Sharing a round streak + * here instead let a brand-new call inherit an earlier call's count: `[a]` + * then `[a, b]` scored `b` as a 2-round repeat and emitted a verdict + * quoting `b`'s own fingerprint, for a call the model had just made for + * the first time. + * + * Note this is NOT identical to the pre-fan-out per-call comparison for + * multi-call rounds — see the order-dependence test below. + */ + const detector = monitor(); + await detector.recordToolCall( + 'server:web_search', + { + q: 'x', + }, + 0, + ); + const repeated = await detector.recordToolCall( + 'server:web_search', + { + q: 'x', + }, + 1, + ); + const fresh = await detector.recordToolCall( + 'server:web_search', + { + q: 'y', + }, + 1, + ); + + /* The genuine repeat still accumulates. */ + expect(repeated.streak).toBe(2); + /* The first-ever call does not inherit it. */ + expect(fresh.streak).toBe(1); + expect(fresh.verdict).toBeUndefined(); + }); + + it('scores an UNDECLARED multi-call round per call, order-independently', async () => { + /* + * Pins the undeclared path (server-tool records, direct callers). The + * round SET is unknowable there, but per-call evidence needs no + * declaration: every repeated fingerprint accumulates its own count, so a + * repeating undeclared fan-out now flags EVERY repeated member instead of + * only whichever happened to be recorded last, and flipping the emission + * order no longer changes any call's outcome. + * + * Still worth pinning: verdicts here can reach `stop` at the default + * ladder's streak 6 (server-tool verdicts cannot `block`, but they can + * stop a run), and a repeat inside a *varying* round accumulates on the + * repeated member exactly as on the declared path. + */ + const undeclared = async (rounds: readonly (readonly string[])[]): Promise => { + const detector = monitor(); + const out: string[][] = []; + for (const [round, paths] of rounds.entries()) { + const scored: string[] = []; + for (const path of paths) { + const record = await detector.recordToolCall( + 'read', + { + path, + }, + round, + ); + scored.push(`${record.streak}:${record.verdict?.action ?? 'none'}`); + } + out.push(scored); + } + return out; + }; + + /* Both repeated members accumulate, not just the last-recorded one. */ + expect( + await undeclared([ + [ + 'a', + 'b', + ], + [ + 'a', + 'b', + ], + ]), + ).toEqual([ + [ + '1:none', + '1:none', + ], + [ + '2:observe', + '2:observe', + ], + ]); + + /* Flipping the order changes nothing about any call's outcome. */ + expect( + await undeclared([ + [ + 'a', + 'b', + ], + [ + 'b', + 'a', + ], + ]), + ).toEqual([ + [ + '1:none', + '1:none', + ], + [ + '2:observe', + '2:observe', + ], + ]); + + /* A varying round accumulates on the repeated member wherever it sits. */ + expect( + await undeclared([ + [ + 'b', + 'a', + ], + [ + 'b', + 'c', + ], + [ + 'b', + 'd', + ], + ]), + ).toEqual([ + [ + '1:none', + '1:none', + ], + [ + '2:observe', + '1:none', + ], + [ + '3:block', + '1:none', + ], + ]); + }); + + it('does not let a call outside the declared set inherit the round streak', async () => { + /* + * `declareRound` drops a call whose key material is unhashable (bigint, + * NaN, circular), but at record time that call still resolves an identity + * through the caller's fallback chain. Sharing the round's streak is keyed + * on the call being a MEMBER of the declaration, not merely on the tool + * having one — otherwise the dropped call inherits the round's accumulated + * count and can be blocked on its first ever appearance, which is the + * failure mode declared/undeclared scoring exists to prevent. + */ + const detector = monitor(); + const hashable = { + path: 'a', + }; + /* Establish a streak on the declared member across two rounds. */ + for (const round of [ + 0, + 1, + ]) { + await detector.declareRound(round, [ + { + toolName: 'read', + keyMaterial: hashable, + }, + /* Unhashable: dropped from the declared set. */ + { + toolName: 'read', + keyMaterial: { + size: 1n, + }, + }, + ]); + await detector.recordToolCall('read', hashable, round); + } + + /* Round 2: the declared member repeats, then the dropped call arrives. */ + await detector.declareRound(2, [ + { + toolName: 'read', + keyMaterial: hashable, + }, + { + toolName: 'read', + keyMaterial: { + size: 1n, + }, + }, + ]); + const member = await detector.recordToolCall('read', hashable, 2); + /* Recorded with a fallback identity, as the engine would — for the + * FIRST time; earlier rounds never recorded it. */ + const dropped = await detector.recordToolCall( + 'read', + { + size: 'fallback-identity', + }, + 2, + ); + + /* The real repeat accumulates. */ + expect(member.streak).toBe(3); + /* The non-member's first-ever appearance inherits nothing — not the + * round streak, and (never having been recorded) no per-call count. */ + expect(dropped.streak).toBe(1); + expect(dropped.verdict).toBeUndefined(); + }); + + it('accumulates per-call evidence for an unhashable call reissued verbatim', async () => { + /* + * A call dropped from the declaration (unhashable key material) records + * under a fallback identity as a NON-member: it can never inherit or move + * the round's counters. Its OWN repetition is still evidence — before + * per-call streaks it was pinned at 1 forever, a documented detection + * loss ("costs detection for its own call only"). Now the fallback + * identity accumulates like any repeat, while the round members are + * unaffected either way. + */ + const detector = monitor(); + const droppedStreaks: number[] = []; + const memberStreaks: number[] = []; + for (const round of [ + 0, + 1, + 2, + ]) { + await detector.declareRound(round, [ + { + toolName: 'read', + keyMaterial: { + path: 'a', + }, + }, + { + toolName: 'read', + keyMaterial: { + size: 1n, + }, + }, + ]); + memberStreaks.push( + ( + await detector.recordToolCall( + 'read', + { + path: 'a', + }, + round, + ) + ).streak, + ); + droppedStreaks.push( + ( + await detector.recordToolCall( + 'read', + { + size: 'fallback-identity', + }, + round, + ) + ).streak, + ); + } + + expect(memberStreaks).toEqual([ + 1, + 2, + 3, + ]); + expect(droppedStreaks).toEqual([ + 1, + 2, + 3, + ]); + }); + + it('keeps accumulating when an unhashable call rides along every round', async () => { + /* + * A non-member must not write the round's identity. It used to store its + * own single-fingerprint set as `roundFingerprints`, so the NEXT round's + * declared member compared against that singleton, failed to match, and + * reset to 1 — permanently, for as long as the unhashable call recurred. + * One unhashable argument therefore disabled detection for that tool for + * the rest of the run, which is the opposite of the fail-open guarantee + * (an unhashable value may only cost detection for ITS OWN call). + * + * Asserted in both emission orders, since which call opens the round is + * up to the model. + */ + const streaksFor = async (droppedFirst: boolean): Promise => { + const detector = monitor(); + const streaks: number[] = []; + for (const round of [ + 0, + 1, + 2, + 3, + ]) { + await detector.declareRound(round, [ + { + toolName: 'read', + keyMaterial: { + path: 'a', + }, + }, + { + toolName: 'read', + keyMaterial: { + size: 1n, + }, + }, + ]); + const recordDropped = (): Promise => + detector.recordToolCall( + 'read', + { + size: 'fallback-identity', + }, + round, + ); + if (droppedFirst) { + await recordDropped(); + } + const member = await detector.recordToolCall( + 'read', + { + path: 'a', + }, + round, + ); + if (!droppedFirst) { + await recordDropped(); + } + streaks.push(member.streak); + } + return streaks; + }; + + const expected = [ + 1, + 2, + 3, + 4, + ]; + expect(await streaksFor(false)).toEqual(expected); + /* Order must not matter: the streak is a function of the sets, not arrival. */ + expect(await streaksFor(true)).toEqual(expected); + }); + + it('persists the streak against the member that earned it, not a non-member', async () => { + /* + * `fingerprint` is the identity that pairs with `streak` in persisted + * state. A non-member recorded LAST in the round used to overwrite it, so + * the saved count was attached to a call that never earned it. Both halves + * then went wrong on resume: the non-member (a call detection is meant to + * ignore) matched, inherited the count, and was BLOCKED on its first + * appearance, while the genuinely repeating call was no longer the saved + * identity and reset to 1, losing its evidence. + */ + const detector = monitor(); + for (const round of [ + 0, + 1, + ]) { + await detector.declareRound(round, [ + { + toolName: 'read', + keyMaterial: { + path: 'a', + }, + }, + /* Unhashable: dropped from the declared set. */ + { + toolName: 'read', + keyMaterial: { + size: 1n, + }, + }, + ]); + await detector.recordToolCall( + 'read', + { + path: 'a', + }, + round, + ); + /* Recorded LAST, so it used to become the persisted identity. */ + await detector.recordToolCall( + 'read', + { + size: 'fallback-identity', + }, + round, + ); + } + + /* + * The non-member was genuinely recorded in both rounds, so on resume its + * OWN per-call count continues (2 -> 3) — earned evidence, not the round + * streak leaking. The K2 bug this test pins was different: the saved + * ROUND count attached to the non-member's fingerprint, so it inherited + * evidence it never earned while the real repeat lost its own. The guard + * for that is the identity pairing, asserted below via the member. + */ + const resumedNonMember = new DoomLoopMonitor(resolveDoomLoopOption(true), detector.getState()); + const ownRepeat = await resumedNonMember.recordToolCall( + 'read', + { + size: 'fallback-identity', + }, + 0, + ); + expect(ownRepeat.streak).toBe(3); + + /* A call NEVER recorded before the save inherits nothing on resume. */ + const resumedFresh = new DoomLoopMonitor(resolveDoomLoopOption(true), detector.getState()); + const fresh = await resumedFresh.recordToolCall( + 'read', + { + size: 'never-seen-before', + }, + 0, + ); + expect(fresh.streak).toBe(1); + expect(fresh.verdict).toBeUndefined(); + + /* And the real repeat keeps the evidence it earned. */ + const resumedMember = new DoomLoopMonitor(resolveDoomLoopOption(true), detector.getState()); + await resumedMember.declareRound(0, [ + { + toolName: 'read', + keyMaterial: { + path: 'a', + }, + }, + ]); + const realRepeat = await resumedMember.recordToolCall( + 'read', + { + path: 'a', + }, + 0, + ); + expect(realRepeat.streak).toBe(3); + }); + + it('keeps counting a recorded call when a declared-but-never-recorded member disappears', async () => { + /* + * A phantom member — declared but never recorded (a manual tool, a + * PermissionRequest denial) — inflates the ROUND's identity, so the round + * streak resets when the phantom stops being emitted. That used to zero + * detection for the sibling that WAS recorded every round; per-call + * evidence is immune, because it follows the call's own fingerprint + * rather than the round set. The engine still filters phantoms out of + * declarations (`isAutoResolvableTool`, `hookDeniedCalls` in + * `beginDoomLoopRound`) so the ROUND streak stays meaningful too. + */ + const detector = monitor(); + const streaks: number[] = []; + for (const round of [ + 0, + 1, + 2, + 3, + ]) { + /* Rounds 0-1 declare a phantom alongside the real call; 2-3 do not. */ + const declaredCalls = [ + { + toolName: 'read', + keyMaterial: { + path: 'a', + }, + }, + ...(round < 2 + ? [ + { + toolName: 'read', + keyMaterial: { + path: 'phantom', + }, + }, + ] + : []), + ]; + await detector.declareRound(round, declaredCalls); + /* Only the real call is ever recorded. */ + streaks.push( + ( + await detector.recordToolCall( + 'read', + { + path: 'a', + }, + round, + ) + ).streak, + ); + } + + /* Identical call, four consecutive rounds: uninterrupted evidence. */ + expect(streaks).toEqual([ + 1, + 2, + 3, + 4, + ]); + }); + + it('gives every call of a repeating round the SAME message so steer dedupes', async () => { + /* + * `queueDoomLoopSteer` dedupes queued guidance by exact message text. Now + * that every call of a repeating round emits a verdict, quoting the + * individual call's fingerprint would queue one near-identical correction + * per call — three user messages for one round of evidence. A multi-call + * round therefore quotes the ROUND's identity instead. + */ + const detector = new DoomLoopMonitor( + resolveDoomLoopOption({ + ladder: { + steer: 2, + }, + }), + ); + const paths = [ + 'a', + 'b', + 'c', + ]; + const messages: string[] = []; + for (const round of [ + 0, + 1, + ]) { + await detector.declareRound( + round, + paths.map((path) => ({ + toolName: 'read', + keyMaterial: { + path, + }, + })), + ); + for (const path of paths) { + const record = await detector.recordToolCall( + 'read', + { + path, + }, + round, + ); + if (round === 1 && record.verdict) { + messages.push(record.verdict.message); + } + } + } + + expect(messages).toHaveLength(3); + expect(new Set(messages).size).toBe(1); + /* Names the shape rather than one member's hash. */ + expect(messages[0]).toContain('3 parallel calls'); + }); + + it('continues a resumed FAN-OUT streak, exactly like a single-call streak', async () => { + /* + * The round SET is persisted (when multi-call), so a doom loop spanning a + * serialize/resume boundary is still a doom loop: the resumed identical + * fan-out picks its count back up instead of getting a fresh grace window. + * Losing this across every save meant approval pauses reset condemned + * fan-outs and per-turn-resume topologies never accumulated at all. + */ + const detector = monitor(); + for (const round of [ + 0, + 1, + ]) { + const paths = [ + 'a', + 'b', + ]; + await detector.declareRound( + round, + paths.map((path) => ({ + toolName: 'read', + keyMaterial: { + path, + }, + })), + ); + for (const path of paths) { + await detector.recordToolCall( + 'read', + { + path, + }, + round, + ); + } + } + + /* JSON round-trip: the set must survive real serialization. */ + const wire = JSON.parse(JSON.stringify(detector.getState())) as unknown; + const resumed = new DoomLoopMonitor(resolveDoomLoopOption(true), wire); + await resumed.declareRound( + 0, + [ + 'a', + 'b', + ].map((path) => ({ + toolName: 'read', + keyMaterial: { + path, + }, + })), + ); + const record = await resumed.recordToolCall( + 'read', + { + path: 'a', + }, + 0, + ); + + /* Streak was 2 at save; the identical resumed fan-out continues to 3. */ + expect(record.streak).toBe(3); + expect(record.verdict?.action).toBe('block'); + }); + + it('keeps counting a repeat when a paused HITL member drops from the resumed round', async () => { + /* + * A HITL member that pauses is recorded in the round where it pauses, but + * the resumed batch no longer contains it — so the tool's ROUND identity + * differs and the round streak resets. Before per-call evidence, that + * granted the loop a fresh grace window on every pause. The repeated + * working call now carries its own count through the membership change. + */ + const detector = monitor(); + const workStreaks: string[] = []; + for (const round of [ + 0, + 1, + 2, + ]) { + /* Round 0 includes the gated call; the resumed rounds do not. */ + const members = + round === 0 + ? [ + 'work', + 'gated', + ] + : [ + 'work', + ]; + await detector.declareRound( + round, + members.map((path) => ({ + toolName: 'deploy', + keyMaterial: { + path, + }, + })), + ); + for (const path of members) { + const record = await detector.recordToolCall( + 'deploy', + { + path, + }, + round, + ); + if (path === 'work') { + workStreaks.push(`${record.streak}:${record.verdict?.action ?? 'none'}`); + } + } + } + + expect(workStreaks).toEqual([ + '1:none', + '2:observe', + '3:block', + ]); + }); + + it('collapses STAGGERED per-call counts in one round to a single message', async () => { + /* + * An expanding fan-out carries different per-call counts in one round — + * `[a]`, `[a,b]`, `[a,b,c]`, `[a,b,c,d]` puts a=4, b=3, c=2 in round 4. + * The per-call message used to interpolate each call's own count, so the + * three verdicts rendered three near-identical strings and the exact-text + * steer dedupe queued all of them: the real bound was the round's WIDTH, + * not the documented two. The per-call text is now count-free (the exact + * count still reaches consumers via the verdict payload's `streak`), so + * every per-call verdict of one tool collapses to one correction. + */ + const detector = new DoomLoopMonitor( + resolveDoomLoopOption({ + ladder: { + steer: 2, + }, + }), + ); + const rounds = [ + [ + 'a', + ], + [ + 'a', + 'b', + ], + [ + 'a', + 'b', + 'c', + ], + [ + 'a', + 'b', + 'c', + 'd', + ], + ]; + const messages: string[] = []; + const streaks: number[] = []; + for (const [round, paths] of rounds.entries()) { + await detector.declareRound( + round, + paths.map((path) => ({ + toolName: 'read', + keyMaterial: { + path, + }, + })), + ); + for (const path of paths) { + const record = await detector.recordToolCall( + 'read', + { + path, + }, + round, + ); + if (round === 3 && record.verdict) { + messages.push(record.verdict.message); + streaks.push(record.verdict.streak); + } + } + } + + /* Three verdicts with distinct counts — the payload keeps the numbers... */ + expect(streaks).toEqual([ + 4, + 3, + 2, + ]); + /* ...but the prose is byte-identical, so steer queues ONE correction. */ + expect(messages).toHaveLength(3); + expect(new Set(messages).size).toBe(1); + }); + + it('bounds a mixed-evidence round to one message per distinct fact', async () => { + /* + * One round can carry TWO pieces of evidence: `[a]`, `[a,b]`, `[a,b]` — + * by round 3, `a` is a 3-peat call (per-call branch) while `{a,b}` is a + * 2-peat set (round branch), so the round legitimately renders two + * DIFFERENT messages stating two different facts. What must hold is the + * bound: same evidence -> byte-identical text, so the steer queue carries + * at most one message per distinct fact per tool per round — never one + * per call. The wide-round test below pins the N-collapses-to-1 case; + * this pins the two-facts case at exactly 2, with the members of each + * fact sharing text. + */ + const detector = new DoomLoopMonitor( + resolveDoomLoopOption({ + ladder: { + steer: 2, + }, + }), + ); + const rounds = [ + [ + 'a', + ], + [ + 'a', + 'b', + ], + [ + 'a', + 'b', + ], + ]; + const lastRound = rounds.length - 1; + const messages: string[] = []; + for (const [round, paths] of rounds.entries()) { + await detector.declareRound( + round, + paths.map((path) => ({ + toolName: 'read', + keyMaterial: { + path, + }, + })), + ); + for (const path of paths) { + const record = await detector.recordToolCall( + 'read', + { + path, + }, + round, + ); + if (round === lastRound && record.verdict) { + messages.push(record.verdict.message); + } + } + } + + /* + * Final round: `a` is a 3-peat call (per-call branch, count 3) while + * `{a,b}` is a 2-peat set (`b` ties the round streak, round-set branch). + * Two verdicts, two distinct messages — one per fact, not one per call. + */ + expect(messages).toHaveLength(2); + expect(new Set(messages).size).toBe(2); + expect(messages[0]).toContain('this exact "read" call has been repeated'); + expect(messages[1]).toContain('same set of 2 parallel calls'); + }); + + it('collapses per-call steer messages across a wide repeating round', async () => { + /* + * When per-call counts decide for MANY members at once (a wide fan-out + * repeated, then widened by one call), each member gets its own verdict. + * The steer rung dedupes by exact message text, so per-call messages must + * not embed the individual fingerprint — same tool + same count must be + * byte-identical, or a 20-wide round queues 20 near-identical corrections + * into one injected prompt. The refused call is identified by the block + * output's position and the verdict payload's `fingerprint`; the message + * text does not need to repeat it. + */ + const detector = new DoomLoopMonitor( + resolveDoomLoopOption({ + ladder: { + steer: 2, + }, + }), + ); + const wide = Array.from( + { + length: 20, + }, + (_, index) => `f${index}`, + ); + const messages = new Set(); + let verdictCount = 0; + for (const [round, paths] of [ + wide, + wide, + [ + ...wide, + 'new', + ], + ].entries()) { + await detector.declareRound( + round, + paths.map((path) => ({ + toolName: 'read', + keyMaterial: { + path, + }, + })), + ); + for (const path of paths) { + const record = await detector.recordToolCall( + 'read', + { + path, + }, + round, + ); + if (round === 2 && record.verdict) { + verdictCount++; + messages.add(record.verdict.message); + } + } + } + + /* All 20 repeated members fire; the steer queue sees ONE correction. */ + expect(verdictCount).toBe(20); + expect(messages.size).toBe(1); + }); + + it('persists per-call evidence when the round has shrunk to a single call', async () => { + /* + * When a round SHRINKS (a paused HITL member drops out), the round streak + * resets while the per-call count keeps climbing — per-call evidence is + * then the ONLY evidence, held by a single-call round. The save-time + * omission of `callStreaks` for "plain" single-call rounds must not fire + * here: dropping the count handed the repeat a fresh grace window on + * resume, reaching block a full round later than the in-memory behavior. + */ + const detector = monitor(); + await detector.declareRound(0, [ + { + toolName: 'deploy', + keyMaterial: { + path: 'work', + }, + }, + { + toolName: 'deploy', + keyMaterial: { + path: 'gated', + }, + }, + ]); + await detector.recordToolCall( + 'deploy', + { + path: 'work', + }, + 0, + ); + await detector.recordToolCall( + 'deploy', + { + path: 'gated', + }, + 0, + ); + /* The gated call paused; the resumed round is just the working call. */ + await detector.declareRound(1, [ + { + toolName: 'deploy', + keyMaterial: { + path: 'work', + }, + }, + ]); + const beforeSave = await detector.recordToolCall( + 'deploy', + { + path: 'work', + }, + 1, + ); + expect(beforeSave.streak).toBe(2); + + /* Save/resume mid-loop (approval pause): the count must survive. */ + const wire = JSON.parse(JSON.stringify(detector.getState())) as unknown; + const resumed = new DoomLoopMonitor(resolveDoomLoopOption(true), wire); + await resumed.declareRound(0, [ + { + toolName: 'deploy', + keyMaterial: { + path: 'work', + }, + }, + ]); + const afterResume = await resumed.recordToolCall( + 'deploy', + { + path: 'work', + }, + 0, + ); + expect(afterResume.streak).toBe(3); + expect(afterResume.verdict?.action).toBe('block'); + }); + + it('scores a resumed SINGLE call on its own earned evidence, never inherited', async () => { + /* + * Both the round set and the per-call counts persist, so evidence follows + * whoever EARNED it. A member of the saved fan-out resumed alone continues + * its own count — it genuinely appeared in consecutive rounds, and which + * member it is no longer matters (the original bug attached the whole + * fan-out count to whichever call was recorded last, arbitrarily by + * emission order). A call never recorded before the save inherits nothing. + */ + const detector = monitor(); + const paths = [ + 'a', + 'b', + 'c', + ]; + for (const round of [ + 0, + 1, + ]) { + await detector.declareRound( + round, + paths.map((path) => ({ + toolName: 'read', + keyMaterial: { + path, + }, + })), + ); + for (const path of paths) { + await detector.recordToolCall( + 'read', + { + path, + }, + round, + ); + } + } + /* The streak survives the save, paired with its set and per-call counts. */ + const saved = detector.getState() as { + tools: Record< + string, + { + streak: number; + roundFingerprints?: string[]; + callStreaks?: Record; + } + >; + }; + expect(saved.tools.read.streak).toBe(2); + expect(saved.tools.read.roundFingerprints).toHaveLength(3); + /* + * Steady state — every member's per-call count equals the round streak — + * is exactly the case where `callStreaks` carries no information beyond + * the set, so getState() omits it (persisting it stored each 64-char hash + * twice). restore() rebuilds `{member: streak}` for the whole set; the + * resume assertions below are what actually pin that reconstruction. + */ + expect(saved.tools.read.callStreaks).toBeUndefined(); + + /* + * ANY member resumed alone continues its own earned count (2 -> 3): a + * third consecutive re-read of the same file is a repeat regardless of + * what happened to its former round-mates. Emission order is irrelevant — + * every member carries the same earned evidence. + */ + for (const path of paths) { + const resumed = new DoomLoopMonitor(resolveDoomLoopOption(true), detector.getState()); + await resumed.declareRound(0, [ + { + toolName: 'read', + keyMaterial: { + path, + }, + }, + ]); + const solo = await resumed.recordToolCall( + 'read', + { + path, + }, + 0, + ); + expect(solo.streak).toBe(3); + expect(solo.verdict?.action).toBe('block'); + } + + /* A call never recorded before the save inherits nothing. */ + const resumedFresh = new DoomLoopMonitor(resolveDoomLoopOption(true), detector.getState()); + await resumedFresh.declareRound(0, [ + { + toolName: 'read', + keyMaterial: { + path: 'never-before', + }, + }, + ]); + const fresh = await resumedFresh.recordToolCall( + 'read', + { + path: 'never-before', + }, + 0, + ); + expect(fresh.streak).toBe(1); + expect(fresh.verdict).toBeUndefined(); + }); + + /* + * The steer rung dedupes queued guidance on exact message text, so one round + * of evidence must render ONE string. On the undeclared path (server-tool + * records, direct monitor consumers, the SDK ports) each call is recorded + * alone, so a round's `callSet` holds only that call: the last-recorded call + * tied at `roundStreak == callStreak` and rendered the fingerprint-bearing + * single-call text, while its round-mates rendered the fingerprint-free + * per-call text — two strings for one round, defeating the dedupe. + */ + it('renders one verdict text per undeclared multi-call round', async () => { + const detector = new DoomLoopMonitor(resolveDoomLoopOption(true)); + for (let round = 1; round <= 4; round++) { + const texts: string[] = []; + for (const cmd of [ + 'a', + 'b', + ]) { + const result = await detector.recordToolCall( + 'sh', + { + cmd, + }, + round, + { + allowBlock: false, + }, + ); + if (result.verdict) { + texts.push(result.verdict.message); + } + } + // Either the round produced no verdict yet, or every member of it + // produced byte-identical text. + expect(new Set(texts).size).toBeLessThanOrEqual(1); + } + }); + + /* A DECLARED single-call round still names the argument fingerprint. */ + it('keeps the fingerprint-bearing text for a genuine single-call round', async () => { + const detector = new DoomLoopMonitor(resolveDoomLoopOption(true)); + let last: string | undefined; + for (let round = 1; round <= 3; round++) { + await detector.declareRound(round, [ + { + toolName: 'read', + keyMaterial: { + path: 'same', + }, + }, + ]); + const result = await detector.recordToolCall( + 'read', + { + path: 'same', + }, + round, + ); + last = result.verdict?.message ?? last; + } + expect(last).toContain('identical arguments (fingerprint'); + }); +}); diff --git a/packages/agent/tests/unit/doom-loop-integration.test.ts b/packages/agent/tests/unit/doom-loop-integration.test.ts index fc790e0..b78301a 100644 --- a/packages/agent/tests/unit/doom-loop-integration.test.ts +++ b/packages/agent/tests/unit/doom-loop-integration.test.ts @@ -21,6 +21,7 @@ vi.mock('@openrouter/sdk/funcs/betaResponsesSend', () => ({ })); import { callModel } from '../../src/inner-loop/call-model.js'; +import { resolveLoopKeyMaterial } from '../../src/lib/doom-loop.js'; import { HooksManager } from '../../src/lib/hooks-manager.js'; import type { DoomLoopDetectedPayload } from '../../src/lib/hooks-schemas.js'; import { tool } from '../../src/lib/tool.js'; @@ -107,6 +108,52 @@ function textTurn(text: string): models.OpenResponsesResult { ]); } +/** A turn fanning one tool out over several argument sets in parallel. */ +function fanOutTurn(name: string, argsList: readonly unknown[]): models.OpenResponsesResult { + return baseResponse( + argsList.map((args) => { + callCounter++; + return { + type: 'function_call', + id: `fc_${callCounter}`, + callId: `call_${callCounter}`, + name, + arguments: JSON.stringify(args), + status: 'completed', + }; + }), + ); +} + +/** A turn with two parallel tool calls, for mixed-batch scenarios. */ +function twoToolCallTurn( + first: [ + string, + unknown, + ], + second: [ + string, + unknown, + ], +): models.OpenResponsesResult { + return baseResponse( + [ + first, + second, + ].map(([name, args]) => { + callCounter++; + return { + type: 'function_call', + id: `fc_${callCounter}`, + callId: `call_${callCounter}`, + name, + arguments: JSON.stringify(args), + status: 'completed', + }; + }), + ); +} + /** Queue scripted turns; the "LLM" plays them back in order. */ function scriptModelTurns(...turns: models.OpenResponsesResult[]) { for (const turn of turns) { @@ -187,6 +234,219 @@ beforeEach(() => { // Scenario 1: identical tool calls, turn after turn // --------------------------------------------------------------------------- +describe('simulated LLM repeating a distinct-argument fan-out', () => { + it('escalates observe -> block through callModel and stops the whole fan-out', async () => { + /* + * End to end through the engine, not the monitor: exercises the + * `beginDoomLoopRound` declaration filter chain that must mirror every + * record-skip path (see the INVARIANT comment there). A regression in + * that mirroring degrades fan-out detection silently; only a scripted + * multi-round fan-out through `callModel` catches it loudly. + */ + const executed: string[] = []; + const readTool = tool({ + name: 'read', + inputSchema: z.object({ + path: z.string(), + }), + outputSchema: z.object({ + content: z.string(), + }), + execute: async ({ path }) => { + executed.push(path); + return { + content: `contents of ${path}`, + }; + }, + }); + + const detections: DoomLoopDetectedPayload[] = []; + const hooks = new HooksManager(); + hooks.on('DoomLoopDetected', { + handler: (payload) => { + detections.push(payload); + }, + }); + + /* The same 3-call fan-out, four rounds running, then recovery. */ + const paths = [ + { + path: 'a', + }, + { + path: 'b', + }, + { + path: 'c', + }, + ]; + scriptModelTurns( + fanOutTurn('read', paths), + fanOutTurn('read', paths), + fanOutTurn('read', paths), + fanOutTurn('read', paths), + textTurn('recovered'), + ); + + const text = await callModel(client, { + model: 'test-model', + input: 'Summarize these files.', + tools: [ + readTool, + ] as const, + doomLoop: true, + hooks, + }).getText(); + + /* The run recovers once the model changes course. */ + expect(text).toBe('recovered'); + + /* + * Round 2 observes all three calls; rounds 3-4 block all three. The + * block rung stops the WHOLE fan-out spending: only rounds 1-2 execute. + */ + expect( + detections.map((d) => [ + d.action, + d.streak, + ]), + ).toEqual([ + [ + 'observe', + 2, + ], + [ + 'observe', + 2, + ], + [ + 'observe', + 2, + ], + [ + 'block', + 3, + ], + [ + 'block', + 3, + ], + [ + 'block', + 3, + ], + [ + 'block', + 4, + ], + [ + 'block', + 4, + ], + [ + 'block', + 4, + ], + ]); + expect(executed).toEqual([ + 'a', + 'b', + 'c', + 'a', + 'b', + 'c', + ]); + + /* + * The verdicts must come from the ROUND detector, not merely per-call + * counts. For an exactly-repeating fan-out both detectors produce the + * same actions and streaks, so a corrupted declaration (a phantom member, + * or a member wrongly filtered out) is invisible to the assertions above + * — the per-call counts mask it. The message form is the discriminator: + * round verdicts name the set ("same set of 3 parallel calls"), per-call + * verdicts name a single repeated call. Asserting the round form pins the + * `beginDoomLoopRound` declaration end to end. + */ + for (const detection of detections) { + expect(detection.message).toContain('same set of 3 parallel calls'); + } + }); +}); + +describe('simulated LLM emitting duplicate call ids', () => { + it('a repeat sharing its id with a varying sibling is still detected', async () => { + /* + * Call ids are model-emitted; nothing upstream enforces uniqueness. The + * loop-key cache is keyed by id, and last-write-wins aliasing meant the + * first call's checkpoint read the SECOND call's key material — its true + * fingerprint was never recorded, so `(id=X, read a), (id=X, read b_i)` + * each round evaded the per-call detector for `a` entirely (measured: + * zero detections in four rounds). A colliding id now poisons its cache + * entry and both calls fall through to per-call resolution. + */ + const readTool = tool({ + name: 'read', + inputSchema: z.object({ + path: z.string(), + }), + outputSchema: z.object({ + content: z.string(), + }), + execute: async ({ path }) => ({ + content: path, + }), + }); + const detections: DoomLoopDetectedPayload[] = []; + const hooks = new HooksManager(); + hooks.on('DoomLoopDetected', { + handler: (payload) => { + detections.push(payload); + }, + }); + + /* Every round: repeated `a` and a fresh `b`, BOTH with callId X. */ + const dupIdTurn = (round: number): models.OpenResponsesResult => + baseResponse( + [ + { + path: 'a', + }, + { + path: `b${round}`, + }, + ].map((args, index) => { + callCounter++; + return { + type: 'function_call', + id: `fc_${callCounter}_${index}`, + callId: 'call_X', + name: 'read', + arguments: JSON.stringify(args), + status: 'completed', + }; + }), + ); + scriptModelTurns(dupIdTurn(0), dupIdTurn(1), dupIdTurn(2), dupIdTurn(3), textTurn('done')); + + await callModel(client, { + model: 'test-model', + input: 'go', + tools: [ + readTool, + ] as const, + doomLoop: true, + hooks, + }) + .getText() + .catch(() => undefined); + + /* The repeated `a` accumulates despite the id collision. */ + const actions = detections.map((detection) => detection.action); + expect(actions).toContain('observe'); + expect(actions).toContain('block'); + }); +}); + describe('simulated LLM repeating the same tool call', () => { it('observes at 2, blocks at 3 with an explanatory tool error, and lets the model recover', async () => { const executeSpy = vi.fn(async ({ query }: { query: string }) => ({ @@ -876,6 +1136,60 @@ describe('tool-declared loopKey', () => { warn.mockRestore(); } }); + + it('resolveLoopKeyMaterial can throw, so its callers must guard it', async () => { + /* + * `resolveLoopKeyMaterial` catches a throwing `loopKey`, but it can still + * throw on its own: the field-list form does `field in args` and + * `args[field]`, so a getter or proxy trap on the arguments object escapes + * it uncaught. Pinned here because `beginDoomLoopRound` resolves the WHOLE + * batch up front — an unguarded throw there fails the round and the run + * over one odd call, instead of costing detection for that call alone, + * which would break the invariant that detection only ever affects a run + * through a ladder action. + * + * NOT reachable through `callModel` today: tool arguments come from + * `JSON.parse` (stream-transformers.ts), so they are always plain objects, + * and `PreToolUse` argument mutation happens after the round is declared. + * It IS reachable for direct callers — `resolveLoopKeyMaterial` and + * `DoomLoopMonitor` are both exported — and for SDK ports that construct + * key material differently. Both call sites are guarded regardless; this + * test pins the throw itself so a future refactor cannot quietly remove + * the need for those guards. + */ + const throwingGetter = {} as Record; + Object.defineProperty(throwingGetter, 'command', { + enumerable: true, + get() { + throw new Error('getter boom'); + }, + }); + expect(() => + resolveLoopKeyMaterial( + [ + 'command', + ], + throwingGetter, + ), + ).toThrow('getter boom'); + + const hostileProxy = new Proxy( + {}, + { + has() { + throw new Error('has boom'); + }, + }, + ) as Record; + expect(() => + resolveLoopKeyMaterial( + [ + 'command', + ], + hostileProxy, + ), + ).toThrow('has boom'); + }); }); // --------------------------------------------------------------------------- @@ -1069,4 +1383,128 @@ describe('doom-loop state persistence', () => { const roundTripped = JSON.parse(JSON.stringify(state)) as ConversationState; expect(roundTripped.doomLoop).toEqual(state.doomLoop); }); + + it('invokes a tool-supplied loopKey exactly once per call', async () => { + /* + * `loopKey` is user code: it may count, log, or return a fresh value each + * time. Declaring a round's identity up front resolves it once for the + * batch, and the per-call checkpoint must reuse that resolution rather + * than invoking the callback a second time — otherwise a callback with + * side effects sees double the activity, and a non-repeatable one makes + * the declared and recorded identities disagree. + */ + const loopKeySpy = vi.fn(({ query }: { query: string }) => query.trim().toLowerCase()); + const countedSearch = tool({ + name: 'web_search', + inputSchema: z.object({ + query: z.string(), + }), + outputSchema: z.object({ + results: z.array(z.string()), + }), + loopKey: loopKeySpy, + execute: async ({ query }) => ({ + results: [ + `result for ${query}`, + ], + }), + }); + + scriptModelTurns( + toolCallTurn('web_search', { + query: 'first', + }), + toolCallTurn('web_search', { + query: 'second', + }), + textTurn('Done.'), + ); + + await callModel(client, { + model: 'test-model', + input: 'Search twice.', + tools: [ + countedSearch, + ] as const, + doomLoop: true, + }).getText(); + + /* Two calls issued, so exactly two invocations — not four. */ + expect(loopKeySpy).toHaveBeenCalledTimes(2); + }); + + it('never invokes loopKey for a manual call the detector will not check', async () => { + /* + * A manual tool (no `execute`, no `onToolCalled`) is handed to the caller + * to resolve externally and is never recorded as loop evidence. Declaring + * a round must therefore skip it: `loopKey` is user code, and running it + * for a call the detector never evaluates is activity the caller cannot + * account for. Every execution path skips these with the same + * `isAutoResolvableTool` predicate the declaration now uses. + */ + const manualLoopKey = vi.fn(({ query }: { query: string }) => query); + const manualTool = tool({ + name: 'ask_human', + inputSchema: z.object({ + query: z.string(), + }), + outputSchema: z.object({ + answer: z.string(), + }), + loopKey: manualLoopKey, + }); + const executableLoopKey = vi.fn(({ query }: { query: string }) => query); + const executableTool = tool({ + name: 'web_search', + inputSchema: z.object({ + query: z.string(), + }), + outputSchema: z.object({ + results: z.array(z.string()), + }), + loopKey: executableLoopKey, + execute: async ({ query }) => ({ + results: [ + query, + ], + }), + }); + + /* + * MIXED batch: an all-manual round never reaches the doom-loop round at + * all (`hasExecutableToolCalls` guards it), so the manual call has to ride + * alongside an executable one for the declaration to be reached. + */ + scriptModelTurns( + twoToolCallTurn( + [ + 'web_search', + { + query: 'go', + }, + ], + [ + 'ask_human', + { + query: 'what now?', + }, + ], + ), + ); + + await callModel(client, { + model: 'test-model', + input: 'Search, and ask me something.', + tools: [ + executableTool, + manualTool, + ] as const, + doomLoop: true, + }).getText(); + + /* The executable call is checked, so its loopKey runs exactly once. */ + expect(executableLoopKey).toHaveBeenCalledTimes(1); + /* The manual call is never recorded, so its loopKey must not run at all. */ + expect(manualLoopKey).not.toHaveBeenCalled(); + }); }); diff --git a/packages/agent/tests/unit/doom-loop-public-api.test.ts b/packages/agent/tests/unit/doom-loop-public-api.test.ts new file mode 100644 index 0000000..afb1f07 --- /dev/null +++ b/packages/agent/tests/unit/doom-loop-public-api.test.ts @@ -0,0 +1,304 @@ +/** + * Consumer-facing contract for driving `DoomLoopMonitor` directly: everything + * here imports from the package entrypoint ONLY, exactly as an npm consumer + * or an SDK port would. `DoomLoopMonitor` was exported without + * `resolveDoomLoopOption`, so the documented construction failed at runtime + * and the class was unusable outside `callModel` — the changeset example was + * only caught because it was executed rather than eyeballed. + */ +import { describe, expect, it } from 'vitest'; + +import { + DEFAULT_DOOM_LOOP_LADDER, + DoomLoopMonitor, + resolveDoomLoopOption, +} from '../../src/index.js'; + +describe('DoomLoopMonitor via the public entrypoint', () => { + it('constructs with defaults, detects a repeating fan-out, and honors the exported ladder', async () => { + const monitor = new DoomLoopMonitor(resolveDoomLoopOption(true)); + const paths = [ + 'a', + 'b', + 'c', + ]; + + let verdictAction: string | undefined; + let verdictStreak: number | undefined; + for (const round of [ + 0, + 1, + 2, + ]) { + await monitor.declareRound( + round, + paths.map((path) => ({ + toolName: 'read', + keyMaterial: { + path, + }, + })), + ); + for (const path of paths) { + const record = await monitor.recordToolCall( + 'read', + { + path, + }, + round, + ); + if (record.verdict) { + verdictAction = record.verdict.action; + verdictStreak = record.verdict.streak; + } + } + } + + /* Third identical round crosses the exported default block threshold. */ + expect(verdictStreak).toBe(DEFAULT_DOOM_LOOP_LADDER.block); + expect(verdictAction).toBe('block'); + }); + + it('accepts a config object and honors a custom ladder', async () => { + const monitor = new DoomLoopMonitor( + resolveDoomLoopOption({ + ladder: { + observe: 2, + block: false, + stop: 4, + }, + }), + ); + + let last: string | undefined; + for (const round of [ + 0, + 1, + 2, + 3, + ]) { + const record = await monitor.recordToolCall( + 'search', + { + q: 'same', + }, + round, + ); + last = record.verdict?.action; + } + /* block disabled; the streak of 4 reaches the custom stop rung. */ + expect(last).toBe('stop'); + }); + + it('accumulates a fan-out streak across per-turn process boundaries', async () => { + /* + * The serverless pattern: one round per callModel run, state persisted + * between turns. A repeating fan-out must accumulate across those + * boundaries — this is why the round's fingerprint SET is persisted. + * Before that, each resume re-baselined the fan-out and a loop that + * repeated once per turn never tripped anything. + */ + const paths = [ + 'a', + 'b', + 'c', + ]; + let wire: unknown; + const perTurn: string[] = []; + for (let turn = 0; turn < 4; turn++) { + const monitor = new DoomLoopMonitor(resolveDoomLoopOption(true), wire); + await monitor.declareRound( + 0, + paths.map((path) => ({ + toolName: 'read', + keyMaterial: { + path, + }, + })), + ); + let last = ''; + for (const path of paths) { + const record = await monitor.recordToolCall( + 'read', + { + path, + }, + 0, + ); + last = `${record.streak}:${record.verdict?.action ?? 'none'}`; + } + perTurn.push(last); + wire = JSON.parse(JSON.stringify(monitor.getState())); + } + + expect(perTurn).toEqual([ + '1:none', + '2:observe', + '3:block', + '4:block', + ]); + }); + + it('restores a legacy blob (no roundFingerprints) with single-call semantics', async () => { + /* + * Pre-existing persisted state has no `roundFingerprints` field. It must + * restore exactly as before: the lone fingerprint describes the round, a + * different call resets to 1, the same call continues. Malformed sets + * (non-string entries) degrade the same way instead of being dropped. + */ + const legacy = { + tools: { + read: { + fingerprint: 'not-a-real-fingerprint', + streak: 2, + }, + }, + }; + const monitor = new DoomLoopMonitor(resolveDoomLoopOption(true), legacy); + const different = await monitor.recordToolCall( + 'read', + { + path: 'x', + }, + 0, + ); + expect(different.streak).toBe(1); + + const hostile = { + tools: { + read: { + fingerprint: 'ab', + streak: 2, + roundFingerprints: [ + 1, + {}, + null, + ], + }, + }, + }; + const survives = new DoomLoopMonitor(resolveDoomLoopOption(true), hostile); + const record = await survives.recordToolCall( + 'read', + { + path: 'x', + }, + 0, + ); + expect(record.streak).toBe(1); + }); + + it('isolates the live detector from mutations of a saved snapshot', async () => { + /* + * `getState()` snapshots are handed to the caller's StateAccessor, and the + * multi-call round set used to be the live array — shared with the running + * streak entry and the round declaration. A caller that pushed into (or + * sorted, or spliced) the saved blob silently corrupted the detector for + * the rest of the run: the next identical fan-out compared against the + * mutated set, never matched, and scored 1 instead of block. + */ + const paths = [ + 'a', + 'b', + ]; + const monitor = new DoomLoopMonitor(resolveDoomLoopOption(true)); + for (const round of [ + 0, + 1, + ]) { + await monitor.declareRound( + round, + paths.map((path) => ({ + toolName: 'read', + keyMaterial: { + path, + }, + })), + ); + for (const path of paths) { + await monitor.recordToolCall( + 'read', + { + path, + }, + round, + ); + } + } + + /* A careless (or hostile) caller mutates the saved snapshot in place. */ + const saved = monitor.getState() as { + tools: Record< + string, + { + roundFingerprints?: string[]; + } + >; + }; + saved.tools.read.roundFingerprints?.push('INJECTED'); + + /* The live detector must be unaffected: round 3 still blocks. */ + await monitor.declareRound( + 2, + paths.map((path) => ({ + toolName: 'read', + keyMaterial: { + path, + }, + })), + ); + let last: { + streak: number; + action?: string; + } = { + streak: 0, + }; + for (const path of paths) { + const record = await monitor.recordToolCall( + 'read', + { + path, + }, + 2, + ); + last = { + streak: record.streak, + action: record.verdict?.action, + }; + } + expect(last.streak).toBe(3); + expect(last.action).toBe('block'); + }); + + it('round-trips state across a process boundary via plain JSON', async () => { + const first = new DoomLoopMonitor(resolveDoomLoopOption(true)); + await first.recordToolCall( + 'search', + { + q: 'same', + }, + 0, + ); + await first.recordToolCall( + 'search', + { + q: 'same', + }, + 1, + ); + + /* Simulate the serverless pattern: serialize, "new process", restore. */ + const wire = JSON.stringify(first.getState()); + const second = new DoomLoopMonitor(resolveDoomLoopOption(true), JSON.parse(wire)); + const resumed = await second.recordToolCall( + 'search', + { + q: 'same', + }, + 0, + ); + + /* Single-call streak continues across the boundary: 2 -> 3. */ + expect(resumed.streak).toBe(3); + }); +});