postrun: tighten unsubmitted-review gate to require create_pull_request_review for Review mode (#724)

The gate at `getUnsubmittedReview` accepted `toolState.finalSummaryWritten`
as a valid Review exit, contradicting the post-failure error message which
already says Review's only valid exit is `create_pull_request_review`.
This let any caller that flipped `finalSummaryWritten` — including a
`task`-dispatched `reviewfrog` subagent calling `pullfrog_report_progress`
in violation of its prose-only read-only contract — silence the gate even
when the orchestrator never submitted a review.

Split per-mode: Review requires `toolState.review`, IncrementalReview keeps
the existing `||` (its post-failure message explicitly accepts
`report_progress` as a "no review warranted" exit). Test split mirrors the
new semantics.

closes #648
This commit is contained in:
Colin McDonnell
2026-05-14 00:01:15 +00:00
committed by pullfrog[bot]
parent b9f0938405
commit d857e06731
2 changed files with 30 additions and 4 deletions
+14 -1
View File
@@ -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();
});