Skip to content

Commit 2183755

Browse files
committed
Stop arming the critique evidence gate for greybeard review intent
1 parent c6be54b commit 2183755

5 files changed

Lines changed: 69 additions & 8 deletions

File tree

‎src/subagent/index.test.ts‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
preferCompletedSubAgentReply,
3535
resolveSubAgentCatchOutcome,
3636
resolveSubAgentDeadlineMs,
37+
shouldRequireEvidence,
3738
subAgentToolName,
3839
SUBAGENT_DEADLINE_MARGIN_MS,
3940
SUBAGENT_PLUGIN_SPAWN_TEARDOWN_LIMITS,
@@ -45,6 +46,8 @@ import {
4546
} from "./index.js";
4647

4748
import { type } from "arktype";
49+
import { formatDirectorSystemPrompt } from "../agent/directors/identity.js";
50+
import { DIRECTOR_REGISTRY } from "../agent/directors/registry.js";
4851
import type {
4952
ReactorAction,
5053
ReactorCapabilities,
@@ -277,6 +280,28 @@ describe("sub-agent stop helpers", () => {
277280
).toBe("complete");
278281
});
279282

283+
test("shouldRequireEvidence is armed for CritiqueDirector prompt", () => {
284+
expect(
285+
shouldRequireEvidence({
286+
systemPromptRole: formatDirectorSystemPrompt(DIRECTOR_REGISTRY.critique),
287+
}),
288+
).toBe(true);
289+
expect(
290+
shouldRequireEvidence({
291+
systemPromptRole: DIRECTOR_REGISTRY.critique.systemPrompt,
292+
}),
293+
).toBe(true);
294+
});
295+
296+
test("shouldRequireEvidence is off for greybeard even with intent=review", () => {
297+
expect(
298+
shouldRequireEvidence({
299+
intent: "review",
300+
systemPromptRole: formatDirectorSystemPrompt(DIRECTOR_REGISTRY.greybeard),
301+
}),
302+
).toBe(false);
303+
});
304+
280305
test("evaluateSubAgentStop does not complete a review/critique with empty readCounts even with a full envelope", () => {
281306
const thrashState = {
282307
totalToolCalls: 1,
@@ -295,7 +320,7 @@ describe("sub-agent stop helpers", () => {
295320
thrashState,
296321
requireEvidence: true,
297322
}),
298-
).not.toBe("complete");
323+
).toBe("incomplete-report");
299324
});
300325

