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.

Author: TheGreatAxios

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<string>(),
+    };
+    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<string>(),
+    };
+    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({

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,
       });

src/subagent/run.ts

@@ -417,6 +417,9 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<string> {
         modelFamilyPolicy.subAgentStallTimeoutMs,
         Date.now,
         params.intent === "implement",
+        params.intent === "review" ||
+          (typeof params.systemPromptRole === "string" &&
+            params.systemPromptRole.includes("CritiqueDirector")),
       ),
   });
 

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.