feat(agent): doom-loop detection for the tool-execution loop

[LukasParke]

Summary

Doom-loop detection baked into the core tool-execution loop, opt-in and configurable via a new doomLoop option on callModel. Design doc: agent-monorepo/planning/doom-loop-detector-design.md; this PR ships the shared machinery + the fingerprint detector (Option A) + the text detectors (Option B) + server-tool coverage, remediated against a 3-reviewer adversarial pass (agent-monorepo/planning/doom-loop-remediation-plan.md — all blockers and defects addressed; commit 2 is the remediation).

A doom loop = a run that stops making progress while continuing to spend. Detected shapes:

1. Identical tool calls across rounds — same tool, same identity, round after round; includes repeated empty {} calls and repeated invalid-JSON calls (which never reach execute).
2. Identical server-tool requests (web_search_call etc.) — detected post-execution at the step checkpoint.
3. Doom-loop text tokens — a token block repeating at the tail of one response, or byte-identical assistant text across consecutive steps.

How it works

Tool-declared call identity (loopKey — a function or a variable)

Tools declare what identifies a call on their definition; the engine owns canonicalization + hashing:

// function: compute the identity
tool({ ..., loopKey: ({ query }) => query.trim().toLowerCase() })

// variable (field list): declarative subset — data, not code, survives tool caches / MCP wire
tool({ ..., loopKey: ['command', 'cwd'] })

// variable (false): statically exempt (polling tools)
tool({ ..., loopKey: false })

Function form may return null to exempt an individual call. undefined, throws, and unhashable material (bigint, circular, >64 deep) fall back to the full-arguments identity with a warning — detection never fails a run. No loopKey ⇒ full validated arguments. MCP-wrapped tools: markMcp(tool, { loopKey }).

Cross-port fingerprints (the contract is now real)

sha256(utf8(toolName + '\n' + jcs(keyMaterial))) — RFC 8785 (JCS) canonicalization, SHA-256 over UTF-8 bytes via WebCrypto (Node ≥18/Bun/Deno/workers/browsers, zero dependencies). Ports MUST use an RFC 8785 implementation (pip jcs, cyberphone/json-canonicalization), not stdlib JSON. Conformance vectors: tests/vectors/doom-loop-fingerprints.json (non-ASCII/emoji, -0, 1e21, key-order permutations, lone surrogates, rejected-input catalog).

Round-scoped streaks

A streak measures the model re-issuing a call after seeing its result — one model round trip = one piece of evidence. N identical calls fanned out in parallel within one round count once and share that round's decision (fixes the review's headline false positive). Evaluations are serialized in model-emission order under the async hash, so verdicts remain a pure function of the transcript. Interleaved calls to other tools don't reset a tool's streak; a different fingerprint for the same tool does.

Graduated ladder (defaults observe@2, block@3, stop@6)

Action	Mechanism
observe	Emit built-in DoomLoopDetected hook (per-event overrideAction, last handler wins)
steer	Inject corrective user message before the next request; guidance queued before a pause persists (doomLoop.pendingSteer) and delivers on resume
block	Refuse the call before execution with an explanatory error output; not applicable to text/server-tool verdicts (downgrade to observe)
stop	Halt before any further model request. Seals state: unresolved calls in the final turn get synthesized halt-error outputs, so persisted history never carries a dangling function_call (no 400s on resume). SessionEnd.reason: 'doom_loop' on every stop path, including text-only no-tools runs. Gates the allowFinalResponse turn and the approval-resume unsent-results request

Resolve-time config warnings: dead rungs (weaker threshold ≥ enabled stronger one) and block with stop: false (unbounded block/re-issue, bounded only by stopWhen).

Persistence semantics

Detector state lives in ConversationState.doomLoop (plain bounded JSON): streaks survive serialize → resume when the resuming call passes doomLoop again; a stop verdict survives decision-only resumes (approveToolCalls/rejectToolCalls) and is cleared by a fresh conversational turn (streaks kept — renewed repetition re-condemns quickly); queued steer guidance survives pauses.

Documented, test-locked limits

• Varying-input loops (model invents a nonce/timestamp field) evade the default whole-args identity — a loopKey closes this per tool; the structural fix is the design doc's Option C progress ledger (follow-up PR-C). A negative test locks the miss so it's visible, not silent.
• Paraphrased repetition does not trip the text detectors (exact token blocks / byte-identical normalized text required). Negative test included.
• Fingerprints are computed pre-PreToolUse-mutation (model repetition is the evidence; a nonce-injecting hook can't mask it — and a normalizing hook can't create repeats).
• Manual/client-executed calls pause the loop and are not recorded.

