Skip to content

Commit c5a8719

Browse files
committed
Stop implement leaves that never edit, and catch ZWSP loops
intent=implement was prompt-only: a leaf that only read/searched then wrote a "plan" looked like a successful complete to the parent. Track editedPaths already recorded by thrash state and salvage as never-edited when requireEdit is on and no write/edit/delete happened. Hard-block identical re-dispatch for that salvage the same way as never-acted. Also strip format characters (ZWSP, BOM, bidi marks, soft hyphen) in the stream repetition normalizer so invisible separators cannot break periodicity detection — observed in live thrash fleets.
1 parent ff84297 commit c5a8719

10 files changed

Lines changed: 158 additions & 30 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ In TUI chat mode there is no completion gate — the session stays open across t
106106
Two directors, selected by role:
107107

108108
- **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.
109-
- **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); 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`) — 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 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.
109+
- **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.
110110

111111

112112

@@ -130,7 +130,7 @@ The ChatDirector counts consecutive assistant turns that contain tool calls and
130130

131131
#### Sub-agent stall management
132132

133-
`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`. Any real activity between pings resets the streak, so a leaf that is genuinely working through a slow single turn is never penalized.
133+
`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.
134134

135135
**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.
136136

‎docs/PRODUCT.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ Corbits Code can fan work out to short-lived **sub-agents** — child agents wit
143143
- **Tasks** are checklist items owned by one agent via `manage_tasks`.
144144
- **Sub-agents** are spawned with the `task` tool (wire name kept; meaning is "spawn a child agent," not "add a checklist item").
145145

