diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index bbd6f2038..624368798 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -106,7 +106,9 @@ 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 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. 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. 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. 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. +- **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 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: + `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. 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. @@ -132,7 +134,8 @@ The ChatDirector counts consecutive assistant turns that contain tool calls and `SubAgentDirector` tracks `lastActivityAt`, updated on every real `inference.done` and `tool.done`. Directors are pure `decide(event, ...)` functions with no timer of their own and the reactor has no proactive "idle" event, so a genuinely silent leaf (e.g. parked on a long-running background command with nothing else to do) produces no event for the director to react to. `runSubAgent` (`src/subagent/index.ts`) arms an external interval, at `subAgentStallTimeoutMs`, that pings the same content-less continuation channel the compaction governor uses to re-enter an idle reactor (`requestContinuation`). The director only acts on a ping if the elapsed time since `lastActivityAt` has crossed the timeout — a ping delivered while a tool call is still executing simply queues until that cycle finishes, so "no pending harness-tracked work" falls out of when the check can run at all rather than needing separate bookkeeping. The first stall past the timeout gets one continuation nudge (asking the leaf to check on the background work or report status); a second **consecutive** stall (no activity since that nudge) escalates to the existing salvage path, returning a `stalled` `forcedStopReport` with the same structured shape (summary/findings/blockers) as `no-progress` / `turn-budget` / `thrash` / `never-acted` / `never-edited`. Any real activity between pings resets the streak, so a leaf that is genuinely working through a slow single turn is never penalized. -**Precedence**: stall detection sits **below** no-progress, thrash, and turn-budget — those are evaluated from real `inference.done` turns inside `evaluateSubAgentStop` and always take priority; the stall check only ever fires on a continuation ping that inference/tool-result handling did not already consume that cycle. Report-forced (the one-shot wrap-up nudge a few turns before the turn-budget cap) and stall nudging are independent one-shot signals that can both fire across a run — one is turn-count driven, the other wall-clock driven — but neither is a competing stop reason in the sense no-progress/thrash/turn-budget are. +**Precedence**: stall detection sits **below** no-progress, thrash, and turn-budget — those are evaluated from real `inference.done` turns inside `evaluateSubAgentStop` and always take priority; the stall check only ever fires on a continuation ping that inference/tool-result handling did not already consume that cycle. Report-forced (near-budget wrap-up) and re-read-nudge (mid-run soft re-read redirect) are independent one-shot signals that can both fire across a run — one is turn-count driven, the other re-read-pressure driven — but neither is a competing stop reason in the sense no-progress/thrash/turn-budget are. Stall nudging is wall-clock driven and likewise independent of both. + The reactor only persists a response turn to `turns.jsonl` on `inference.done`, so a cycle that is cancelled, aborted, errors, or is otherwise interrupted mid-stream would leave nothing behind. A cycle-text recorder (`src/session/stream-journal.ts`) closes that gap by buffering the in-flight cycle's streamed text in memory — no writes on the happy path — and appending one JSON record (`{reason, chars, text}`) to `partial.jsonl`, alongside `turns.jsonl` in the session context dir, on abnormal cycle end. It is wired into the sub-agent run loop, the exec runner (flushed on failed sends), and the TUI runner (flushed on interrupt and on session rotation, before the context dir is repointed). diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index 85f0fa9cd..9d3719e4d 100644 --- a/docs/PRODUCT.md +++ b/docs/PRODUCT.md @@ -143,7 +143,8 @@ Corbits Code can fan work out to short-lived **sub-agents** — child agents wit - **Tasks** are checklist items owned by one agent via `manage_tasks`. - **Sub-agents** are spawned with the `task` tool (wire name kept; meaning is "spawn a child agent," not "add a checklist item"). -Dispatch uses a structured brief (context / goal / optional goals seed) and returns a structured report. The TUI Agents strip shows who is running; live tool progress updates the status bar without dumping the child transcript into the parent chat. Leaf workers hard-stop after 2 consecutive identical tool calls, when their inference-turn budget is exhausted (default 30; parent can pass `maxTurns` per dispatch; profiles and global settings can raise the default; cap 100), when they finish without ever using tools (never-acted salvage — planning/prose only is not a successful implement), or when `intent=implement` finishes after tools but without any file write/edit/delete (never-edited salvage — a pure-explore plan is not a successful implement). Each hard stop returns a salvage report so a runaway or idle child cannot quietly burn a large token budget or look done after prose alone. The parent tracks same-brief fingerprints for the session (`src/subagent/brief-dispatch.ts`): after thrash / no-progress / repetition / never-acted / never-edited salvage, an identical re-dispatch is refused — change prompt, agent, intent, success_criteria, and/or do_not to unlock a new run (`maxTurns` or tier alone does not). Turn-budget salvage still allows a few same-brief retries with a higher `maxTurns`, then flips the parent hint to stop and change approach; a successful complete resets the same-brief retry budget. +Dispatch uses a structured brief (context / goal / optional goals seed) and returns a structured report. The TUI Agents strip shows who is running; live tool progress updates the status bar without dumping the child transcript into the parent chat. Leaf workers hard-stop after 2 consecutive identical tool calls, when their inference-turn budget is exhausted (default 30; parent can pass `maxTurns` per dispatch; profiles and global settings can raise the default; cap 100), when they finish without ever using tools (never-acted salvage — planning/prose only is not a successful implement), or when `intent=implement` finishes after tools but without any file write/edit/delete (never-edited salvage — a pure-explore plan is not a successful implement). Progressive re-read thrash also hard-stops a leaf that keeps re-reading the same path past a limit; before that hard stop, a soft mid-run nudge asks implement leaves to edit or wrap up (explore leaves: expand findings / change approach — never forced to edit). Each hard stop returns a salvage report so a runaway or idle child cannot quietly burn a large token budget or look done after prose alone. + The parent tracks same-brief fingerprints for the session (`src/subagent/brief-dispatch.ts`): after thrash / no-progress / repetition / never-acted / never-edited salvage, an identical re-dispatch is refused — change prompt, agent, intent, success_criteria, and/or do_not to unlock a new run (`maxTurns` or tier alone does not). Turn-budget salvage still allows a few same-brief retries with a higher `maxTurns`, then flips the parent hint to stop and change approach; a successful complete resets the same-brief retry budget. ## Roadmap (planned, not yet shipped) diff --git a/src/subagent/index.test.ts b/src/subagent/index.test.ts index 330df428c..cc7c60aef 100644 --- a/src/subagent/index.test.ts +++ b/src/subagent/index.test.ts @@ -902,6 +902,202 @@ describe("SubAgentDirector report-forced wiring", () => { }); }); +describe("SubAgentDirector re-read-nudge wiring (CL-5813)", () => { + const mockState: ReactorState = { turns: [] } as unknown as ReactorState; + + function makeCapabilities(): ReactorCapabilities { + return { + infer: (options) => + ({ type: "infer", ...(options !== undefined ? { options } : {}) }) as ReactorAction, + executeTools: (calls, parallel, addToHistory) => + ({ type: "execute_tools", calls, parallel, addToHistory }) as ReactorAction, + suspend: (gate) => ({ type: "suspend", gate }) as ReactorAction, + fork: (mode, forkId) => ({ type: "fork", mode, forkId }) as ReactorAction, + emit: (eventType, data) => ({ type: "emit", eventType, data }) as ReactorAction, + reply: (content) => ({ type: "reply", content }) as ReactorAction, + checkpoint: (message = "") => ({ type: "checkpoint", message }) as ReactorAction, + compact: (compactor, reason) => ({ type: "compact", compactor, reason }) as ReactorAction, + wait: () => ({ type: "wait" }) as ReactorAction, + done: () => ({ type: "done" }) as ReactorAction, + }; + } + + function makeInferenceDoneEvent( + toolCalls: Array<{ id: string; name: string; args?: Record }>, + ): ReactorInboundEvent { + return { + type: "inference.done", + turn: { + role: "assistant", + model: "test", + timestamp: 0, + content: toolCalls.map((tc) => ({ + type: "tool_call", + id: tc.id, + name: tc.name, + arguments: tc.args ?? {}, + })), + }, + usage: { input: 0, output: 0 }, + source: "test", + } as unknown as ReactorInboundEvent; + } + + function makeToolDoneEvent(callId: string): ReactorInboundEvent { + return { + type: "tool.done", + result: { callId, content: "ok" }, + } as unknown as ReactorInboundEvent; + } + + function actionsArray(result: ReactorAction | ReactorAction[]): ReactorAction[] { + return Array.isArray(result) ? result : [result]; + } + + /** + * Soft re-read needs count>=3 on one path and total tools >= 8. Drive that + * over a few turns, then assert the follow-up infer carries the implement + * nudge, and that a further climb to hard thrash still stops the leaf. + */ + test("soft re-read injects implement wording once, then hard thrash still stops", async () => { + // requireEdit=true → implement wording + const director = new SubAgentDirector("system", [], undefined, 30, 2, undefined, Date.now, true); + const capabilities = makeCapabilities(); + + // Turns 1–3: three reads of the same path (still under soft min tools). + for (let i = 1; i <= 3; i++) { + await director.decide( + makeInferenceDoneEvent([{ id: `r${i}`, name: "read_file", args: { path: "hot.ts" } }]), + mockState, + capabilities, + ); + await director.decide(makeToolDoneEvent(`r${i}`), mockState, capabilities); + } + + // Turns 4–7: greps to clear reReadMinTotalTools (3 reads + 5 greps = 8). + // Soft fires on the turn that crosses total=8 with count=3. + for (let i = 1; i <= 4; i++) { + await director.decide( + makeInferenceDoneEvent([{ id: `g${i}`, name: "grep", args: { pattern: `p${i}` } }]), + mockState, + capabilities, + ); + await director.decide(makeToolDoneEvent(`g${i}`), mockState, capabilities); + } + + // 5th grep: total tools = 8, soft re-read should arm. + const softDone = makeInferenceDoneEvent([ + { id: "g5", name: "grep", args: { pattern: "p5" } }, + ]); + const softTurn = actionsArray(await director.decide(softDone, mockState, capabilities)); + // Soft is not a stop — tools still execute. + expect(softTurn.find((a) => a.type === "execute_tools")).toBeDefined(); + expect(softTurn.some((a) => a.type === "reply")).toBe(false); + + const afterSoft = actionsArray( + await director.decide(makeToolDoneEvent("g5"), mockState, capabilities), + ); + const softInfer = afterSoft.find((a) => a.type === "infer"); + expect(softInfer).toBeDefined(); + if (softInfer === undefined || softInfer.type !== "infer") throw new Error("expected infer"); + const softEphemeral = ( + softInfer.options as { ephemeralTurns?: Array<{ content: Array<{ text?: string }> }> } + )?.ephemeralTurns; + expect(softEphemeral?.[0]?.content?.[0]?.text).toContain("Edit a file"); + expect(softEphemeral?.[0]?.content?.[0]?.text).not.toContain("Expand Findings"); + + // One more read of hot.ts → hard thrash stop. + const hardDone = makeInferenceDoneEvent([ + { id: "r4", name: "read_file", args: { path: "hot.ts" } }, + ]); + const hardTurn = actionsArray(await director.decide(hardDone, mockState, capabilities)); + expect(hardTurn.some((a) => a.type === "reply")).toBe(true); + const checkpoint = hardTurn.find((a) => a.type === "checkpoint"); + expect(checkpoint).toBeDefined(); + if (checkpoint === undefined || checkpoint.type !== "checkpoint") { + throw new Error("expected checkpoint"); + } + expect(checkpoint.message).toBe("subagent-thrash"); + }); + + test("explore intent uses non-edit soft re-read wording", async () => { + // requireEdit=false (default) → explore wording + const director = new SubAgentDirector("system", [], undefined, 30); + const capabilities = makeCapabilities(); + + for (let i = 1; i <= 3; i++) { + await director.decide( + makeInferenceDoneEvent([{ id: `r${i}`, name: "read_file", args: { path: "hot.ts" } }]), + mockState, + capabilities, + ); + await director.decide(makeToolDoneEvent(`r${i}`), mockState, capabilities); + } + for (let i = 1; i <= 4; i++) { + await director.decide( + makeInferenceDoneEvent([{ id: `g${i}`, name: "grep", args: { pattern: `p${i}` } }]), + mockState, + capabilities, + ); + await director.decide(makeToolDoneEvent(`g${i}`), mockState, capabilities); + } + await director.decide( + makeInferenceDoneEvent([{ id: "g5", name: "grep", args: { pattern: "p5" } }]), + mockState, + capabilities, + ); + const afterSoft = actionsArray( + await director.decide(makeToolDoneEvent("g5"), mockState, capabilities), + ); + const infer = afterSoft.find((a) => a.type === "infer"); + expect(infer).toBeDefined(); + if (infer === undefined || infer.type !== "infer") throw new Error("expected infer"); + const text = ( + infer.options as { ephemeralTurns?: Array<{ content: Array<{ text?: string }> }> } + )?.ephemeralTurns?.[0]?.content?.[0]?.text; + expect(text).toContain("Expand Findings"); + expect(text).not.toContain("Edit a file"); + }); + + test("soft re-read nudge fires only once even while pressure stays soft", async () => { + const director = new SubAgentDirector("system", [], undefined, 30); + const capabilities = makeCapabilities(); + + for (let i = 1; i <= 3; i++) { + await director.decide( + makeInferenceDoneEvent([{ id: `r${i}`, name: "read_file", args: { path: "hot.ts" } }]), + mockState, + capabilities, + ); + await director.decide(makeToolDoneEvent(`r${i}`), mockState, capabilities); + } + for (let i = 1; i <= 5; i++) { + await director.decide( + makeInferenceDoneEvent([{ id: `g${i}`, name: "grep", args: { pattern: `p${i}` } }]), + mockState, + capabilities, + ); + await director.decide(makeToolDoneEvent(`g${i}`), mockState, capabilities); + } + + // Soft already fired on g5. Another grep keeps soft pressure (still 3 reads) + // but the follow-up infer must not re-nudge. + await director.decide( + makeInferenceDoneEvent([{ id: "g6", name: "grep", args: { pattern: "p6" } }]), + mockState, + capabilities, + ); + const second = actionsArray( + await director.decide(makeToolDoneEvent("g6"), mockState, capabilities), + ); + const infer = second.find((a) => a.type === "infer"); + expect(infer).toBeDefined(); + if (infer === undefined || infer.type !== "infer") throw new Error("expected infer"); + const ephemeral = (infer.options as { ephemeralTurns?: unknown[] } | undefined)?.ephemeralTurns; + expect(ephemeral).toBeUndefined(); + }); +}); + describe("SubAgentDirector stall management", () => { const mockState: ReactorState = { turns: [] } as unknown as ReactorState; diff --git a/src/subagent/index.ts b/src/subagent/index.ts index ff0f7e5ef..b4a79df52 100644 --- a/src/subagent/index.ts +++ b/src/subagent/index.ts @@ -24,6 +24,7 @@ export { nextThrashState, thrashForceReport, thrashFromReRead, + thrashSoftReRead, type ThrashConfig, type ThrashState, type ThrashStopReason, diff --git a/src/subagent/nudge-director.ts b/src/subagent/nudge-director.ts index eff0598f3..51d1d0196 100644 --- a/src/subagent/nudge-director.ts +++ b/src/subagent/nudge-director.ts @@ -33,10 +33,21 @@ import { const REPORT_FORCED_WRAP_UP_NUDGE = "You are close to your turn budget. Stop calling tools and write your final report now: summarize what you did, your findings, and any blockers."; -function reportForcedNudgeTurn(): ConversationTurn { +/** Implement leaves: soft re-read pressure should push toward edit or wrap-up. */ +const RE_READ_NUDGE_IMPLEMENT = + "You are re-reading the same paths without finishing. Edit a file to make progress, or stop tooling and write your final report now."; + +/** + * Explore / non-implement leaves: same soft re-read pressure, but do not force + * edit behavior — expand findings, change approach, or report. + */ +const RE_READ_NUDGE_EXPLORE = + "You are re-reading the same paths. Expand Findings, change approach, or write your final report — do not keep re-reading the same files."; + +function ephemeralNudgeTurn(text: string): ConversationTurn { return { role: "user", - content: [{ type: "text", text: REPORT_FORCED_WRAP_UP_NUDGE }], + content: [{ type: "text", text }], timestamp: Date.now(), }; } @@ -61,8 +72,11 @@ function inferWithSubAgentNudge(capabilities: ReactorCapabilities, text: string) * never a bare user turn, so the nudge can only ride the infer that follows * once the pending tool calls have actually executed. */ -function withReportForcedNudge(options: InferenceOptions | undefined): ExtendedInferenceOptions { - return { ...(options ?? {}), ephemeralTurns: [reportForcedNudgeTurn()] }; +function withEphemeralNudge( + options: InferenceOptions | undefined, + text: string, +): ExtendedInferenceOptions { + return { ...(options ?? {}), ephemeralTurns: [ephemeralNudgeTurn(text)] }; } export class SubAgentDirector extends DefaultDirector { @@ -78,12 +92,15 @@ export class SubAgentDirector extends DefaultDirector { consecutiveIdentical: 0, }; private thrashState: ThrashState = EMPTY_THRASH_STATE; - // Set on a report-forced turn so the follow-up infer (after the pending - // tool calls from THIS turn have executed) carries the wrap-up nudge. + // Set on a report-forced or re-read-nudge turn so the follow-up infer (after + // the pending tool calls from THIS turn have executed) carries the nudge. // Cannot attach the nudge to this turn's own infer: the model just emitted // tool_use blocks, and every provider requires tool_result before the next // turn — a bare nudge here would send an invalid conversation. - private pendingWrapUpNudge = false; + private pendingNudgeText: string | null = null; + // Soft re-read-nudge is one-shot per run; thrash hard-stop still fires later + // if the leaf ignores it and keeps re-reading. + private reReadNudgeFired = false; // Stall management: a leaf that goes quiet (e.g. parked on a long-running // background command with nothing else to do) produces no inbound events @@ -188,7 +205,16 @@ export class SubAgentDirector extends DefaultDirector { // to super.decide below), and arm the nudge for the infer that // follows once their results land. Turn-budget stays reachable — // this fires once, forceReportWithin turns before the cap. - this.pendingWrapUpNudge = true; + this.pendingNudgeText = REPORT_FORCED_WRAP_UP_NUDGE; + } else if (stop === "re-read-nudge") { + // Soft mid-run redirect (CL-5813). One-shot; hard thrash still stops + // the leaf if re-read pressure keeps climbing after the nudge. + if (!this.reReadNudgeFired) { + this.reReadNudgeFired = true; + this.pendingNudgeText = this.requireEdit + ? RE_READ_NUDGE_IMPLEMENT + : RE_READ_NUDGE_EXPLORE; + } } else if ( stop === "no-progress" || stop === "turn-budget" || @@ -221,7 +247,7 @@ export class SubAgentDirector extends DefaultDirector { this.consecutiveStalls = 0; } const base = await super.decide(event, state, capabilities); - const actions = this.applyPendingWrapUpNudge( + const actions = this.applyPendingNudge( Array.isArray(base) ? base : [base], capabilities, ); @@ -271,21 +297,22 @@ export class SubAgentDirector extends DefaultDirector { /** * Rewrite the infer action in a fall-through actions batch to carry the - * armed wrap-up nudge, once — this only ever matches the infer that - * follows the report-forced turn's tool results (super.decide only emits + * armed nudge, once — this only ever matches the infer that follows a + * report-forced or re-read-nudge turn's tool results (super.decide only emits * infer once pendingToolResults reaches zero). */ - private applyPendingWrapUpNudge( + private applyPendingNudge( actions: ReactorAction[], capabilities: ReactorCapabilities, ): ReactorAction[] { - if (!this.pendingWrapUpNudge) return actions; + if (this.pendingNudgeText === null) return actions; const inferIndex = actions.findIndex((action) => action.type === "infer"); if (inferIndex === -1) return actions; - this.pendingWrapUpNudge = false; + const text = this.pendingNudgeText; + this.pendingNudgeText = null; const existing = actions[inferIndex] as Extract; const rewritten = [...actions]; - rewritten[inferIndex] = capabilities.infer(withReportForcedNudge(existing.options)); + rewritten[inferIndex] = capabilities.infer(withEphemeralNudge(existing.options, text)); return rewritten; } } diff --git a/src/subagent/stop-policy.ts b/src/subagent/stop-policy.ts index c897d9d1e..0ef46910c 100644 --- a/src/subagent/stop-policy.ts +++ b/src/subagent/stop-policy.ts @@ -134,16 +134,17 @@ export type SubAgentStopReason = | "never-acted" | "never-edited" | "thrash" - | "report-forced"; + | "report-forced" + | "re-read-nudge"; /** * Pure stop decision for leaf workers. Null means keep running tools. * * Precedence when tools are still firing: * no-progress (identical fingerprints) > thrash (re-read pressure) > - * turn-budget (hard cap). "report-forced" is not a competing stop reason — - * it is a one-shot signal, forceReportWithin turns before the cap, telling - * the caller to inject a wrap-up nudge and keep running; turn-budget remains + * turn-budget (hard cap). "report-forced" and "re-read-nudge" 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 and thrash remain * reachable afterward. Tool-less turns always end the leaf as complete, * never-acted, or never-edited. */ diff --git a/src/subagent/thrash.test.ts b/src/subagent/thrash.test.ts index 322299ec8..fac001f28 100644 --- a/src/subagent/thrash.test.ts +++ b/src/subagent/thrash.test.ts @@ -6,6 +6,7 @@ import { nextThrashState, thrashForceReport, thrashFromReRead, + thrashSoftReRead, type ThrashState, type ThrashToolCallBlock, } from "./thrash.js"; @@ -39,8 +40,10 @@ function applyAll(calls: ReadonlyArray): ThrashState { } describe("thrash pure module", () => { - test("defaults are conservative (reReadLimit 4, forceReportWithin 2)", () => { + test("defaults are conservative (reReadLimit 4, soft 3, forceReportWithin 2)", () => { expect(DEFAULT_THRASH_CONFIG.reReadLimit).toBe(4); + expect(DEFAULT_THRASH_CONFIG.reReadSoftLimit).toBe(3); + expect(DEFAULT_THRASH_CONFIG.reReadSoftLimit).toBeLessThan(DEFAULT_THRASH_CONFIG.reReadLimit); expect(DEFAULT_THRASH_CONFIG.forceReportWithin).toBe(2); expect(DEFAULT_THRASH_CONFIG.reReadMinTotalTools).toBeGreaterThanOrEqual( DEFAULT_THRASH_CONFIG.reReadLimit, @@ -209,6 +212,68 @@ describe("thrash pure module", () => { expect(thrashForceReport(3, 3, true)).toBe(false); }); + test("soft re-read pressure fires re-read-nudge before hard thrash (CL-5813)", () => { + // 3 reads of one path + 5 greps = 8 tools → soft (limit 3), not hard (limit 4). + const softCalls: ThrashToolCallBlock[] = []; + for (let i = 0; i < 3; i++) softCalls.push(read("hot.ts")); + for (let i = 0; i < 5; i++) softCalls.push(grep(`p${i}`)); + const soft = applyAll(softCalls); + expect(soft.totalToolCalls).toBe(8); + expect(thrashFromReRead(soft)).toBe(false); + expect(thrashSoftReRead(soft)).toBe(true); + expect( + evaluateThrashStop({ + state: soft, + hasToolCalls: true, + turnsCompleted: 5, + maxTurns: 30, + }), + ).toBe("re-read-nudge"); + + // One more read of the same path crosses hard thrash. + const hard = nextThrashState(soft, [read("hot.ts")]); + expect(thrashFromReRead(hard)).toBe(true); + expect(thrashSoftReRead(hard)).toBe(false); + expect( + evaluateThrashStop({ + state: hard, + hasToolCalls: true, + turnsCompleted: 6, + maxTurns: 30, + }), + ).toBe("thrash"); + }); + + test("soft re-read still requires min total tools", () => { + // 3 reads only — under reReadMinTotalTools. + const under = applyAll([read("a.ts"), read("a.ts"), read("a.ts")]); + expect(thrashSoftReRead(under)).toBe(false); + expect( + evaluateThrashStop({ + state: under, + hasToolCalls: true, + turnsCompleted: 3, + maxTurns: 30, + }), + ).toBeNull(); + }); + + test("report-forced is preferred over re-read-nudge when both apply", () => { + const softCalls: ThrashToolCallBlock[] = []; + for (let i = 0; i < 3; i++) softCalls.push(read("hot.ts")); + for (let i = 0; i < 5; i++) softCalls.push(grep(`p${i}`)); + const soft = applyAll(softCalls); + // maxTurns=10, forceReportWithin=2 → report-forced at turnsCompleted === 8. + expect( + evaluateThrashStop({ + state: soft, + hasToolCalls: true, + turnsCompleted: 8, + maxTurns: 10, + }), + ).toBe("report-forced"); + }); + test("thrash is preferred over report-forced when both apply", () => { const path = "hot.ts"; const state = applyAll([ diff --git a/src/subagent/thrash.ts b/src/subagent/thrash.ts index 62cdc4898..39c38ba2a 100644 --- a/src/subagent/thrash.ts +++ b/src/subagent/thrash.ts @@ -5,16 +5,23 @@ * identical tool fingerprints. Wired into SubAgentDirector via evaluateSubAgentStop. * * Precedence when both thrash signals and existing stop helpers apply: - * no-progress > thrash > turn-budget. report-forced is not a competing stop — - * it fires once, at forceReportWithin turns before the cap, as a signal to - * inject a wrap-up nudge; the leaf keeps running toward turn-budget after that. - * Tool-less turns stay owned by evaluateSubAgentStop (complete / never-acted). + * no-progress > thrash > turn-budget. Soft re-read-nudge and report-forced are + * not competing stops — they fire as one-shot wrap-up / redirect nudges; the leaf + * keeps running. report-forced is preferred over re-read-nudge when both apply + * (near-budget wrap-up is more urgent than a mid-run redirect). Tool-less turns + * stay owned by evaluateSubAgentStop (complete / never-acted / never-edited). */ /** Tunable thresholds for thrash / force-report detection. */ export type ThrashConfig = { - /** Same path read this many times triggers re-read pressure. */ + /** Same path read this many times triggers hard re-read thrash stop. */ reReadLimit: number; + /** + * Soft re-read pressure threshold (must be < reReadLimit). Crossing it injects + * a one-shot mid-run nudge without stopping the leaf; hard thrash still fires + * if the leaf keeps re-reading past reReadLimit. + */ + reReadSoftLimit: number; /** * Without a prior edit of the path, re-read pressure also requires at least * this many total tool calls in the run (keeps multi-chunk legitimate reads @@ -31,6 +38,7 @@ export type ThrashConfig = { /** Conservative defaults: 20 unique single-path reads must not thrash. */ export const DEFAULT_THRASH_CONFIG: ThrashConfig = { reReadLimit: 4, + reReadSoftLimit: 3, reReadMinTotalTools: 8, forceReportWithin: 2, }; @@ -48,8 +56,13 @@ export const EMPTY_THRASH_STATE: ThrashState = { totalToolCalls: 0, }; -/** Thrash-module stop reasons; "thrash" is a real stop, "report-forced" a wrap-up-nudge signal. */ -export type ThrashStopReason = "thrash" | "report-forced"; +/** + * Thrash-module stop reasons. + * - "thrash" is a real stop + * - "report-forced" is a near-budget wrap-up-nudge signal + * - "re-read-nudge" is a mid-run soft re-read redirect (one-shot, not a stop) + */ +export type ThrashStopReason = "thrash" | "report-forced" | "re-read-nudge"; /** Content block shape compatible with fingerprintToolCalls / inference turns. */ export type ThrashToolCallBlock = { @@ -149,6 +162,22 @@ export function nextThrashState( }; } +/** + * True when any path's re-read count meets `limit` and total tool volume clears + * the min-tools gate. Shared by hard thrash and soft re-read-nudge. + */ +function reReadPressureAt( + state: ThrashState, + limit: number, + minTotalTools: number, +): boolean { + if (state.totalToolCalls < minTotalTools) return false; + for (const count of state.readCounts.values()) { + if (count >= limit) return true; + } + return false; +} + /** * True when re-read pressure indicates progressive thrash. Gated on total * tool volume regardless of whether the path was edited — an ordinary @@ -161,12 +190,22 @@ export function thrashFromReRead( state: ThrashState, config: ThrashConfig = DEFAULT_THRASH_CONFIG, ): boolean { - const { reReadLimit, reReadMinTotalTools } = config; - if (state.totalToolCalls < reReadMinTotalTools) return false; - for (const count of state.readCounts.values()) { - if (count >= reReadLimit) return true; - } - return false; + return reReadPressureAt(state, config.reReadLimit, config.reReadMinTotalTools); +} + +/** + * True when re-read pressure has crossed the soft threshold but not yet hard + * thrash. Used to inject a one-shot mid-run redirect before the leaf burns + * the rest of its budget re-reading the same paths. + */ +export function thrashSoftReRead( + state: ThrashState, + config: ThrashConfig = DEFAULT_THRASH_CONFIG, +): boolean { + const soft = Math.min(config.reReadSoftLimit, config.reReadLimit - 1); + if (soft < 1) return false; + if (thrashFromReRead(state, config)) return false; + return reReadPressureAt(state, soft, config.reReadMinTotalTools); } /** @@ -194,6 +233,7 @@ function resolveConfig(partial?: Partial): ThrashConfig { if (partial === undefined) return DEFAULT_THRASH_CONFIG; return { reReadLimit: partial.reReadLimit ?? DEFAULT_THRASH_CONFIG.reReadLimit, + reReadSoftLimit: partial.reReadSoftLimit ?? DEFAULT_THRASH_CONFIG.reReadSoftLimit, reReadMinTotalTools: partial.reReadMinTotalTools ?? DEFAULT_THRASH_CONFIG.reReadMinTotalTools, forceReportWithin: partial.forceReportWithin ?? DEFAULT_THRASH_CONFIG.forceReportWithin, @@ -201,13 +241,13 @@ function resolveConfig(partial?: Partial): ThrashConfig { } /** - * Pure thrash / force-report decision. Null means keep running (or defer to - * evaluateSubAgentStop for tool-less / fingerprint / hard budget). "thrash" is - * a real stop; "report-forced" is a one-shot wrap-up-nudge signal, not a stop - * — the caller injects a nudge and keeps running toward turn-budget. + * Pure thrash / force-report / soft re-read decision. Null means keep running + * (or defer to evaluateSubAgentStop for tool-less / fingerprint / hard budget). + * "thrash" is a real stop; "report-forced" and "re-read-nudge" are one-shot + * nudge signals, not stops — the caller injects a nudge and keeps running. * * Only evaluates when hasToolCalls is true — tool-less endings are not thrash. - * Prefers thrash over report-forced when both apply. + * Prefers thrash > report-forced > re-read-nudge when multiple apply. */ export function evaluateThrashStop(input: { state: ThrashState; @@ -224,5 +264,6 @@ export function evaluateThrashStop(input: { ) { return "report-forced"; } + if (thrashSoftReRead(input.state, config)) return "re-read-nudge"; return null; }