Tests (all simulate the LLM — no live model)

93 doom-loop tests across three files; scripted betaResponsesSend responses drive the real callModel loop; verdicts assert exact firing turns.

• doom-loop.test.ts (57): JCS canonicalization (key order, -0, 1e21, undefined-dropping, bigint/NaN/circular/depth rejection), cross-port vector conformance, loopKey resolution (all four forms + undefined/throw fallbacks), text repetition (incl. paraphrase-miss and multi-MB perf-bound input), ladder + config warnings (dead rungs, block-without-stop), round-scoped monitor semantics (parallel duplicates, interleave, resume round-trip, corrupt-blob tolerance), server-tool detector labeling.
• doom-loop-integration.test.ts (16): the original scenario matrix — identical calls (observe@2/block@3, stop halts at the exact turn, SessionEnd.reason: 'doom_loop'), repeated empty calls, repeated invalid JSON, degenerate text, loopKey contracts, hook overrides (de-escalate/escalate), state persistence.
• doom-loop-remediation.test.ts (20, new): review-finding regression suite — B3 parallel fan-out executes fully with zero detections / blocked duplicates share one decision; B1 text-stop with tool calls leaves well-formed history + resumed request input validated; D7 text-only stop reports doom_loop; D1 no final-response request after a stop; D2/D3 approval-resume gating, verdict persistence across decision resumes, clearing on fresh input; D4 in-loop steer injection + steer persisted at HITL pause; D5/API1 field-list identity, false exemption, undefined-fallback; H1 nonce evasion locked as a documented miss + caught with a field-list loopKey; H2 server-tool streaks stop the run / distinct queries don't; ON→OFF/OFF→ON config lifecycle.

Verification

• pnpm test: 49 files, 614 passed
• pnpm typecheck, pnpm lint, pnpm build: clean

Surface changes

• callModel option doomLoop?: boolean | DoomLoopConfig (client-only; stripped from API requests on both resolve paths)
• tool() config field loopKey?: ToolLoopKey<TInput> = function | readonly string[] | false (all tool kinds); markMcp(tool, { loopKey? })
• New built-in hook DoomLoopDetected; SessionEnd.reason + 'doom_loop'
• ConversationState.doomLoop?: DoomLoopSerializedState (streaks + stopVerdict + pendingSteer; additive within state v1)
• ModelResult.getDoomLoopVerdict(); @openrouter/agent/doom-loop subpath (DoomLoopMonitor, fingerprintToolCall, canonicalizeKeyMaterial, resolveLoopKeyMaterial, detectTextRepetition, …)
• Cross-port vector file tests/vectors/doom-loop-fingerprints.json
• Changeset: minor

Follow-ups (tracked in the design doc)

• PR-C: Option C progress ledger (outcome hashing — the structural fix for varying-input loops) + loopOutputKey
• @openrouter/mcp stacked PR: per-tool loopKeys map in the wrapper config + _meta['openrouter/loopKey'] field-list transport (the markMcp injection point ships here)
• Port sync (Python/Go) against the vector file once TS settles

Escalation recovery (added on team request)

New escalate ladder rung between steer and block: on detection, unblock the run by throwing more intelligence at the next turn instead of refusing or halting — then revert.

doomLoop: {
  ladder: { observe: 2, escalate: 3, block: 5, stop: 8 },
  escalation: {
    model: 'anthropic/claude-opus-4.6', // one-turn model swap, automatic revert
    advisor: true,                      // and/or force an openrouter:advisor consult
    maxEscalations: 2,                  // spend cap per conversation
  },
}

