diff --git a/CHANGELOG.md b/CHANGELOG.md index fae9ddf83..54c133503 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,15 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename independent copies of the same `AbortController` + `setTimeout` race. - `runtime-bridge.ts` now re-exports `mapReactorLike` from `stream-event-map.ts` instead of wrapping it in an identical local function. +- Removed degenerate-repetition detection outright: the streamed-text loop + detector, the tool-fingerprint period/cycle thrash check, the + turns-since-user-message backstop, and the leaf no-progress (identical + tool-call) counter. These were pattern-matching heuristics layered on top + of the transport/policy line the harness actually needs — provider stream + error handling, connection retry/backoff, and the turn budget — and had + become a source of false-positive stalls without a clear win rate. The + turn budget stop and the director's soft tool-only check-in nudge are + unchanged; nothing else in this run/stop chain was touched. ## [0.2.108] - 2026-08-24 diff --git a/src/agent/director.test.ts b/src/agent/director.test.ts index c4587758e..6cc6c3db5 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; @@ -34,7 +28,7 @@ function makeCapabilities(): ReactorCapabilities { // Varied arguments per call so the fingerprint changes turn to turn — the // shape of genuine, varied tool-only orchestration (Linear lookups, reading -// different files, ...), as opposed to repeatedToolOnlyTurn below. +// different files, ...). function toolOnlyTurn(id: string): ReactorInboundEvent { return { type: "inference.done", @@ -49,39 +43,6 @@ function toolOnlyTurn(id: string): ReactorInboundEvent { } as unknown as ReactorInboundEvent; } -// Identical tool name + arguments on every call regardless of id — the shape -// of genuine no-progress thrash (fingerprintToolCalls ignores call id). -function repeatedToolOnlyTurn(id: string): ReactorInboundEvent { - return { - type: "inference.done", - turn: { - role: "assistant", - model: "test", - timestamp: 0, - content: [{ type: "tool_call", id, name: "read_file", arguments: { path: "a.ts" } }], - }, - usage: { input: 0, output: 0 }, - source: "test", - } 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,59 +50,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; stopReason?: string } = {}, -): ReactorInboundEvent { - return { - type: "tool.done", - result: { - callId, - isError: options.isError ?? false, - content: options.content ?? "ok", - ...(options.stopReason !== undefined ? { detail: { stopReason: options.stopReason } } : {}), - }, - } 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]; } @@ -216,880 +124,6 @@ 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 () => { - const director = createChatDirector("system", [], { - onTasksChange: () => {}, - provider: providerlessPolicy, - }); - const capabilities = makeCapabilities(); - - await runToolOnlyStreak(director, capabilities, 4, repeatedToolOnlyTurn); - const actions = await runToolOnlyStreak(director, capabilities, 3, toolOnlyTurn); - 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, detail: { stopReason: "no-ship" } }, - } 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"), - stopReason: "no-progress", - }), - 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 2041f6b4b..e8e8c4e72 100644 --- a/src/agent/director.ts +++ b/src/agent/director.ts @@ -20,29 +20,12 @@ 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 type { ForcedStopReason } from "../subagent/stop-policy.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( @@ -405,62 +388,12 @@ 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). Drives the soft + // check-in nudge at toolOnlyTurnNudgeAt (see applyToolOnlyLoopProtection) — + // a turn-count nudge, not a stop. 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; @@ -531,56 +464,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, attaching the soft check-in nudge once the raw + * tool-only streak reaches toolOnlyTurnNudgeAt. A turn-count nudge, not a + * stop — the session keeps running either way. */ 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; @@ -740,23 +637,11 @@ 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 + // Only a message carrying OPERATOR_ORIGINATED_FLAG resets the salvage + // nudge — 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). + // also fire message.received but are not a genuine operator checkpoint. 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(); @@ -812,30 +697,9 @@ 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++; + // toolOnlyStreak is narration-sensitive: any turn with text clears it + // (same as a fresh user message), and it only drives the soft + // check-in nudge at toolOnlyTurnNudgeAt, never a stop. const turnContent = event.turn.content as readonly { type: string; name?: string; @@ -848,54 +712,13 @@ 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 ( + if ( this.toolOnlyStreak === this.modelFamilyPolicy.toolOnlyTurnNudgeAt && !this.toolOnlyNudgeFired ) { @@ -943,38 +766,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, independent of the - // structured salvage classification above. - 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/agent/model-family-policy.ts b/src/agent/model-family-policy.ts index b3367bc39..08d135bde 100644 --- a/src/agent/model-family-policy.ts +++ b/src/agent/model-family-policy.ts @@ -12,10 +12,8 @@ export interface ModelFamilyPolicy { * Consecutive tool-only assistant turns (tool calls, no text) before the * main chat director injects a one-shot wrap-up nudge. A long tool-only * streak is normal orchestration (Linear lookups, code reads, etc.) and - * must not by itself stop the session — this is a soft check-in, not a - * loop-protection trigger. The real stop signal is a repeating cycle in - * the tool-fingerprint history, independent of this threshold — see - * detectToolFingerprintThrash in subagent/stop-policy.ts. + * must not by itself stop the session — this is a soft check-in, and it + * never escalates to a pause on its own. */ toolOnlyTurnNudgeAt: number; /** Ephemeral nudge text injected at toolOnlyTurnNudgeAt. */ diff --git a/src/agent/prompts.ts b/src/agent/prompts.ts index f673128ab..bf3721fab 100644 --- a/src/agent/prompts.ts +++ b/src/agent/prompts.ts @@ -153,7 +153,7 @@ export function buildGuidelines( "- Prefer the typed spawn contract on every worker: `intent`, `success_criteria` (done-when), `do_not` (scope fence), and `report_focus` so workers finish instead of thrashing. Free-form `prompt` alone is weaker.", "- After workers return, merge their Summary/Findings into a coherent answer for the operator; do not paste raw sub-agent dumps.", "- Pass `maxTurns` on `task` when a job needs a bounded inference budget (unset is unbounded). On turn-budget salvage, re-dispatch with continuation context and a higher maxTurns only a few times on the same brief — after the re-dispatch cap, change approach instead of bumping turns again.", - "- After thrash / no-progress / repetition / never-acted salvage, do not re-dispatch an identical brief (prompt/agent/intent/success_criteria/do_not) — it is refused. Change the brief to force a re-run; maxTurns alone does not unlock it.", + "- After a thrash / no-ship / never-acted / never-edited salvage, do not re-dispatch an identical brief (prompt/agent/intent/success_criteria/do_not) — it is refused. Change the brief to force a re-run; maxTurns alone does not unlock it.", "- Use manage_tasks for your own coordination checklist; spawning workers is `task`, not manage_tasks.", "- If context is compacted automatically, do not stop tasks early due to token fear; persist progress via manage_tasks and worker reports.", ]), 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 7fca171d3..2d5f3f00b 100644 --- a/src/subagent/brief-dispatch.ts +++ b/src/subagent/brief-dispatch.ts @@ -15,8 +15,7 @@ import type { TaskIntent } from "./report.js"; import type { ForcedStopReason } 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" | "never-acted" | "never-edited"; // Every forced-stop reason a leaf can report maps 1:1 onto a salvage kind // the parent ledger cares about. @@ -44,13 +43,7 @@ export interface BriefDispatchRecord { */ export const TURN_BUDGET_STOP_AFTER_DISPATCHES = 3; -const HARD_BLOCK_SALVAGES = new Set([ - "no-ship", - "no-progress", - "repetition", - "never-acted", - "never-edited", -]); +const HARD_BLOCK_SALVAGES = new Set(["no-ship", "never-acted", "never-edited"]); export function isHardBlockSalvage(kind: BriefSalvageKind): kind is HardBlockSalvage { return HARD_BLOCK_SALVAGES.has(kind); diff --git a/src/subagent/index.test.ts b/src/subagent/index.test.ts index 03971cf38..5c4db5292 100644 --- a/src/subagent/index.test.ts +++ b/src/subagent/index.test.ts @@ -10,15 +10,11 @@ import { createSubAgentRunController, createSubAgentSessionStore, createSubAgentSpawnRegistryPlugin, - DEFAULT_SUBAGENT_REPEAT_LIMIT, disposeSubAgentSession, evaluateSubAgentStop, - fingerprintToolCalls, forcedStopReport, formatSubAgentReport, - nextToolCallStreak, parseSubAgentReport, - repetitionStopDetail, stopReasonFromReport, appendSubAgentParentHints, createBriefDispatchLedger, @@ -36,7 +32,6 @@ import { subAgentToolName, SUBAGENT_DEADLINE_MARGIN_MS, SUBAGENT_PLUGIN_SPAWN_TEARDOWN_LIMITS, - subAgentNoProgress, subAgentTurnLimitExceeded, SubAgentDirector, TaskToolArgs, @@ -153,81 +148,6 @@ describe("sub-agent stop helpers", () => { expect(subAgentTurnLimitExceeded(1_000_000, Infinity)).toBe(false); }); - test("no-progress trips at the default repeat limit", () => { - expect(DEFAULT_SUBAGENT_REPEAT_LIMIT).toBe(5); - expect(subAgentNoProgress(4, DEFAULT_SUBAGENT_REPEAT_LIMIT)).toBe(false); - expect(subAgentNoProgress(5, DEFAULT_SUBAGENT_REPEAT_LIMIT)).toBe(true); - }); - - test("legitimate polling (2-4 identical fingerprints) does not hard-stop (CL-6776)", () => { - // A worker rerunning `git status` or polling a build a few times while - // waiting must not be hard-blocked on identical re-dispatch. - for (const consecutive of [2, 3, 4]) { - expect(subAgentNoProgress(consecutive, DEFAULT_SUBAGENT_REPEAT_LIMIT)).toBe(false); - expect( - evaluateSubAgentStop({ - hasToolCalls: true, - everHadToolCalls: true, - turnsCompleted: consecutive, - maxTurns: TEST_MAX_TURNS, - consecutiveIdentical: consecutive, - repeatLimit: DEFAULT_SUBAGENT_REPEAT_LIMIT, - }), - ).toBeNull(); - } - }); - - test("a true runaway (>5 identical fingerprints) still hard-stops", () => { - expect(subAgentNoProgress(6, DEFAULT_SUBAGENT_REPEAT_LIMIT)).toBe(true); - expect( - evaluateSubAgentStop({ - hasToolCalls: true, - everHadToolCalls: true, - turnsCompleted: 6, - maxTurns: TEST_MAX_TURNS, - consecutiveIdentical: 6, - repeatLimit: DEFAULT_SUBAGENT_REPEAT_LIMIT, - }), - ).toBe("no-progress"); - }); - - test("fingerprint is null when a turn has no tool calls", () => { - expect(fingerprintToolCalls([{ type: "text" }])).toBeNull(); - }); - - test("fingerprint is stable across argument key order and multi-call order", () => { - const a = fingerprintToolCalls([ - { type: "tool_call", name: "read_file", arguments: { path: "a.ts", offset: 1 } }, - { type: "tool_call", name: "grep", arguments: { pattern: "x", path: "src" } }, - ]); - const b = fingerprintToolCalls([ - { type: "tool_call", name: "grep", arguments: { path: "src", pattern: "x" } }, - { type: "tool_call", name: "read_file", arguments: { offset: 1, path: "a.ts" } }, - ]); - expect(a).toBe(b); - expect(a).not.toBeNull(); - }); - - test("fingerprint normalizes JSON-string arguments", () => { - const objectArgs = fingerprintToolCalls([ - { type: "tool_call", name: "read_file", arguments: { path: "a.ts" } }, - ]); - const stringArgs = fingerprintToolCalls([ - { type: "tool_call", name: "read_file", arguments: JSON.stringify({ path: "a.ts" }) }, - ]); - expect(objectArgs).toBe(stringArgs); - }); - - test("changing arguments produces a different fingerprint", () => { - const first = fingerprintToolCalls([ - { type: "tool_call", name: "read_file", arguments: { path: "a.ts" } }, - ]); - const second = fingerprintToolCalls([ - { type: "tool_call", name: "read_file", arguments: { path: "b.ts" } }, - ]); - expect(first).not.toBe(second); - }); - test("evaluateSubAgentStop returns complete when tools were used and the final turn has none", () => { expect( evaluateSubAgentStop({ @@ -235,8 +155,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 2, maxTurns: 10, - consecutiveIdentical: 0, - repeatLimit: 2, }), ).toBe("complete"); }); @@ -268,8 +186,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 2, maxTurns: 10, - consecutiveIdentical: 0, - repeatLimit: 2, lastAssistantText: SUMMARY_ONLY_NARRATION, }), ).toBe("incomplete-report"); @@ -282,8 +198,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 3, maxTurns: 10, - consecutiveIdentical: 0, - repeatLimit: 2, lastAssistantText: SUMMARY_ONLY_NARRATION, incompleteReportNudgeFired: true, }), @@ -297,8 +211,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 2, maxTurns: 10, - consecutiveIdentical: 0, - repeatLimit: 2, lastAssistantText: FULL_REPORT_ENVELOPE, }), ).toBe("complete"); @@ -333,8 +245,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 2, maxTurns: 10, - consecutiveIdentical: 0, - repeatLimit: 2, lastAssistantText: FULL_REPORT_ENVELOPE, thrashState, requireEvidence: true, @@ -354,8 +264,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 2, maxTurns: 10, - consecutiveIdentical: 0, - repeatLimit: 2, lastAssistantText: FULL_REPORT_ENVELOPE, thrashState, requireEvidence: true, @@ -375,8 +283,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 2, maxTurns: 10, - consecutiveIdentical: 0, - repeatLimit: 2, lastAssistantText: FULL_REPORT_ENVELOPE, thrashState, requireEvidence: false, @@ -391,8 +297,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: false, turnsCompleted: 1, maxTurns: 10, - consecutiveIdentical: 0, - repeatLimit: 2, }), ).toBe("never-acted"); }); @@ -409,8 +313,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 5, maxTurns: 30, - consecutiveIdentical: 0, - repeatLimit: 2, thrashState, requireEdit: true, }), @@ -430,8 +332,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 40, maxTurns: 60, - consecutiveIdentical: 0, - repeatLimit: 2, thrashState: thrash, requireEdit: true, }), @@ -442,8 +342,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 40, maxTurns: 60, - consecutiveIdentical: 0, - repeatLimit: 2, thrashState: thrash, }), ).toBeNull(); @@ -461,8 +359,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 5, maxTurns: 30, - consecutiveIdentical: 0, - repeatLimit: 2, thrashState, requireEdit: true, }), @@ -476,26 +372,11 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: false, turnsCompleted: 1, maxTurns: 10, - consecutiveIdentical: 0, - repeatLimit: 2, requireEdit: true, }), ).toBe("never-acted"); }); - test("evaluateSubAgentStop prefers no-progress over turn-budget", () => { - expect( - evaluateSubAgentStop({ - hasToolCalls: true, - everHadToolCalls: true, - turnsCompleted: 10, - maxTurns: 10, - consecutiveIdentical: 2, - repeatLimit: 2, - }), - ).toBe("no-progress"); - }); - test("evaluateSubAgentStop trips turn-budget when the leaf is still making progress", () => { expect( evaluateSubAgentStop({ @@ -503,8 +384,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 10, maxTurns: 10, - consecutiveIdentical: 1, - repeatLimit: 2, }), ).toBe("turn-budget"); }); @@ -516,38 +395,10 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 5, maxTurns: 10, - consecutiveIdentical: 1, - repeatLimit: 2, }), ).toBeNull(); }); - test("evaluateSubAgentStop prefers no-progress over the turn budget", () => { - let thrash = EMPTY_THRASH_STATE; - for (let i = 0; i < 4; i++) { - thrash = nextThrashState(thrash, [ - { type: "tool_call", name: "read_file", arguments: { path: "a.ts" } }, - ]); - } - thrash = nextThrashState(thrash, [ - { type: "tool_call", name: "edit_file", arguments: { path: "a.ts" } }, - ]); - thrash = nextThrashState(thrash, [ - { type: "tool_call", name: "read_file", arguments: { path: "a.ts" } }, - ]); - expect( - evaluateSubAgentStop({ - hasToolCalls: true, - everHadToolCalls: true, - turnsCompleted: 5, - maxTurns: 20, - consecutiveIdentical: 2, - repeatLimit: 2, - thrashState: thrash, - }), - ).toBe("no-progress"); - }); - test("shell-only work is not never-edited or incomplete-report (CL-6937)", () => { const shellState = nextThrashState(EMPTY_THRASH_STATE, [ { type: "tool_call", name: "run_shell", arguments: { command: "cat src/a.ts" } }, @@ -565,8 +416,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 4, maxTurns: 30, - consecutiveIdentical: 0, - repeatLimit: 5, thrashState: shellState, requireEdit: true, requireEvidence: true, @@ -591,8 +440,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 10, maxTurns: 10, - consecutiveIdentical: 1, - repeatLimit: 2, thrashState: thrash, }), ).toBe("turn-budget"); @@ -605,8 +452,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 8, maxTurns: 10, - consecutiveIdentical: 1, - repeatLimit: 2, thrashState: EMPTY_THRASH_STATE, }), ).toBe("report-forced"); @@ -617,8 +462,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 9, maxTurns: 10, - consecutiveIdentical: 1, - repeatLimit: 2, thrashState: EMPTY_THRASH_STATE, }), ).toBeNull(); @@ -628,8 +471,6 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 10, maxTurns: 10, - consecutiveIdentical: 1, - repeatLimit: 2, thrashState: EMPTY_THRASH_STATE, }), ).toBe("turn-budget"); @@ -648,35 +489,12 @@ describe("sub-agent stop helpers", () => { everHadToolCalls: true, turnsCompleted: 5, maxTurns: 20, - consecutiveIdentical: 1, - repeatLimit: 2, thrashState: thrash, }), ).toBeNull(); }); - test("nextToolCallStreak increments on identical fingerprints and resets on change", () => { - let streak = nextToolCallStreak( - { lastFingerprint: undefined, consecutiveIdentical: 0 }, - 'read_file:{"path":"a.ts"}', - ); - expect(streak.consecutiveIdentical).toBe(1); - streak = nextToolCallStreak(streak, 'read_file:{"path":"a.ts"}'); - expect(streak.consecutiveIdentical).toBe(2); - streak = nextToolCallStreak(streak, 'read_file:{"path":"b.ts"}'); - expect(streak.consecutiveIdentical).toBe(1); - streak = nextToolCallStreak(streak, null); - expect(streak).toEqual({ lastFingerprint: undefined, consecutiveIdentical: 0 }); - }); - test("forcedStopReport is a real envelope with salvage findings, not a summarize instruction", () => { - const noProgress = forcedStopReport("no-progress", "Found auth in gate.ts"); - const parsed = parseSubAgentReport(noProgress); - expect(parsed.summary).toContain("no progress"); - expect(parsed.findings).toContain("gate.ts"); - expect(parsed.blockers.length).toBeGreaterThan(0); - expect(noProgress.toLowerCase()).not.toContain("summarize what you found"); - const budget = forcedStopReport("turn-budget", ""); const budgetParsed = parseSubAgentReport(budget); expect(budgetParsed.summary).toContain("Turn budget"); @@ -770,22 +588,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, "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. @@ -805,18 +607,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); @@ -910,27 +700,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, "repetition"); - 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", @@ -1007,8 +776,6 @@ describe("thrash edge cases", () => { everHadToolCalls: true, turnsCompleted, maxTurns, - consecutiveIdentical: 0, - repeatLimit: 2, thrashState, }); @@ -1134,8 +901,8 @@ describe("SubAgentDirector report-forced wiring", () => { }); }); - // Turn 1 of 3 fires report-forced (a nudge); repeating one identical call - // to the repeat limit then fires no-progress (a stop). + // Turn 1 of 3 fires report-forced (a nudge); continuing past maxTurns + // then fires turn-budget (a stop). for (let i = 0; i < 6; i++) { await director.decide( makeInferenceDoneEvent([{ id: "r1", name: "read_file", args: { path: "a.ts" } }]), @@ -1146,7 +913,7 @@ describe("SubAgentDirector report-forced wiring", () => { const nudge = recorded.find((r) => r.id === "report-forced"); expect(nudge?.class).toBe("nudge"); - const stop = recorded.find((r) => r.id === "no-progress"); + const stop = recorded.find((r) => r.id === "turn-budget"); expect(stop?.class).toBe("stop"); expect(stop?.value).toBeGreaterThanOrEqual(stop?.threshold ?? 0); }); @@ -1264,7 +1031,7 @@ describe("SubAgentDirector stall management", () => { test("no nudge fires before the stall timeout elapses", async () => { let now = 0; - const director = new SubAgentDirector("system", [], undefined, 30, undefined, 1000, () => now); + const director = new SubAgentDirector("system", [], undefined, 30, 1000, () => now); const capabilities = makeCapabilities(); await director.decide(toolCallDoneEvent("tc-1"), mockState, capabilities); @@ -1283,7 +1050,7 @@ describe("SubAgentDirector stall management", () => { test("first stall past the timeout gets one continuation nudge", async () => { let now = 0; - const director = new SubAgentDirector("system", [], undefined, 30, undefined, 1000, () => now); + const director = new SubAgentDirector("system", [], undefined, 30, 1000, () => now); const capabilities = makeCapabilities(); await director.decide(toolCallDoneEvent("tc-1"), mockState, capabilities); @@ -1302,7 +1069,7 @@ describe("SubAgentDirector stall management", () => { test("a second consecutive stall escalates to the salvage report", async () => { let now = 0; - const director = new SubAgentDirector("system", [], undefined, 30, undefined, 1000, () => now); + const director = new SubAgentDirector("system", [], undefined, 30, 1000, () => now); const capabilities = makeCapabilities(); await director.decide(toolCallDoneEvent("tc-1"), mockState, capabilities); @@ -1322,7 +1089,7 @@ describe("SubAgentDirector stall management", () => { test("real activity between pings resets the stall streak", async () => { let now = 0; - const director = new SubAgentDirector("system", [], undefined, 30, undefined, 1000, () => now); + const director = new SubAgentDirector("system", [], undefined, 30, 1000, () => now); const capabilities = makeCapabilities(); await director.decide(toolCallDoneEvent("tc-1"), mockState, capabilities); @@ -2155,27 +1922,27 @@ describe("brief re-dispatch ledger (CL-4343 / CL-5203)", () => { expect(changed).not.toBe(a); }); - test("hard-blocks identical brief after no-progress salvage; allows changed brief", () => { + test("hard-blocks identical brief after no-ship salvage; allows changed brief", () => { const ledger = createBriefDispatchLedger(); - const fp = fingerprintTaskBrief({ prompt: "fix no-progress job", intent: "implement" }); + const fp = fingerprintTaskBrief({ prompt: "fix no-ship job", intent: "implement" }); expect(ledger.admit(fp).ok).toBe(true); - ledger.recordOutcome(fp, "no-progress"); + ledger.recordOutcome(fp, "no-ship"); const blocked = ledger.admit(fp); expect(blocked.ok).toBe(false); if (blocked.ok) throw new Error("expected block"); expect(blocked.message).toContain("refused re-dispatch"); - expect(blocked.message).toContain("no-progress"); + expect(blocked.message).toContain("no-ship"); const other = fingerprintTaskBrief({ - prompt: "fix no-progress job with narrower scope", + prompt: "fix no-ship job with narrower scope", intent: "implement", successCriteria: ["one file only"], }); 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-ship, never-acted, never-edited; not turn-budget", () => { + for (const salvage of ["no-ship", "never-acted", "never-edited"] as const) { const ledger = createBriefDispatchLedger(); const fp = fingerprintTaskBrief({ prompt: `job ${salvage}` }); expect(ledger.admit(fp).ok).toBe(true); @@ -2215,7 +1982,7 @@ describe("brief re-dispatch ledger (CL-4343 / CL-5203)", () => { expect(ledger.admit(fp).ok).toBe(true); // One sibling salvages (hard-block class)... - ledger.recordOutcome(fp, "no-progress"); + ledger.recordOutcome(fp, "no-ship"); // ...but the other sibling succeeds in the same wave. ledger.recordOutcome(fp, null); @@ -2239,13 +2006,9 @@ describe("brief re-dispatch ledger (CL-4343 / CL-5203)", () => { // classifyBriefSalvage takes no report text at all — only the structured // stopReason and an independently-observed wasCancelled flag. expect(classifyBriefSalvage({ wasCancelled: false })).toBeNull(); - expect(classifyBriefSalvage({ stopReason: "no-progress", wasCancelled: false })).toBe( - "no-progress", - ); + expect(classifyBriefSalvage({ stopReason: "no-ship", wasCancelled: false })).toBe("no-ship"); // An operator cancel wins even when the run's own reason disagrees. - expect(classifyBriefSalvage({ stopReason: "no-progress", wasCancelled: true })).toBe( - "cancelled", - ); + expect(classifyBriefSalvage({ stopReason: "no-ship", wasCancelled: true })).toBe("cancelled"); expect(classifyBriefSalvage({ stopReason: "deadline", wasCancelled: false })).toBe("deadline"); }); @@ -2261,10 +2024,10 @@ describe("brief re-dispatch ledger (CL-4343 / CL-5203)", () => { expect(third).toContain(TURN_BUDGET_STOP_PARENT_HINT.slice(1, 40)); }); - test("createTaskTool refuses identical re-dispatch after no-progress salvage", async () => { + test("createTaskTool refuses identical re-dispatch after no-ship salvage", async () => { const thrash = { - report: forcedStopReport("no-progress", "Repeated the same call"), - stopReason: "no-progress" as const, + report: forcedStopReport("no-ship", "Repeated the same call"), + stopReason: "no-ship" as const, }; let runs = 0; const sessions = createSubAgentSessionStore(); @@ -2285,14 +2048,14 @@ describe("brief re-dispatch ledger (CL-4343 / CL-5203)", () => { intent: "implement", }; const first = await callTask(tool, args); - expect(first).toContain("no progress"); + expect(first).toContain("without writing any"); expect(first).toContain("identical brief"); expect(runs).toBe(1); expect(sessions.list().filter((s) => s.status === "running")).toHaveLength(0); const second = await callTask(tool, args); expect(second).toContain("refused re-dispatch"); - expect(second).toContain("no-progress"); + expect(second).toContain("no-ship"); expect(runs).toBe(1); // Refuse must not leave a ghost running session on the Agents strip. expect(sessions.list().filter((s) => s.status === "running")).toHaveLength(0); @@ -2310,7 +2073,7 @@ describe("brief re-dispatch ledger (CL-4343 / CL-5203)", () => { prompt: "do the thrashy work with a narrower scope", success_criteria: ["one file"], }); - expect(third).toContain("no progress"); + expect(third).toContain("without writing any"); expect(runs).toBe(2); }); diff --git a/src/subagent/index.ts b/src/subagent/index.ts index f733ebb2d..2ef21897a 100644 --- a/src/subagent/index.ts +++ b/src/subagent/index.ts @@ -45,25 +45,20 @@ export { type TaskIntent, } from "./report.js"; export { - DEFAULT_SUBAGENT_REPEAT_LIMIT, SUBAGENT_DEADLINE_MARGIN_MS, appendSubAgentParentHints, evaluateSubAgentStop, - fingerprintToolCalls, forcedStopReport, - nextToolCallStreak, partialTextFromEvent, preferCompletedSubAgentReply, resolveSubAgentCatchOutcome, resolveSubAgentDeadlineMs, - subAgentNoProgress, subAgentTurnLimitExceeded, TURN_BUDGET_STOP_PARENT_HINT, type ForcedStopReason, type SubAgentCatchOutcome, type SubAgentParentHintOptions, type SubAgentStopReason, - type ToolCallStreak, } from "./stop-policy.js"; export { @@ -105,7 +100,6 @@ export { buildSubAgentPrimarySource, coreSubAgentWebTools, createSubAgentRunController, - repetitionStopDetail, runSubAgent, shouldRequireEvidence, type SubAgentRunController, diff --git a/src/subagent/nudge-director.ts b/src/subagent/nudge-director.ts index db74e152f..c72aaab0b 100644 --- a/src/subagent/nudge-director.ts +++ b/src/subagent/nudge-director.ts @@ -23,14 +23,10 @@ import { } from "./thrash.js"; import { NOOP_INTERVENTION_SINK, type InterventionSink } from "./intervention-log.js"; import { - DEFAULT_SUBAGENT_REPEAT_LIMIT, evaluateSubAgentStop, - fingerprintToolCalls, forcedStopReport, lastText, - nextToolCallStreak, type ForcedStopReason, - type ToolCallStreak, } from "./stop-policy.js"; const REPORT_FORCED_WRAP_UP_NUDGE = @@ -79,17 +75,12 @@ function withEphemeralNudge( export class SubAgentDirector extends DefaultDirector { private readonly compaction: CompactionGovernor; private readonly maxTurns: number; - private readonly repeatLimit: number; /** When true (intent=implement), tool-less finish without edits salvages as never-edited. */ private readonly requireEdit: boolean; /** When true (CritiqueDirector), empty readCounts is not a successful complete. */ private readonly requireEvidence: boolean; private turnsCompleted = 0; private everHadToolCalls = false; - private streak: ToolCallStreak = { - lastFingerprint: undefined, - consecutiveIdentical: 0, - }; private thrashState: ThrashState = EMPTY_THRASH_STATE; // Armed for wrap-up (report-forced) or failed-tool recovery so the // follow-up infer (after pending tool calls from THIS turn have executed) @@ -116,11 +107,11 @@ export class SubAgentDirector extends DefaultDirector { // (directors are pure decide(event, ...) functions — see requestContinuation // above), so the run loop periodically pings this same continuation channel // and the director only acts on a ping if genuinely nothing happened since - // the last one. Precedence: this check sits below no-progress / - // turn-budget (evaluateSubAgentStop, above) — those fire from real - // inference.done turns and always take priority; stall pings only ever - // fire on a continuation message that inference.done/tool.done handling - // did not already consume this cycle. + // the last one. Precedence: this check sits below turn-budget + // (evaluateSubAgentStop, above) — that fires from real inference.done turns + // and always takes priority; stall pings only ever fire on a continuation + // message that inference.done/tool.done handling did not already consume + // this cycle. private readonly stallTimeoutMs: number | undefined; private readonly now: () => number; private lastActivityAt: number; @@ -167,7 +158,6 @@ export class SubAgentDirector extends DefaultDirector { toolDefinitions: ToolDefinition[], requestContinuation: (() => void) | undefined, maxTurns: number, - repeatLimit: number = DEFAULT_SUBAGENT_REPEAT_LIMIT, stallTimeoutMs?: number, now: () => number = Date.now, requireEdit = false, @@ -176,7 +166,6 @@ export class SubAgentDirector extends DefaultDirector { super(systemPrompt, toolDefinitions, {}); this.compaction = createCompactionGovernor(requestContinuation, systemPrompt, toolDefinitions); this.maxTurns = maxTurns; - this.repeatLimit = repeatLimit; this.stallTimeoutMs = stallTimeoutMs; this.now = now; this.lastActivityAt = now(); @@ -232,9 +221,7 @@ export class SubAgentDirector extends DefaultDirector { text?: string; }[]; this.lastAssistantText = lastText(content); - const fingerprint = fingerprintToolCalls(content); - this.streak = nextToolCallStreak(this.streak, fingerprint); - const hasToolCalls = fingerprint !== null; + const hasToolCalls = content.some((block) => block.type === "tool_call"); if (hasToolCalls) { this.everHadToolCalls = true; this.thrashState = nextThrashState(this.thrashState, content); @@ -245,8 +232,6 @@ export class SubAgentDirector extends DefaultDirector { everHadToolCalls: this.everHadToolCalls, turnsCompleted: this.turnsCompleted, maxTurns: this.maxTurns, - consecutiveIdentical: this.streak.consecutiveIdentical, - repeatLimit: this.repeatLimit, thrashState: this.thrashState, requireEdit: this.requireEdit, requireEvidence: this.requireEvidence, @@ -312,37 +297,23 @@ export class SubAgentDirector extends DefaultDirector { }, state: this.interventionState(), }); - } else if ( - stop === "no-progress" || - stop === "turn-budget" || - stop === "never-acted" || - stop === "never-edited" - ) { + } else if (stop === "turn-budget" || stop === "never-acted" || stop === "never-edited") { const checkpoint = - stop === "no-progress" - ? "subagent-no-progress" - : stop === "never-acted" - ? "subagent-never-acted" - : stop === "never-edited" - ? "subagent-never-edited" - : "subagent-turn-budget"; + stop === "never-acted" + ? "subagent-never-acted" + : stop === "never-edited" + ? "subagent-never-edited" + : "subagent-turn-budget"; const detail = - stop === "no-progress" - ? `identical tool call × ${this.streak.consecutiveIdentical}` - : stop === "turn-budget" - ? `${this.turnsCompleted}/${this.maxTurns} turns` - : undefined; + stop === "turn-budget" ? `${this.turnsCompleted}/${this.maxTurns} turns` : undefined; this.interventions({ id: stop, class: "stop", - measurement: - stop === "no-progress" - ? { - metric: "consecutiveIdentical", - value: this.streak.consecutiveIdentical, - threshold: this.repeatLimit, - } - : { metric: "turnsCompleted", value: this.turnsCompleted, threshold: this.maxTurns }, + measurement: { + metric: "turnsCompleted", + value: this.turnsCompleted, + threshold: this.maxTurns, + }, state: this.interventionState(), ...(detail !== undefined ? { detail } : {}), }); @@ -389,7 +360,7 @@ export class SubAgentDirector extends DefaultDirector { * First stall past the timeout: one continuation nudge, asking the leaf to * report status or keep going. A second consecutive stall (no activity * since the nudge) escalates to the existing salvage path, same shape as - * no-progress/turn-budget above. Returns null when this event is not + * turn-budget above. Returns null when this event is not * a stall check the director should act on (let it fall through as an * ordinary continuation). */ 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/report.ts b/src/subagent/report.ts index 8983f67e6..6eaf52ebf 100644 --- a/src/subagent/report.ts +++ b/src/subagent/report.ts @@ -124,7 +124,7 @@ export interface SubAgentReport { paths: string; /** * Machine-readable termination reason for a forced stop (e.g. - * `repetition — window "Groaning. " × 1363`). Rendered as a dedicated + * `turn-budget — 40/40 turns`). Rendered as a dedicated * `Stopped:` line above the envelope; absent on successful completes. */ stopped?: string; diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 8c9b1c852..7fff343f0 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"; @@ -96,7 +83,6 @@ import { subAgentToolName, } from "./report.js"; import { - DEFAULT_SUBAGENT_REPEAT_LIMIT, forcedStopReport, partialTextFromEvent, preferCompletedSubAgentReply, @@ -207,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; } @@ -257,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; @@ -641,7 +603,6 @@ export async function runSubAgent(params: RunSubAgentParams): Promise 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) { @@ -842,66 +753,6 @@ export async function runSubAgent(params: RunSubAgentParams): Promise= 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); @@ -985,12 +836,10 @@ export async function runSubAgent(params: RunSubAgentParams): Promise 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 { - report: appendActivitySummary(forcedStopReport(reason, partial, detail), toolNamesUsed), + report: appendActivitySummary(forcedStopReport(reason, tail, detail), toolNamesUsed), stopReason: reason, }; } 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 40511bade..e941ec9f6 100644 --- a/src/subagent/stop-policy.ts +++ b/src/subagent/stop-policy.ts @@ -1,23 +1,13 @@ /** - * Pure stop / salvage policy for leaf sub-agents: turn budget, no-progress, - * thrash, deadlines, and parent-facing salvage reports. + * Pure stop / salvage policy for leaf sub-agents: turn budget, thrash, + * deadlines, and parent-facing salvage reports. */ 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, formatSubAgentReport, hasReportEnvelope } from "./report.js"; -// Consecutive identical tool-call fingerprints before a leaf is forced to -// stop. Mirrors IDENTICAL_REPEAT_MIN below (the director-level period-1 -// thrash threshold): the same forensic scan found zero occurrences of even -// two consecutive identical fingerprints in local trace history, and CL-5611 -// found the previous 4-repeat hard pause false-positived on legitimate -// polling (rerunning a flaky test, polling a build) — hence a threshold set -// above 4, not at 2. -export const DEFAULT_SUBAGENT_REPEAT_LIMIT = 5; - // Minimum gap kept between an opt-in internal deadline and the outer // tool-execution watchdog, so there is time left for the salvage report to // unwind and return before the outer watchdog would discard the run wholesale. @@ -59,8 +49,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. @@ -73,12 +62,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"; @@ -88,177 +72,9 @@ export function subAgentTurnLimitExceeded(turnsCompleted: number, maxTurns: numb return turnsCompleted >= maxTurns; } -export function subAgentNoProgress(consecutiveIdentical: number, repeatLimit: number): boolean { - return consecutiveIdentical >= repeatLimit; -} - -// Stable JSON so key insertion order does not create false progress between turns. -function stableJson(value: unknown): string { - if (value === null || typeof value !== "object") return JSON.stringify(value); - if (Array.isArray(value)) return `[${value.map(stableJson).join(",")}]`; - const obj = value as Record; - const keys = Object.keys(obj).sort(); - return `{${keys.map((k) => `${JSON.stringify(k)}:${stableJson(obj[k])}`).join(",")}}`; -} - -/** Fingerprint of a turn's tool calls, or null when the turn has none. */ -export function fingerprintToolCalls( - content: readonly { type: string; name?: string; arguments?: unknown }[], -): string | null { - const parts: string[] = []; - for (const block of content) { - if (block.type !== "tool_call") continue; - const name = typeof block.name === "string" ? block.name : ""; - let args: unknown = block.arguments ?? {}; - // Some adapters hand arguments as a JSON string; normalize so fingerprints match. - if (typeof args === "string") { - try { - args = JSON.parse(args) as unknown; - } catch { - // Keep the raw string when it is not valid JSON. - } - } - parts.push(`${name}:${stableJson(args)}`); - } - if (parts.length === 0) return null; - parts.sort(); - 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" - | "no-progress" | "never-acted" | "never-edited" | "report-forced" @@ -268,9 +84,6 @@ export type SubAgentStopReason = /** * Pure stop decision for leaf workers. Null means keep running tools. * - * Precedence when tools are still firing: - * no-progress (identical fingerprints) > turn-budget (hard cap). - * Look volume never hard-stops. * "report-forced" and "incomplete-report" * are not competing stop reasons — they are one-shot signals telling the * caller to inject a wrap-up / redirect nudge and keep running; turn-budget @@ -290,9 +103,7 @@ export function evaluateSubAgentStop(input: { everHadToolCalls: boolean; turnsCompleted: number; maxTurns: number; - consecutiveIdentical: number; - repeatLimit: number; - /** When set, the near-budget force-report nudge is evaluated after no-progress. */ + /** When set, the near-budget force-report nudge is evaluated after tool-budget checks. */ thrashState?: ThrashState; thrashConfig?: Partial; /** @@ -347,8 +158,6 @@ export function evaluateSubAgentStop(input: { } return "complete"; } - // No-progress is more specific than the turn budget when both could apply. - if (subAgentNoProgress(input.consecutiveIdentical, input.repeatLimit)) return "no-progress"; if (input.thrashState !== undefined) { const thrashStop = evaluateThrashStop({ hasToolCalls: true, @@ -363,39 +172,16 @@ export function evaluateSubAgentStop(input: { return null; } -export interface ToolCallStreak { - lastFingerprint: string | undefined; - consecutiveIdentical: number; -} - -/** Advance consecutive-identical bookkeeping for one inference.done turn. */ -export function nextToolCallStreak( - prev: ToolCallStreak, - fingerprint: string | null, -): ToolCallStreak { - if (fingerprint === null) { - return { lastFingerprint: undefined, consecutiveIdentical: 0 }; - } - if (fingerprint === prev.lastFingerprint) { - return { - lastFingerprint: fingerprint, - consecutiveIdentical: prev.consecutiveIdentical + 1, - }; - } - return { lastFingerprint: fingerprint, consecutiveIdentical: 1 }; -} - // A sub-agent is a worker, not a chat partner: it runs until it stops calling // tools, at which point its final assistant text is the result handed back to // the dispatcher — unless it never called tools at all, in which case the // result is a never-acted salvage report rather than a successful implement. // It has no submit_output or ask_operator; consequential tools still go through -// the parent's permission gate (grants, auto mode, or prompts). Hard stops also -// fire on identical tool fingerprints (no-progress) and the hard turn budget -// so a looping leaf cannot burn the full budget -// with no parent-visible report. Near the budget the leaf gets a one-shot -// wrap-up nudge (report-forced) rather than a stop, so turn-budget stays -// reachable for a leaf that is genuinely still making progress. +// the parent's permission gate (grants, auto mode, or prompts). The hard turn +// budget stops a leaf that would otherwise burn the full budget with no +// parent-visible report. Near the budget the leaf gets a one-shot wrap-up +// nudge (report-forced) rather than a stop, so turn-budget stays reachable +// for a leaf that is genuinely still making progress. export function lastText(content: readonly { type: string }[]): string { for (let i = content.length - 1; i >= 0; i--) { @@ -419,7 +205,6 @@ export function partialTextFromEvent(event: ReactorEmittedEvent): string | null } export type ForcedStopReason = - | "no-progress" | "turn-budget" | "never-acted" | "never-edited" @@ -427,7 +212,6 @@ export type ForcedStopReason = | "deadline" | "no-ship" | "stalled" - | "repetition" | "incomplete-report"; // Exact Summary text rendered for each forced-stop reason. Human-facing only — @@ -435,7 +219,6 @@ export type ForcedStopReason = // structured ForcedStopReason value itself (see run.ts/task-tool.ts), never by // parsing this text back out of the report. const FORCED_STOP_SUMMARIES: Record = { - "no-progress": "Stopped: repeated the same tool calls with no progress.", "no-ship": "Stopped: implement intent searched many files without writing any.", "never-acted": "Stopped: completed without using any tools.", "never-edited": "Stopped: implement intent finished without writing any files.", @@ -443,7 +226,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.", }; @@ -452,9 +234,9 @@ const FORCED_STOP_SUMMARIES: Record = { * Build the parent-facing report when a leaf is force-stopped. There is no * further inference, so this must already be a full envelope — not an * instruction asking the finished worker to summarize. `detail` is the - * path-specific specifics (looped window × count, turn counts, cancel reason) - * rendered verbatim on the report's `Stopped:` line so the parent and the TUI - * see the cause, not just that the worker stopped. + * path-specific specifics (turn counts, cancel reason) rendered verbatim on + * the report's `Stopped:` line so the parent and the TUI see the cause, not + * just that the worker stopped. */ export function forcedStopReport( reason: ForcedStopReason, @@ -463,25 +245,21 @@ export function forcedStopReport( ): string { const summary = FORCED_STOP_SUMMARIES[reason]; const blockers = - reason === "no-progress" - ? "Identical tool-call fingerprint repeated consecutively; parent must not re-dispatch the identical brief (it will be refused) — tighten success_criteria/do_not or change approach." - : reason === "no-ship" - ? "Implement searched many files without writing any; parent must not re-dispatch the identical brief (it will be refused) — re-dispatch with an edit-first brief, tighter success_criteria, and do_not. Do not search the repo yourself first." - : reason === "never-acted" - ? "Worker returned planning/prose only (zero tool calls in the run); parent must not re-dispatch the identical brief (it will be refused) — re-dispatch only with a tighter brief, or treat findings as unexecuted." - : reason === "never-edited" - ? "Worker used tools but never called edit_file/write_file/delete_file under intent=implement; parent must not re-dispatch the identical brief (it will be refused) — re-dispatch with an edit-first brief, or treat findings as unexecuted." - : reason === "cancelled" - ? "Operator or parent cancelled the worker mid-run; parent may re-dispatch with the partial findings below." - : reason === "deadline" - ? "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 === "no-ship" + ? "Implement searched many files without writing any; parent must not re-dispatch the identical brief (it will be refused) — re-dispatch with an edit-first brief, tighter success_criteria, and do_not. Do not search the repo yourself first." + : reason === "never-acted" + ? "Worker returned planning/prose only (zero tool calls in the run); parent must not re-dispatch the identical brief (it will be refused) — re-dispatch only with a tighter brief, or treat findings as unexecuted." + : reason === "never-edited" + ? "Worker used tools but never called edit_file/write_file/delete_file under intent=implement; parent must not re-dispatch the identical brief (it will be refused) — re-dispatch with an edit-first brief, or treat findings as unexecuted." + : reason === "cancelled" + ? "Operator or parent cancelled the worker mid-run; parent may re-dispatch with the partial findings below." + : reason === "deadline" + ? "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 === "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 @@ -518,12 +296,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.]"; - /** Options for parent-hint stacking (session re-dispatch ledger state). */ export interface SubAgentParentHintOptions { /** @@ -565,10 +337,6 @@ export function appendSubAgentParentHints( return `${DEADLINE_PARENT_HINT}\n\n${report}`; case "no-ship": return `${NO_SHIP_PARENT_HINT}\n\n${report}`; - case "repetition": - return `${REPETITION_PARENT_HINT}\n\n${report}`; - case "no-progress": - return `${NO_PROGRESS_PARENT_HINT}\n\n${report}`; default: return report; } diff --git a/src/subagent/task-tool.ts b/src/subagent/task-tool.ts index 736cdda2b..acd8214ad 100644 --- a/src/subagent/task-tool.ts +++ b/src/subagent/task-tool.ts @@ -76,7 +76,7 @@ export const TaskToolArgs = type({ export const taskToolDefinition: ToolDefinition = { name: "task", description: - 'Spawn a sub-agent (a short-lived child agent) for one self-contained job. This is not a checklist item — use manage_tasks for your own work list. The sub-agent has the full file, search, and shell toolset, uses this session\'s permission gate (saved grants and auto mode when eligible; you may be prompted for other consequential actions), and returns a structured report (Summary / Findings / Blockers / Paths). Use it to parallelize exploration ("map every caller of X") or hand off a well-scoped implementation so your own context stays focused. Fire several task calls in one turn to run sub-agents in parallel. When launching multiple agents with the same profile, assign each a distinct lens in description and prompt so they do not duplicate work. The sub-agent cannot ask you questions. Depending on dispatch configuration it either shares your working tree directly, or runs isolated in its own git worktree snapshotted from your last commit — in the isolated case, any uncommitted or untracked changes in your working tree are excluded. Write a clear brief: context = durable background; prompt = actionable goal; goals = optional manage_tasks seeds. Prefer the typed spawn contract so workers finish without thrashing: intent (explore|implement|review|plan|general), success_criteria (done-when checklist), do_not (scope fence), report_focus (what Findings must cover). After no-progress / repetition / never-acted salvage, re-dispatching the identical brief (same prompt/agent/intent/success_criteria/do_not) is refused — change the brief to retry; maxTurns alone does not unlock it. Turn-budget salvage may invite a higher maxTurns a few times, then stops recommending re-dispatch until a successful complete resets the same-brief retry budget.', + 'Spawn a sub-agent (a short-lived child agent) for one self-contained job. This is not a checklist item — use manage_tasks for your own work list. The sub-agent has the full file, search, and shell toolset, uses this session\'s permission gate (saved grants and auto mode when eligible; you may be prompted for other consequential actions), and returns a structured report (Summary / Findings / Blockers / Paths). Use it to parallelize exploration ("map every caller of X") or hand off a well-scoped implementation so your own context stays focused. Fire several task calls in one turn to run sub-agents in parallel. When launching multiple agents with the same profile, assign each a distinct lens in description and prompt so they do not duplicate work. The sub-agent cannot ask you questions. Depending on dispatch configuration it either shares your working tree directly, or runs isolated in its own git worktree snapshotted from your last commit — in the isolated case, any uncommitted or untracked changes in your working tree are excluded. Write a clear brief: context = durable background; prompt = actionable goal; goals = optional manage_tasks seeds. Prefer the typed spawn contract so workers finish without thrashing: intent (explore|implement|review|plan|general), success_criteria (done-when checklist), do_not (scope fence), report_focus (what Findings must cover). After a no-ship / never-acted / never-edited salvage, re-dispatching the identical brief (same prompt/agent/intent/success_criteria/do_not) is refused — change the brief to retry; maxTurns alone does not unlock it. Turn-budget salvage may invite a higher maxTurns a few times, then stops recommending re-dispatch until a successful complete resets the same-brief retry budget.', inputSchema: { type: "object", properties: { 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..ba6e0b339 100644 --- a/src/tui/turn-state.ts +++ b/src/tui/turn-state.ts @@ -11,22 +11,12 @@ 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 the current cycle. Comfortably +// larger than what the cross-cycle fingerprint comparison needs. 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 @@ -106,20 +96,12 @@ export interface TurnState { * 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. + * ordinary and must not accumulate into an apparent loop. Bounded to + * `STREAM_TEXT_BUFFER_CHARS`; feeds the cross-cycle fingerprint comparison + * in `runningTool`, nothing else. */ 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 `consecutiveMatchingCycles` has crossed its threshold this turn. */ readonly repeating: boolean; /** * `streamTokenCount` at the moment repetition was first observed this turn. @@ -137,8 +119,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 on its own. */ readonly consecutiveMatchingCycles: number; /** @@ -166,8 +147,6 @@ export function initialTurnState(nowMs: number): TurnState { activeToolCalls: [], callIdByName: {}, streamText: "", - streamCharsSeen: 0, - repetitionCheckedAt: 0, repeating: false, repeatingSinceTokenCount: null, cycleFingerprint: null, @@ -210,8 +189,6 @@ export function turnStateOnSubmit(state: TurnState, nowMs: number): TurnState { activeToolCalls: [], callIdByName: {}, streamText: "", - streamCharsSeen: 0, - repetitionCheckedAt: 0, repeating: false, repeatingSinceTokenCount: null, cycleFingerprint: null, @@ -452,13 +429,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 +438,6 @@ const streaming = ( streamTokenCount, lastActivityAt: nowMs, streamText, - streamCharsSeen, - repetitionCheckedAt: due ? streamCharsSeen : state.repetitionCheckedAt, - repeating, - repeatingSinceTokenCount: - repeating && state.repeatingSinceTokenCount === null - ? streamTokenCount - : state.repeatingSinceTokenCount, }; }; @@ -513,8 +476,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..eae1acb6c 100644 --- a/tests/unit/director.test.ts +++ b/tests/unit/director.test.ts @@ -248,18 +248,3 @@ test("a grok provider no longer pauses a 10-turn productive tool-only streak", a false, ); }); - -test("a grok provider still pauses when the same tool call repeats without progress", 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); - expect(grokActions.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))).toBe( - true, - ); -}); 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 },