146-
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), or when they finish without ever using tools (never-acted salvage — planning/prose only 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 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.
146+
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.
147147

148148
## Roadmap (planned, not yet shipped)
149149

‎src/subagent/brief-dispatch.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { parseSubAgentReport } from "./report.js";
1616
import {
1717
isDeadlineSubAgentReport,
1818
isNeverActedSubAgentReport,
19+
isNeverEditedSubAgentReport,
1920
isNoProgressSubAgentReport,
2021
isRepetitionSubAgentReport,
2122
isThrashSubAgentReport,
@@ -27,7 +28,8 @@ export type HardBlockSalvage =
2728
| "thrash"
2829
| "no-progress"
2930
| "repetition"
30-
| "never-acted";
31+
| "never-acted"
32+
| "never-edited";
3133

3234
export type BriefSalvageKind =
3335
| HardBlockSalvage
@@ -63,6 +65,7 @@ const HARD_BLOCK_SALVAGES = new Set<BriefSalvageKind>([
6365
"no-progress",
6466
"repetition",
6567
"never-acted",
68+
"never-edited",
6669
]);
6770

6871
export function isHardBlockSalvage(kind: BriefSalvageKind): kind is HardBlockSalvage {
@@ -89,6 +92,7 @@ export function classifyBriefSalvage(report: string): BriefSalvageKind | null {
8992
// Order: more specific salvage phrases first.
9093
if (isThrashSubAgentReport(report)) return "thrash";
9194
if (isRepetitionSubAgentReport(report)) return "repetition";
95+
if (isNeverEditedSubAgentReport(report)) return "never-edited";
9296
if (isNeverActedSubAgentReport(report)) return "never-acted";
9397
if (isNoProgressSubAgentReport(report)) return "no-progress";
9498
if (isTurnBudgetSubAgentReport(report)) return "turn-budget";

‎src/subagent/index.test.ts‎

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,60 @@ describe("sub-agent stop helpers", () => {
227227
).toBe("never-acted");
228228
});
229229

230+
test("evaluateSubAgentStop returns never-edited when requireEdit and tools never wrote files", () => {
231+
const thrashState = {
232+
totalToolCalls: 4,
233+
readCounts: new Map([["src/a.ts", 2]]),
234+
editedPaths: new Set<string>(),
235+
};
236+
expect(
237+
evaluateSubAgentStop({
238+
hasToolCalls: false,
239+
everHadToolCalls: true,
240+
turnsCompleted: 5,
241+
maxTurns: 30,
242+
consecutiveIdentical: 0,
243+
repeatLimit: 2,
244+
thrashState,
245+
requireEdit: true,
246+
}),
247+
).toBe("never-edited");
248+
});
249+
250+
test("evaluateSubAgentStop still completes implement when an edit path was recorded", () => {
251+
const thrashState = {
252+
totalToolCalls: 4,
253+
readCounts: new Map([["src/a.ts", 1]]),
254+
editedPaths: new Set(["src/a.ts"]),
255+
};
256+
expect(
257+
evaluateSubAgentStop({
258+
hasToolCalls: false,
259+
everHadToolCalls: true,
260+
turnsCompleted: 5,
261+
maxTurns: 30,
262+
consecutiveIdentical: 0,
263+
repeatLimit: 2,
264+
thrashState,
265+
requireEdit: true,
266+
}),
267+
).toBe("complete");
268+
});
269+
270+
test("evaluateSubAgentStop ignores requireEdit when the run never used tools (never-acted wins)", () => {
271+
expect(
272+
evaluateSubAgentStop({
273+
hasToolCalls: false,
274+
everHadToolCalls: false,
275+
turnsCompleted: 1,
276+
maxTurns: 10,
277+
consecutiveIdentical: 0,
278+
repeatLimit: 2,
279+
requireEdit: true,
280+
}),
281+
).toBe("never-acted");
282+
});
283+
230284
test("evaluateSubAgentStop prefers no-progress over turn-budget", () => {
231285
expect(
232286
evaluateSubAgentStop({
@@ -414,6 +468,10 @@ describe("sub-agent stop helpers", () => {
414468
expect(neverParsed.blockers).toContain("unexecuted");
415469
expect(neverActed.toLowerCase()).not.toContain("summarize what you found");
416470

471+
const neverEdited = forcedStopReport("never-edited", "I mapped the files; ready to code next");
472+
expect(neverEdited).toContain("without writing any files");
473+
expect(appendSubAgentParentHints(neverEdited)).toContain("edit-first");
474+
417475
const thrashReport = forcedStopReport("thrash", "Re-read a.ts after edit");
418476
const thrashParsed = parseSubAgentReport(thrashReport);
419477
expect(thrashParsed.summary).toContain("progressive thrash");
@@ -1658,8 +1716,8 @@ describe("brief re-dispatch ledger (CL-4343 / CL-5203)", () => {
16581716
expect(ledger.admit(other).ok).toBe(true);
16591717
});
16601718

1661-
test("hard-blocks no-progress, repetition, never-acted; not turn-budget", () => {
1662-
for (const salvage of ["no-progress", "repetition", "never-acted"] as const) {
1719+
test("hard-blocks no-progress, repetition, never-acted, never-edited; not turn-budget", () => {
1720+
for (const salvage of ["no-progress", "repetition", "never-acted", "never-edited"] as const) {
16631721
const ledger = createBriefDispatchLedger();
16641722
const fp = fingerprintTaskBrief({ prompt: `job ${salvage}` });
16651723
expect(ledger.admit(fp).ok).toBe(true);
@@ -1713,6 +1771,7 @@ describe("brief re-dispatch ledger (CL-4343 / CL-5203)", () => {
17131771
expect(classifyBriefSalvage(forcedStopReport("no-progress", "x"))).toBe("no-progress");
17141772
expect(classifyBriefSalvage(forcedStopReport("repetition", "x"))).toBe("repetition");
17151773
expect(classifyBriefSalvage(forcedStopReport("never-acted", "x"))).toBe("never-acted");
1774+
expect(classifyBriefSalvage(forcedStopReport("never-edited", "x"))).toBe("never-edited");
17161775
expect(classifyBriefSalvage(forcedStopReport("turn-budget", "x"))).toBe("turn-budget");
17171776
expect(classifyBriefSalvage("## Summary\nDone\n\n## Findings\nok\n\n## Blockers\nNone\n\n## Paths\n")).toBeNull();
17181777
});

‎src/subagent/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ export {
5454
forcedStopReport,
5555
isDeadlineSubAgentReport,
5656
isNeverActedSubAgentReport,
57+
isNeverEditedSubAgentReport,
5758
isNoProgressSubAgentReport,
5859
isRepetitionSubAgentReport,
5960
isThrashSubAgentReport,

0 commit comments

Comments
 (0)