Skip to content

Commit fd9ac4e

Browse files
committed
Stop treating mid-run worker narration as a finished report
A tool-less turn after tools is complete only when the four-heading envelope is present. Missing headings get one wrap-up nudge, then an incomplete-report salvage so identical re-dispatch is blocked. Greybeard is told to review itself and never spawn a parallel diagnostic fleet.
1 parent b207592 commit fd9ac4e

12 files changed

Lines changed: 470 additions & 40 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 1 addition & 1 deletion
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). 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:
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 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:
110110
`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.
111111
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.
112112

‎src/agent/directors/greybeard/package.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ describe("greybeardPackage", () => {
3232
expect(allow).not.toContain("plan");
3333
});
3434

35+
test("systemPrompt forbids parallel diagnostic fleets", () => {
36+
expect(greybeardPackage.systemPrompt).toMatch(/do the review yourself/i);
37+
expect(greybeardPackage.systemPrompt).toMatch(/spawn at most one intern/i);
38+
expect(greybeardPackage.systemPrompt).toMatch(/never spawn a parallel diagnostic fleet/i);
39+
});
40+
3541
test("tools.allow is orchestrator surface without product writes", () => {
3642
const allow = greybeardPackage.tools?.allow ?? [];
3743
expect(allow).toContain("task");

‎src/agent/directors/greybeard/package.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,16 @@ export const greybeardPackage: DirectorPackage = {
2222
nudge: { maxTurns: 50 },
2323
report: { requiredSections: ["Summary", "Findings", "Blockers", "Paths"] },
2424
modelRole: "review",
25-
systemPrompt: `You are GreybeardDirector, a leaf director in Corbits Code.
25+
systemPrompt: `You are GreybeardDirector, a specialist in Corbits Code.
2626
2727
PRIMARY INTENT: architecture review. Judge soundness, constraint ownership, and backward-compatibility implications. Do not fix or ship product code.
2828
2929
Load style and philosophy when reviewing plans or approaches — skills are active constraints, not background docs.
3030
3131
You may spawn only intern, explore, and critique for evidence gathering. Do not spawn implement, plan, skywalker, or other directors. Your value is analysis, not legwork or implementation.
3232
33+
Do the review yourself. Spawn at most one intern, explore, or critique evidence leaf when a single unknown path blocks you. Never spawn a parallel diagnostic fleet.
34+
3335
Focus on:
3436
- Architectural holes, anti-patterns, missing invariants
3537
- Constraint ownership (fixed at the right layer, not symptom-chasing)

‎src/subagent/brief-dispatch.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
isNeverActedSubAgentReport,
1919
isNeverEditedSubAgentReport,
2020
isNoProgressSubAgentReport,
21+
isNoShipSubAgentReport,
2122
isRepetitionSubAgentReport,
2223
isThrashSubAgentReport,
2324
isTurnBudgetSubAgentReport,
@@ -26,6 +27,7 @@ import {
2627
/** Salvage classes that must not be re-dispatched with an identical brief. */
2728
export type HardBlockSalvage =
2829
| "thrash"
30+
| "no-ship"
2931
| "no-progress"
3032
| "repetition"
3133
| "never-acted"
@@ -36,7 +38,8 @@ export type BriefSalvageKind =
3638
| "turn-budget"
3739
| "deadline"
3840
| "stalled"
39-
| "cancelled";
41+
| "cancelled"
42+
| "incomplete-report";
4043

4144
export type TaskBriefFingerprintInput = {
4245
prompt: string;
@@ -62,6 +65,7 @@ export const TURN_BUDGET_STOP_AFTER_DISPATCHES = 3;
6265

6366
const HARD_BLOCK_SALVAGES = new Set<BriefSalvageKind>([
6467
"thrash",
68+
"no-ship",
6569
"no-progress",
6670
"repetition",
6771
"never-acted",
@@ -84,13 +88,20 @@ export function isCancelledSubAgentReport(report: string): boolean {
8488
return parsed.summary.toLowerCase().includes("cancelled");
8589
}
8690

91+
/** True when the worker returned an incomplete-report salvage (narration, no envelope). */
92+
export function isIncompleteReportSubAgentReport(report: string): boolean {
93+
const parsed = parseSubAgentReport(report);
94+
return parsed.summary.toLowerCase().includes("narrated instead of writing a report envelope");
95+
}
96+
8797
/**
8898
* Classify a sub-agent tool result body as a salvage kind the parent ledger cares
8999
* about. Returns null for normal completes (or unrecognized envelopes).
90100
*/
91101
export function classifyBriefSalvage(report: string): BriefSalvageKind | null {
92102
// Order: more specific salvage phrases first.
93103
if (isThrashSubAgentReport(report)) return "thrash";
104+
if (isNoShipSubAgentReport(report)) return "no-ship";
94105
if (isRepetitionSubAgentReport(report)) return "repetition";
95106
if (isNeverEditedSubAgentReport(report)) return "never-edited";
96107
if (isNeverActedSubAgentReport(report)) return "never-acted";
@@ -99,6 +110,7 @@ export function classifyBriefSalvage(report: string): BriefSalvageKind | null {
99110
if (isDeadlineSubAgentReport(report)) return "deadline";
100111
if (isStalledSubAgentReport(report)) return "stalled";
101112
if (isCancelledSubAgentReport(report)) return "cancelled";
113+
if (isIncompleteReportSubAgentReport(report)) return "incomplete-report";
102114
return null;
103115
}
104116

‎src/subagent/index.test.ts‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,69 @@ describe("sub-agent stop helpers", () => {
214214
).toBe("complete");
215215
});
216216

217+
const SUMMARY_ONLY_NARRATION = [
218+
"## Summary",
219+
"Checking whether Skywalker write-tool unmount is tested...",
220+
"Checking those next.",
221+
].join("\n");
222+
223+
const FULL_REPORT_ENVELOPE = [
224+
"## Summary",
225+
"Reviewed gate.ts.",
226+
"",
227+
"## Findings",
228+
"Auth lives in gate.ts.",
229+
"",
230+
"## Blockers",
231+
"None.",
232+
"",
233+
"## Paths",
234+
"src/gate.ts",
235+
].join("\n");
236+
237+
test("evaluateSubAgentStop returns incomplete-report for Summary-only tool-less narration after tools", () => {
238+
expect(
239+
evaluateSubAgentStop({
240+
hasToolCalls: false,
241+
everHadToolCalls: true,
242+
turnsCompleted: 2,
243+
maxTurns: 10,
244+
consecutiveIdentical: 0,
245+
repeatLimit: 2,
246+
lastAssistantText: SUMMARY_ONLY_NARRATION,
247+
}),
248+
).toBe("incomplete-report");
249+
});
250+
251+
test("evaluateSubAgentStop returns incomplete-report-stop for Summary-only after the wrap-up nudge", () => {
252+
expect(
253+
evaluateSubAgentStop({
254+
hasToolCalls: false,
255+
everHadToolCalls: true,
256+
turnsCompleted: 3,
257+
maxTurns: 10,
258+
consecutiveIdentical: 0,
259+
repeatLimit: 2,
260+
lastAssistantText: SUMMARY_ONLY_NARRATION,
261+
incompleteReportNudgeFired: true,
262+
}),
263+
).toBe("incomplete-report-stop");
264+
});
265+
266+
test("evaluateSubAgentStop returns complete for tool-less after tools with all four headings", () => {
267+
expect(
268+
evaluateSubAgentStop({
269+
hasToolCalls: false,
270+
everHadToolCalls: true,
271+
turnsCompleted: 2,
272+
maxTurns: 10,
273+
consecutiveIdentical: 0,
274+
repeatLimit: 2,
275+
lastAssistantText: FULL_REPORT_ENVELOPE,
276+
}),
277+
).toBe("complete");
278+
});
279+
217280
test("evaluateSubAgentStop returns never-acted when the run never used tools", () => {
218281
expect(
219282
evaluateSubAgentStop({
@@ -247,6 +310,38 @@ describe("sub-agent stop helpers", () => {
247310
).toBe("never-edited");
248311
});
249312

313+
test("evaluateSubAgentStop does not hard-stop implement for many unique reads", () => {
314+
let thrash = EMPTY_THRASH_STATE;
315+
for (let i = 0; i < 200; i++) {
316+
thrash = nextThrashState(thrash, [
317+
{ type: "tool_call", name: "read_file", arguments: { path: `src/f${i}.ts` } },
318+
]);
319+
}
320+
expect(
321+
evaluateSubAgentStop({
322+
hasToolCalls: true,
323+
everHadToolCalls: true,
324+
turnsCompleted: 40,
325+
maxTurns: 60,
326+
consecutiveIdentical: 0,
327+
repeatLimit: 2,
328+
thrashState: thrash,
329+
requireEdit: true,
330+
}),
331+
).toBeNull();
332+
expect(
333+
evaluateSubAgentStop({
334+
hasToolCalls: true,
335+
everHadToolCalls: true,
336+
turnsCompleted: 40,
337+
maxTurns: 60,
338+
consecutiveIdentical: 0,
339+
repeatLimit: 2,
340+
thrashState: thrash,
341+
}),
342+
).toBeNull();
343+
});
344+
250345
test("evaluateSubAgentStop still completes implement when an edit path was recorded", () => {
251346
const thrashState = {
252347
totalToolCalls: 4,
@@ -1227,6 +1322,44 @@ describe("SubAgentDirector stall management", () => {
12271322
});
12281323

12291324
describe("createTaskTool", () => {
1325+
test("handler does not resolve until run() resolves; result includes the full report", async () => {
1326+
let release!: () => void;
1327+
const gate = new Promise<void>((resolve) => {
1328+
release = resolve;
1329+
});
1330+
const report = "## Summary\nThe work is done.";
1331+
const tool = createTaskTool({
1332+
permissionGate: testPermissionGate,
1333+
cwd: "/repo",
1334+
getWorkdirBase: () => "/repo/.corbits",
1335+
provider,
1336+
profiles: [{ id: "leaf" }],
1337+
run: async () => {
1338+
await gate;
1339+
return report;
1340+
},
1341+
});
1342+
1343+
const pending = callTask(tool, {
1344+
description: "Investigate",
1345+
prompt: "Do the work",
1346+
agent: "leaf",
1347+
});
1348+
let settled = false;
1349+
void pending.then(() => {
1350+
settled = true;
1351+
});
1352+
await Promise.resolve();
1353+
expect(settled).toBe(false);
1354+
1355+
release();
1356+
const result = await pending;
1357+
expect(settled).toBe(true);
1358+
expect(result).toContain('Sub-agent "');
1359+
expect(result).toContain(report);
1360+
expect(result).toContain("## Summary");
1361+
});
1362+
12301363
test("does not inherit a bogus parent-session maxTurns dep on the task tool", async () => {
12311364
let captured: RunSubAgentParams | undefined;
12321365
const tool = createTaskTool({
@@ -1983,10 +2116,15 @@ describe("brief re-dispatch ledger (CL-4343 / CL-5203)", () => {
19832116
expect(classifyBriefSalvage(forcedStopReport("repetition", "x"))).toBe("repetition");
19842117
expect(classifyBriefSalvage(forcedStopReport("never-acted", "x"))).toBe("never-acted");
19852118
expect(classifyBriefSalvage(forcedStopReport("never-edited", "x"))).toBe("never-edited");
2119+
expect(classifyBriefSalvage(forcedStopReport("no-ship", "x"))).toBe("no-ship");
19862120
expect(classifyBriefSalvage(forcedStopReport("turn-budget", "x"))).toBe("turn-budget");
19872121
expect(classifyBriefSalvage("## Summary\nDone\n\n## Findings\nok\n\n## Blockers\nNone\n\n## Paths\n")).toBeNull();
19882122
});
19892123

2124+
test("classifyBriefSalvage maps incomplete-report salvage", () => {
2125+
expect(classifyBriefSalvage(forcedStopReport("incomplete-report", "x"))).toBe("incomplete-report");
2126+
});
2127+
19902128
test("turn-budget parent hint flips after re-dispatch threshold", () => {
19912129
const report = forcedStopReport("turn-budget", "partial");
19922130
const first = appendSubAgentParentHints(report, { dispatchCount: 1 });

‎src/subagent/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export {
3434
buildDispatchBrief,
3535
demoteNestedReportHeadings,
3636
formatSubAgentReport,
37+
hasReportEnvelope,
3738
parseSubAgentReport,
3839
subAgentToolName,
3940
type DispatchBrief,

0 commit comments

Comments
 (0)