From c6be54b7654ec9e6ed672ee301915a34dee11ba1 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 17:49:30 -0700 Subject: [PATCH 1/2] Stop treating a wrap-up envelope as a finished critique without reads Review/critique leaves now require at least one read or search in readCounts before evaluateSubAgentStop can return complete, even when the four-heading report is present. --- src/subagent/index.test.ts | 42 ++++++++++++++++++++++++++++++++++ src/subagent/nudge-director.ts | 5 ++++ src/subagent/run.ts | 3 +++ src/subagent/stop-policy.ts | 21 ++++++++++++++++- 4 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/subagent/index.test.ts b/src/subagent/index.test.ts index cc3f9b773..b18948a46 100644 --- a/src/subagent/index.test.ts +++ b/src/subagent/index.test.ts @@ -277,6 +277,48 @@ describe("sub-agent stop helpers", () => { ).toBe("complete"); }); + test("evaluateSubAgentStop does not complete a review/critique with empty readCounts even with a full envelope", () => { + const thrashState = { + totalToolCalls: 1, + readCounts: new Map(), + editedPaths: new Set(), + }; + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + everHadToolCalls: true, + turnsCompleted: 2, + maxTurns: 10, + consecutiveIdentical: 0, + repeatLimit: 2, + lastAssistantText: FULL_REPORT_ENVELOPE, + thrashState, + requireEvidence: true, + }), + ).not.toBe("complete"); + }); + + test("evaluateSubAgentStop completes a review when readCounts has file evidence", () => { + const thrashState = { + totalToolCalls: 1, + readCounts: new Map([["src/gate.ts", 1]]), + editedPaths: new Set(), + }; + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + everHadToolCalls: true, + turnsCompleted: 2, + maxTurns: 10, + consecutiveIdentical: 0, + repeatLimit: 2, + lastAssistantText: FULL_REPORT_ENVELOPE, + thrashState, + requireEvidence: true, + }), + ).toBe("complete"); + }); + test("evaluateSubAgentStop returns never-acted when the run never used tools", () => { expect( evaluateSubAgentStop({ diff --git a/src/subagent/nudge-director.ts b/src/subagent/nudge-director.ts index 24c021ede..78c2a29d0 100644 --- a/src/subagent/nudge-director.ts +++ b/src/subagent/nudge-director.ts @@ -93,6 +93,8 @@ export class SubAgentDirector extends DefaultDirector { private readonly repeatLimit: number; /** When true (intent=implement), tool-less finish without edits salvages as never-edited. */ private readonly requireEdit: boolean; + /** When true (intent=review / critique), empty readCounts is not a successful complete. */ + private readonly requireEvidence: boolean; private turnsCompleted = 0; private everHadToolCalls = false; private streak: ToolCallStreak = { @@ -148,6 +150,7 @@ export class SubAgentDirector extends DefaultDirector { stallTimeoutMs?: number, now: () => number = Date.now, requireEdit: boolean = false, + requireEvidence: boolean = false, ) { super(systemPrompt, toolDefinitions, {}); this.compaction = createCompactionGovernor(requestContinuation, systemPrompt, toolDefinitions); @@ -157,6 +160,7 @@ export class SubAgentDirector extends DefaultDirector { this.now = now; this.lastActivityAt = now(); this.requireEdit = requireEdit; + this.requireEvidence = requireEvidence; } override async decide( @@ -217,6 +221,7 @@ export class SubAgentDirector extends DefaultDirector { repeatLimit: this.repeatLimit, thrashState: this.thrashState, requireEdit: this.requireEdit, + requireEvidence: this.requireEvidence, lastAssistantText: this.lastAssistantText, incompleteReportNudgeFired: this.incompleteReportNudgeFired, }); diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 59426369c..a5e2a1f5c 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -417,6 +417,9 @@ export async function runSubAgent(params: RunSubAgentParams): Promise { modelFamilyPolicy.subAgentStallTimeoutMs, Date.now, params.intent === "implement", + params.intent === "review" || + (typeof params.systemPromptRole === "string" && + params.systemPromptRole.includes("CritiqueDirector")), ), }); diff --git a/src/subagent/stop-policy.ts b/src/subagent/stop-policy.ts index 102b294b2..4b7fe12e3 100644 --- a/src/subagent/stop-policy.ts +++ b/src/subagent/stop-policy.ts @@ -275,6 +275,9 @@ export type SubAgentStopReason = * (Summary, Findings, Blockers, Paths). Omitting `lastAssistantText` * still completes (back-compat). Missing envelope nudges * once (`incomplete-report`) then salvages (`incomplete-report-stop`). + * When `requireEvidence` is set (review/critique), an empty `readCounts` + * is not complete even with all four headings — same incomplete-report + * nudge then salvage, so a wrap-up envelope cannot fake a real review. */ export function evaluateSubAgentStop(input: { hasToolCalls: boolean; @@ -293,6 +296,13 @@ export function evaluateSubAgentStop(input: { * does not treat a pure-explore "plan" as shipped work. */ requireEdit?: boolean; + /** + * When true (intent=review / critique leaf), a tool-using run that never + * read or searched a file is not a successful complete — even a four-heading + * envelope is incomplete-report so the parent does not treat a wrap-up + * narration as a finished review. + */ + requireEvidence?: boolean; /** * Final assistant text of this turn. When omitted, a tool-less turn after * tools still completes (back-compat for existing unit tests). When provided, @@ -307,7 +317,8 @@ export function evaluateSubAgentStop(input: { // read/searched (no edit_file/write_file/delete_file) is never-edited — // both hard-block identical re-dispatch. After those, a tool-less turn // following tools is complete only with a report envelope (or when - // lastAssistantText is omitted). + // lastAssistantText is omitted). Review/critique additionally requires + // at least one read/search in thrashState.readCounts. if (!input.hasToolCalls) { if (!input.everHadToolCalls) return "never-acted"; if ( @@ -324,6 +335,14 @@ export function evaluateSubAgentStop(input: { ? "incomplete-report-stop" : "incomplete-report"; } + if ( + input.requireEvidence === true && + (input.thrashState === undefined || input.thrashState.readCounts.size === 0) + ) { + return input.incompleteReportNudgeFired === true + ? "incomplete-report-stop" + : "incomplete-report"; + } return "complete"; } // No-progress is more specific than thrash or the turn budget when both could apply. From 21837551d70b9bac5228efc0c583ad58a02fea07 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 18:06:39 -0700 Subject: [PATCH 2/2] Stop arming the critique evidence gate for greybeard review intent --- src/subagent/index.test.ts | 48 +++++++++++++++++++++++++++++++++- src/subagent/index.ts | 1 + src/subagent/nudge-director.ts | 2 +- src/subagent/run.ts | 20 +++++++++++--- src/subagent/stop-policy.ts | 6 ++--- 5 files changed, 69 insertions(+), 8 deletions(-) diff --git a/src/subagent/index.test.ts b/src/subagent/index.test.ts index b18948a46..9c13e359e 100644 --- a/src/subagent/index.test.ts +++ b/src/subagent/index.test.ts @@ -34,6 +34,7 @@ import { preferCompletedSubAgentReply, resolveSubAgentCatchOutcome, resolveSubAgentDeadlineMs, + shouldRequireEvidence, subAgentToolName, SUBAGENT_DEADLINE_MARGIN_MS, SUBAGENT_PLUGIN_SPAWN_TEARDOWN_LIMITS, @@ -45,6 +46,8 @@ import { } from "./index.js"; import { type } from "arktype"; +import { formatDirectorSystemPrompt } from "../agent/directors/identity.js"; +import { DIRECTOR_REGISTRY } from "../agent/directors/registry.js"; import type { ReactorAction, ReactorCapabilities, @@ -277,6 +280,28 @@ describe("sub-agent stop helpers", () => { ).toBe("complete"); }); + test("shouldRequireEvidence is armed for CritiqueDirector prompt", () => { + expect( + shouldRequireEvidence({ + systemPromptRole: formatDirectorSystemPrompt(DIRECTOR_REGISTRY.critique), + }), + ).toBe(true); + expect( + shouldRequireEvidence({ + systemPromptRole: DIRECTOR_REGISTRY.critique.systemPrompt, + }), + ).toBe(true); + }); + + test("shouldRequireEvidence is off for greybeard even with intent=review", () => { + expect( + shouldRequireEvidence({ + intent: "review", + systemPromptRole: formatDirectorSystemPrompt(DIRECTOR_REGISTRY.greybeard), + }), + ).toBe(false); + }); + test("evaluateSubAgentStop does not complete a review/critique with empty readCounts even with a full envelope", () => { const thrashState = { totalToolCalls: 1, @@ -295,7 +320,7 @@ describe("sub-agent stop helpers", () => { thrashState, requireEvidence: true, }), - ).not.toBe("complete"); + ).toBe("incomplete-report"); }); test("evaluateSubAgentStop completes a review when readCounts has file evidence", () => { @@ -319,6 +344,27 @@ describe("sub-agent stop helpers", () => { ).toBe("complete"); }); + test("evaluateSubAgentStop completes greybeard spawn-only envelope when requireEvidence is off", () => { + const thrashState = { + totalToolCalls: 1, + readCounts: new Map(), + editedPaths: new Set(), + }; + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + everHadToolCalls: true, + turnsCompleted: 2, + maxTurns: 10, + consecutiveIdentical: 0, + repeatLimit: 2, + lastAssistantText: FULL_REPORT_ENVELOPE, + thrashState, + requireEvidence: false, + }), + ).toBe("complete"); + }); + test("evaluateSubAgentStop returns never-acted when the run never used tools", () => { expect( evaluateSubAgentStop({ diff --git a/src/subagent/index.ts b/src/subagent/index.ts index a18d73f9e..c212f04b1 100644 --- a/src/subagent/index.ts +++ b/src/subagent/index.ts @@ -116,6 +116,7 @@ export { coreSubAgentWebTools, createSubAgentRunController, runSubAgent, + shouldRequireEvidence, type SubAgentRunController, } from "./run.js"; diff --git a/src/subagent/nudge-director.ts b/src/subagent/nudge-director.ts index 78c2a29d0..b2728e154 100644 --- a/src/subagent/nudge-director.ts +++ b/src/subagent/nudge-director.ts @@ -93,7 +93,7 @@ export class SubAgentDirector extends DefaultDirector { private readonly repeatLimit: number; /** When true (intent=implement), tool-less finish without edits salvages as never-edited. */ private readonly requireEdit: boolean; - /** When true (intent=review / critique), empty readCounts is not a successful complete. */ + /** When true (CritiqueDirector), empty readCounts is not a successful complete. */ private readonly requireEvidence: boolean; private turnsCompleted = 0; private everHadToolCalls = false; diff --git a/src/subagent/run.ts b/src/subagent/run.ts index a5e2a1f5c..c8b776411 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -89,6 +89,7 @@ import { } from "./dispose.js"; import { createTaskTool } from "./task-tool.js"; import type { RunSubAgentParams, SubAgentProvider } from "./types.js"; +import type { TaskIntent } from "./report.js"; import { runWithSubAgentIdentity } from "./identity-context.js"; export type { @@ -223,6 +224,21 @@ export function createSubAgentRunController( }; } +/** + * Arm requireEvidence only for CritiqueDirector. Greybeard is also + * intent=review and may spawn-only then envelope; that is not a fake + * review — do not pull it into the empty-readCounts gate. + */ +export function shouldRequireEvidence(input: { + intent?: TaskIntent; + systemPromptRole?: string; +}): boolean { + return ( + typeof input.systemPromptRole === "string" && + input.systemPromptRole.includes("CritiqueDirector") + ); +} + // Spin up an isolated, autonomous agent loop, hand it one task, and return // its final report. `params.cwd` is either the dispatcher's own cwd (shared // mode) or a worktree snapshotted from the dispatcher's last commit @@ -417,9 +433,7 @@ export async function runSubAgent(params: RunSubAgentParams): Promise { modelFamilyPolicy.subAgentStallTimeoutMs, Date.now, params.intent === "implement", - params.intent === "review" || - (typeof params.systemPromptRole === "string" && - params.systemPromptRole.includes("CritiqueDirector")), + shouldRequireEvidence(params), ), }); diff --git a/src/subagent/stop-policy.ts b/src/subagent/stop-policy.ts index 4b7fe12e3..ba16bcbb7 100644 --- a/src/subagent/stop-policy.ts +++ b/src/subagent/stop-policy.ts @@ -275,7 +275,7 @@ export type SubAgentStopReason = * (Summary, Findings, Blockers, Paths). Omitting `lastAssistantText` * still completes (back-compat). Missing envelope nudges * once (`incomplete-report`) then salvages (`incomplete-report-stop`). - * When `requireEvidence` is set (review/critique), an empty `readCounts` + * When `requireEvidence` is set (CritiqueDirector), an empty `readCounts` * is not complete even with all four headings — same incomplete-report * nudge then salvage, so a wrap-up envelope cannot fake a real review. */ @@ -297,7 +297,7 @@ export function evaluateSubAgentStop(input: { */ requireEdit?: boolean; /** - * When true (intent=review / critique leaf), a tool-using run that never + * When true (CritiqueDirector leaf), a tool-using run that never * read or searched a file is not a successful complete — even a four-heading * envelope is incomplete-report so the parent does not treat a wrap-up * narration as a finished review. @@ -317,7 +317,7 @@ export function evaluateSubAgentStop(input: { // read/searched (no edit_file/write_file/delete_file) is never-edited — // both hard-block identical re-dispatch. After those, a tool-less turn // following tools is complete only with a report envelope (or when - // lastAssistantText is omitted). Review/critique additionally requires + // lastAssistantText is omitted). CritiqueDirector additionally requires // at least one read/search in thrashState.readCounts. if (!input.hasToolCalls) { if (!input.everHadToolCalls) return "never-acted";