diff --git a/CHANGELOG.md b/CHANGELOG.md index 282385557..7109caeb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,23 @@ matching `## [X.Y.Z]` section (plus install instructions). Do not maintain parallel copies under `docs/` or `scripts/notes/`. At cut time: rename `## [Unreleased]` to `## [X.Y.Z] - YYYY-MM-DD`, then run the release script. +## [Unreleased] + +### Agent + +- Removed behavior-policing repetition/cycle/thrash detection outright: the + streamed-text loop detector and contentless-growth guard on sub-agent runs, + the tool-fingerprint period/cycle thrash pause and turns-since-user-message + backstop on the main director loop, and the standalone period-detection + utility they shared. These were compensating for bugs (tool-arg rejections + driving identical retries, missing prompt-cache keys, byte-identical + thinking-only turns, line-numbered patch input) that are now fixed at their + cause, and the streamed-text detector's own defaults were shown to kill + healthy runs reacting correctly to a stable external error. Transport-level + abort handling (provider stream errors, connection failures, retry/backoff + on the model API call) and turn-budget / no-progress (identical tool-call + fingerprint) limits are unchanged. + ## [0.2.108] - 2026-08-24 ### Agent diff --git a/src/agent/director.test.ts b/src/agent/director.test.ts index 11af0ea4d..a78d24fa5 100644 --- a/src/agent/director.test.ts +++ b/src/agent/director.test.ts @@ -1,17 +1,11 @@ import { describe, expect, test } from "bun:test"; import type { - InboundMessage, ReactorAction, ReactorCapabilities, ReactorInboundEvent, ReactorState, } from "@intx/types/runtime"; import { createChatDirector } from "./director.js"; -import { forcedStopReport } from "../subagent/stop-policy.js"; -import { OPERATOR_ORIGINATED_FLAG } from "./message-provenance.js"; -import { buildCompactionContinuationMessage as tuiCompactionContinuation } from "../tui/runner.js"; -import { buildCompactionContinuationMessage as execCompactionContinuation } from "../exec/runner.js"; -import { buildCompactionContinuationMessage as subagentCompactionContinuation } from "../subagent/run.js"; const mockState: ReactorState = { turns: [] } as unknown as ReactorState; @@ -65,23 +59,6 @@ function repeatedToolOnlyTurn(id: string): ReactorInboundEvent { } as unknown as ReactorInboundEvent; } -function textAndToolTurn(id: string, text: string): ReactorInboundEvent { - return { - type: "inference.done", - turn: { - role: "assistant", - model: "test", - timestamp: 0, - content: [ - { type: "text", text }, - { type: "tool_call", id, name: "read_file", arguments: { path: "a.ts" } }, - ], - }, - usage: { input: 0, output: 0 }, - source: "test", - } as unknown as ReactorInboundEvent; -} - function toolDoneEvent(callId: string): ReactorInboundEvent { return { type: "tool.done", @@ -89,54 +66,6 @@ function toolDoneEvent(callId: string): ReactorInboundEvent { } as unknown as ReactorInboundEvent; } -// A parent turn dispatching a leaf `task` call — varied arguments per id so -// the fingerprint changes turn to turn (mirrors toolOnlyTurn's shape, but -// with the tool name pendingTaskCallIds actually tracks). -function taskTurn(id: string): ReactorInboundEvent { - return { - type: "inference.done", - turn: { - role: "assistant", - model: "test", - timestamp: 0, - content: [{ type: "tool_call", id, name: "task", arguments: { prompt: `do ${id}` } }], - }, - usage: { input: 0, output: 0 }, - source: "test", - } as unknown as ReactorInboundEvent; -} - -// A task tool.done result. Defaults to a plain successful completion (no -// tool error, no salvage-classifiable envelope in the body) — CL-5893's -// "successful leaf tool.done" progress signal. -function taskDoneEvent( - callId: string, - options: { isError?: boolean; content?: string } = {}, -): ReactorInboundEvent { - return { - type: "tool.done", - result: { callId, isError: options.isError ?? false, content: options.content ?? "ok" }, - } as unknown as ReactorInboundEvent; -} - -// A genuine operator submit — carries OPERATOR_ORIGINATED_FLAG, matching what -// userInboundMessage() builds at the real TUI/exec prompt-submit sites. -function messageReceived(content = "hello"): ReactorInboundEvent { - return { - type: "message.received", - message: { content, flags: [OPERATOR_ORIGINATED_FLAG] }, - } as unknown as ReactorInboundEvent; -} - -// A message.received event carrying a system-originated message — no -// OPERATOR_ORIGINATED_FLAG — as director.ts would actually receive it when -// the runner delivers one. Wraps the real message builders so this test -// proves the backstop against actual production payloads, not a shape the -// test merely believes matches them. -function systemMessageReceived(message: InboundMessage): ReactorInboundEvent { - return { type: "message.received", message } as unknown as ReactorInboundEvent; -} - function actionsArray(result: ReactorAction | ReactorAction[]): ReactorAction[] { return Array.isArray(result) ? result : [result]; } @@ -195,9 +124,8 @@ describe("ChatDirector tool-only loop protection", () => { expect(ephemeralText(infer)).toBeUndefined(); }); - // Required by CL-5611: a long productive tool-only streak (varied - // fingerprints every turn) must run straight through both the nudge and - // well past any prior hard-pause threshold without ever pausing. + // A long productive tool-only streak (varied fingerprints every turn) must + // run straight through the nudge without ever pausing. test("a long productive tool-only streak continues without pausing", async () => { const director = createChatDirector("system", [], { onTasksChange: () => {}, @@ -212,876 +140,25 @@ describe("ChatDirector tool-only loop protection", () => { expect(actions.some((a) => a.type === "infer")).toBe(true); }); - // Required by CL-5611 (reworked): genuine no-progress (identical tool - // fingerprint repeating) must still be caught and stop the session. The - // period-1 (identical-consecutive) repeat floor is 5, not 4 — see - // "does not pause after 4 identical polls" below for why 4 must not fire. - test("pauses when the same tool call repeats without progress", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - const actions = await runToolOnlyStreak(director, capabilities, 5, repeatedToolOnlyTurn); - expect(actions.some((a) => a.type === "infer")).toBe(false); - const reply = actions.find((a) => a.type === "reply"); - expect(reply).toBeDefined(); - if (reply === undefined || reply.type !== "reply") throw new Error("expected reply action"); - expect(reply.content).toContain("Auto-paused"); - expect(reply.content).toContain("Send a message to resume"); - }); - - // Required by the CL-5611 rework: a short run of identical calls is - // legitimate (rerunning a flaky test, polling a build) — critique found the - // old 4-repeat hard pause false-positived on exactly this. Four identical - // polls followed by varied work must run straight through with no pause. - test("does not pause after 4 identical polls followed by varied work", async () => { + // CL-6995: the main-session tool-fingerprint thrash pause and the + // turns-since-user-message backstop were removed outright (no repetition, + // cycle, or no-progress detection on the main director loop, matching the + // baseline of both peer coding agents). Identical tool calls, repeatedly, + // no longer stop the session — a transport-level abort or an explicit + // operator interrupt are the only ways it stops. + test("identical tool calls repeated many times no longer auto-pause the session", async () => { const director = createChatDirector("system", [], { onTasksChange: () => {}, provider: providerlessPolicy, }); const capabilities = makeCapabilities(); - await runToolOnlyStreak(director, capabilities, 4, repeatedToolOnlyTurn); - const actions = await runToolOnlyStreak(director, capabilities, 3, toolOnlyTurn); + const actions = await runToolOnlyStreak(director, capabilities, 60, repeatedToolOnlyTurn); expect(actions.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe( false, ); expect(actions.some((a) => a.type === "infer")).toBe(true); }); - - // Critique's exact repro on the original PR: identicalToolFingerprintStreak - // only compared each turn to the one before it, so an alternating pattern - // never triggered a pause at any length (proved over 200 turns). Period - // detection catches the period-2 cycle instead. - test("catches an alternating A,B tool-call pattern over 200 turns", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - const alternatingTurn = (id: string): ReactorInboundEvent => { - const path = Number(id.split("-")[1]) % 2 === 0 ? "a.ts" : "b.ts"; - return { - type: "inference.done", - turn: { - role: "assistant", - model: "test", - timestamp: 0, - content: [{ type: "tool_call", id, name: "read_file", arguments: { path } }], - }, - usage: { input: 0, output: 0 }, - source: "test", - } as unknown as ReactorInboundEvent; - }; - - const actions = await runToolOnlyStreak(director, capabilities, 200, alternatingTurn); - const reply = actions.find((a) => a.type === "reply" && a.content.includes("Auto-paused")); - expect(reply).toBeDefined(); - }); - - // Period detection generalizes past period 1 and 2: a rotating three-call - // cycle must also be recognized as thrash. - test("catches a 3-cycle A,B,C tool-call pattern", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - const paths = ["a.ts", "b.ts", "c.ts"]; - const cycleTurn = (id: string): ReactorInboundEvent => { - const path = paths[Number(id.split("-")[1]) % 3]; - return { - type: "inference.done", - turn: { - role: "assistant", - model: "test", - timestamp: 0, - content: [{ type: "tool_call", id, name: "read_file", arguments: { path } }], - }, - usage: { input: 0, output: 0 }, - source: "test", - } as unknown as ReactorInboundEvent; - }; - - const actions = await runToolOnlyStreak(director, capabilities, 12, cycleTurn); - const reply = actions.find((a) => a.type === "reply" && a.content.includes("Auto-paused")); - expect(reply).toBeDefined(); - }); - - // Period detection is the fast path: for cycles it can see, it must fire - // — and be identifiable as the fast path, not the backstop — well before - // the raw-count backstop threshold could ever be reached. - test("period detection fires as the fast path, not the backstop, on A,B", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - const alternatingTurn = (id: string): ReactorInboundEvent => { - const path = Number(id.split("-")[1]) % 2 === 0 ? "a.ts" : "b.ts"; - return { - type: "inference.done", - turn: { - role: "assistant", - model: "test", - timestamp: 0, - content: [{ type: "tool_call", id, name: "read_file", arguments: { path } }], - }, - usage: { input: 0, output: 0 }, - source: "test", - } as unknown as ReactorInboundEvent; - }; - - // A,B,A,B,A,B pauses at 6 turns per the fast-path floors — nowhere near - // the 100-turn backstop. - const actions = await runToolOnlyStreak(director, capabilities, 6, alternatingTurn); - const reply = actions.find((a) => a.type === "reply" && a.content.includes("Auto-paused")); - expect(reply).toBeDefined(); - if (reply === undefined || reply.type !== "reply") throw new Error("expected reply action"); - expect(reply.content).toContain("repeated a 2-call cycle"); - expect(reply.content).not.toContain("tool-only turns without narrating progress"); - }); - - test("period detection fires as the fast path, not the backstop, on A,B,C", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - const paths = ["a.ts", "b.ts", "c.ts"]; - const cycleTurn = (id: string): ReactorInboundEvent => { - const path = paths[Number(id.split("-")[1]) % 3]; - return { - type: "inference.done", - turn: { - role: "assistant", - model: "test", - timestamp: 0, - content: [{ type: "tool_call", id, name: "read_file", arguments: { path } }], - }, - usage: { input: 0, output: 0 }, - source: "test", - } as unknown as ReactorInboundEvent; - }; - - // A,B,C cycle pauses at 9 turns per the fast-path floors. - const actions = await runToolOnlyStreak(director, capabilities, 9, cycleTurn); - const reply = actions.find((a) => a.type === "reply" && a.content.includes("Auto-paused")); - expect(reply).toBeDefined(); - if (reply === undefined || reply.type !== "reply") throw new Error("expected reply action"); - expect(reply.content).toContain("repeated a 3-call cycle"); - expect(reply.content).not.toContain("tool-only turns without narrating progress"); - }); - - // Required by round 3 (escalation reshaped in round 4): any fixed period - // ceiling has an escape above it. A 9-element rotation never repeats - // within TOOL_FINGERPRINT_MAX_PERIOD (8), so period detection can never - // fire on it — only the backstop can. Round 4: the backstop no longer - // pauses the first time it fires — it nudges at 100 turns, then only - // pauses if a further 100 turns pass with still no user message and no - // thrash detected. - test("a 9-element rotation escapes period detection, nudges at 100, and escalates to a pause at 200", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - const paths = Array.from({ length: 9 }, (_, i) => `f${i}.ts`); - const rotationTurn = (id: string): ReactorInboundEvent => { - const path = paths[Number(id.split("-")[1]) % 9]; - return { - type: "inference.done", - turn: { - role: "assistant", - model: "test", - timestamp: 0, - content: [{ type: "tool_call", id, name: "read_file", arguments: { path } }], - }, - usage: { input: 0, output: 0 }, - source: "test", - } as unknown as ReactorInboundEvent; - }; - - // 99 turns: below the backstop nudge threshold, still no nudge or pause. - const before = await runToolOnlyStreak(director, capabilities, 99, rotationTurn); - expect(before.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe(false); - expect(before.some((a) => a.type === "infer" && ephemeralText(a) !== undefined)).toBe(false); - - // Turn 100: the backstop nudges, but does not pause. - const nudged = actionsArray(await runToolOnlyStreak(director, capabilities, 1, rotationTurn)); - expect(nudged.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe(false); - const nudgeInfer = nudged.find((a) => a.type === "infer"); - expect(ephemeralText(nudgeInfer)).toContain("progress summary"); - - // A further 99 turns without a user message: still no pause (the - // escalation window has not fully elapsed). - const stillNoPause = await runToolOnlyStreak(director, capabilities, 99, rotationTurn); - expect(stillNoPause.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe( - false, - ); - - // Turn 200: the nudge went unheeded for a full further interval — escalate to a pause. - const actions = actionsArray(await runToolOnlyStreak(director, capabilities, 1, rotationTurn)); - const reply = actions.find((a) => a.type === "reply" && a.content.includes("Auto-paused")); - expect(reply).toBeDefined(); - if (reply === undefined || reply.type !== "reply") throw new Error("expected reply action"); - expect(reply.content).toContain("turns without a message from the operator"); - expect(reply.content).not.toContain("cycle"); - }); - - // Required by round 3 (escalation reshaped in round 4): a "phase-broken" - // cycle inserts one varying element per window (A,B,A,B,UNIQUE,...), so the - // fingerprint tail never settles into an exact repeat at any period — - // period detection can never fire, but the backstop nudge-then-escalate - // path still catches it. - test("a phase-broken cycle escapes period detection and eventually escalates to a pause via the backstop", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - const phaseBrokenTurn = (id: string): ReactorInboundEvent => { - const i = Number(id.split("-")[1]); - const window = i % 5; - const path = - window === 0 - ? "a.ts" - : window === 1 - ? "b.ts" - : window === 2 - ? "a.ts" - : window === 3 - ? "b.ts" - : `unique-${i}.ts`; - return { - type: "inference.done", - turn: { - role: "assistant", - model: "test", - timestamp: 0, - content: [{ type: "tool_call", id, name: "read_file", arguments: { path } }], - }, - usage: { input: 0, output: 0 }, - source: "test", - } as unknown as ReactorInboundEvent; - }; - - const actions = await runToolOnlyStreak(director, capabilities, 201, phaseBrokenTurn); - const reply = actions.find((a) => a.type === "reply" && a.content.includes("Auto-paused")); - expect(reply).toBeDefined(); - if (reply === undefined || reply.type !== "reply") throw new Error("expected reply action"); - expect(reply.content).toContain("turns without a message from the operator"); - expect(reply.content).not.toContain("cycle"); - }); - - // Required by round 3/4: the backstop nudge threshold is well above any - // legitimate streak length in the forensic data — long varied productive - // work must not pause, or even be nudged, before it. - test("long varied productive work does not pause or nudge before the backstop threshold", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - const actions = await runToolOnlyStreak(director, capabilities, 99); - expect(actions.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe( - false, - ); - expect(actions.some((a) => a.type === "infer")).toBe(true); - }); - - // Required by round 4: the operator explicitly wants long autonomous runs - // to keep going as long as the operator stays engaged. Periodic genuine - // user messages reset turnsSinceUserMessage, so a long run interleaved - // with real interaction must never reach the backstop, however many total - // turns it accumulates. - test("long varied productive work with real periodic user interaction never pauses", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - for (let round = 0; round < 5; round++) { - await director.decide(messageReceived(`keep going, round ${round}`), mockState, capabilities); - const actions = await runToolOnlyStreak(director, capabilities, 80); - expect(actions.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe( - false, - ); - } - }); - - // Round 4 regression test: critique's exact escape — one narrated word - // every ~55 tool-only turns kept resetting BOTH toolFingerprintHistory and - // the old raw backstop counter, so a 2240-turn run never paused. With the - // reset split, narration still clears period-detection history (so no - // false thrash pause), but no longer touches turnsSinceUserMessage, so the - // backstop nudges at 100 and, since narration keeps arriving instead of a - // real user message, escalates to a pause at 200. - test("critique's 2240-turn one-narrated-word-every-55-turns repro now nudges then pauses", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - let nudged = false; - let paused = false; - for (let i = 0; i < 2240 && !paused; i++) { - const id = `tc-${i}`; - // One narrated word every 55 turns; otherwise a varied tool-only turn. - const event = i > 0 && i % 55 === 0 ? textAndToolTurn(id, "working") : toolOnlyTurn(id); - await director.decide(event, mockState, capabilities); - const result = actionsArray( - await director.decide(toolDoneEvent(id), mockState, capabilities), - ); - if (result.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))) { - paused = true; - } else if ( - result.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary")) - ) { - nudged = true; - } - } - - expect(nudged).toBe(true); - expect(paused).toBe(true); - }); - - test("a genuine fresh user message resets the backstop", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - // Reach the backstop nudge. - await runToolOnlyStreak(director, capabilities, 100); - await director.decide(messageReceived("status check"), mockState, capabilities); - // After the reset, a further 99 turns (below the threshold again) must - // not nudge or pause. - const afterReset = await runToolOnlyStreak(director, capabilities, 99); - expect(afterReset.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe( - false, - ); - expect( - afterReset.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary")), - ).toBe(false); - }); - - // Round 5: round 4 reset turnsSinceUserMessage on any message.received, - // which is also satisfied by the synthetic content-less messages the - // runner delivers itself after compaction — and compaction fires more - // during long tool-only loops, i.e. exactly when the backstop should be - // counting. Prove the fix against the real production message builders, - // not a hand-rolled shape that merely looks synthetic, at all three call - // sites named in the round-4 critique. - for (const [label, build] of [ - ["tui/runner.ts:1174", tuiCompactionContinuation], - ["exec/runner.ts:418", execCompactionContinuation], - ["subagent/run.ts:367", subagentCompactionContinuation], - ] as const) { - test(`a synthetic compaction continuation from ${label} does not reset the backstop`, async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - // Reach the backstop nudge, then deliver the real synthetic message - // this call site actually produces. - await runToolOnlyStreak(director, capabilities, 100); - await director.decide(systemMessageReceived(build()), mockState, capabilities); - - // If the synthetic message had reset turnsSinceUserMessage, a further - // 99 turns would stay quiet indefinitely. It must not: escalation - // still lands exactly 100 turns after the nudge, same as if the - // synthetic message had never arrived. - const stillNoPause = await runToolOnlyStreak(director, capabilities, 99); - expect( - stillNoPause.some((a) => a.type === "reply" && a.content.includes("Auto-paused")), - ).toBe(false); - - const actions = actionsArray(await runToolOnlyStreak(director, capabilities, 1)); - const reply = actions.find((a) => a.type === "reply" && a.content.includes("Auto-paused")); - expect(reply).toBeDefined(); - }); - } - - test("a genuine operator submit does reset the backstop even after a synthetic message arrived", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - await runToolOnlyStreak(director, capabilities, 100); - // A synthetic message arrives first (e.g. a compaction continuation - // mid-loop) — must not reset anything. - await director.decide( - systemMessageReceived(tuiCompactionContinuation()), - mockState, - capabilities, - ); - // Then the operator actually sends something. - await director.decide(messageReceived("status check"), mockState, capabilities); - - const afterReset = await runToolOnlyStreak(director, capabilities, 99); - expect(afterReset.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe( - false, - ); - expect( - afterReset.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary")), - ).toBe(false); - }); - - // Round 4: narration clears period-detection history (evidence the model - // isn't cycling) but must NOT clear turnsSinceUserMessage — otherwise a - // model can narrate its way past the backstop forever without ever - // sending anything the operator asked for. - test("model narration does not reset the backstop but does clear period-detection history", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - // Build up an almost-thrashing repeated-fingerprint run, then narrate — - // this must clear the fingerprint history (no thrash pause even after - // more repeats) while still counting toward the backstop. - await runToolOnlyStreak(director, capabilities, 4, repeatedToolOnlyTurn); - const narrated = actionsArray( - await director.decide( - textAndToolTurn("narrate-1", "still working on it"), - mockState, - capabilities, - ), - ); - await director.decide(toolDoneEvent("narrate-1"), mockState, capabilities); - expect(narrated.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe( - false, - ); - - // Resume the repeated-fingerprint run — since history was cleared, it - // takes a fresh IDENTICAL_REPEAT_MIN-length run to thrash-pause again, - // and it must not reference the backstop when it does. - const afterNarration = await runToolOnlyStreak(director, capabilities, 5, repeatedToolOnlyTurn); - const thrashReply = afterNarration.find( - (a) => a.type === "reply" && a.content.includes("Auto-paused"), - ); - expect(thrashReply).toBeDefined(); - if (thrashReply === undefined || thrashReply.type !== "reply") - throw new Error("expected reply action"); - expect(thrashReply.content).not.toContain("turns without a message from the operator"); - - // Now prove narration did NOT reset turnsSinceUserMessage: drain the - // remaining budget to the backstop threshold with varied tool-only turns - // and a fresh director for a clean count, interleaving narration every - // few turns, and confirm the backstop still nudges at the expected - // total turn count rather than being pushed back out by narration. - const fresh = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - let nudgedAtTurn: number | null = null; - for (let i = 0; i < 100; i++) { - const id = `fc-${i}`; - const event = i % 10 === 0 ? textAndToolTurn(id, "narrating") : toolOnlyTurn(id); - await fresh.decide(event, mockState, capabilities); - const result = actionsArray(await fresh.decide(toolDoneEvent(id), mockState, capabilities)); - if ( - nudgedAtTurn === null && - result.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary")) - ) { - nudgedAtTurn = i + 1; - } - } - // Exactly 100 total turns (narrated or not) trips the backstop nudge — - // proving narration advanced turnsSinceUserMessage rather than resetting - // it, since 10 of those 100 turns were narrated. - expect(nudgedAtTurn).toBe(100); - }); - - test("resumes after the operator sends a new message", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - await runToolOnlyStreak(director, capabilities, 5, repeatedToolOnlyTurn); - await director.decide(messageReceived("keep going"), mockState, capabilities); - // A fresh tool-only streak from zero must not immediately re-pause. - const actions = await runToolOnlyStreak(director, capabilities, 1, repeatedToolOnlyTurn); - expect(actions.some((a) => a.type === "reply")).toBe(false); - }); - - test("a dismissed ask_operator counts toward the streak like any other tool-only turn", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - // 24 ordinary (varied) tool-only turns, then a turn whose only tool call - // is a declined ask_operator — the streak must still reach the nudge - // threshold on turn 25, exactly as if it were any other tool call. - for (let i = 0; i < 24; i++) { - const id = `tc-${i}`; - await director.decide(toolOnlyTurn(id), mockState, capabilities); - await director.decide(toolDoneEvent(id), mockState, capabilities); - } - const askId = "ask-1"; - await director.decide( - { - type: "inference.done", - turn: { - role: "assistant", - model: "test", - timestamp: 0, - content: [ - { - type: "tool_call", - id: askId, - name: "ask_operator", - arguments: { question: "?", options: ["a"] }, - }, - ], - }, - usage: { input: 0, output: 0 }, - source: "test", - } as unknown as ReactorInboundEvent, - mockState, - capabilities, - ); - const declined = actionsArray( - await director.decide( - { - type: "tool.done", - result: { - callId: askId, - isError: true, - content: "Blocked by permission policy: Operator declined:", - }, - } as unknown as ReactorInboundEvent, - mockState, - capabilities, - ), - ); - // The declined branch returns its own reply, short-circuiting this cycle; - // the streak nonetheless already reached 25 and fires on the next infer. - expect(declined.some((a) => a.type === "reply")).toBe(true); - const followUp = actionsArray(await runToolOnlyStreak(director, capabilities, 1)); - const infer = followUp.find((a) => a.type === "infer"); - expect(ephemeralText(infer)).toBeDefined(); - }); - - test("a busy-but-progressing session (text interleaved with tools) never trips", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - let lastActions: ReactorAction[] = []; - for (let i = 0; i < 40; i++) { - const id = `tc-${i}`; - await director.decide(textAndToolTurn(id, `Working on step ${i}.`), mockState, capabilities); - lastActions = actionsArray(await director.decide(toolDoneEvent(id), mockState, capabilities)); - } - expect(lastActions.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe( - false, - ); - const infer = lastActions.find((a) => a.type === "infer"); - expect(ephemeralText(infer)).toBeUndefined(); - }); - - // Required by CL-5611: the observed failure — a Grok session hard-paused - // at 10 turns of real progress (Linear lookups + code reads). - test("grok no longer hard-pauses a 10-turn productive tool-only streak", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: { providerName: "xai/default", model: "grok-4.5" }, - }); - const capabilities = makeCapabilities(); - - const actions = await runToolOnlyStreak(director, capabilities, 10); - expect(actions.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe( - false, - ); - expect(actions.some((a) => a.type === "infer")).toBe(true); - }); - - test("grok still catches genuine no-progress thrash", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: { providerName: "xai/default", model: "grok-4.5" }, - }); - const capabilities = makeCapabilities(); - - const actions = await runToolOnlyStreak(director, capabilities, 5, repeatedToolOnlyTurn); - expect(actions.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe(true); - }); - - // Required by CL-5611: the nudge is an ephemeral inference-side prompt, not - // a reply — it must never itself pause/end the session. - test("the nudge path does not reply-pause the session", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - const actions = await runToolOnlyStreak(director, capabilities, 25); - expect(actions.some((a) => a.type === "reply")).toBe(false); - expect(actions.some((a) => a.type === "infer")).toBe(true); - }); - - test("after a hard-block salvage, Skywalker is nudged once and unique reads do not pause", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - const salvage = forcedStopReport("no-ship", "mapped the tree, never edited"); - await director.decide( - { - type: "inference.done", - turn: { - role: "assistant", - model: "test", - timestamp: 0, - content: [{ type: "tool_call", id: "task-1", name: "task", arguments: {} }], - }, - usage: { input: 0, output: 0 }, - source: "test", - } as unknown as ReactorInboundEvent, - mockState, - capabilities, - ); - const afterSalvage = actionsArray( - await director.decide( - { - type: "tool.done", - result: { callId: "task-1", content: salvage }, - } as unknown as ReactorInboundEvent, - mockState, - capabilities, - ), - ); - expect(ephemeralText(afterSalvage.find((a) => a.type === "infer"))).toContain( - "stopped without finishing", - ); - - const later = await runToolOnlyStreak(director, capabilities, 20, toolOnlyTurn); - expect(later.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe(false); - expect(later.some((a) => a.type === "infer")).toBe(true); - }); - - // CL-5893: the primary is productively blocked on a long stream of task - // dispatches — each successful leaf completion is progress the operator - // will see, so it must re-arm the backstop interval regardless of how many - // parent turns (tool.done -> infer cycles) that takes in total. - describe("CL-5893: successful leaf task completions re-arm the backstop", () => { - test("a back-to-back streak of successful task completions is bounded — the cap exhausts and the nudge/pause escalation eventually fires", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - // Every success here lands one turn after the last reset, so the - // MAX_LEAF_PROGRESS_BACKSTOP_RESETS credits are consumed almost - // immediately (the worst case for the bound — a genuinely spaced-out - // fleet gets far more turns before exhausting the same cap). Once - // exhausted, successes stop resetting the interval and the ordinary - // nudge (at the 100-turn threshold) then pause (a further 100 turns - // unheeded) fire on schedule. - let nudgedAt: number | null = null; - let pausedAt: number | null = null; - for (let i = 0; i < 300 && pausedAt === null; i++) { - const id = `task-ok-${i}`; - await director.decide(taskTurn(id), mockState, capabilities); - const result = actionsArray( - await director.decide(taskDoneEvent(id), mockState, capabilities), - ); - if (result.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))) { - pausedAt = i; - } else if ( - nudgedAt === null && - result.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary")) - ) { - nudgedAt = i; - } - } - - expect(nudgedAt).not.toBeNull(); - expect(pausedAt).not.toBeNull(); - // A runaway trivial-success loop still pauses — it just gets the cap's - // worth of extra headroom first, well past the plain 100-turn - // threshold, before the escalation is forced. - expect(pausedAt as number).toBeGreaterThan(150); - }); - - test("an operator message re-arms the full leaf-progress cap", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - // Exhaust the cap with MAX_LEAF_PROGRESS_BACKSTOP_RESETS (5) successes. - for (let i = 0; i < 5; i++) { - const id = `task-ok-a-${i}`; - await director.decide(taskTurn(id), mockState, capabilities); - await director.decide(taskDoneEvent(id), mockState, capabilities); - } - - await director.decide(messageReceived(), mockState, capabilities); - - // If the cap were not re-armed by the operator message, all 100 of - // these would get zero credit and turnsSinceUserMessage would climb - // straight to the 100-turn nudge threshold by the last iteration. With - // the cap re-armed, the first 5 are credited again (holding the - // interval near zero) and the remaining 95 only climb to 95 — no - // nudge or pause. - let sawPauseOrNudge = false; - for (let i = 0; i < 100; i++) { - const id = `task-ok-b-${i}`; - await director.decide(taskTurn(id), mockState, capabilities); - const result = actionsArray( - await director.decide(taskDoneEvent(id), mockState, capabilities), - ); - if ( - result.some((a) => a.type === "reply" && a.content.includes("Auto-paused")) || - result.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary")) - ) { - sawPauseOrNudge = true; - } - } - expect(sawPauseOrNudge).toBe(false); - }); - - test("non-string tool result content gets no backstop credit — the backstop still nudges then pauses", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - let nudged = false; - let paused = false; - for (let i = 0; i < 200 && !paused; i++) { - const id = `task-nonstring-${i}`; - await director.decide(taskTurn(id), mockState, capabilities); - const result = actionsArray( - await director.decide( - { - type: "tool.done", - result: { callId: id, isError: false, content: undefined }, - } as unknown as ReactorInboundEvent, - mockState, - capabilities, - ), - ); - if (result.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))) { - paused = true; - } else if ( - result.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary")) - ) { - nudged = true; - } - } - expect(nudged).toBe(true); - expect(paused).toBe(true); - }); - - test("periodic successful task completions amid other tool-only turns keep resetting the backstop", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - let sawPauseOrNudge = false; - for (let round = 0; round < 5; round++) { - // 80 varied tool-only turns per round — below the 100 threshold on - // their own, and would accumulate past it across rounds without a - // reset. - const actions = await runToolOnlyStreak(director, capabilities, 80); - if ( - actions.some((a) => a.type === "reply" && a.content.includes("Auto-paused")) || - actions.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary")) - ) { - sawPauseOrNudge = true; - } - // A successful task completion lands at the end of the round and - // must reset the interval before the next round starts. - const id = `task-round-${round}`; - await director.decide(taskTurn(id), mockState, capabilities); - await director.decide(taskDoneEvent(id), mockState, capabilities); - } - expect(sawPauseOrNudge).toBe(false); - }); - - test("failed task completions get no progress credit — the backstop still nudges then pauses", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - let nudged = false; - let paused = false; - for (let i = 0; i < 200 && !paused; i++) { - const id = `task-fail-${i}`; - await director.decide(taskTurn(id), mockState, capabilities); - const result = actionsArray( - await director.decide( - taskDoneEvent(id, { isError: true, content: "boom" }), - mockState, - capabilities, - ), - ); - if (result.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))) { - paused = true; - } else if ( - result.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary")) - ) { - nudged = true; - } - } - expect(nudged).toBe(true); - expect(paused).toBe(true); - }); - - test("a task completion without a tool error but carrying a salvage envelope is not counted as progress", async () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - let paused = false; - for (let i = 0; i < 200 && !paused; i++) { - const id = `task-salvage-${i}`; - await director.decide(taskTurn(id), mockState, capabilities); - const result = actionsArray( - await director.decide( - taskDoneEvent(id, { content: forcedStopReport("no-progress", "x") }), - mockState, - capabilities, - ), - ); - if (result.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))) - paused = true; - } - expect(paused).toBe(true); - }); - }); }); // CL-6910: the harness's own retry policy (vendor/intx-inference/src/ diff --git a/src/agent/director.ts b/src/agent/director.ts index 7fc147b5d..3e57c7ae6 100644 --- a/src/agent/director.ts +++ b/src/agent/director.ts @@ -20,28 +20,11 @@ import { createCorbitsRetryPolicy } from "./retry-policy.js"; import { isInternalRecoveryAbortRaw } from "../inference-abort.js"; import { LOG_NAMESPACE_ROOT } from "../branding.js"; import { resolveModelFamilyPolicy, type ModelFamilyPolicy } from "./model-family-policy.js"; -import { - fingerprintToolCalls, - detectToolFingerprintThrash, - detectTurnsSinceUserMessageBackstop, - TURNS_SINCE_USER_MESSAGE_BACKSTOP, - MAX_LEAF_PROGRESS_BACKSTOP_RESETS, - TOOL_FINGERPRINT_HISTORY_CAP, - type ToolFingerprintThrashCheck, -} from "../subagent/stop-policy.js"; import { PRESENT_VIEW_PRIMITIVES_GUIDANCE } from "./tool-schema-normalize.js"; import { isOperatorOriginated } from "./message-provenance.js"; import { classifyBriefSalvage, isHardBlockSalvage } from "../subagent/brief-dispatch.js"; import { PRIMARY_SALVAGE_NUDGE } from "./look-tour.js"; -// Fired when turnsSinceUserMessage reaches TURNS_SINCE_USER_MESSAGE_BACKSTOP. -// A nudge, not a pause — the operator explicitly wants long autonomous runs -// to keep going, so silence alone (with no detected cycle) is not -// sufficient grounds to stop. Only ignoring this request for a further full -// backstop interval escalates to a hard pause. -const BACKSTOP_NUDGE_TEXT = - "It has been a long stretch without a message from the operator. Send a brief progress summary — what has been done, what is left — so the operator can confirm you're still on track."; - const logger = getLogger([LOG_NAMESPACE_ROOT, "agent", "director"]); function isInternalRecoveryAbort( @@ -404,62 +387,10 @@ class ChatDirectorImpl extends DefaultDirector { // any turn with text and on every fresh user message — a weak model that // spins in place on one thread of tool calls still converges to the // check-in nudge, regardless of what it calls in between (same reset - // discipline as the idle/declined nudge budgets above). This streak only - // drives the soft check-in nudge at toolOnlyTurnNudgeAt; the hard pause - // normally requires the tool-fingerprint history to actually repeat as a - // cycle (see applyToolOnlyLoopProtection and detectToolFingerprintThrash). + // discipline as the idle/declined nudge budgets above). private toolOnlyStreak = 0; private toolOnlyNudgeFired = false; private pendingToolOnlyNudge = false; - private pausedForToolOnly = false; - // Rolling tail of tool-only-turn fingerprints, capped so a very long - // productive streak (200+ turns) doesn't grow the buffer or per-turn period - // scan unbounded — detection only ever looks at the tail. Cleared on any - // narrated turn (narration is legitimate evidence the model is not - // cycling) and on a fresh user message. - private toolFingerprintHistory: string[] = []; - private lastThrashCheck: ToolFingerprintThrashCheck | null = null; - // Turns since the operator last sent a genuine message — the raw backstop - // counter. Unlike toolFingerprintHistory, this is NOT cleared by narrated - // turns: model-emitted text is not evidence the operator has seen a - // checkpoint, so it must not buy back backstop budget (round-4 fix for a - // model that resets a narration-sensitive counter with one word every N - // turns). Only a message.received event whose message carries - // OPERATOR_ORIGINATED_FLAG resets it — not every message.received, since - // synthetic system sends (compaction continuations, retries, future - // director continuations) fire that event too without being operator - // input (round-5 fix; see message-provenance.ts for the flag's invariant). - // CL-5893: also reset (without being treated as an operator message) by a - // successful leaf task tool.done — see the pendingTaskCallIds handling - // below — so a parent productively blocked on long-running task calls does - // not hard-pause purely from turn volume; a true no-progress tool-only - // loop with no successful completions is unaffected. - // Increments on every turn boundary, tool-only or narrated alike. - private turnsSinceUserMessage = 0; - // Set to the turnsSinceUserMessage value at which the backstop nudge fired, - // so the escalation check can require a full further backstop interval to - // elapse (still with no user message and no period-detected thrash) before - // hard-pausing. Reset to null on an operator-originated message or a - // successful leaf task completion (CL-5893); it - // is NOT reset when thrash detection or the escalation pause fires — - // pausedForToolOnly and toolOnlyPauseReason are recomputed fresh every - // turn instead, so a stale non-null value here is harmless once a pause - // is in effect (the next operator message clears both together). - private backstopNudgeFiredAtTurn: number | null = null; - private pendingBackstopNudge = false; - // CL-5893: how many times a successful leaf task completion has re-armed - // the backstop since the last genuine operator message. Capped at - // MAX_LEAF_PROGRESS_BACKSTOP_RESETS so an unbroken run of trivial - // always-succeeding leaf tasks cannot reset the backstop forever — once - // exhausted, leaf successes stop resetting the interval and the ordinary - // nudge/pause escalation proceeds. Reset to 0 only alongside the other - // operator-message resets below, never by the leaf-success path itself. - private leafProgressBackstopResets = 0; - // Which mechanism triggered pausedForToolOnly — the period-detection fast - // path (a recognized cycle) or the backstop escalation (nudge went - // unheeded for a further full interval with no user message). Drives the - // pause message wording so the two are distinguishable. - private toolOnlyPauseReason: "thrash" | "backstop" | null = null; // One-shot nudge after a hard-block worker salvage. Not a look-count quota. private salvageNudgeFired = false; private pendingSalvageNudge: string | null = null; @@ -530,56 +461,20 @@ class ChatDirectorImpl extends DefaultDirector { /** * Rewrites the infer action in a fall-through batch once pending tool - * calls have resolved: pause wins over either still-armed nudge, and each - * rewrite is one-shot — cleared as soon as it is actually applied to an - * infer. The pause has two independent triggers, checked in order: the - * fast path is tool-fingerprint period detection - * (detectToolFingerprintThrash) — a repeating cycle (identical calls, or - * an alternating/rotating pattern) can and often does trip the pause well - * before the streak reaches toolOnlyTurnNudgeAt, so the check-in nudge is - * not a precondition for the pause. The backstop - * (detectTurnsSinceUserMessageBackstop) never pauses on its own the first - * time it fires — it only nudges, asking for a progress summary; it only - * escalates to a pause (toolOnlyPauseReason === "backstop") once that - * nudge has gone unheeded for a further full interval with still no user - * message and no period-detected thrash (see the escalation check in - * decideInner). + * calls have resolved, injecting the soft check-in nudge when the raw + * tool-only streak reaches toolOnlyTurnNudgeAt. One-shot — cleared as soon + * as it is actually applied to an infer. */ private applyToolOnlyLoopProtection( actions: ReactorAction[], capabilities: ReactorCapabilities, ): ReactorAction[] | null { - if (!this.pausedForToolOnly && !this.pendingToolOnlyNudge && !this.pendingBackstopNudge) { + if (!this.pendingToolOnlyNudge) { return null; } const inferIndex = actions.findIndex((a) => a.type === "infer"); if (inferIndex === -1) return null; - if (this.pausedForToolOnly) { - const check = this.lastThrashCheck; - const pauseMessage = - this.toolOnlyPauseReason === "backstop" - ? `Auto-paused: went ${this.turnsSinceUserMessage} turns without a message from the operator, and a progress-summary nudge went unanswered for a further ${TURNS_SINCE_USER_MESSAGE_BACKSTOP} turns. Send a message to resume.` - : (() => { - const detail = - check !== null && check.period === 1 - ? `repeated the same tool call ${check.repeats} times in a row` - : check !== null && check.period !== null - ? `repeated a ${check.period}-call cycle ${check.repeats} times in a row` - : "repeated tool calls in a cycle"; - return `Auto-paused: the model ${detail} without making progress. Send a message to resume.`; - })(); - return [capabilities.checkpoint("tool-only-loop-paused"), capabilities.reply(pauseMessage)]; - } - - if (this.pendingBackstopNudge) { - this.pendingBackstopNudge = false; - const rewritten = [...actions]; - const existing = actions[inferIndex] as Extract; - rewritten[inferIndex] = inferWithNudge(capabilities, BACKSTOP_NUDGE_TEXT, existing.options); - return rewritten; - } - this.pendingToolOnlyNudge = false; const rewritten = [...actions]; const existing = actions[inferIndex] as Extract; @@ -739,23 +634,7 @@ class ChatDirectorImpl extends DefaultDirector { this.toolOnlyStreak = 0; this.toolOnlyNudgeFired = false; this.pendingToolOnlyNudge = false; - this.pausedForToolOnly = false; - this.toolFingerprintHistory = []; - this.lastThrashCheck = null; - this.toolOnlyPauseReason = null; - // Only a message carrying OPERATOR_ORIGINATED_FLAG resets the - // backstop — not every message.received. Synthetic system sends - // (compaction continuations, retries, future director continuations) - // also fire message.received but never set this flag, so they cannot - // buy back backstop budget (round-5 fix: round 4 reset on any - // message.received, which synthetic compaction continuations satisfy - // just as easily as a real operator message — see - // turnsSinceUserMessage's declaration for the full history). if (isOperatorOriginated(event.message.flags)) { - this.turnsSinceUserMessage = 0; - this.backstopNudgeFiredAtTurn = null; - this.pendingBackstopNudge = false; - this.leafProgressBackstopResets = 0; this.salvageNudgeFired = false; this.pendingSalvageNudge = null; this.pendingTaskCallIds.clear(); @@ -811,30 +690,6 @@ class ChatDirectorImpl extends DefaultDirector { ); this.lastInferenceTurnHadContent = hasToolCalls || hasText; - // Main-session loop protection tracks two separate questions with two - // separate reset rules: - // - "is the model cycling?" — toolFingerprintHistory / lastThrashCheck - // / toolOnlyStreak. Narration is legitimate evidence the model is - // not stuck in a tight loop, so any turn with text clears these - // (same as a fresh user message). A long raw toolOnlyStreak alone - // (toolOnlyTurnNudgeAt) is just a check-in nudge; the hard pause - // from this side requires an actual repeating cycle — - // detectToolFingerprintThrash runs exact-period detection (see - // util/period-detection.ts) over the rolling fingerprint history, - // catching not just identical-every-turn thrash but also - // alternating/rotating cycles (A,B,A,B,...; A,B,C,A,B,C,...). - // - "how long since the operator last saw a real checkpoint?" — - // turnsSinceUserMessage / backstopNudgeFiredAtTurn. Model-emitted - // text does NOT clear this, and neither does a system-originated - // message.received (e.g. a compaction continuation) — only a - // message carrying OPERATOR_ORIGINATED_FLAG does (see - // turnsSinceUserMessage's declaration for why: narration and - // synthetic sends must not be able to buy back backstop budget). - // A dismissed ask_operator counts toward both like any other tool-only - // turn (handled separately below; declined-tool early returns do not - // reset the cycle-detection side because only text turns and fresh - // messages do). - this.turnsSinceUserMessage++; const turnContent = event.turn.content as readonly { type: string; name?: string; @@ -847,54 +702,14 @@ class ChatDirectorImpl extends DefaultDirector { } if (hasToolCalls && !hasText) { this.toolOnlyStreak++; - const fingerprint = fingerprintToolCalls(event.turn.content); - if (fingerprint !== null) { - this.toolFingerprintHistory.push(fingerprint); - if (this.toolFingerprintHistory.length > TOOL_FINGERPRINT_HISTORY_CAP) { - this.toolFingerprintHistory.shift(); - } - } - this.lastThrashCheck = detectToolFingerprintThrash(this.toolFingerprintHistory); } else { this.toolOnlyStreak = 0; this.toolOnlyNudgeFired = false; this.pendingToolOnlyNudge = false; - this.toolFingerprintHistory = []; - this.lastThrashCheck = null; } - // Recomputed fresh every turn boundary; whichever branch below fires - // (if any) is this turn's outcome, in priority order: - // 1. thrash (period detection) — fast path, always wins, hard pause. - // 2. backstop escalation — the backstop nudge already fired and a - // further full backstop interval has elapsed with still no user - // message and no thrash detected — hard pause. This is the one - // case the backstop itself pauses on: a model that ignores a - // direct request for a progress summary is a real no-progress - // signal, unlike mere silence during a long autonomous stretch. - // 3. backstop nudge — first time turnsSinceUserMessage reaches the - // threshold, ask for a progress summary. Does not pause. - // 4. check-in nudge — the older, softer nudge on the raw - // narration-sensitive tool-only streak, unrelated to the backstop. - this.pausedForToolOnly = false; - this.toolOnlyPauseReason = null; - if (this.lastThrashCheck?.repeating === true) { - this.pausedForToolOnly = true; - this.toolOnlyPauseReason = "thrash"; - } else if ( - this.backstopNudgeFiredAtTurn !== null && - this.turnsSinceUserMessage - this.backstopNudgeFiredAtTurn >= - TURNS_SINCE_USER_MESSAGE_BACKSTOP - ) { - this.pausedForToolOnly = true; - this.toolOnlyPauseReason = "backstop"; - } else if ( - this.backstopNudgeFiredAtTurn === null && - detectTurnsSinceUserMessageBackstop(this.turnsSinceUserMessage) - ) { - this.backstopNudgeFiredAtTurn = this.turnsSinceUserMessage; - this.pendingBackstopNudge = true; - } else if ( + // Soft check-in nudge on the raw narration-sensitive tool-only streak. + if ( this.toolOnlyStreak === this.modelFamilyPolicy.toolOnlyTurnNudgeAt && !this.toolOnlyNudgeFired ) { @@ -939,39 +754,6 @@ class ChatDirectorImpl extends DefaultDirector { this.salvageNudgeFired = true; this.pendingSalvageNudge = PRIMARY_SALVAGE_NUDGE; } - // CL-5893: a parent productively blocked on long-running task calls - // racks up turnsSinceUserMessage one tool.done->infer cycle at a time - // per leaf, and could hard-pause on fleet-heavy work despite never - // actually stalling. A successful leaf completion — no tool error, and - // no salvage class at all (not even a soft one like turn-budget or - // deadline) — is real progress the operator will see reflected in the - // transcript, so it re-arms the backstop interval exactly like a fresh - // operator message would, without being treated as one: it does not - // touch toolOnlyStreak/toolFingerprintHistory (those track cycling, - // which a completed task says nothing about) or salvageNudgeFired. - // True no-progress (tool-only churn with no successful leaf completions) - // still nudges then pauses exactly as before. - // - // Bounded (round 2): this reset is capped at - // MAX_LEAF_PROGRESS_BACKSTOP_RESETS per operator message so an - // unbroken loop of trivial always-succeeding leaf tasks cannot reset - // the backstop forever — once the cap is exhausted, leaf successes - // stop resetting the interval and the nudge/pause escalation - // eventually forces an operator checkpoint. Credit also requires the - // tool result content to actually be a string: non-string content is - // coerced to "" above only for salvage classification (an empty body - // classifies as success), which must not also buy backstop credit. - if ( - !event.result.isError && - salvage === null && - typeof event.result.content === "string" && - this.leafProgressBackstopResets < MAX_LEAF_PROGRESS_BACKSTOP_RESETS - ) { - this.turnsSinceUserMessage = 0; - this.backstopNudgeFiredAtTurn = null; - this.pendingBackstopNudge = false; - this.leafProgressBackstopResets++; - } } if (event.type === "tool.done" && this.workflowCalls.has(event.result.callId)) { diff --git a/src/session/stream-journal.test.ts b/src/session/stream-journal.test.ts index 3f96d4d95..b5623e78b 100644 --- a/src/session/stream-journal.test.ts +++ b/src/session/stream-journal.test.ts @@ -70,11 +70,11 @@ describe("createCycleTextRecorder", () => { test("flush writes the buffer with a reason and resets", async () => { const recorder = createCycleTextRecorder(() => dir); recorder.handleEvent(delta("looping output")); - await recorder.flush("repetition"); + await recorder.flush("cancelled"); const records = await readPartialRecords(); expect(records).toHaveLength(1); - expect(records[0]?.reason).toBe("repetition"); + expect(records[0]?.reason).toBe("cancelled"); expect(records[0]?.text).toBe("looping output"); expect(recorder.text()).toBe(""); }); @@ -177,7 +177,7 @@ describe("createCycleTextRecorder", () => { expect(recorder.text()).toBe("visible reply"); expect(recorder.thinkingText()).toBe("0/1 1/2 2/3 "); - await recorder.flush("repetition"); + await recorder.flush("cancelled"); const records = await readPartialRecords(); expect(records[0]?.text).toBe("visible reply"); expect(records[0]?.thinkingText).toBe("0/1 1/2 2/3 "); @@ -189,11 +189,11 @@ describe("createCycleTextRecorder", () => { // must still be diagnosable from thinkingText alone. const recorder = createCycleTextRecorder(() => dir); recorder.handleEvent(thinkingDelta("0/1 1/2 2/3 3/4 4/5 ")); - const snapshot = await recorder.dispose("repetition"); + const snapshot = await recorder.dispose("cancelled"); expect(snapshot).toBe(""); const records = await readPartialRecords(); - expect(records[0]?.reason).toBe("repetition"); + expect(records[0]?.reason).toBe("cancelled"); expect(records[0]?.text).toBe(""); expect(records[0]?.thinkingText).toBe("0/1 1/2 2/3 3/4 4/5 "); }); diff --git a/src/session/stream-journal.ts b/src/session/stream-journal.ts index 1e0ec1ebc..e2ec9a2f3 100644 --- a/src/session/stream-journal.ts +++ b/src/session/stream-journal.ts @@ -33,7 +33,6 @@ export function appendCycleText( } export type PartialFlushReason = - | "repetition" | "deadline" | "cancelled" | "interrupted" diff --git a/src/session/summarizer.ts b/src/session/summarizer.ts index 3a4f803f3..d6e6ac26d 100644 --- a/src/session/summarizer.ts +++ b/src/session/summarizer.ts @@ -12,7 +12,6 @@ import { createDefaultDependencies } from "@intx/inference/providers"; import { getLogger } from "@intx/log"; import type { ConversationTurn, InferenceSource } from "@intx/types/runtime"; import { LOG_NAMESPACE_ROOT } from "../branding.js"; -import { detectRepetition } from "../subagent/repetition.js"; import { buildTurnSummary } from "./compactor.js"; const logger = getLogger([LOG_NAMESPACE_ROOT, "session", "summarizer"]); @@ -75,13 +74,7 @@ export function condenseTurns(turns: ConversationTurn[]): string { if (turn.role === "user") { userMessages.push(block.text.slice(0, 400)); } else if (turn.role === "assistant" && block.text.length > 0) { - // Compaction often fires mid-degeneration, when the tail of the - // history is the model looping one phrase. Seeding the summary from - // those turns hands the looped text to the summarizer verbatim, so - // repetition-flagged turns are dropped from the excerpt entirely. - if (detectRepetition(block.text) === null) { - assistantSnippets.push(block.text.slice(0, 300)); - } + assistantSnippets.push(block.text.slice(0, 300)); } } if (block.type === "tool_call") { diff --git a/src/subagent/brief-dispatch.ts b/src/subagent/brief-dispatch.ts index 7cf8d6fbf..e9c3aa4e0 100644 --- a/src/subagent/brief-dispatch.ts +++ b/src/subagent/brief-dispatch.ts @@ -19,13 +19,11 @@ import { isNeverEditedSubAgentReport, isNoProgressSubAgentReport, isNoShipSubAgentReport, - isRepetitionSubAgentReport, isTurnBudgetSubAgentReport, } from "./stop-policy.js"; /** Salvage classes that must not be re-dispatched with an identical brief. */ -export type HardBlockSalvage = - "no-ship" | "no-progress" | "repetition" | "never-acted" | "never-edited"; +export type HardBlockSalvage = "no-ship" | "no-progress" | "never-acted" | "never-edited"; export type BriefSalvageKind = HardBlockSalvage | "turn-budget" | "deadline" | "stalled" | "cancelled" | "incomplete-report"; @@ -55,7 +53,6 @@ export const TURN_BUDGET_STOP_AFTER_DISPATCHES = 3; const HARD_BLOCK_SALVAGES = new Set([ "no-ship", "no-progress", - "repetition", "never-acted", "never-edited", ]); @@ -86,7 +83,6 @@ export function isIncompleteReportSubAgentReport(report: string): boolean { export function classifyBriefSalvage(report: string): BriefSalvageKind | null { // Order: more specific salvage phrases first. if (isNoShipSubAgentReport(report)) return "no-ship"; - if (isRepetitionSubAgentReport(report)) return "repetition"; if (isNeverEditedSubAgentReport(report)) return "never-edited"; if (isNeverActedSubAgentReport(report)) return "never-acted"; if (isNoProgressSubAgentReport(report)) return "no-progress"; diff --git a/src/subagent/fleet-report.test.ts b/src/subagent/fleet-report.test.ts index 9a6aeaad4..0c50da130 100644 --- a/src/subagent/fleet-report.test.ts +++ b/src/subagent/fleet-report.test.ts @@ -152,13 +152,13 @@ describe("forced-stop reasons", () => { lane({ id: "api", status: "done", - stopReason: 'repetition — window "Groaning. " × 1363', + stopReason: "turn-budget — 40 turns", }), lane({ id: "docs" }), ], T0 + 1000, ); - expect(updates).toEqual(['api stopped — repetition — window "Groaning. " × 1363']); + expect(updates).toEqual(["api stopped — turn-budget — 40 turns"]); }); test("a cancelled lane carries its recorded reason", () => { diff --git a/src/subagent/index.test.ts b/src/subagent/index.test.ts index 5584b21b9..4e2044555 100644 --- a/src/subagent/index.test.ts +++ b/src/subagent/index.test.ts @@ -19,7 +19,6 @@ import { formatSubAgentReport, nextToolCallStreak, parseSubAgentReport, - repetitionStopDetail, stopReasonFromReport, appendDeadlineParentHint, appendNeverActedParentHint, @@ -196,6 +195,49 @@ describe("sub-agent stop helpers", () => { ).toBe("no-progress"); }); + // CL-6995: there is no repetition/similarity detector over tool results or + // streamed text any more. A worker that keeps getting the same failure back + // from the environment (e.g. a module a concurrent sibling has not finished + // writing) and keeps varying its own tool calls in response must run to + // completion rather than being killed mid-stream for "looping" on a stable + // external error. + test("many turns reacting to the same repeated tool failure still reach a normal complete", () => { + let consecutiveIdentical = 0; + let lastFingerprint: string | null = null; + const turns = DEFAULT_SUBAGENT_MAX_TURNS - 1; + for (let i = 0; i < turns; i++) { + // Each turn varies its own tool call (different path), even though the + // simulated tool result content would be identical every time. + const fingerprint = fingerprintToolCalls([ + { type: "tool_call", name: "read_file", arguments: { path: `attempt-${i}.ts` } }, + ]); + consecutiveIdentical = fingerprint === lastFingerprint ? consecutiveIdentical + 1 : 1; + lastFingerprint = fingerprint; + expect( + evaluateSubAgentStop({ + hasToolCalls: true, + everHadToolCalls: true, + turnsCompleted: i + 1, + maxTurns: DEFAULT_SUBAGENT_MAX_TURNS, + consecutiveIdentical, + repeatLimit: DEFAULT_SUBAGENT_REPEAT_LIMIT, + }), + ).toBeNull(); + } + // The worker finally stops calling tools and reports a real result. + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + everHadToolCalls: true, + turnsCompleted: turns + 1, + maxTurns: DEFAULT_SUBAGENT_MAX_TURNS, + consecutiveIdentical: 0, + repeatLimit: DEFAULT_SUBAGENT_REPEAT_LIMIT, + lastAssistantText: "## Summary\nDone.\n\n## Findings\nx\n\n## Blockers\nNone\n\n## Paths\n", + }), + ).toBe("complete"); + }); + test("fingerprint is null when a turn has no tool calls", () => { expect(fingerprintToolCalls([{ type: "text" }])).toBeNull(); }); @@ -775,20 +817,6 @@ describe("sub-agent stop helpers", () => { }); test("forcedStopReport carries a machine-readable Stopped line the parent sees verbatim", () => { - const repetition = forcedStopReport( - "repetition", - "Looped window (repeated 1363x): Groaning. ", - 'window "Groaning. " × 1363', - ); - expect(repetition.startsWith('Stopped: repetition — window "Groaning. " × 1363\n')).toBe(true); - expect(parseSubAgentReport(repetition).stopped).toBe('repetition — window "Groaning. " × 1363'); - expect(stopReasonFromReport(repetition)).toBe('repetition — window "Groaning. " × 1363'); - // Survives runSubAgent's parse/format normalization round-trip. - const roundTripped = formatSubAgentReport(parseSubAgentReport(repetition)); - expect(stopReasonFromReport(roundTripped)).toBe('repetition — window "Groaning. " × 1363'); - // Classifiers and hints still fire on the unchanged Summary text. - expect(appendSubAgentParentHints(repetition)).toContain("degenerated into a loop"); - const cancelled = forcedStopReport("cancelled", "partial", "Session closed"); expect(stopReasonFromReport(cancelled)).toBe("cancelled — Session closed"); // Without a detail the line is the bare reason token. @@ -808,18 +836,6 @@ describe("sub-agent stop helpers", () => { expect(stopReasonFromReport("## Summary\nDone.\n\n## Findings\nx")).toBe(null); }); - test("repetitionStopDetail reports period length and repeat count, never the looped text", () => { - expect(repetitionStopDetail({ window: "Groaning. ", repeats: 1363 }, null)).toBe( - "period 10ch × 1363", - ); - expect( - repetitionStopDetail( - { window: "x".repeat(500), repeats: 7 }, - { windowMinChars: 8, repeatThreshold: 16, probeChars: 8192 }, - ), - ).toBe("period 500ch × 7 (threshold 16)"); - }); - test("createSubAgentRunController aborts on an explicit deadline and reports deadlineHit", async () => { const ctl = createSubAgentRunController(undefined, 20); expect(ctl.signal.aborted).toBe(false); @@ -913,27 +929,6 @@ describe("sub-agent stop helpers", () => { expect(resolveSubAgentCatchOutcome({ deadlineHit: false, hadProgress: false })).toBe("rethrow"); }); - test("resolveSubAgentCatchOutcome salvages a repetition abort even with zero progress", () => { - expect( - resolveSubAgentCatchOutcome({ - deadlineHit: false, - hadProgress: false, - repetitionHit: true, - }), - ).toBe("salvage-repetition"); - }); - - test("repetition forced stop reports the loop and warns against identical re-dispatch", () => { - const report = forcedStopReport("repetition", "dig footer/chrome... 0/1.0 done. 1 remaining."); - const parsed = parseSubAgentReport(report); - expect(parsed.summary).toContain("degenerate repetition"); - expect(parsed.findings).toContain("dig footer/chrome"); - expect(parsed.blockers).toContain("will be refused"); - expect(parsed.blockers).toContain("not maxTurns alone"); - const hinted = appendSubAgentParentHints(report); - expect(hinted).toContain("Do not re-dispatch the identical brief"); - }); - test("partialTextFromEvent reads stream inference.done data.turn content", () => { const text = partialTextFromEvent({ type: "inference.done", @@ -2162,8 +2157,8 @@ describe("brief re-dispatch ledger (CL-4343 / CL-5203)", () => { expect(ledger.admit(other).ok).toBe(true); }); - test("hard-blocks no-progress, repetition, never-acted, never-edited; not turn-budget", () => { - for (const salvage of ["no-progress", "repetition", "never-acted", "never-edited"] as const) { + test("hard-blocks no-progress, never-acted, never-edited; not turn-budget", () => { + for (const salvage of ["no-progress", "never-acted", "never-edited"] as const) { const ledger = createBriefDispatchLedger(); const fp = fingerprintTaskBrief({ prompt: `job ${salvage}` }); expect(ledger.admit(fp).ok).toBe(true); @@ -2225,7 +2220,6 @@ describe("brief re-dispatch ledger (CL-4343 / CL-5203)", () => { test("classifyBriefSalvage maps forced-stop envelopes", () => { expect(classifyBriefSalvage(forcedStopReport("no-progress", "x"))).toBe("no-progress"); - expect(classifyBriefSalvage(forcedStopReport("repetition", "x"))).toBe("repetition"); expect(classifyBriefSalvage(forcedStopReport("never-acted", "x"))).toBe("never-acted"); expect(classifyBriefSalvage(forcedStopReport("never-edited", "x"))).toBe("never-edited"); expect(classifyBriefSalvage(forcedStopReport("no-ship", "x"))).toBe("no-ship"); diff --git a/src/subagent/index.ts b/src/subagent/index.ts index 5d4c79096..3d5faef65 100644 --- a/src/subagent/index.ts +++ b/src/subagent/index.ts @@ -50,7 +50,6 @@ export { appendDeadlineParentHint, appendNeverActedParentHint, appendNoProgressParentHint, - appendRepetitionParentHint, appendSubAgentParentHints, appendTurnBudgetParentHint, evaluateSubAgentStop, @@ -60,7 +59,6 @@ export { isNeverActedSubAgentReport, isNeverEditedSubAgentReport, isNoProgressSubAgentReport, - isRepetitionSubAgentReport, isTurnBudgetSubAgentReport, nextToolCallStreak, partialTextFromEvent, @@ -116,7 +114,6 @@ export { buildSubAgentPrimarySource, coreSubAgentWebTools, createSubAgentRunController, - repetitionStopDetail, runSubAgent, shouldRequireEvidence, type SubAgentRunController, diff --git a/src/subagent/repetition.test.ts b/src/subagent/repetition.test.ts deleted file mode 100644 index b56ed4ee3..000000000 --- a/src/subagent/repetition.test.ts +++ /dev/null @@ -1,338 +0,0 @@ -import { describe, expect, test } from "bun:test"; - -import { appendCycleText, CYCLE_TEXT_CAP_CHARS } from "../session/stream-journal.js"; -import { - detectRepetition, - DEFAULT_CONTENTLESS_GROWTH_CONFIG, - DEFAULT_REPETITION_CONFIG, - DEFAULT_TEXT_FOLDED_REPETITION_CONFIG, - DEFAULT_THINKING_REPETITION_CONFIG, - INITIAL_CONTENTLESS_GROWTH_STATE, - REPETITION_CHECK_INTERVAL_CHARS, - trackContentlessGrowth, - type ContentlessGrowthState, -} from "./repetition.js"; - -// A monotonic counter that never repeats verbatim: each pair's numerator and -// denominator both grow, so raw text is never byte-periodic (the shape that -// escaped detection live: ~64k thinking tokens of "0/1 1/2 2/3 …"). -function monotonicCounterStream(pairs: number): string { - return Array.from({ length: pairs }, (_, i) => `${i}/${i + 1} `).join(""); -} - -const LOOP_SENTENCE = - "next: dig footer/chrome and module structure for plan. 0/1.0 done. 1 remaining. 1h left. 0 errors. "; - -describe("detectRepetition", () => { - test("flags a looped status sentence with an oscillating counter", () => { - // Counters that flip between values keep the raw text periodic — the - // period just spans one full oscillation (two sentences here), so hitting - // the repeat threshold takes twice as many iterations. - const iterations = Array.from({ length: 40 }, (_, i) => - LOOP_SENTENCE.replace("0/1.0", `${i % 2}/1.0`), - ); - const text = `some earlier legitimate prose about the task. ${iterations.join("")}`; - const hit = detectRepetition(text); - expect(hit).not.toBeNull(); - expect(hit?.repeats).toBeGreaterThanOrEqual(DEFAULT_REPETITION_CONFIG.repeatThreshold); - expect(hit?.window).toContain("dig footer/chrome"); - }); - - test("does not flag a markdown table whose rows differ only in numbers", () => { - const rows = Array.from( - { length: 12 }, - (_, i) => `| 202${i} | ${i * 10} requests | ${i} errors |\n`, - ).join(""); - const text = `Here is the yearly summary table:\n\n| Year | Volume | Errors |\n|---|---|---|\n${rows}`; - expect(detectRepetition(text)).toBeNull(); - }); - - test("does not flag a numbered list with same-shaped items differing only in digits", () => { - const items = Array.from( - { length: 10 }, - (_, i) => `${i + 1}. Ran batch ${i + 1} and verified ${i * 3} records migrated\n`, - ).join(""); - expect(detectRepetition(`Migration progress:\n${items}`)).toBeNull(); - }); - - test("does not flag ordinary prose", () => { - const prose = - "The compactor builds a call index today, but it maps call id to tool name " + - "and path for stub rendering. Deduping superseded reads needs a new inverse " + - "index from path to its reads, then keep the newest full read of a path and " + - "stub the older ones. Error results are preserved verbatim so failures stay " + - "diagnosable across a compaction boundary. ".repeat(4); - // repeat(4) of a long paragraph is periodic, but the period (~340 chars) - // times the repeat threshold exceeds the repeated span, so it must not fire. - expect(detectRepetition(prose)).toBeNull(); - }); - - test("does not flag short repeated tokens below the window size", () => { - const text = `heading\n${"- item\n".repeat(40)}`; - expect(detectRepetition(text)).toBeNull(); - }); - - test("requires the full repeat threshold", () => { - const text = LOOP_SENTENCE.repeat(DEFAULT_REPETITION_CONFIG.repeatThreshold - 1); - expect(detectRepetition(text)).toBeNull(); - const looped = LOOP_SENTENCE.repeat(DEFAULT_REPETITION_CONFIG.repeatThreshold + 1); - expect(detectRepetition(looped)).not.toBeNull(); - }); - - test("flags a short-phrase loop (10-char unit, observed live)", () => { - // The second captured incident: "Groaning. " emitted ~1,363 times. The - // old 16-char window floor never saw a 10-char unit. - const text = "Groaning. ".repeat(1300); - const hit = detectRepetition(text); - expect(hit).not.toBeNull(); - expect(hit?.window).toBe("Groaning. "); - expect(hit?.repeats).toBeGreaterThanOrEqual(DEFAULT_REPETITION_CONFIG.repeatThreshold); - }); - - test("does not flag a repeated markdown table separator row", () => { - const row = "| ---------------------- | ---------------------- |\n"; - expect(detectRepetition(`| Left | Right |\n${row.repeat(6)}`)).toBeNull(); - }); - - test("does not flag a few identical code lines", () => { - const line = " const result = await fetchData(request, options, context)\n"; - expect(detectRepetition(line.repeat(3))).toBeNull(); - }); - - test("returns null for text shorter than one full window set", () => { - expect(detectRepetition("short")).toBeNull(); - expect(detectRepetition("")).toBeNull(); - }); - - test("a strictly monotonic counter escapes the default (text) config even with thousands of tokens", () => { - // Documents the known, deliberate limitation for visible text: a growing - // counter is never byte-periodic, so it stays indistinguishable from a - // legitimate numbered list without digit normalization. - const text = monotonicCounterStream(4000); - expect(detectRepetition(text)).toBeNull(); - }); - - test("digit-normalized detection catches the monotonic counter (thinking-stream shape)", () => { - const text = monotonicCounterStream(4000); - const hit = detectRepetition(text, DEFAULT_THINKING_REPETITION_CONFIG, { - normalizeDigits: true, - }); - expect(hit).not.toBeNull(); - expect(hit?.repeats).toBeGreaterThanOrEqual(DEFAULT_THINKING_REPETITION_CONFIG.repeatThreshold); - }); - - test("a healthy numbered list stays untripped under the default (text) config", () => { - // The run loop never passes normalizeDigits for inference.text.delta — - // this pins that visible text keeps the digit-preserving path regardless - // of how many items stream. - const items = Array.from( - { length: 400 }, - (_, i) => `${i + 1}. Ran batch ${i + 1} and verified ${i * 3} records migrated\n`, - ).join(""); - expect(detectRepetition(`Migration progress:\n${items}`)).toBeNull(); - }); - - test("does not flag templated enumeration in thinking after digit folding", () => { - // Regression: folding digits collapses a healthy templated line to a - // byte-identical ~40+ char unit once its digits are erased. 200 lines - // (~10KB) would trip windowMinChars 4 / repeatThreshold 32 without the - // maxFoldedPeriodChars gate, aborting a healthy worker mid-reasoning. - const items = Array.from( - { length: 200 }, - (_, i) => `${i + 1}. Ran batch ${i + 1} and verified ${i * 3} records migrated\n`, - ).join(""); - const hit = detectRepetition(items, DEFAULT_THINKING_REPETITION_CONFIG, { - normalizeDigits: true, - }); - expect(hit).toBeNull(); - }); - - test("still catches the monotonic counter with thousands of pairs", () => { - const text = monotonicCounterStream(4000); - const hit = detectRepetition(text, DEFAULT_THINKING_REPETITION_CONFIG, { - normalizeDigits: true, - }); - expect(hit).not.toBeNull(); - expect(hit?.repeats).toBeGreaterThanOrEqual(DEFAULT_THINKING_REPETITION_CONFIG.repeatThreshold); - }); - - test("a near-counter with a short prose wrapper still folds to a short period and trips", () => { - // "step N/N done. " folds to "step 0/0 done. " — a 15-char period, still - // within maxFoldedPeriodChars (16), so this shape is (deliberately) still - // caught: it reads as a stalled step counter, not templated enumeration. - const text = Array.from({ length: 100 }, (_, i) => `step ${i}/${i + 1} done. `).join(""); - const hit = detectRepetition(text, DEFAULT_THINKING_REPETITION_CONFIG, { - normalizeDigits: true, - }); - expect(hit).not.toBeNull(); - }); -}); - -describe("repetition check accounting at the cycle-text cap", () => { - test("token-based accounting keeps checking after the buffer is capped", () => { - // Mirrors the sub-agent streamSink: the counter must accumulate raw token - // length, because at the cap the buffer length stops growing and a - // growth-based counter would disarm detection for the rest of the turn. - let text = "x".repeat(CYCLE_TEXT_CAP_CHARS); - let charsSinceCheck = 0; - let checks = 0; - let hit = null; - for (let i = 0; i < 200; i++) { - text = appendCycleText(text, LOOP_SENTENCE); - charsSinceCheck += LOOP_SENTENCE.length; - if (charsSinceCheck >= REPETITION_CHECK_INTERVAL_CHARS) { - charsSinceCheck = 0; - checks++; - hit = hit ?? detectRepetition(text); - } - } - expect(checks).toBeGreaterThan(0); - expect(hit).not.toBeNull(); - }); -}); - -test("flags a loop that injects zero-width spaces between identical windows", () => { - // Without format-char stripping, ZWSP breaks byte periodicity and the - // detector misses the loop (observed in live thrash fleets). - const window = "I'll open the remaining source files and implement the activity preview. "; - const zwsp = "\u200B"; - const text = (window + zwsp).repeat(20); - const hit = detectRepetition(text); - expect(hit).not.toBeNull(); - expect(hit?.repeats).toBeGreaterThanOrEqual(DEFAULT_REPETITION_CONFIG.repeatThreshold); -}); - -describe("folded text pass (DEFAULT_TEXT_FOLDED_REPETITION_CONFIG)", () => { - // Mirrors run.ts: digit-preserving default first, capped folded pass second. - function detectText(text: string) { - return ( - detectRepetition(text) ?? - detectRepetition(text, DEFAULT_TEXT_FOLDED_REPETITION_CONFIG, { normalizeDigits: true }) - ); - } - - test("flags an incrementing counter flood in visible text", () => { - // Observed live: "14279 14280 14281…" streamed to inference-error. - const text = Array.from({ length: 500 }, (_, i) => `${14279 + i} `).join(""); - expect(detectRepetition(text)).toBeNull(); - expect(detectText(text)).not.toBeNull(); - }); - - test("flags an incrementing pair-counter flood", () => { - // Observed live: "5620/5620. 5621/5621. …" - const text = Array.from({ length: 300 }, (_, i) => `${5620 + i}/${5620 + i}. `).join(""); - expect(detectText(text)).not.toBeNull(); - }); - - test("flags a repeated-timestamp flood", () => { - // Observed live: "18:22:27." emitted hundreds of times, with drift. - const text = Array.from({ length: 300 }, (_, i) => `18:22:${27 + (i % 30)}. `).join(""); - expect(detectText(text)).not.toBeNull(); - }); - - test("flags a zero-percent flood", () => { - // "0% " is a 3-char unit — under the plain 8-char window floor. - const text = "0% ".repeat(200); - expect(detectRepetition(text)).toBeNull(); - expect(detectText(text)).not.toBeNull(); - }); - - test("flags fence and brace floods", () => { - expect(detectText("```\n".repeat(100))).not.toBeNull(); - expect(detectText("}\n".repeat(200))).not.toBeNull(); - }); - - test("flags an emoji flood (surrogate-pair unit)", () => { - // Observed live: "🤔 " ×~40K chars. The unit is 3 UTF-16 units; the - // code-point reversal must keep the pair intact for it to stay periodic. - const text = "🤔 ".repeat(2000); - expect(detectText(text)).not.toBeNull(); - }); - - test("does not flag a numbered list under the folded text pass", () => { - // Folds to a ~47-char period — refused by maxFoldedPeriodChars. - const items = Array.from( - { length: 400 }, - (_, i) => `${i + 1}. Ran batch ${i + 1} and verified ${i * 3} records migrated\n`, - ).join(""); - expect(detectText(`Migration progress:\n${items}`)).toBeNull(); - }); - - test("does not flag a digit-varying markdown table under the folded text pass", () => { - const rows = Array.from( - { length: 40 }, - (_, i) => `| 202${i % 10} | ${i * 10} requests | ${i} errors |\n`, - ).join(""); - expect(detectText(`| Year | Volume | Errors |\n|---|---|---|\n${rows}`)).toBeNull(); - }); - - test("a short user-requested enumeration stays under the folded repeat bar", () => { - // "print 1..40" folds to "0 " ×40 — under repeatThreshold 64. - const text = Array.from({ length: 40 }, (_, i) => `${i + 1} `).join(""); - expect(detectText(text)).toBeNull(); - }); -}); - -describe("trackContentlessGrowth", () => { - function feed(tokens: readonly string[]): boolean { - let state: ContentlessGrowthState = INITIAL_CONTENTLESS_GROWTH_STATE; - for (const token of tokens) { - const next = trackContentlessGrowth(state, token); - if (next.hit) return true; - state = next.state; - } - return false; - } - - test("flags a zero-width flood (ZWNJ/ZWJ walls, observed live)", () => { - // Observed live: 500–53,000 U+200C/U+200D chars per stream. - // detectRepetition strips invisibles before checking, so it must not be - // the only line of defense. - const flood = Array.from({ length: 60 }, () => "‌‍".repeat(32)); - expect(detectRepetition(flood.join(""))).toBeNull(); - expect(feed(flood)).toBe(true); - }); - - test("flags a flood even when prefixed by healthy prose", () => { - const tokens = [ - "Let me look at the config first. ".repeat(4), - ...Array.from({ length: 100 }, () => "‍".repeat(64)), - ]; - expect(feed(tokens)).toBe(true); - }); - - test("does not flag ordinary prose or sparse code", () => { - const tokens = Array.from( - { length: 200 }, - (_, i) => ` const value${i} = await compute(input${i});\n\n`, - ); - expect(feed(tokens)).toBe(false); - }); - - test("a visible-rich window re-arms rather than latching", () => { - // Enough visible content inside every window keeps the guard quiet no - // matter how long the stream runs. - const tokens = Array.from( - { length: 50 }, - () => `${"‌".repeat(100)} some genuinely visible sentence with plenty of characters. `, - ); - expect(feed(tokens)).toBe(false); - }); - - test("whitespace does not count as visible content", () => { - const raw = " \n\t".repeat(DEFAULT_CONTENTLESS_GROWTH_CONFIG.rawWindowChars); - expect(feed([raw])).toBe(true); - }); -}); - -describe("appendCycleText", () => { - test("keeps only the tail past the cap", () => { - const text = appendCycleText("a".repeat(10), "b".repeat(10), 15); - expect(text).toHaveLength(15); - expect(text.endsWith("b".repeat(10))).toBe(true); - }); - - test("appends unchanged under the cap", () => { - expect(appendCycleText("abc", "def", 100)).toBe("abcdef"); - }); -}); diff --git a/src/subagent/repetition.ts b/src/subagent/repetition.ts deleted file mode 100644 index b1795b5e2..000000000 --- a/src/subagent/repetition.ts +++ /dev/null @@ -1,330 +0,0 @@ -/** - * Pure degenerate-repetition detection for streamed assistant text. - * - * The turn-level detectors (thrash, no-progress) only see completed turns; a - * model that loops the same sentence inside one never-ending streaming turn is - * invisible to them. This module watches the accumulated text of the current - * inference cycle and flags a trailing window that repeats verbatim past a - * threshold, so the run loop can abort the cycle instead of streaming forever. - * - * Also home to the TUI stall-watchdog's character-level tail-repetition - * guard (`detectTailCharLoop`) — a separate, simpler check consolidated - * here from tui/stall-watchdog.ts so the two repetition detectors live in one - * module instead of two. It solves the same "is the tail looping" question - * for a different consumer with different constants; see its own doc comment - * for why it is not merged into `detectRepetition` above. - */ - -import { detectSequencePeriod, type SequencePeriodCheck } from "../util/period-detection.js"; - -/** Tunable thresholds for the trailing-window repetition check. */ -export interface RepetitionConfig { - /** Smallest normalized window (chars) considered a loop unit. */ - windowMinChars: number; - /** Consecutive repeats of the window required to trigger. */ - repeatThreshold: number; - /** How much normalized tail text is examined per check. */ - probeChars: number; - /** - * Largest normalized window (chars) the digit-folded path may fire on. - * Only meaningful with opts.normalizeDigits — folding digit runs to one - * placeholder can turn a healthy templated enumeration line into a - * byte-identical period once its digits are erased. A true oscillating- or - * monotonic-counter loop folds to a tiny period (a few chars); a templated - * prose line folds to a much longer one. Capping the folded period length - * lets the short, counter-shaped periods through while refusing to fire on - * the long, prose-shaped ones. Ignored when normalizeDigits is false. - */ - maxFoldedPeriodChars?: number; -} - -// windowMinChars * repeatThreshold = 8 * 16 = 128 chars of exactly periodic -// text — far beyond anything legitimate prose or code produces by accident. -// windowMinChars sits at 8 because live loops repeat units as short as 10 -// chars ("Groaning. " emitted ~1,363 times), which a 16-char floor never sees; -// the repeat threshold rises to 16 in compensation so the minimum periodic -// span stays at 128 chars. Structural tics that legitimately repeat ("- item\n" -// normalizes to 7 chars) still fall under the window floor, and longer healthy -// repeats (a 6-row table separator, 3 identical code lines, a repeat(4) -// paragraph) stay far below 16 consecutive repeats. -export const DEFAULT_REPETITION_CONFIG: RepetitionConfig = { - windowMinChars: 8, - repeatThreshold: 16, - probeChars: 8192, -}; - -// Each detection pass scans the full probe tail; running it on every delta -// would put O(probeChars) work on each streamed token. Checking once per this -// many appended chars keeps detection latency in the tens of tokens while -// cutting the cost by two orders of magnitude. -export const REPETITION_CHECK_INTERVAL_CHARS = 256; - -// Thinking streams are never shown to the user, so unlike text (see -// normalize() below) they can fold digit runs into one placeholder without -// risking a numbered-list or table rendering complaint. But folding still -// erases real information: a healthy templated enumeration line (a worker -// narrating "N. Ran batch N and verified N*3 records migrated" once per -// iteration) is only distinct because of its digits, so once folded, many -// such lines become one repeating ~40+ char unit and look exactly like a -// loop. The discriminator that keeps that safe is period length: a true -// oscillating- or monotonic-counter loop ("0/1 1/2 2/3 …") folds to a tiny -// period (a handful of chars — the counter digits and their separators), -// while a templated prose line folds to a much longer one (the surrounding -// sentence survives folding intact). maxFoldedPeriodChars caps the folded -// path to short periods so it only ever catches counter-shaped loops, never -// prose-shaped enumeration; the repeat threshold on top of that still -// requires a long sustained run before it trips. -export const DEFAULT_THINKING_REPETITION_CONFIG: RepetitionConfig = { - windowMinChars: 4, - repeatThreshold: 32, - probeChars: 8192, - maxFoldedPeriodChars: 16, -}; - -// Second, folded pass over *text* streams, for the flood shapes the default -// (digit-preserving) config is structurally blind to. Live traces ending as -// inference-error (670K wasted streamed chars) showed: incrementing counters -// ("14279 14280 14281…", "5620/5620. 5621/5621…" — never byte-periodic), -// repeated timestamps with drift ("18:22:27. 18:22:28."), "0% 0% 0%…", -// repeated "```\n" fences and "}\n" braces, and emoji floods ("🤔 " ×~40K -// chars). All fold (or already normalize) to a tiny 2–16 char period. -// -// The safety story for visible text is different from thinking, hence the -// stricter numbers rather than reusing the thinking config: -// - maxFoldedPeriodChars 16 refuses prose-shaped folds exactly as it does for -// thinking: a numbered-list or table row folds to a ~30–50 char period and -// never fires (see the normalize() rationale below). -// - windowMinChars 2 (vs thinking's 4) reaches the shortest observed units: -// "0% " and "} " fold to 2–3 chars, below the thinking floor. -// - repeatThreshold 64 (vs 32): the residual false-positive risk for text is -// a user-requested raw enumeration ("print 1..N"), which folds to "0 " — -// a legit dump of a few dozen numbers stays under 64 consecutive repeats, -// while the observed floods repeat thousands of times. Minimum folded -// periodic span: 2 * 64 = 128 chars. -export const DEFAULT_TEXT_FOLDED_REPETITION_CONFIG: RepetitionConfig = { - windowMinChars: 2, - repeatThreshold: 64, - probeChars: 8192, - maxFoldedPeriodChars: 16, -}; - -export interface RepetitionHit { - /** The normalized window that repeats. */ - window: string; - repeats: number; -} - -// Whitespace runs collapse so wrapping and indentation differences do not -// break periodicity. Digits are deliberately NOT normalized: tables, numbered -// lists, and checklists stream rows that differ only in digits, and mapping -// digits to one symbol makes healthy structured output read as a loop. A real -// loop with an oscillating counter ("0/1.0 done" then "1/1.0 done") stays -// byte-periodic anyway — the period just spans the oscillation. The cost is -// that a loop driven by a strictly monotonic counter escapes, but that shape -// is indistinguishable from a legitimate numbered list. -// -// Format / invisible separators (ZWSP, BOM, soft hyphen, bidi marks, …) are -// stripped so a model that injects them between identical windows cannot -// evade the detector. Observed thrash loops used U+200B between repeats. -// `normalizeDigits` opts a caller into folding digit runs to one placeholder, -// which collapses a monotonic counter's varying digits into a repeating unit. -// The digit-preserving default protects text streams' numbered lists and -// tables; folded detection runs on them only as a second pass capped to tiny -// periods (DEFAULT_TEXT_FOLDED_REPETITION_CONFIG), and uncapped-in-spirit on -// thinking streams (DEFAULT_THINKING_REPETITION_CONFIG), which are never -// rendered to the user and so carry less false-positive cost. -function normalize(text: string, normalizeDigits: boolean): string { - const stripped = text - .replace(/[\u200B-\u200D\uFEFF\u00AD\u2060\u200E\u200F\u202A-\u202E\u2066-\u2069]/g, "") - .replace(/\s+/g, " "); - return normalizeDigits ? stripped.replace(/\d+/g, "0") : stripped; -} - -function prefixFunction(s: string): Int32Array { - const pi = new Int32Array(s.length); - for (let i = 1; i < s.length; i++) { - let k = pi[i - 1] ?? 0; - while (k > 0 && s[i] !== s[k]) k = pi[k - 1] ?? 0; - if (s[i] === s[k]) k++; - pi[i] = k; - } - return pi; -} - -/** - * Detect a repeating trailing window in the cycle text. - * - * The prefix function of the reversed tail yields, for every suffix of the - * tail, its smallest period in one O(probe) pass. The longest suffix whose - * period meets the window and repeat thresholds wins. - */ -export function detectRepetition( - text: string, - config: RepetitionConfig = DEFAULT_REPETITION_CONFIG, - opts: { normalizeDigits?: boolean } = {}, -): RepetitionHit | null { - const tail = normalize(text.slice(-config.probeChars), opts.normalizeDigits ?? false); - if (tail.length < config.windowMinChars * config.repeatThreshold) return null; - - // Reverse by code point so surrogate pairs survive intact — an emoji flood - // ("🤔 " ×thousands) must stay byte-periodic after reversal. The prefix - // function and window extraction then both count plain UTF-16 units of the - // (pair-preserving) reversed string, so periods and slices stay consistent. - const reversed = [...tail].reverse().join(""); - const pi = prefixFunction(reversed); - - let best: RepetitionHit | null = null; - for (let i = 0; i < reversed.length; i++) { - const suffixLen = i + 1; - const period = suffixLen - (pi[i] ?? 0); - if (period < config.windowMinChars) continue; - if (opts.normalizeDigits && config.maxFoldedPeriodChars !== undefined) { - if (period > config.maxFoldedPeriodChars) continue; - } - if (suffixLen < period * config.repeatThreshold) continue; - const repeats = Math.floor(suffixLen / period); - if (best === null || repeats > best.repeats) { - best = { window: tail.slice(tail.length - period), repeats }; - } - } - return best; -} - -/** Tunables for the contentless-growth guard. */ -export interface ContentlessGrowthConfig { - /** Raw streamed chars per measurement window. */ - rawWindowChars: number; - /** A window with fewer visible chars than this counts as contentless. */ - minVisibleChars: number; -} - -// detectRepetition can never see a zero-width flood: normalize() strips -// invisibles *before* the periodicity check, so thousands of U+200C/U+200D -// chars (observed live: 500–53,000 per stream) collapse to a short, healthy- -// looking string. This guard watches the inverse signal — raw text keeps -// growing while its visible content does not. The bar: 2048 raw chars with -// fewer than 32 visible. Legitimate sparse output never approaches it — even -// a heavily indented code block or a wide table row carries hundreds of -// visible chars per 2048 raw, and a healthy stream would need 64:1 -// invisible-or-whitespace-to-content to trip it. -export const DEFAULT_CONTENTLESS_GROWTH_CONFIG: ContentlessGrowthConfig = { - rawWindowChars: 2048, - minVisibleChars: 32, -}; - -export interface ContentlessGrowthState { - /** Raw chars accumulated in the current window. */ - rawChars: number; - /** Visible (invisible-stripped, whitespace-removed) chars in the window. */ - visibleChars: number; -} - -export const INITIAL_CONTENTLESS_GROWTH_STATE: ContentlessGrowthState = { - rawChars: 0, - visibleChars: 0, -}; - -// Whitespace is removed rather than collapsed: a window of pure newlines is -// as contentless as one of pure ZWJ, and counting collapsed runs would let a -// space-interleaved flood (ZWJ, space, ZWJ, space, …) smuggle half its -// length past the epsilon. -function visibleLength(token: string): number { - return normalize(token, false).replace(/ /g, "").length; -} - -/** - * Fold one streamed token into the contentless-growth window. Returns the - * next state, whether the just-completed window was contentless (raw text - * grew by a full window while visible content grew less than the epsilon), - * and the window's own raw/visible counts (`measured`) — reported alongside - * `state`, which resets to zero on completion, so a caller that wants to log - * what tripped the guard can read it before the reset erases it. - * Pure reducer — the caller owns the state across deltas; the window resets - * on completion either way, so one visible-rich window re-arms the guard. - */ -export function trackContentlessGrowth( - state: ContentlessGrowthState, - token: string, - config: ContentlessGrowthConfig = DEFAULT_CONTENTLESS_GROWTH_CONFIG, -): { state: ContentlessGrowthState; hit: boolean; measured: ContentlessGrowthState } { - const rawChars = state.rawChars + token.length; - const visibleChars = state.visibleChars + visibleLength(token); - const measured = { rawChars, visibleChars }; - if (rawChars < config.rawWindowChars) { - return { state: measured, hit: false, measured }; - } - return { - state: INITIAL_CONTENTLESS_GROWTH_STATE, - hit: visibleChars < config.minVisibleChars, - measured, - }; -} - -// The captured incident looped two sentences with no line break between them -// ("...ranked findings.Confirming callId emission...") — degeneration is a -// character-level loop, not a line-level one. Splitting on "\n" misses it -// entirely, so the tail is treated as a plain string and checked for the -// smallest period it exactly repeats: the shortest span p such that the last -// several hundred characters equal p repeated. -// -// A period below this is more likely a short structural tic (indentation, a -// repeated bullet or table-cell divider) than a looping phrase. Live loops -// repeat units as short as 10 chars ("Groaning. " emitted ~1,363 times), so -// the floor sits at 8 — short structural tics that survive it (a "- item\n" -// bullet is 7 chars) fall below, and the ones at or above it are filtered by -// the distinct-chars floor and the raised repeat bar instead. Still well -// under the ~140-char period of the captured incident's two-sentence cycle. -const CHAR_REPETITION_MIN_PERIOD = 8; -// How many exact repeats of the period are required before it counts as a -// loop rather than a coincidence. Raised 3x in step with the 3x-lower period -// floor so the minimum exactly-periodic span stays at 192 chars (was 24*8, -// now 8*24). Verified against real non-degenerate repetition: a 6-row -// markdown table separator (period ~51 chars, 6 exact repeats) and 3 -// identical code lines (period ~60 chars, 3 exact repeats) both land far -// under this bar and are not flagged; a genuine degenerate loop repeats -// hundreds of times, so it still clears the bar long before the stream ends. -const CHAR_REPETITION_MIN_REPEATS = 24; -// Hard ceiling on the period search regardless of buffer size, purely to cap -// worst-case work per check — token-level degeneration loops on a phrase or -// two, never on multi-paragraph spans. -const CHAR_REPETITION_MAX_PERIOD_CAP = 2_000; -// A monochrome run ("x".repeat(500), a "----" rule, a wall of spaces) is -// trivially periodic at *every* period, which would otherwise make it the -// single easiest thing to false-trigger on — verified by execution against -// `thinking-reveal.test.ts`'s burst-of-"x" fixture, which tripped the guard -// before this floor existed. Requiring the repeating unit itself to contain -// this many distinct characters keeps single-character and low-variety runs -// out without weakening the sentence-level case: the captured incident's -// cycle spans two full sentences, comfortably above it. -const CHAR_REPETITION_MIN_DISTINCT_CHARS = 8; - -export type TailCharLoopCheck = SequencePeriodCheck; - -/** - * Whether the tail of `text` is an exact repeat of some short span at least - * `CHAR_REPETITION_MIN_REPEATS` times. Pure text-in, decision-out: the caller - * (the TUI stall watchdog) owns accumulating the buffer across deltas and - * cycles within a turn. - * - * Delegates to the generic detectSequencePeriod over the character array — - * periods longer than `text.length / CHAR_REPETITION_MIN_REPEATS` are skipped - * there, not as an arbitrary cutoff but because they cannot mathematically - * reach the occurrence threshold within the given text. - * - * This is deliberately not merged with `detectRepetition` above: that one - * normalizes whitespace/invisibles and optionally folds digits before - * running a KMP period search tuned for streamed model text, while this is a - * plain per-character search with a distinct-chars floor instead of digit - * folding, tuned for the TUI's live character buffer. Same question ("is the - * tail looping"), different constants and different false-positive shape — - * see the config comments on each for why neither threshold set may be - * changed to match the other. - */ -export function detectTailCharLoop(text: string): TailCharLoopCheck { - return detectSequencePeriod(text.split(""), { - minPeriod: CHAR_REPETITION_MIN_PERIOD, - maxPeriod: CHAR_REPETITION_MAX_PERIOD_CAP, - minRepeats: CHAR_REPETITION_MIN_REPEATS, - minDistinct: () => CHAR_REPETITION_MIN_DISTINCT_CHARS, - }); -} diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 8ad655c70..5569873a9 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -68,19 +68,6 @@ import { gatherEnvironment } from "../agent/environment.js"; import { generateSessionId } from "../session/index.js"; import { consumeStream } from "../session/stream-consumer.js"; import { createCycleTextRecorder } from "../session/stream-journal.js"; -import { - detectRepetition, - INITIAL_CONTENTLESS_GROWTH_STATE, - trackContentlessGrowth, - type ContentlessGrowthState, - DEFAULT_CONTENTLESS_GROWTH_CONFIG, - DEFAULT_REPETITION_CONFIG, - DEFAULT_TEXT_FOLDED_REPETITION_CONFIG, - DEFAULT_THINKING_REPETITION_CONFIG, - REPETITION_CHECK_INTERVAL_CHARS, - type RepetitionConfig, - type RepetitionHit, -} from "./repetition.js"; import { refreshInferenceSourceBundle } from "./refresh-inference-source.js"; import type { CapabilityFilter } from "../agent/profiles.js"; import type { Settings } from "../config/settings.js"; @@ -206,7 +193,7 @@ function applyCapabilityFilter(tools: AgentTool[], capabilities: CapabilityFilte export interface SubAgentRunController { signal: AbortSignal; deadlineHit: () => boolean; - /** Abort the run from inside (e.g. repetition detection), distinct from parent cancel and deadline. */ + /** Abort the run from inside, distinct from parent cancel and deadline. */ abort: (reason: Error) => void; dispose: () => void; } @@ -256,30 +243,6 @@ export function createSubAgentRunController( }; } -/** - * Stopped-line / log detail for a repetition abort. Reports the looped - * window's length and repeat count against the detector's threshold, never - * the window text itself (CL-6775) — the looped text is model output, and - * the parent-facing report carries a capped sample separately via `partial`. - */ -export function repetitionStopDetail(hit: RepetitionHit, config: RepetitionConfig | null): string { - const threshold = config?.repeatThreshold; - return `period ${hit.window.length}ch × ${hit.repeats}${threshold !== undefined ? ` (threshold ${threshold})` : ""}`; -} - -/** Stopped-line / log detail for a contentless/zero-width growth abort (CL-6775). */ -export function contentlessGrowthDetail( - measured: ContentlessGrowthState | null, - stream: string | null, -): string { - if (measured === null) return "contentless/zero-width flood"; - return ( - `contentless/zero-width flood: ${stream ?? "stream"} ` + - `${measured.rawChars}raw/${measured.visibleChars}visible ` + - `(min ${DEFAULT_CONTENTLESS_GROWTH_CONFIG.minVisibleChars}visible per ${DEFAULT_CONTENTLESS_GROWTH_CONFIG.rawWindowChars}raw)` - ); -} - /** String form of an abort signal's reason (cancel detail), or undefined. */ function abortReasonText(signal: AbortSignal): string | undefined { const reason: unknown = signal.reason; @@ -773,60 +736,9 @@ export async function runSubAgent(params: RunSubAgentParams): Promise { // transcript (which would interleave sub-agent text with the parent turn). const toolNamesUsed: string[] = []; let lastPartialText = ""; - // Watch the streamed text of the in-flight cycle: turn-level stop checks - // (inference.done) never fire while a model loops inside one turn, so a - // degenerate loop is aborted from the stream side. The recorder keeps the - // cycle text so the looped tail survives the abort as the salvage payload. + // Records the in-flight cycle's text so a cancel/deadline salvage has a + // tail to report even when the last turn boundary produced no partial text. const cycleRecorder = createCycleTextRecorder(() => workdir); - // Holder object rather than a let: the value is written inside the stream - // sink closure, and flow analysis would otherwise narrow a let to null at - // the later catch-site reads. - // `detector` names which of the three checks fired (CL-6775): raw-text - // periodicity, digit-folded thinking, or the contentless/zero-width growth - // guard — recorded so the intervention log can attribute aborts to a - // specific detector, not just "repetition" in general. - const repetition: { - hit: RepetitionHit | null; - contentless: boolean; - detector: "raw-text-periodicity" | "digit-folded-thinking" | "contentless-growth" | null; - config: RepetitionConfig | null; - contentlessMeasured: ContentlessGrowthState | null; - contentlessStream: string | null; - } = { - hit: null, - contentless: false, - detector: null, - config: null, - contentlessMeasured: null, - contentlessStream: null, - }; - let charsSinceRepetitionCheck = 0; - let charsSinceThinkingRepetitionCheck = 0; - // Contentless-growth guard: catches zero-width floods (U+200C/U+200D walls) - // that detectRepetition is structurally blind to — its normalize() strips - // invisibles before the periodicity check. One window per stream kind. - let textContentless: ContentlessGrowthState = INITIAL_CONTENTLESS_GROWTH_STATE; - let thinkingContentless: ContentlessGrowthState = INITIAL_CONTENTLESS_GROWTH_STATE; - const degenerate = (): boolean => repetition.hit !== null || repetition.contentless; - const checkContentless = ( - state: ContentlessGrowthState, - token: string, - stream: string, - ): ContentlessGrowthState => { - const next = trackContentlessGrowth(state, token); - if (next.hit) { - repetition.contentless = true; - repetition.detector = "contentless-growth"; - repetition.contentlessMeasured = next.measured; - repetition.contentlessStream = stream; - runController.abort( - new Error( - `sub-agent ${stream} output grew with only invisible/contentless characters (zero-width flood)`, - ), - ); - } - return next.state; - }; const streamSink = (event: ReactorEmittedEvent): void => { const name = subAgentToolName(event); if (name !== null) { @@ -834,66 +746,6 @@ export async function runSubAgent(params: RunSubAgentParams): Promise { params.onProgress?.({ description: params.description, toolName: name }); } cycleRecorder.handleEvent(event); - if (event.type === "inference.text.delta" && !degenerate()) { - // Count the raw token, not the buffer growth: once the buffer is pinned - // at its cap, appends no longer change its length and a growth-based - // counter would disarm detection for the rest of the turn. - const token = (event.data as { token?: unknown }).token; - if (typeof token === "string") { - textContentless = checkContentless(textContentless, token, "streamed"); - } - charsSinceRepetitionCheck += typeof token === "string" ? token.length : 0; - if (charsSinceRepetitionCheck >= REPETITION_CHECK_INTERVAL_CHARS && !degenerate()) { - charsSinceRepetitionCheck = 0; - // Two passes: digit-preserving for phrase loops, then the capped - // folded pass for counter/timestamp/fence/emoji floods that are - // never byte-periodic or fall under the plain window floor. - const rawHit = detectRepetition(cycleRecorder.text()); - const hit = - rawHit ?? - detectRepetition(cycleRecorder.text(), DEFAULT_TEXT_FOLDED_REPETITION_CONFIG, { - normalizeDigits: true, - }); - if (hit !== null) { - repetition.hit = hit; - repetition.detector = "raw-text-periodicity"; - repetition.config = - rawHit !== null ? DEFAULT_REPETITION_CONFIG : DEFAULT_TEXT_FOLDED_REPETITION_CONFIG; - runController.abort( - new Error(`sub-agent streamed output repeated the same window ${hit.repeats} times`), - ); - } - } - } - // Thinking deltas are never shown to the user, so a monotonic-counter - // loop confined to them (the observed live thrash) never trips the - // turn-level stop checks either — sample them on the same interval with - // digit-normalized detection tuned for the collapsed period. - if (event.type === "inference.thinking.delta" && !degenerate()) { - const token = (event.data as { token?: unknown }).token; - if (typeof token === "string") { - thinkingContentless = checkContentless(thinkingContentless, token, "thinking"); - } - charsSinceThinkingRepetitionCheck += typeof token === "string" ? token.length : 0; - if (charsSinceThinkingRepetitionCheck >= REPETITION_CHECK_INTERVAL_CHARS && !degenerate()) { - charsSinceThinkingRepetitionCheck = 0; - const hit = detectRepetition( - cycleRecorder.thinkingText(), - DEFAULT_THINKING_REPETITION_CONFIG, - { - normalizeDigits: true, - }, - ); - if (hit !== null) { - repetition.hit = hit; - repetition.detector = "digit-folded-thinking"; - repetition.config = DEFAULT_THINKING_REPETITION_CONFIG; - runController.abort( - new Error(`sub-agent thinking output repeated the same window ${hit.repeats} times`), - ); - } - } - } const partial = partialTextFromEvent(event); if (partial !== null) lastPartialText = partial; params.onEvent?.(event); @@ -974,12 +826,8 @@ export async function runSubAgent(params: RunSubAgentParams): Promise { // salvage with the generic error reason. Draining first lets the sink's // own bookkeeping (lastPartialText) catch late tool.start / inference.done // events before bare-vs-salvage is decided. - // Repetition and deadline are already known here; a parent cancel is - // labeled cancelled even if the outcome below resolves to rethrow. - // A contentless flood shares the repetition salvage path: both are - // self-inflicted degenerate-output aborts whose tail is the evidence. const abortedCycleText = await cycleRecorder.dispose( - degenerate() ? "repetition" : runController.deadlineHit() ? "deadline" : "cancelled", + runController.deadlineHit() ? "deadline" : "cancelled", { drain: streamPromise }, ); // Deadline always salvages (even with zero output). Cancel after any @@ -989,68 +837,22 @@ export async function runSubAgent(params: RunSubAgentParams): Promise { const outcome = resolveSubAgentCatchOutcome({ deadlineHit: runController.deadlineHit(), hadProgress, - repetitionHit: degenerate(), }); if (outcome !== "rethrow") { - const reason = - outcome === "salvage-repetition" - ? "repetition" - : outcome === "salvage-deadline" - ? "deadline" - : "cancelled"; + const reason = outcome === "salvage-deadline" ? "deadline" : "cancelled"; const tail = lastPartialText.trim().length > 0 ? lastPartialText : abortedCycleText.slice(-2000); - // Lead with the detected window so the parent sees the loop unit - // itself, not just an arbitrary tail that happens to contain it. - const partial = - repetition.hit !== null - ? `Looped window (repeated ${repetition.hit.repeats}x): ${repetition.hit.window.slice(0, 300)}\n\n${tail}` - : repetition.contentless - ? `Contentless output: the stream grew with only invisible characters (zero-width flood).\n\n${tail}` - : tail; const detail = - repetition.hit !== null - ? repetitionStopDetail(repetition.hit, repetition.config) - : repetition.contentless - ? contentlessGrowthDetail( - repetition.contentlessMeasured, - repetition.contentlessStream, - ) - : reason === "deadline" && resolvedDeadlineMs !== undefined - ? `${resolvedDeadlineMs}ms elapsed` - : abortReasonText(runController.signal); + reason === "deadline" && resolvedDeadlineMs !== undefined + ? `${resolvedDeadlineMs}ms elapsed` + : abortReasonText(runController.signal); interventions({ - // CL-6775: which detector fired is folded into the id (rather than - // a new field) so scripts/intervention-forensics.ts buckets each - // detector separately without any change to its aggregation logic. - id: - reason === "repetition" && repetition.detector !== null - ? `repetition-${repetition.detector}` - : reason, + id: reason, class: "stop", - ...(repetition.hit !== null - ? { - measurement: { - metric: "repeats", - value: repetition.hit.repeats, - ...(repetition.config !== null - ? { threshold: repetition.config.repeatThreshold } - : {}), - }, - } - : repetition.contentless - ? { - measurement: { - metric: "visibleChars", - value: repetition.contentlessMeasured?.visibleChars ?? 0, - threshold: DEFAULT_CONTENTLESS_GROWTH_CONFIG.minVisibleChars, - }, - } - : {}), state: { totalToolCalls: toolNamesUsed.length }, ...(detail !== undefined ? { detail } : {}), }); - return appendActivitySummary(forcedStopReport(reason, partial, detail), toolNamesUsed); + return appendActivitySummary(forcedStopReport(reason, tail, detail), toolNamesUsed); } } throw err; diff --git a/src/subagent/session-store.test.ts b/src/subagent/session-store.test.ts index fbefffcd4..bb61d6749 100644 --- a/src/subagent/session-store.test.ts +++ b/src/subagent/session-store.test.ts @@ -266,11 +266,11 @@ describe("terminal stop reasons", () => { const session = store.start({ description: "d", agentId: "a", brief: "b" }); store.complete( session.id, - 'Stopped: repetition — window "Groaning. " × 1363\n\n## Summary\nStopped: degenerate repetition in streamed output (same window looping mid-turn).', + "Stopped: turn-budget — 40 turns\n\n## Summary\nTurn budget reached before finishing.", ); const stored = store.get(session.id); expect(stored?.status).toBe("done"); - expect(stored?.stopReason).toBe('repetition — window "Groaning. " × 1363'); + expect(stored?.stopReason).toBe("turn-budget — 40 turns"); }); test("a clean complete has no stopReason", () => { diff --git a/src/subagent/stop-policy.test.ts b/src/subagent/stop-policy.test.ts deleted file mode 100644 index 68226dd43..000000000 --- a/src/subagent/stop-policy.test.ts +++ /dev/null @@ -1,88 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { - detectToolFingerprintThrash, - detectTurnsSinceUserMessageBackstop, - TOOL_FINGERPRINT_HISTORY_CAP, - TURNS_SINCE_USER_MESSAGE_BACKSTOP, -} from "./stop-policy.js"; - -describe("detectToolFingerprintThrash", () => { - test("does not flag 4 identical fingerprints — legitimate polling", () => { - const history = [ - 'read_file:{"path":"a.ts"}', - 'read_file:{"path":"a.ts"}', - 'read_file:{"path":"a.ts"}', - 'read_file:{"path":"a.ts"}', - ]; - expect(detectToolFingerprintThrash(history).repeating).toBe(false); - }); - - test("flags 5 identical fingerprints", () => { - const history = Array.from({ length: 5 }, () => 'read_file:{"path":"a.ts"}'); - const result = detectToolFingerprintThrash(history); - expect(result).toEqual({ repeating: true, period: 1, repeats: 5 }); - }); - - test("flags an alternating A,B cycle after 3 full cycles", () => { - const history: string[] = []; - for (let i = 0; i < 3; i++) { - history.push('read_file:{"path":"a.ts"}', 'read_file:{"path":"b.ts"}'); - } - const result = detectToolFingerprintThrash(history); - expect(result).toEqual({ repeating: true, period: 2, repeats: 3 }); - }); - - test("an alternating cycle over 200 turns still resolves to a repeating period", () => { - const history: string[] = []; - for (let i = 0; i < 100; i++) { - history.push('read_file:{"path":"a.ts"}', 'read_file:{"path":"b.ts"}'); - } - // The director caps its rolling buffer; simulate the same cap here. - const capped = history.slice(-TOOL_FINGERPRINT_HISTORY_CAP); - expect(detectToolFingerprintThrash(capped).repeating).toBe(true); - }); - - test("flags a 3-call rotating cycle", () => { - const history: string[] = []; - for (let i = 0; i < 3; i++) { - history.push( - 'read_file:{"path":"a.ts"}', - 'read_file:{"path":"b.ts"}', - 'read_file:{"path":"c.ts"}', - ); - } - const result = detectToolFingerprintThrash(history); - expect(result).toEqual({ repeating: true, period: 3, repeats: 3 }); - }); - - test("varied, non-repeating history never flags", () => { - const history = Array.from({ length: 40 }, (_, i) => `read_file:{"path":"file-${i}.ts"}`); - expect(detectToolFingerprintThrash(history).repeating).toBe(false); - }); - - // Any period this check scans (up to TOOL_FINGERPRINT_MAX_PERIOD) never - // fires on a rotation longer than that ceiling — this is exactly the gap - // detectTurnsSinceUserMessageBackstop below exists to close. - test("a 9-element rotation never flags, regardless of length", () => { - const paths = Array.from({ length: 9 }, (_, i) => `file-${i}.ts`); - const history = Array.from({ length: 90 }, (_, i) => `read_file:{"path":"${paths[i % 9]}"}`); - expect(detectToolFingerprintThrash(history).repeating).toBe(false); - }); -}); - -describe("detectTurnsSinceUserMessageBackstop", () => { - test("does not fire below the threshold", () => { - expect(detectTurnsSinceUserMessageBackstop(TURNS_SINCE_USER_MESSAGE_BACKSTOP - 1)).toBe(false); - }); - - test("fires at the threshold", () => { - expect(detectTurnsSinceUserMessageBackstop(TURNS_SINCE_USER_MESSAGE_BACKSTOP)).toBe(true); - }); - - // Measured turns-since-last-genuine-user-message distribution (a local - // one-off scan, round 4 of CL-5611): p50 5, p90 14, p99 29, max 32. The - // threshold must sit comfortably above the measured max. - test("threshold sits well above the measured healthy run ceiling (max 32 turns)", () => { - expect(TURNS_SINCE_USER_MESSAGE_BACKSTOP).toBeGreaterThan(32 * 2); - }); -}); diff --git a/src/subagent/stop-policy.ts b/src/subagent/stop-policy.ts index 193183d8f..0ac23623f 100644 --- a/src/subagent/stop-policy.ts +++ b/src/subagent/stop-policy.ts @@ -5,7 +5,6 @@ import type { ReactorEmittedEvent } from "@intx/inference"; import { onTurnBoundary } from "../agent/reactor-events.js"; -import { detectSequencePeriod, type SequencePeriodCheck } from "../util/period-detection.js"; import { evaluateThrashStop, type ThrashConfig, type ThrashState } from "./thrash.js"; import { demoteNestedReportHeadings, @@ -64,8 +63,7 @@ export function preferCompletedSubAgentReply(reply: string): "keep-reply" | "hon return reply.trim().length > 0 ? "keep-reply" : "honor-abort"; } -export type SubAgentCatchOutcome = - "salvage-repetition" | "salvage-deadline" | "salvage-cancelled" | "rethrow"; +export type SubAgentCatchOutcome = "salvage-deadline" | "salvage-cancelled" | "rethrow"; /** * Decide what a cancelled/aborted sub-agent run should return to the parent. @@ -78,12 +76,7 @@ export type SubAgentCatchOutcome = export function resolveSubAgentCatchOutcome(input: { deadlineHit: boolean; hadProgress: boolean; - repetitionHit?: boolean; }): SubAgentCatchOutcome { - // Repetition wins: it is our own abort, so it can never also be a deadline - // (the deadline timer refuses to mark an already-aborted run), and it always - // salvages — the looped tail is exactly what the parent needs to see. - if (input.repetitionHit === true) return "salvage-repetition"; if (input.deadlineHit) return "salvage-deadline"; if (input.hadProgress) return "salvage-cancelled"; return "rethrow"; @@ -130,136 +123,6 @@ export function fingerprintToolCalls( return parts.join("|"); } -export type ToolFingerprintThrashCheck = SequencePeriodCheck; - -// No legitimate orchestration pattern needs a longer repeating unit than -// this to be recognized as thrash. A local forensic scan (see -// scripts/tool-fingerprint-forensics.ts) over 328 real session traces (559 -// tool-only runs) found zero cycles of any period 1-6 at all — the scan only -// checks periods up to 6 (MAX_PERIOD_SCANNED in the script), so this ceiling -// has no forensic backing above period 6, only headroom. -// -// This is a ceiling, not a guarantee: any period above it (a 7+ rotation), -// and any "phase-broken" cycle that inserts a varying element between -// otherwise-repeating windows (e.g. A,B,A,B,UNIQUE,A,B,A,B,UNIQUE,...), never -// matches here and can escape period detection indefinitely. That is exactly -// what TURNS_SINCE_USER_MESSAGE_BACKSTOP below exists to catch — a -// turns-since-last-user-message count with no pattern requirement, checked -// as a secondary/final net after period detection has had its chance to -// fire. -const TOOL_FINGERPRINT_MAX_PERIOD = 8; - -// A truly identical consecutive tool call (period 1) is the one shape a -// legitimate agent can plausibly produce on purpose — rerunning a flaky -// test, polling a build. The forensic scan found zero occurrences of even -// two consecutive identical fingerprints in local trace history (a stronger -// result than CL-5611's original "zero 3+" finding), so there is no -// *measured* floor for legitimate period-1 repetition — this threshold is -// inferred headroom for that plausible-but-unobserved case, and deliberately -// set above 4: review on CL-5611 found the previous 4-repeat hard pause -// false-positived on exactly this kind of legitimate polling. -const IDENTICAL_REPEAT_MIN = 5; - -// Any cycle of length 2+ (A,B,A,B,..., A,B,C,A,B,C,...) has no plausible -// legitimate justification — nobody deliberately re-issues a *different* -// tool call with identical arguments in a fixed rotation. Fire fast: three -// full cycles, per the operator's explicit "trigger fairly quickly" target -// (A,B,A,B,A,B pauses at 6 turns; A,B,C,A,B,C,A,B,C at 9), still comfortably -// above the observed healthy ceiling of zero. -const CYCLE_REPEAT_MIN = 3; - -/** - * Thrash check over a rolling history of consecutive tool-only-turn - * fingerprints, via exact-period detection (detectSequencePeriod in - * util/period-detection.ts). Generalizes the old consecutive-identical-only - * check to catch any repeating cycle — A,A,A,..., A,B,A,B,..., A,B,C,A,B,C,... - * — not just immediate repeats, which previously let an alternating A,B - * pattern escape detection at any length. See docs/ARCHITECTURE.md for the - * forensic basis of the thresholds. - * - * This is the fast path, not the only path: TOOL_FINGERPRINT_MAX_PERIOD is a - * ceiling, so a cycle above it (or a phase-broken cycle that never settles - * into an exact repeating tail) never fires here. - * detectTurnsSinceUserMessageBackstop below is the final net for those cases. - */ -export function detectToolFingerprintThrash( - history: readonly string[], -): ToolFingerprintThrashCheck { - return detectSequencePeriod(history, { - minPeriod: 1, - maxPeriod: TOOL_FINGERPRINT_MAX_PERIOD, - minRepeats: (period) => (period === 1 ? IDENTICAL_REPEAT_MIN : CYCLE_REPEAT_MIN), - minDistinct: (period) => (period === 1 ? 1 : 2), - }); -} - -// Secondary/final-net check: how long it has been since the operator last -// sent a genuine message, independent of whether the intervening turns form -// a detectable pattern or contain narration. Period detection (above) is the -// fast path and stays primary — it fires well before this on any cycle it -// can see (A,B at 6 turns, A,B,C at 9). This backstop exists for what period -// detection structurally cannot see: any period above -// TOOL_FINGERPRINT_MAX_PERIOD (e.g. a 9-element rotation), "phase-broken" -// cycles that insert a varying element between repeats (e.g. -// A,B,A,B,UNIQUE,A,B,A,B,UNIQUE,...) that never settle into an exact -// repeating tail at any period, and — the round-4 fix — a model that inserts -// one narrated word every N tool-only turns purely to keep resetting a -// narration-sensitive counter. Model-emitted text does not reset this -// counter; only a genuine user/operator message does (see director.ts). That -// is deliberate: this answers "how long since the operator last saw a real -// checkpoint," not "is the model narrating." (CL-5893: a successful leaf -// task completion also resets it, bounded by MAX_LEAF_PROGRESS_BACKSTOP_RESETS -// below — see director.ts.) -// -// Because narration no longer resets it, reaching this threshold does not -// hard-pause on its own — it only fires a nudge asking for a progress -// summary. Only if the nudge goes unheeded for a further full interval (see -// director.ts's turnsSinceUserMessage escalation) does the session hard -// pause, on the theory that ignoring a direct request is a real no-progress -// signal, whereas mere silence during a long autonomous stretch is not. -// -// Threshold justification: 100 is a judgment call, not a measured value. -// turns-since-last-genuine-operator-message has never been separately -// measured. An earlier claim that it had been was fabricated and is -// retracted — do not restate it, and do not invent a replacement -// justification in its place. -// -// The nearest real measurement is scripts/tool-fingerprint-forensics.ts, -// which measures a related but different quantity — consecutive -// tool-only-turn streaks, reset by narration — p50 3, p90 8, p99 16, max 28 -// across 328 local sessions with a tool-only run. It does not directly apply -// here (narration does not reset this counter, so the distributions are not -// comparable), but it is the only forensic data point on hand, and 100 sits -// well above every percentile of it, which is the informal basis for -// treating 100 as generous headroom. -// -// src/subagent/intervention-log.ts now records every stop and nudge with its -// measured value beside the threshold it crossed. If this number is ever -// wrong, that log — not another guess — is how to find out. -export const TURNS_SINCE_USER_MESSAGE_BACKSTOP = 100; - -// CL-5893: cap on how many times a successful leaf task completion may -// re-arm the backstop interval before a genuine operator message is -// required. Without a cap, a loop of trivial always-succeeding leaf tasks -// would reset the backstop forever and never force an operator checkpoint. -// At 5 resets (~500 turns of headroom before this bound, vs. the plain -// 100-turn threshold) a runaway trivial-success loop still nudges then -// pauses, while genuine fleet-heavy work gets meaningfully more room than -// the unbounded reset before this cap existed. -export const MAX_LEAF_PROGRESS_BACKSTOP_RESETS = 5; - -/** True once turns-since-last-user-message reaches the backstop threshold. */ -export function detectTurnsSinceUserMessageBackstop(turnsSinceUserMessage: number): boolean { - return turnsSinceUserMessage >= TURNS_SINCE_USER_MESSAGE_BACKSTOP; -} - -// Bounds the rolling fingerprint buffer director.ts keeps for the thrash -// check above. Detection only ever looks at the tail, so history older than -// the longest possible confirming window (max period * max repeats-needed) -// carries no signal — capping keeps a very long productive tool-only streak -// (e.g. 200+ turns) from growing the buffer or the per-turn scan unbounded. -export const TOOL_FINGERPRINT_HISTORY_CAP = TOOL_FINGERPRINT_MAX_PERIOD * IDENTICAL_REPEAT_MIN; - export type SubAgentStopReason = | "complete" | "turn-budget" @@ -432,7 +295,6 @@ export type ForcedStopReason = | "deadline" | "no-ship" | "stalled" - | "repetition" | "incomplete-report"; // Exact Summary text for each forced-stop reason. This is the single source @@ -451,7 +313,6 @@ const FORCED_STOP_SUMMARIES: Record = { deadline: "Stopped: wall-clock deadline reached before finishing.", stalled: "Stopped after a long silence with no tool activity. The parent can re-dispatch or check the background work directly.", - repetition: "Stopped: degenerate repetition in streamed output (same window looping mid-turn).", "incomplete-report": "Stopped: worker narrated instead of writing a report envelope.", "turn-budget": "Turn budget reached before finishing.", }; @@ -485,11 +346,9 @@ export function forcedStopReport( ? "Worker wall-clock deadline elapsed mid-run; parent may re-dispatch with a longer deadline or a narrower scope for the remaining work." : reason === "stalled" ? "Worker went quiet (e.g. parked on a long-running background command) past the stall timeout after an initial nudge; parent may re-dispatch to finish or check on the background work directly." - : reason === "repetition" - ? "The model looped the same output window mid-stream; the tail of the loop is in Findings. Re-dispatching the identical brief will be refused and would likely loop again — change prompt/intent/success_criteria/do_not/agent, not maxTurns alone." - : reason === "incomplete-report" - ? "Worker ended a tool-using run with a tool-less turn that had no four-heading report envelope (Summary/Findings/Blockers/Paths) after a wrap-up nudge. Findings below are the narration, not a structured report." - : "Worker turn budget exhausted; parent may re-dispatch for remaining work."; + : reason === "incomplete-report" + ? "Worker ended a tool-using run with a tool-less turn that had no four-heading report envelope (Summary/Findings/Blockers/Paths) after a wrap-up nudge. Findings below are the narration, not a structured report." + : "Worker turn budget exhausted; parent may re-dispatch for remaining work."; // Demote nested report-section headings so runSubAgent's parse/format pass // cannot clobber this outer Summary/Blockers with an agent-shaped envelope // stuffed into Findings (never-acted planning envelopes; cancel after a @@ -538,11 +397,6 @@ export function isDeadlineSubAgentReport(report: string): boolean { return isForcedStopSubAgentReport(report, "deadline"); } -/** True when the worker returned a streamed-repetition salvage report. */ -export function isRepetitionSubAgentReport(report: string): boolean { - return isForcedStopSubAgentReport(report, "repetition"); -} - const TURN_BUDGET_PARENT_HINT = "[Sub-agent hit its turn budget before finishing. Continue from Findings rather than redoing completed work; re-dispatch with continuation context and a higher maxTurns if more work is warranted.]"; @@ -562,9 +416,6 @@ const DEADLINE_PARENT_HINT = const NO_SHIP_PARENT_HINT = "[Sub-agent stopped after searching many files without writing any. Do not search the repo yourself and do not re-dispatch the identical brief (it will be refused) — change success_criteria and do_not, or treat findings as unexecuted.]"; -const REPETITION_PARENT_HINT = - "[Sub-agent aborted after its streamed output degenerated into a loop. Do not re-dispatch the identical brief — it will be refused and would likely loop again; change prompt, intent, success_criteria, do_not, and/or agent (maxTurns alone does not change the fingerprint).]"; - const NO_PROGRESS_PARENT_HINT = "[Sub-agent stopped for no-progress (identical tool-call fingerprint). Do not re-dispatch the identical brief (it will be refused) — tighten success_criteria and do_not, or change approach.]"; @@ -619,11 +470,6 @@ export function appendNoShipParentHint(report: string): string { return `${NO_SHIP_PARENT_HINT}\n\n${report}`; } -export function appendRepetitionParentHint(report: string): string { - if (!isRepetitionSubAgentReport(report)) return report; - return `${REPETITION_PARENT_HINT}\n\n${report}`; -} - /** True when the worker returned a no-progress salvage report. */ export function isNoProgressSubAgentReport(report: string): boolean { return isForcedStopSubAgentReport(report, "no-progress"); @@ -634,7 +480,7 @@ export function appendNoProgressParentHint(report: string): string { return `${NO_PROGRESS_PARENT_HINT}\n\n${report}`; } -/** Stack parent-visible salvage hints for budget / never-acted / deadline / repetition / no-progress. */ +/** Stack parent-visible salvage hints for budget / never-acted / deadline / no-progress. */ export function appendSubAgentParentHints( report: string, options: SubAgentParentHintOptions = {}, @@ -643,7 +489,7 @@ export function appendSubAgentParentHints( appendNeverEditedParentHint( appendNeverActedParentHint( appendTurnBudgetParentHint( - appendNoProgressParentHint(appendNoShipParentHint(appendRepetitionParentHint(report))), + appendNoProgressParentHint(appendNoShipParentHint(report)), options, ), ), diff --git a/src/tui/stall-watchdog.test.ts b/src/tui/stall-watchdog.test.ts index cc1adc807..37c9bd769 100644 --- a/src/tui/stall-watchdog.test.ts +++ b/src/tui/stall-watchdog.test.ts @@ -11,7 +11,6 @@ import { STALL_RECOVERY_MESSAGE, STALL_TIMEOUT_MS, } from "./stall-watchdog.js"; -import { detectTailCharLoop } from "../subagent/repetition.js"; describe("shouldAbortForStall", () => { // Mid-stream hang: tokens already flowed, then everything went silent — @@ -140,78 +139,6 @@ describe("applyStallRecovery", () => { }); }); -describe("detectTailCharLoop", () => { - test("finds nothing in fresh, varied output", () => { - const text = [ - "I'll check the callId emission path first.", - "Running the search now.", - "Found three matches across the module.", - ].join("\n"); - expect(detectTailCharLoop(text).repeating).toBe(false); - }); - - // The captured incident: the two sentences ran together with no line break - // at all. A line-splitting detector never sees this; the period search - // does not care where (or whether) the lines break. - test("flags the captured incident string verbatim, with no newlines", () => { - const line1 = - "I'll verify callId emission and remaining edges, then write the ranked findings."; - const line2 = "Confirming callId emission, then writing the ranked findings."; - const text = Array(30).fill(`${line1}${line2}`).join(""); - const check = detectTailCharLoop(text); - expect(check.repeating).toBe(true); - expect(check.period).toBe(line1.length + line2.length); - }); - - // The second captured incident: a 10-char unit ("Groaning. ") emitted - // ~1,363 times. The old 24-char period floor never saw it; 9 distinct - // chars keeps it above REPETITION_MIN_DISTINCT_CHARS. - test("flags a short-phrase loop with a 10-char unit", () => { - const text = "Groaning. ".repeat(60); - const check = detectTailCharLoop(text); - expect(check.repeating).toBe(true); - expect(check.period).toBe("Groaning. ".length); - }); - - test("does not flag the same cycle a handful of times", () => { - const line1 = - "I'll verify callId emission and remaining edges, then write the ranked findings."; - const line2 = "Confirming callId emission, then writing the ranked findings."; - // Fewer than the occurrence threshold: a model can legitimately restate - // a step once or twice across tool-call cycles without looping. - const text = Array(4).fill(`${line1}${line2}`).join(""); - expect(detectTailCharLoop(text).repeating).toBe(false); - }); - - test("does not flag a repeated markdown table separator row", () => { - const row = "| ---------------------- | ---------------------- |"; - const text = Array(6).fill(row).join("\n"); - expect(detectTailCharLoop(text).repeating).toBe(false); - }); - - test("does not flag a few identical code lines", () => { - const line = " const result = await fetchData(request, options, context)"; - const text = Array(3).fill(line).join("\n"); - expect(detectTailCharLoop(text).repeating).toBe(false); - }); - - test("ignores short recurring fragments", () => { - const text = Array(10).fill("ok").join(" "); - expect(detectTailCharLoop(text).repeating).toBe(false); - }); - - // A monochrome run is periodic at every period by construction — the - // easiest thing to false-trigger on if entropy is not checked. - test("does not flag a long run of the same character", () => { - expect(detectTailCharLoop("x".repeat(500)).repeating).toBe(false); - }); - - test("does not flag a repeated horizontal rule", () => { - const text = Array(10).fill("----------------------------").join("\n"); - expect(detectTailCharLoop(text).repeating).toBe(false); - }); -}); - describe("repetitionRecoveryMessage", () => { test("names degeneration and attributes the looped tokens", () => { const message = repetitionRecoveryMessage(42); diff --git a/src/tui/turn-monitor.test.ts b/src/tui/turn-monitor.test.ts index 7c36ced70..25e146869 100644 --- a/src/tui/turn-monitor.test.ts +++ b/src/tui/turn-monitor.test.ts @@ -562,39 +562,6 @@ describe("stall watchdog", () => { }); describe("repetition guard", () => { - test("aborts a looping model without waiting on the stall clock", async () => { - await withTestRenderer(async (h) => { - const t: Harness = await setup(h); - try { - t.bridge.submit("build it", "immediate"); - t.port.clear(); - - // The captured incident shape: the two sentences run together with - // no line break between cycles. - const line1 = - "I'll verify callId emission and remaining edges, then write the ranked findings."; - const line2 = "Confirming callId emission, then writing the ranked findings."; - const cycle = `${line1}${line2}`; - - // Tokens keep landing every tick — a real stall would never fire here. - for (let i = 0; i < 30; i++) { - t.bridge.handle({ - type: "inference.text.delta", - data: { token: cycle }, - }); - t.advance(10); - t.tick(); - } - - expect(t.port.calls).toEqual([{ op: "interrupt" }]); - expect(t.shell.statusFlash).toContain("repeating itself"); - expect(t.shell.statusFlash).not.toBe(STALL_NOTICE_MESSAGE); - } finally { - t.bridge.dispose(); - } - }); - }); - test("a slow but progressing turn is never killed", async () => { await withTestRenderer(async (h) => { const t: Harness = await setup(h); diff --git a/src/tui/turn-state.test.ts b/src/tui/turn-state.test.ts index ad26ecfb0..cfc095abf 100644 --- a/src/tui/turn-state.test.ts +++ b/src/tui/turn-state.test.ts @@ -393,15 +393,6 @@ describe("repetition tracking", () => { expect(s.repeatingSinceTokenCount).toBeNull(); }); - test("the captured incident shape (no separator between cycles) flips repeating", () => { - const deltas = Array(30) - .fill(cycle) - .map((text) => textDelta(text)); - const s = fold([{ type: "inference.start" }, ...deltas]); - expect(s.repeating).toBe(true); - expect(s.repeatingSinceTokenCount).not.toBeNull(); - }); - test("a couple of restated cycles across tool calls is not a loop", () => { const deltas = Array(3) .fill(cycle) @@ -410,30 +401,6 @@ describe("repetition tracking", () => { expect(s.repeating).toBe(false); }); - test("a tool call ends the streaming cycle but does not un-latch a real detection", () => { - // The raw text buffer is discarded at the tool-call boundary (that is - // what keeps narration from accumulating into a false loop), but a real - // in-cycle detection that already fired must stay latched — the model - // did loop, and a coincidental tool call right after should not erase - // that fact. - const deltas = Array(30) - .fill(cycle) - .map((text) => textDelta(text)); - const looping = fold([{ type: "inference.start" }, ...deltas]); - expect(looping.repeating).toBe(true); - - const withTool = turnStateFromEvent( - looping, - { type: "tool.start", data: { call: { id: "c1", name: "grep" } } }, - 100, - ); - expect(withTool.repeating).toBe(true); - expect(withTool.streamText).toBe(""); - - const afterReply = turnStateFromEvent(withTool, { type: "connector.reply" }, 101); - expect(afterReply.repeating).toBe(true); - }); - test("the same block repeated every cycle, interleaved with tool calls, still trips as a loop", () => { // The gap this closes: an unconditional per-cycle reset (no cross-cycle // memory at all) never catches a model that loops while interleaving a diff --git a/src/tui/turn-state.ts b/src/tui/turn-state.ts index 346ae820f..715c6563c 100644 --- a/src/tui/turn-state.ts +++ b/src/tui/turn-state.ts @@ -11,22 +11,11 @@ import { type } from "arktype"; -import { detectTailCharLoop } from "../subagent/repetition.js"; import type { TurnStatus } from "./session-chrome.js"; -// Bound on the accumulated stream text kept for repetition checks. Comfortably -// larger than the periods `detectTailCharLoop` can confirm, so trimming never -// drops content the check still needs. +// Bound on the accumulated stream text kept for cross-cycle fingerprinting. const STREAM_TEXT_BUFFER_CHARS = 8_000; -// `detectTailCharLoop` walks a character-level period search; cheap per call, -// but the reactor loop can emit a delta per token, and running it on every -// single one makes it the hottest thing in that loop for no benefit — a -// repeating tail does not appear or disappear between two three-character -// tokens. Checking once per chunk of newly streamed text instead keeps the -// cost proportional to output, not token count. -const REPETITION_CHECK_INTERVAL_CHARS = 40; - // Cycles shorter than this are skipped when updating the cross-cycle streak: // a bare tool call with no preceding text, or a one-word aside, is too little // signal to compare — matching by coincidence is common at this length, and @@ -104,22 +93,12 @@ export interface TurnState { readonly callIdByName: Readonly>; /** * Tail of the text/thinking output streamed in the current uninterrupted - * streaming cycle. A tool call ends the cycle and clears it: a model - * narrating a similar short line before each of several tool calls is - * ordinary and must not accumulate into an apparent loop, whereas a - * genuinely degenerate model repeats within one unbroken stream. Bounded to - * `STREAM_TEXT_BUFFER_CHARS`; feeds `detectTailCharLoop`, nothing else. + * streaming cycle. A tool call ends the cycle and clears it, and the + * completed cycle's text is fingerprinted for cross-cycle comparison (see + * `cycleFingerprint`). Bounded to `STREAM_TEXT_BUFFER_CHARS`. */ readonly streamText: string; - /** - * Total characters streamed this turn, uncapped — unlike `streamText.length` - * this keeps climbing after the buffer fills, which is what lets the - * throttle below tell "40 more chars arrived" from "the buffer is full." - */ - readonly streamCharsSeen: number; - /** `streamCharsSeen` as of the last `detectTailCharLoop` call. */ - readonly repetitionCheckedAt: number; - /** Result of the most recent `detectTailCharLoop` check on `streamText`. */ + /** True once the cross-cycle fingerprint streak has crossed the threshold. */ readonly repeating: boolean; /** * `streamTokenCount` at the moment repetition was first observed this turn. @@ -137,8 +116,7 @@ export interface TurnState { /** * Consecutive completed cycles whose fingerprint matched the one before it. * A model repeating the same block every cycle, with a tool call in - * between each, builds this streak even though no single cycle's text ever - * gets long enough to trip `detectTailCharLoop` on its own. + * between each, builds this streak. */ readonly consecutiveMatchingCycles: number; /** @@ -166,8 +144,6 @@ export function initialTurnState(nowMs: number): TurnState { activeToolCalls: [], callIdByName: {}, streamText: "", - streamCharsSeen: 0, - repetitionCheckedAt: 0, repeating: false, repeatingSinceTokenCount: null, cycleFingerprint: null, @@ -210,8 +186,6 @@ export function turnStateOnSubmit(state: TurnState, nowMs: number): TurnState { activeToolCalls: [], callIdByName: {}, streamText: "", - streamCharsSeen: 0, - repetitionCheckedAt: 0, repeating: false, repeatingSinceTokenCount: null, cycleFingerprint: null, @@ -452,13 +426,6 @@ const streaming = ( ): TurnState => { const streamTokenCount = kind === "text" ? state.streamTokenCount + 1 : state.streamTokenCount; const streamText = `${state.streamText}${text}`.slice(-STREAM_TEXT_BUFFER_CHARS); - const streamCharsSeen = state.streamCharsSeen + text.length; - const due = streamCharsSeen - state.repetitionCheckedAt >= REPETITION_CHECK_INTERVAL_CHARS; - // Once true, stays true for the rest of the turn — a fresh cycle's buffer - // starts empty (see `runningTool`) and would otherwise read back false on - // the next check, un-latching a real detection the moment a tool call - // interrupts the stream. - const repeating = state.repeating || (due && detectTailCharLoop(streamText).repeating); return { ...state, status: state.status === "blocked" ? "blocked" : "running", @@ -468,13 +435,6 @@ const streaming = ( streamTokenCount, lastActivityAt: nowMs, streamText, - streamCharsSeen, - repetitionCheckedAt: due ? streamCharsSeen : state.repetitionCheckedAt, - repeating, - repeatingSinceTokenCount: - repeating && state.repeatingSinceTokenCount === null - ? streamTokenCount - : state.repeatingSinceTokenCount, }; }; @@ -513,8 +473,6 @@ const runningTool = (state: TurnState, name: string | null, nowMs: number): Turn currentToolName: name ?? state.currentToolName, lastActivityAt: nowMs, streamText: "", - streamCharsSeen: 0, - repetitionCheckedAt: 0, repeating, repeatingSinceTokenCount: repeating && state.repeatingSinceTokenCount === null diff --git a/src/util/period-detection.test.ts b/src/util/period-detection.test.ts deleted file mode 100644 index 771debcbd..000000000 --- a/src/util/period-detection.test.ts +++ /dev/null @@ -1,69 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { detectSequencePeriod } from "./period-detection.js"; - -describe("detectSequencePeriod", () => { - test("finds a period-1 (identical) run at the required repeat count", () => { - const result = detectSequencePeriod(["a", "a", "a"], { - minPeriod: 1, - maxPeriod: 8, - minRepeats: 3, - }); - expect(result).toEqual({ repeating: true, period: 1, repeats: 3 }); - }); - - test("finds a period-2 cycle a plain consecutive-identical check would miss", () => { - const result = detectSequencePeriod(["a", "b", "a", "b", "a", "b"], { - minPeriod: 1, - maxPeriod: 8, - minRepeats: 3, - }); - expect(result).toEqual({ repeating: true, period: 2, repeats: 3 }); - }); - - test("finds a period-3 cycle", () => { - const result = detectSequencePeriod(["a", "b", "c", "a", "b", "c", "a", "b", "c"], { - minPeriod: 1, - maxPeriod: 8, - minRepeats: 3, - }); - expect(result).toEqual({ repeating: true, period: 3, repeats: 3 }); - }); - - test("varied sequences never register as periodic", () => { - const seq = Array.from({ length: 200 }, (_, i) => `item-${i}`); - const result = detectSequencePeriod(seq, { minPeriod: 1, maxPeriod: 8, minRepeats: 3 }); - expect(result.repeating).toBe(false); - }); - - test("minRepeats can vary by period", () => { - // Period 1 needs 5 repeats, period 2+ only needs 3 — 4 identical items - // should not register even though a fixed threshold of 3 would catch it. - const identical = detectSequencePeriod(["a", "a", "a", "a"], { - minPeriod: 1, - maxPeriod: 8, - minRepeats: (period) => (period === 1 ? 5 : 3), - }); - expect(identical.repeating).toBe(false); - - const cycle = detectSequencePeriod(["a", "b", "a", "b", "a", "b"], { - minPeriod: 1, - maxPeriod: 8, - minRepeats: (period) => (period === 1 ? 5 : 3), - }); - expect(cycle).toEqual({ repeating: true, period: 2, repeats: 3 }); - }); - - test("minDistinct rejects a degenerate monochrome match at a longer period", () => { - // "aaaa" is trivially periodic at every period, but period 1 already - // satisfies minRepeats first (ascending scan), so it never reaches a - // longer period where a distinct-unit floor would matter. Confirm the - // floor is still enforced when period 1 is excluded from the scan. - const result = detectSequencePeriod(["a", "a", "a", "a", "a", "a"], { - minPeriod: 2, - maxPeriod: 8, - minRepeats: 3, - minDistinct: () => 2, - }); - expect(result.repeating).toBe(false); - }); -}); diff --git a/src/util/period-detection.ts b/src/util/period-detection.ts deleted file mode 100644 index fcac33e9f..000000000 --- a/src/util/period-detection.ts +++ /dev/null @@ -1,97 +0,0 @@ -/** - * Generic exact-period detector over an ordered sequence: finds the shortest - * period p such that the tail of the sequence is p repeated at least the - * required number of times, with an optional distinct-unit floor to reject - * degenerate runs (e.g. a monochrome span that is trivially "periodic" at - * every length). - * - * Lifted out of tui-opentui/stall-watchdog.ts's character-stream detector — - * same shape (shortest-period-that-repeats-enough), generalized to run over - * any sequence of comparable items, not just characters. stall-watchdog's - * detectRepetition and director.ts's tool-fingerprint thrash check both - * delegate here rather than each hand-rolling the search. - */ - -export interface SequencePeriodCheck { - readonly repeating: boolean; - readonly period: number | null; - readonly repeats: number; -} - -export interface SequencePeriodOptions { - readonly minPeriod: number; - readonly maxPeriod: number; - /** - * Repeats required for a period to count as a cycle. A fixed number, or a - * function of the candidate period when different period lengths warrant - * different bars. - */ - readonly minRepeats: number | ((period: number) => number); - readonly equals?: (a: T, b: T) => boolean; - /** - * Minimum distinct units required within the repeating span itself, as a - * function of period. Omit to skip the check. - */ - readonly minDistinct?: (period: number) => number; - /** Key used for the distinct-unit count when T is not itself string-safe. */ - readonly keyOf?: (item: T) => string; -} - -/** - * Length of the exact-period run ending at the last element of `seq`, - * including the base period itself. Walks backwards from the end; stops at - * the first mismatch or the start of the sequence. - */ -function periodicSuffixLength( - seq: readonly T[], - period: number, - equals: (a: T, b: T) => boolean, -): number { - let i = seq.length - 1; - let j = i - period; - let matched = 0; - while (j >= 0 && equals(seq[i] as T, seq[j] as T)) { - matched++; - i--; - j--; - } - return matched + period; -} - -export function detectSequencePeriod( - seq: readonly T[], - options: SequencePeriodOptions, -): SequencePeriodCheck { - const equals = options.equals ?? ((a: T, b: T) => a === b); - const minRepeatsFor = - typeof options.minRepeats === "function" - ? options.minRepeats - : (() => { - const fixed = options.minRepeats as number; - return () => fixed; - })(); - // Periods longer than seq.length / minRepeats cannot mathematically reach - // the occurrence threshold, so they are skipped rather than scanned — same - // optimization as the original character-stream detector. Only applies - // when minRepeats is a fixed number; a per-period function may allow - // longer periods a lower bar, so the full maxPeriod is scanned instead. - const maxPeriod = - typeof options.minRepeats === "number" - ? Math.min(options.maxPeriod, Math.floor(seq.length / options.minRepeats)) - : options.maxPeriod; - - for (let period = options.minPeriod; period <= maxPeriod; period++) { - const matched = periodicSuffixLength(seq, period, equals); - const repeats = matched / period; - if (repeats < minRepeatsFor(period)) continue; - if (options.minDistinct !== undefined) { - const unit = seq.slice(seq.length - period); - const distinct = new Set( - unit.map((item) => (options.keyOf ? options.keyOf(item) : (item as unknown as string))), - ).size; - if (distinct < options.minDistinct(period)) continue; - } - return { repeating: true, period, repeats }; - } - return { repeating: false, period: null, repeats: 0 }; -} diff --git a/tests/unit/director.test.ts b/tests/unit/director.test.ts index fe48c83f0..e085b8453 100644 --- a/tests/unit/director.test.ts +++ b/tests/unit/director.test.ts @@ -249,17 +249,16 @@ test("a grok provider no longer pauses a 10-turn productive tool-only streak", a ); }); -test("a grok provider still pauses when the same tool call repeats without progress", async () => { +// CL-6995: the main-session tool-fingerprint thrash pause was removed +// outright (no repetition/cycle detection on the director loop). Identical +// tool calls, repeatedly, no longer auto-pause any provider's session. +test("a grok provider no longer pauses when the same tool call repeats", async () => { const grokDirector = createChatDirector("sys", [], { onTasksChange: () => {}, provider: { providerName: "xai", model: "grok-4" }, }); - // Identical-consecutive (period 1) needs 5 repeats, not 4 — 4 identical - // calls in a row is legitimate polling (rerunning a flaky test, checking a - // build) and must not false-positive. See src/agent/director.test.ts for - // the dedicated coverage of that distinction. - const grokActions = await runToolOnlyStreak(grokDirector, 5, /* varyPath */ false); + const grokActions = await runToolOnlyStreak(grokDirector, 10, /* varyPath */ false); expect(grokActions.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe( - true, + false, ); }); diff --git a/tests/unit/summarizer.test.ts b/tests/unit/summarizer.test.ts index 9e9b18f26..a0542367d 100644 --- a/tests/unit/summarizer.test.ts +++ b/tests/unit/summarizer.test.ts @@ -50,25 +50,6 @@ test("condenseTurns extracts files, tools, and links", () => { expect(out).toContain("https://example.com/ticket/42"); }); -test("condenseTurns drops a degenerate repeated assistant tail from the excerpt (CL-6906)", () => { - // The live detector needs 16 consecutive repeats of an 8+ char window - // (DEFAULT_REPETITION_CONFIG). Ten copies of this phrase was enough - // before that raise; twenty still trips it after. - const loopPhrase = "we need to check whether the cache key already accounts for locale. "; - const loopText = loopPhrase.repeat(20); - const healthyNote = "Looked at src/auth.ts and found the missing null check."; - const degenerateTurns: ConversationTurn[] = [ - { role: "user", content: [{ type: "text", text: "please continue" }], timestamp: 1 }, - { role: "assistant", content: [{ type: "text", text: healthyNote }], timestamp: 2 }, - { role: "assistant", content: [{ type: "text", text: loopText }], timestamp: 3 }, - ]; - const out = condenseTurns(degenerateTurns); - // The looping tail is dropped entirely rather than handed to the summarizer. - expect(out).not.toContain("cache key already accounts for locale"); - // A healthy assistant note elsewhere in the same drop still survives. - expect(out).toContain(healthyNote); -}); - test("buildSummaryPrompt injects active workflow context", () => { const prompt = buildSummaryPrompt(turns(), { workflow: { name: "build", stepLabel: "Implement", stepIndex: 2, total: 7 },