diff --git a/agents/postRun.test.ts b/agents/postRun.test.ts index 1f271d1..d05a10f 100644 --- a/agents/postRun.test.ts +++ b/agents/postRun.test.ts @@ -29,9 +29,22 @@ describe("getUnsubmittedReview", () => { ).toBeNull(); }); - it("returns null when report_progress wrote a final summary", () => { + it("fires for Review even when report_progress wrote a final summary", () => { + // Review's only valid exit is `create_pull_request_review`. a summary + // comment is not a substitute, and accepting it here previously let + // subagent-flipped `finalSummaryWritten` silence the gate. expect( getUnsubmittedReview(makeToolState({ selectedMode: "Review", finalSummaryWritten: true })) + ).toBe("Review"); + }); + + it("returns null for IncrementalReview when report_progress wrote a final summary", () => { + // IncrementalReview treats `report_progress` as a legitimate + // "no review warranted" exit, matching the post-failure error message. + expect( + getUnsubmittedReview( + makeToolState({ selectedMode: "IncrementalReview", finalSummaryWritten: true }) + ) ).toBeNull(); }); diff --git a/agents/postRun.ts b/agents/postRun.ts index 2ffb74c..cd7b035 100644 --- a/agents/postRun.ts +++ b/agents/postRun.ts @@ -30,13 +30,26 @@ import { * * the gate is anchored to `hadProgressComment` so silent runs (non-issue * events, dispatcher skipped seeding) don't fire a nudge there's no UI for. + * + * `Review` and `IncrementalReview` have different valid exits: + * - Review: only `create_pull_request_review` counts. `report_progress` is + * not a substitute — a Review run that exits with just a summary comment + * has produced nothing reviewable on the PR. matches the hard-fail + * message at `expected = "create_pull_request_review"` below. + * - IncrementalReview: `report_progress` is a legitimate "no review + * warranted" exit, so either toolState flag short-circuits. + * splitting per mode also closes the bypass where a subagent (e.g. a + * `task`-dispatched `reviewfrog` lens) calls `report_progress` and silences + * the gate even though the orchestrator never submitted a review. */ export function getUnsubmittedReview(toolState: ToolState): "Review" | "IncrementalReview" | null { const mode = toolState.selectedMode; - if (mode !== "Review" && mode !== "IncrementalReview") return null; - if (toolState.review || toolState.finalSummaryWritten) return null; if (!toolState.hadProgressComment) return null; - return mode; + if (mode === "Review") return toolState.review ? null : "Review"; + if (mode === "IncrementalReview") { + return toolState.review || toolState.finalSummaryWritten ? null : "IncrementalReview"; + } + return null; } /**