301326
test("evaluateSubAgentStop completes a review when readCounts has file evidence", () => {
@@ -319,6 +344,27 @@ describe("sub-agent stop helpers", () => {
319344
).toBe("complete");
320345
});
321346

347+
test("evaluateSubAgentStop completes greybeard spawn-only envelope when requireEvidence is off", () => {
348+
const thrashState = {
349+
totalToolCalls: 1,
350+
readCounts: new Map(),
351+
editedPaths: new Set<string>(),
352+
};
353+
expect(
354+
evaluateSubAgentStop({
355+
hasToolCalls: false,
356+
everHadToolCalls: true,
357+
turnsCompleted: 2,
358+
maxTurns: 10,
359+
consecutiveIdentical: 0,
360+
repeatLimit: 2,
361+
lastAssistantText: FULL_REPORT_ENVELOPE,
362+
thrashState,
363+
requireEvidence: false,
364+
}),
365+
).toBe("complete");
366+
});
367+
322368
test("evaluateSubAgentStop returns never-acted when the run never used tools", () => {
323369
expect(
324370
evaluateSubAgentStop({

‎src/subagent/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ export {
116116
coreSubAgentWebTools,
117117
createSubAgentRunController,
118118
runSubAgent,
119+
shouldRequireEvidence,
119120
type SubAgentRunController,
120121
} from "./run.js";
121122

‎src/subagent/nudge-director.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export class SubAgentDirector extends DefaultDirector {
9393
private readonly repeatLimit: number;
9494
/** When true (intent=implement), tool-less finish without edits salvages as never-edited. */
9595
private readonly requireEdit: boolean;
96-
/** When true (intent=review / critique), empty readCounts is not a successful complete. */
96+
/** When true (CritiqueDirector), empty readCounts is not a successful complete. */
9797
private readonly requireEvidence: boolean;
9898
private turnsCompleted = 0;
9999
private everHadToolCalls = false;

‎src/subagent/run.ts‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ import {
8989
} from "./dispose.js";
9090
import { createTaskTool } from "./task-tool.js";
9191
import type { RunSubAgentParams, SubAgentProvider } from "./types.js";
92+
import type { TaskIntent } from "./report.js";
9293
import { runWithSubAgentIdentity } from "./identity-context.js";
9394

9495
export type {
@@ -223,6 +224,21 @@ export function createSubAgentRunController(
223224
};
224225
}
225226

227+
/**
228+
* Arm requireEvidence only for CritiqueDirector. Greybeard is also
229+
* intent=review and may spawn-only then envelope; that is not a fake
230+
* review — do not pull it into the empty-readCounts gate.
231+
*/
232+
export function shouldRequireEvidence(input: {
233+
intent?: TaskIntent;
234+
systemPromptRole?: string;
235+
}): boolean {
236+
return (
237+
typeof input.systemPromptRole === "string" &&
238+
input.systemPromptRole.includes("CritiqueDirector")
239+
);
240+
}
241+
226242
// Spin up an isolated, autonomous agent loop, hand it one task, and return
227243
// its final report. `params.cwd` is either the dispatcher's own cwd (shared
228244
// mode) or a worktree snapshotted from the dispatcher's last commit
@@ -417,9 +433,7 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<string> {
417433
modelFamilyPolicy.subAgentStallTimeoutMs,
418434
Date.now,
419435
params.intent === "implement",
420-
params.intent === "review" ||
421-
(typeof params.systemPromptRole === "string" &&
422-
params.systemPromptRole.includes("CritiqueDirector")),
436+
shouldRequireEvidence(params),
423437
),
424438
});
425439

‎src/subagent/stop-policy.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ export type SubAgentStopReason =
275275
* (Summary, Findings, Blockers, Paths). Omitting `lastAssistantText`
276276
* still completes (back-compat). Missing envelope nudges
277277
* once (`incomplete-report`) then salvages (`incomplete-report-stop`).
278-
* When `requireEvidence` is set (review/critique), an empty `readCounts`
278+
* When `requireEvidence` is set (CritiqueDirector), an empty `readCounts`
279279
* is not complete even with all four headings — same incomplete-report
280280
* nudge then salvage, so a wrap-up envelope cannot fake a real review.
281281
*/
@@ -297,7 +297,7 @@ export function evaluateSubAgentStop(input: {
297297
*/
298298
requireEdit?: boolean;
299299
/**
300-
* When true (intent=review / critique leaf), a tool-using run that never
300+
* When true (CritiqueDirector leaf), a tool-using run that never
301301
* read or searched a file is not a successful complete — even a four-heading
302302
* envelope is incomplete-report so the parent does not treat a wrap-up
303303
* narration as a finished review.
@@ -317,7 +317,7 @@ export function evaluateSubAgentStop(input: {
317317
// read/searched (no edit_file/write_file/delete_file) is never-edited —
318318
// both hard-block identical re-dispatch. After those, a tool-less turn
319319
// following tools is complete only with a report envelope (or when
320-
// lastAssistantText is omitted). Review/critique additionally requires
320+
// lastAssistantText is omitted). CritiqueDirector additionally requires
321321
// at least one read/search in thrashState.readCounts.
322322
if (!input.hasToolCalls) {
323323
if (!input.everHadToolCalls) return "never-acted";

0 commit comments

Comments
 (0)