diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a59b8da94..14989ff30 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -106,7 +106,7 @@ In TUI chat mode there is no completion gate — the session stays open across t Two directors, selected by role: - **ChatDirector** (interactive, `src/agent/director.ts`) — Extends `DefaultDirector` with task list tracking, workflow nudges, LSP auto-activation, and multi-turn chat semantics. It never terminates the session: operator declines are surfaced as replies and the reactor stays alive for the next message. Auto mode is toggled by CLI flags (`--auto` / `--no-auto`); there is currently no in-session key to toggle it (default on; constrained envelope — workspace writes and unconstrained shell auto-allow; installs, recursive rm, force/uncontained worktree changes, sensitive-path and opaque-wrapper shell still ask; contained non-force `git worktree add`/`remove`/`prune` and `list` auto-allow; shell file-mutation denied). It is not a separate edit/plan mode. -- **SubAgentDirector** (delegated work, `src/subagent/index.ts`) — Drives a dispatched worker until a turn arrives with no tool calls, then replies with the final assistant text and ends the run. A tool-less turn **after tools** completes only with the four-heading envelope (Summary, Findings, Blockers, Paths); a missing envelope nudges once then salvages as **incomplete-report**. A tool-less completion with **zero tool calls in the entire run** is returned as a **never-acted** salvage report (not a successful implement). When `task(intent="implement")` is set, a tool-using run that never wrote/edited/deleted a file is returned as **never-edited** instead of complete — so a pure-explore "plan" cannot look shipped to the parent (tracked via `thrashState.editedPaths` from `edit_file` / `write_file` / `delete_file`). Explore/read-only workers that used tools then replied with findings remain normal completes. Hard stops also fire after 2 consecutive identical tool-call fingerprints (**no-progress**), on progressive re-read pressure (**thrash** — the same path re-read past a limit amid enough tool volume, tracked by `src/subagent/thrash.ts`), or after the leaf turn budget (**turn-budget**, default 30, overridable via `task(maxTurns)`, agent profile `maxTurns`, or `settings.subagentMaxTurns`, capped at 100), each returning a structured salvage report (reason, partial findings, blockers) so a thrashing child cannot burn tokens indefinitely. Before hard thrash, a one-shot **re-read-nudge** fires when re-read pressure crosses a soft threshold (default 3 same-path reads with enough tool volume, still below the hard re-read limit of 4): the director injects an ephemeral redirect — implement leaves are asked to edit or wrap up; explore leaves are asked to expand findings / change approach / report, never forced into edit — then keeps running so hard thrash remains reachable if the leaf ignores it. A fourth hard stop, **repetition**, is detected outside the director entirely: +- **SubAgentDirector** (delegated work, `src/subagent/index.ts`) — Drives a dispatched worker until a turn arrives with no tool calls, then replies with the final assistant text and ends the run. A tool-less turn **after tools** completes only with the four-heading envelope (Summary, Findings, Blockers, Paths); a missing envelope nudges once then salvages as **incomplete-report**. A tool-less completion with **zero tool calls in the entire run** is returned as a **never-acted** salvage report (not a successful implement). When `task(intent="implement")` is set, a tool-using run that never wrote/edited/deleted a file is returned as **never-edited** instead of complete — so a pure-explore "plan" cannot look shipped to the parent (tracked via `thrashState.editedPaths` from `edit_file` / `write_file` / `delete_file`). Explore/read-only workers that used tools then replied with findings remain normal completes. Hard stops also fire after 5 consecutive identical tool-call fingerprints (**no-progress**, mirroring the director-level `IDENTICAL_REPEAT_MIN` threshold), on progressive re-read pressure (**thrash** — the same path re-read past a limit amid enough tool volume, tracked by `src/subagent/thrash.ts`), or after the leaf turn budget (**turn-budget**, default 30, overridable via `task(maxTurns)`, agent profile `maxTurns`, or `settings.subagentMaxTurns`, capped at 100), each returning a structured salvage report (reason, partial findings, blockers) so a thrashing child cannot burn tokens indefinitely. Before hard thrash, a one-shot **re-read-nudge** fires when re-read pressure crosses a soft threshold (default 3 same-path reads with enough tool volume, still below the hard re-read limit of 4): the director injects an ephemeral redirect — implement leaves are asked to edit or wrap up; explore leaves are asked to expand findings / change approach / report, never forced into edit — then keeps running so hard thrash remains reachable if the leaf ignores it. A fourth hard stop, **repetition**, is detected outside the director entirely: `runSubAgent`'s stream sink watches the streamed text of the in-flight cycle for degenerate token loops (`src/subagent/repetition.ts`) — format chars (ZWSP, BOM, bidi marks, soft hyphen, …) stripped then whitespace-collapsed raw text, a smallest-period KMP check over the probe tail, default window >= 16 chars repeated >= 8 times, evaluated every 256 streamed chars — and on a hit aborts the run controller mid-cycle, returning a `repetition` salvage report that leads with the looped window and warns the parent against re-dispatching the identical brief. `inference.thinking.delta` is sampled the same way on its own buffer, but with digit runs folded to one placeholder and a shorter window (>= 4 chars repeated >= 32 times), gated to periods <= 16 chars once folded: thinking is never rendered to the user, so a monotonic counter (e.g. `0/1 1/2 2/3 …`, which stays non-periodic and escapes the raw-text check) can be caught, but folding still erases real information — a healthy templated enumeration line becomes byte-identical to its neighbors once digits are erased, so the period-length cap only lets counter-shaped folded periods (a handful of chars) through and refuses the much longer periods a folded prose line produces. Because directors only see completed turns, this is the only stop that can catch a loop inside a single turn that never finishes. A one-shot **report-forced** signal fires a few turns before the cap while the leaf is still tooling — it is not a stop: the director injects a wrap-up nudge and lets the leaf finish on its own, so turn-budget stays reachable for a leaf still making progress. When both report-forced and re-read-nudge apply, report-forced wins (near-budget wrap-up is more urgent than a mid-run redirect). Operator/parent cancel after any progress likewise returns a **cancelled** salvage report (partial findings + tool activity) instead of a bare cancel string; cancel before progress still surfaces as cancelled-by-operator. Optional `task(tier=)` (`fast` | `standard` | `clever`) overrides profile inference, profile tier, and the parent provider for that spawn only, and fails closed when the tier is unconfigured. The parent `task` tool keeps a session-scoped brief-dispatch ledger (`src/subagent/brief-dispatch.ts`): fingerprints cover prompt + agent + intent + success_criteria + do_not (not maxTurns/description/tier). After thrash / no-progress / repetition / never-acted / never-edited salvage, an identical re-dispatch is hard-blocked for the rest of the parent chat; change at least one fingerprint field to force a re-run. Turn-budget salvage still invites a higher maxTurns for a few same-brief retries without a successful complete, then flips the parent hint to stop and change approach (soft — further identical dispatches are still admitted). A successful complete resets the same-brief retry budget. diff --git a/src/subagent/index.test.ts b/src/subagent/index.test.ts index 19bbc39f0..9feca3b2d 100644 --- a/src/subagent/index.test.ts +++ b/src/subagent/index.test.ts @@ -162,9 +162,41 @@ describe("sub-agent stop helpers", () => { }); test("no-progress trips at the default repeat limit", () => { - expect(DEFAULT_SUBAGENT_REPEAT_LIMIT).toBe(2); - expect(subAgentNoProgress(1, DEFAULT_SUBAGENT_REPEAT_LIMIT)).toBe(false); - expect(subAgentNoProgress(2, DEFAULT_SUBAGENT_REPEAT_LIMIT)).toBe(true); + 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: DEFAULT_SUBAGENT_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: DEFAULT_SUBAGENT_MAX_TURNS, + consecutiveIdentical: 6, + repeatLimit: DEFAULT_SUBAGENT_REPEAT_LIMIT, + }), + ).toBe("no-progress"); }); test("fingerprint is null when a turn has no tool calls", () => { diff --git a/src/subagent/stop-policy.ts b/src/subagent/stop-policy.ts index 1291c45e7..061e58e65 100644 --- a/src/subagent/stop-policy.ts +++ b/src/subagent/stop-policy.ts @@ -18,8 +18,14 @@ import { parseSubAgentReport, } from "./report.js"; -/** Consecutive identical tool-call fingerprints before a leaf is forced to stop. */ -export const DEFAULT_SUBAGENT_REPEAT_LIMIT = 2; +// 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