• Model swap applies to the escalated dispatch only (the base request is never mutated → automatic revert; fallback models list cleared for that turn so the override can't be shadowed).
• Advisor forcing appends the openrouter:advisor server tool with forwardTranscript: true + loop-diagnosing instructions, and pins toolChoice (allowed_tools/required) so the stuck model must consult before acting. Object form passes through as advisor parameters.
• A steer notice naming the detected loop accompanies the escalated turn.
• Budgeted: maxEscalations (default 2), consumed at application time, escalationsUsed persisted in ConversationState.doomLoop (resumes can't reset it), first-verdict latch (concurrent detectors escalate once). Exhausted/unconfigured → falls through to weaker rungs; resolve-time warnings flag rung/mechanism mismatches.
• DoomLoopDetected action/overrideAction enums gain 'escalate'; an override without config/budget downgrades to observe.

15 new tests (doom-loop-escalation.test.ts): config resolution + warnings, ladder placement and budget fall-through, one-turn swap + revert asserted per dispatch, advisor injection/pinning/revert, parameter merging, steer notice, application-time budget consumption + persistence, hook-override honor/downgrade, text-detector escalation, double-spend prevention. Totals: 638 unit tests across 51 files.

[devin-ai-integration]

Devin Review found 0 potential issues.

[devin-ai-integration]

Devin Review found 0 new potential issues.

[cortex-github-agent]

Summary

This update's only functional delta versus the prior reviewed revision is a cosmetic refactor in call-model.ts (building engineOptions via a plain object + a delete-if-undefined loop instead of per-key conditional spreads) — behaviorally equivalent, no new risk. All doom-loop logic (doom-loop.ts, model-result.ts doom-loop wiring) is byte-identical to what I reviewed last round, so the one outstanding major finding from that review is still unresolved. Everything else (round-scoped streaks, cross-port fingerprint contract, seal-on-stop, steer/condemnation persistence, cancellation/timeout composition) remains as previously assessed: well-designed and heavily tested.

Findings (2)

🟠 major · packages/agent/src/lib/doom-loop.ts:600-650
OPEN (unresolved from prior round): DoomLoopMonitor.restore() builds tools: Record<string, StreakEntry> = {} (a plain object with the default Object.prototype) and assigns tools[name] = {...} for every key in the untrusted persisted blob after only checking isValidStreak(entry). A persisted ConversationState.doomLoop containing {"tools":{"__proto__":{"fingerprint":"x","streak":999}}} passes that check and pollutes the object's prototype via the bracket-notation setter, so this.tools[anyUnseenToolName] subsequently inherits the attacker-seeded streak instead of returning undefined — corrupting streak bookkeeping (false-positive stop / detection bypass) for every tool once that state round-trips through a resume. This code region is unchanged since the previous review pass (the remediation commit did not touch it); still needs a name === '__proto__' / 'constructor' / 'prototype' guard or Object.create(null).

⚪ nit · packages/agent/src/inner-loop/call-model.ts:140-175
Refactor from per-key conditional spreads to building engineOptions as a mutable record and deleting undefined-valued keys is functionally equivalent (verified against GetResponseOptions consumers, which all use optional chaining / !== undefined checks, not in/hasOwnProperty). No behavior change; no action needed.

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[devin-ai-integration]

Devin Review found 0 new potential issues.

[devin-ai-integration]

Devin Review found 0 new potential issues.

[devin-ai-integration]

Devin Review found 0 new potential issues.

[LukasParke]

Both outstanding findings are resolved as of 165fcb7:

• Prototype pollution in restore() — streak store is now Map<string, StreakEntry> (4860771); getState() emits via Object.fromEntries. Regression tests cover the crafted __proto__ blob and a legitimately-named __proto__ tool (doom-loop.test.ts:938, :967).
• loopKey subset __proto__ field drop — resolveLoopKeyMaterial builds the subset with Object.create(null) (165fcb7), so declared __proto__ fields are preserved as own properties.

All 111 doom-loop tests pass locally. Re-requesting review to clear the stale CHANGES_REQUESTED state.

Both findings fixed: tools map is now Map<string, StreakEntry> (4860771) and resolveLoopKeyMaterial subset uses Object.create(null) (165fcb7); regression tests at doom-loop.test.ts:938/:967.

[devin-ai-integration]

Devin Review found 0 new potential issues.

[devin-ai-integration]

Devin's fresh review (head 1b48987)

Verdict: LGTM — approve. Full re-review of the diff at head, since the prior review was dismissed.

Prior findings — verified fixed at head

• Prototype pollution in DoomLoopMonitor.restore(): streak store is Map<string, StreakEntry> (doom-loop.ts:803), snapshot via Object.fromEntries (:847). Regression tests cover both the crafted __proto__ blob and a legitimately-named __proto__ tool.
• loopKey field-list __proto__ drop: resolveLoopKeyMaterial builds the subset on Object.create(null) (doom-loop.ts:455), so declared __proto__ fields survive as own data properties.

Verification (local, at 1b48987)

• pnpm install / pnpm run typecheck / pnpm run lint: clean (Biome, both workspaces).
• pnpm --filter @openrouter/agent test: 51 files, 639 tests passed; the 4 doom-loop suites (111 tests) pass in isolation too.

Design points I checked and found sound

• Signal composition in dispatchRequestOptions: verified against the SDK's merge semantics — createRequest does {...options.fetchOptions, ...options}, so the composed top-level signal correctly wins over a caller fetchOptions.signal, and setting a signal disables the SDK's own timeout wiring, which the method faithfully recreates per dispatch (AbortSignal.timeout is correctly not cached). client._options is a public readonly property, so the timeout fallback is a supported access.
• Round scoping: server-tool evaluations recorded post-response reuse the previous batch's round counter, but each loop iteration is preceded by a beginDoomLoopRound(), so consecutive identical server-tool requests get distinct rounds and streaks increment as intended; the server: name prefix prevents decision-map collisions with client tools.
• Ordering determinism: the doomLoopChain promise chain serializes parallel-call evaluations in model-emission order (the .map() runs synchronously to its first await), so verdicts remain a pure function of the transcript despite async hashing.
• Stop sealing: every stop site (pre-round, post-round, no-tools, final-response gate, approval-resume gate) synthesizes halt outputs for unresolved function_calls, so persisted history can't 400 on resume.
• Escalation budget: consumed at application time, persisted via escalationsUsed, first-verdict latch prevents double-spend — matches the description.

Minor, non-blocking observations

1. Alternating-fingerprint loops evade detection: a model ping-ponging A, B, A, B, … on one tool resets the streak to 1 each round (recordToolCall: a different fingerprint resets). Same family as the documented varying-input limit; the design doc's Option C progress ledger (PR-C) is the right structural home, but worth a line in the "documented limits" list.
2. markMcp(tool, { loopKey }) silently ignores loopKey for non-client tools (tool.ts: the isClientTool(marked) guard). A dev-time warning would make the no-op visible, though the stacked MCP PR may make this moot.
3. takeDoomLoopEscalationOverrides sets overrides['models'] = undefined to clear the fallback list — works because the SDK treats present-as-undefined as absent (and tests lock it), but an explicit delete after spread would be more self-evidently correct.

None of these block merge. Nice work on the remediation-test discipline — the review-finding regression suite (doom-loop-remediation.test.ts) locking each prior finding is exactly the right pattern.

[perry-the-pr-reviewer]

Perry's Review

Verdict: 💬 Comments / questions
Risk: 🟡 Medium

Details

Fresh review on commit 1b48987 (previous Perry review was DISMISSED on 1723d17). The prior blocker (prototype pollution in DoomLoopMonitor.restore()) and the prior nit (__proto__ setter in resolveLoopKeyMaterial's field-list subset) are both resolved. The only new change since the last review is the fix commit 165fcb7 (applying Object.create(null) to the subset) plus two merge-from-main commits that brought in publish.yaml/changeset/CHANGELOG updates from already-merged PRs.

Previous blocker — resolved

Prototype pollution via __proto__ in DoomLoopMonitor.restore() — fixed in 4860771. The streak store is now Map<string, StreakEntry>; hostile keys from persisted JSON are inert map entries. getState() emits via Object.fromEntries (own properties, safe). A regression test locks the attack case and a legitimate __proto__-named tool (doom-loop.test.ts:919).

Previous nit — resolved

resolveLoopKeyMaterial field-list subset — fixed in 165fcb7. The subset is now Object.create(null), so the __proto__ setter is bypassed. A test verifies a declared __proto__ field is preserved as an own property with a null prototype.

One consistency nit (non-blocking)

extractServerToolIdentity (model-result.ts:128) still builds its identity object as a plain {}. If a server-tool output item carries a __proto__ key (possible from JSON-parsed API responses), identity['__proto__'] = value hits the prototype setter — the same pattern the resolveLoopKeyMaterial fix just closed. The impact is minimal (the object is temporary; canonicalizeKeyMaterial uses Object.keys which skips non-own properties, so the fingerprint is just slightly wrong, not a security hole), but Object.create(null) here would make the codebase consistent and remove the edge case entirely. Inline comment posted.

Assessment

• Architecture: well-designed throughout. Deterministic detection (pure function of the transcript), round-scoped streaks (parallel duplicates count once), graduated ladder with hook overrides, cross-port fingerprint contract (RFC 8785 JCS + SHA-256 via WebCrypto with conformance vectors), and clean state persistence semantics (streaks survive resume, stop verdict survives decision-only resumes and clears on fresh input).
• Escalation recovery: one-turn model swap (resolvedRequest never mutated, so revert is automatic), advisor forcing with pinned toolChoice, budget consumed at application time and persisted. Thoroughly tested (15 tests).
• Run-level cancellation: properly composes run signal + caller signal + per-request timeout via AbortSignal.any per dispatch. Fresh timeout per request (not cached). Pre-aborted signal fails before network I/O. E2E tests validate against a live transport.
• Test coverage: 93+ doom-loop tests, 15 escalation tests, cancellation tests (unit + e2e), integration and remediation suites. All use scripted LLM responses driving the real callModel loop with exact-turn assertions. CI is green (638 unit tests, 51 files).
• All features are opt-in (doomLoop defaults to off; signal defaults to off). No behavior change for existing callers.
• .sentrux/baseline.json: updated in the same PR (max_depth 8→11, import edges 133→306). Process nit from the previous review still applies — the structural gate runs against this new baseline so it won't flag the complexity increase the PR introduces. Ideally the baseline would be updated separately or by the gate's own post-merge hook.

[perry-the-pr-reviewer]

▶ Nit — use Object.create(null) for consistency.

extractServerToolIdentity builds identity as a plain {}. If a server-tool output item carries a __proto__ key (possible from JSON-parsed API responses), identity['__proto__'] = value hits the prototype setter — the same pattern resolveLoopKeyMaterial just fixed in 165fcb7.

Impact is minimal (the object is temporary; canonicalizeKeyMaterial uses Object.keys which skips non-own properties, so the fingerprint is just slightly wrong, not a security hole). But Object.create(null) here would make the codebase consistent and remove the edge case.

Suggested change

	const identity: Record<string, unknown> = {};
	const identity: Record<string, unknown> = Object.create(null) as Record<string, unknown>;

[devin-ai-integration]

Re-review of b686e7a

LGTM. The reclassification from "consistency nit" to "false-positive source" is correct, and I verified it empirically rather than by reading — including that the new test is non-vacuous.

The mechanism, confirmed

Reverting just the one line to {} and rerunning the new test:

AssertionError: expected [ { …(6) }, { …(6) } ] to deeply equal []
+ "fingerprint": "d7b9e650f44d43d406a8897d6f74bcecb772a35c14c0ca7f371425b2b9756e35"
+ "streak": 2
+ "streak": 3

Three distinct server-tool requests collapsing onto one fingerprint and climbing a streak to 3 — with block@3 that is a healthy run being condemned, not a fingerprint that is "slightly wrong". A dropped identity field is a hash collision across distinct inputs, and collision is precisely the doom-loop detector's false-positive mechanism; "temporary object, Object.keys skips non-own properties" describes the cause accurately and then stops one inference short of the consequence.

Where the earlier severity call still holds: reachability is essentially nil in practice. It needs a server-tool output item carrying a literal __proto__ key, which no provider API emits — so this is a correctness-invariant fix, not a live bug. Both readings were partly right; the class was misjudged, the urgency wasn't.

The same revert-and-rerun check on 165fcb7's field-list subset also fails as expected (keyMaterial: {} vs { __proto__: 'declared' }), so neither test is vacuous.

The one thing I went looking for and want to flag as a latent invariant

The default identity path ("no loopKey ⇒ full arguments") is immune — but by placement, not by construction. The checkpoint fingerprints the raw JSON-parsed toolCall.arguments (model-result.ts:1164-1171 runs before execution; :1458-1463 passes toolCall.arguments through), while Zod validation happens later in tool-executor.ts:214-219. That ordering is load-bearing, because the Zod output object has exactly the bug this commit just fixed:

$ node -e 'z.object({}).passthrough().parse(JSON.parse("{\"__proto__\":\"x\"}"))'
{"own":false,"keys":[],"proto":true,"value":{}}

A passthrough/z.record tool's __proto__ field survives JSON.parse and is swallowed by Zod's plain-{} accumulator. I confirmed with a throwaway test that three calls differing only in that field produce no detections today. But if fingerprinting ever moves to validated arguments, the collision class returns through the Zod layer instead. Worth one line in the doom-loop docstring stating that key material must be taken pre-validation — the reason is non-obvious and the failure is silent.

Rest of the sweep

• Dropping type from the identity is right, not a collision: it's carried in the tool name (server:${item.type}).
• The extractor is deny-list-only over Object.entries (no primitive-only filter), so object-valued fields do participate — no other silent-drop path. The docstring's "every primitive/JSON field" understates what the code does.
• Function-form and false-form loopKey build no engine-side accumulator from untrusted keys; the remaining plain-{} accumulators (tool-context.ts:118, model-result.ts:1605, tool-executor.ts:63, next-turn-params.ts:178) are all off the fingerprint path.

Verification at b686e7a

pnpm run lint and pnpm run typecheck clean (both workspaces); pnpm --filter @openrouter/agent test → 51 files, 640 tests passed.

[devin-ai-integration]

Devin Review found 3 new potential issues.

[devin-ai-integration]

Re-review at 68eb0bc

Verified locally at this head: pnpm typecheck, pnpm lint clean; full suite 682 tests across 60 files (643 agent + 39 mcp).

advisor: false fix — correct, and the tests are non-vacuous

advisor is boolean | Record<string, unknown>, so false is the documented opt-out, and the recovery step already skipped it (config.advisor !== undefined && config.advisor !== false). Counting it as a mechanism at resolve time produced the worst combination: a resolved escalate rung that, on trigger, ran consumeEscalation() before the advisor check and then applied no override — spending one of a run's two escalations and announcing recovery while doing nothing. Gating both the hasMechanism test and the spread on the same hasAdvisor predicate makes { advisor: false } read identically to {} and drops the disabled advisor from the resolved config rather than carrying it through inert, so the redundant guard in model-result.ts can never be reached with a false.

Reverting the source change and rerunning the doom-loop suite fails exactly the two tests that should fail (the advisor: true case still passes):

× advisor: false is not a recovery mechanism
    AssertionError: expected { advisor: false, maxEscalations: 2 } to be null
× advisor: false does not resolve alongside a real mechanism
    AssertionError: expected { model: 'stronger-model', …(2) } to not have property "advisor"

No asymmetry left in the sibling mechanism: model?: string has no falsy sentinel, so model needs no equivalent predicate.

One standing item: changeset code examples

The three changesets (doom-loop-detector.md, doom-loop-escalation.md, run-cancellation.md) contain no fenced code block. .agents/skills/public-api-examples/SKILL.md requires one in the changeset ("Always") for any public-API change, and these add doomLoop, doomLoop.escalation, loopKey, the DoomLoopDetected hook, signal on callModel, and a new @openrouter/agent/doom-loop subpath. The prose is unusually thorough, which is exactly why the omission is easy to miss — the rule is about the generated CHANGELOG carrying usage, not about completeness of description. Three short ts blocks mirroring the README usage close it.

Verdict: ✅ LGTM on the code. The changeset examples are a repo-rule gap, not a correctness one.

[devin-ai-integration]

Re-review — 4eef251

The commit is stranded. This PR was squash-merged as 78c562e, and 4eef251 landed on feat/doom-loop-detector afterwards — so the examples are not in main and will not ship.

main today still has all three changesets prose-only:

$ git show origin/main:.changeset/run-cancellation.md | grep -c '```'
0

Same for doom-loop-detector.md and doom-loop-escalation.md. Since publishing is driven by the pending .changeset/*.md files on main, the CHANGELOG.md that npm consumers read will be the example-free version — which is the outcome .agents/skills/public-api-examples/SKILL.md exists to prevent. This needs a fresh PR against main; the three changeset edits cherry-pick cleanly.

The content itself is right. I compiled all three examples against main (tsc --noEmit, clean): loopKey as a tool() field, the ladder rung names, escalation: { model, advisor, maxEscalations }, text: false, and await result.getDoomLoopVerdict() all typecheck as written. Your correction on the argument position is also right — the public surface is client.callModel(request, options), so RequestOptions is second.

One thing the commit missed while fixing exactly that: run-cancellation.md's prose still reads

RequestOptions.timeoutMs (the third callModel argument)

directly above the new example that (correctly) passes it as the second argument. The same wrong "third" is in packages/agent/src/lib/async-params.ts:143-144 on main. Both are worth folding into the follow-up PR — a changeset that contradicts its own example inside one file is worse than one with no example.