Skip to content

A failed-review verdict is permanent, but the branch it judged is not #571

Description

@khaliqgant

The Software Garden drafts a pull request when its adversarial review withholds signoff, via FLOW_REVIEW_BLOCKED_COMMAND: it writes review-blocked.md, runs gh pr ready --undo, posts the findings, and returns done("step_failed").

That state is set once and never revisited. The flow does not re-run against an existing pull request, so nothing re-marks it ready when the findings are fixed, and nothing withdraws or amends the verdict comment. The author pushes fixes; the PR stays a draft with **Relayflow: the adversarial review did not pass.** pinned at the top.

Why this is more than cosmetic

The verdict outranks the evidence. It is the most authoritative-looking thing on the PR, it names a specific head, and it is wrong the moment the next commit lands.

Concretely, today (2026-09-23) in this repository:

PR verdict said reality
#545 "not approved", P1 credential disclosure three fix commits had already landed after the reviewed head, with the exact regression coverage the review asked for; mutation-proved and merged
#521 not approved both Medium findings fixed on the branch; merged
#512 not approved both findings fixed; merged
#519 P2 open on step-evidence.ts fixed in current code
#517 "not approved" verdict itself says "No remaining production-code correctness finding identified"
#518 "not approved" verdict itself says "No new actionable code defects found"
#520 "not approved" verdict itself says "no production-code defect identified in this diff"

Seven PRs, none blocked by an open code defect, all presenting as blocked. Three of them were mergeable and sat in the queue. A reviewer scanning the list cannot distinguish "this has real findings" from "this had findings in the past", so the signal is worth nothing without re-deriving it by hand each time.

Worse, #517, #518 and #520 were never drafted for defects at all. Their reviews withheld review.clean for verification reasons — a missing kernel/target/debug/relayflowd, Bun 1.4.0 required against 1.3.6 supplied, an external Cloud dependency the reviewed plan explicitly permits to stay pending. "The sandbox could not run the suite" and "this code is wrong" produce an identical, indistinguishable outcome.

Suggested directions

  1. Make the verdict self-invalidating. Record the reviewed head in the comment (several already do) and edit or strike the comment when head != reviewed_head, so a superseded verdict reads as superseded.
  2. Re-review on push. Let a new commit on a flow-authored PR re-run the review step, which is the only thing that can actually clear a draft.
  3. Separate "cannot verify" from "found a defect". A review blocked on a missing binary or a wrong runtime version should not produce the same terminal state as one that found a bug — at minimum it should say so in the first line, where the draft state is decided.

(1) is the cheapest and removes most of the harm on its own.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    garden-readyScoped and ready for an agent to pick up

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions