From 46d51496b4cc808adf99c137cd68c14748d508d3 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 9 Nov 2026 17:43:01 -0800 Subject: [PATCH] Let an owner's ruling clear new blockers that an extra review round found, so /ship can finish (GST-2251) (#4260) * Let an owner's ruling clear the new blockers a granted fix-check found (GST-2251) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Name every finding that still holds a fix-check open, with the full rule command (GST-2251) Review-Finding: p1/adversarial-native#1 reader=adversarial-native Review-Finding: p1/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Describe the wider FIX_CHECK_OPEN list in the release note (GST-2251) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Keep a skipped blocking finding open until the owner rules it, even beside a ruled CRITICAL (GST-2251) A ruled open CRITICAL could clear a fix-check row while an unruled non-CRITICAL finding logged skipped with blocking:true stayed unruled. Rulings now clear a row only when one covers every unresolved or blocking finding, and FIX_CHECK_OPEN names the rest. Review-Finding: captain-audit/Q1 reader=outside-voice Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- ...rulings-close-a-fix-checks-own-blockers.md | 4 + docs/review-policy.md | 3 +- docs/ship-policy.md | 13 ++- docs/troubleshooting.md | 23 +++++ lib/review-evidence.ts | 24 +++-- lib/ship-release.ts | 9 +- test/ship-finish-ruled-notes.test.ts | 93 +++++++++++++++++++ 7 files changed, 155 insertions(+), 14 deletions(-) create mode 100644 changelog.d/let-owner-rulings-close-a-fix-checks-own-blockers.md diff --git a/changelog.d/let-owner-rulings-close-a-fix-checks-own-blockers.md b/changelog.d/let-owner-rulings-close-a-fix-checks-own-blockers.md new file mode 100644 index 000000000..77d4cf1d0 --- /dev/null +++ b/changelog.d/let-owner-rulings-close-a-fix-checks-own-blockers.md @@ -0,0 +1,4 @@ +--- +important: blocker +--- +- **An owner's ruling now clears the new blockers a granted fix-check found, so /ship can finish with them listed as open notes.** When a granted fix-check finds a new CRITICAL, it logs the finding `open` and stops for the owner. The owner can rule it a disclosed note with `gstack-approvals rule`, but `gstack-ship-release finish` rejected the fix-check as unfinished before it looked at any ruling. It also ignored rulings on `open` findings, so the stop could only end with another fix-check (harness-evals#361, GST-2251). Now, once an approved ruling covers every finding that still holds it open, the fix-check counts at the tree it read, and each ruling is printed and listed in the PR body and receipt. A fix-check with a finding still unruled prints `FIX_CHECK_OPEN:`, which names every finding that still holds it open, including below-floor ones, and the full `gstack-approvals rule` command to run. A finding the lane only marks as accepted in its own review-log row still stays open, and so does a ruling at a head outside the fix-check's range. Edits made after the fix-check still route as post-review changes. diff --git a/docs/review-policy.md b/docs/review-policy.md index 92c2e6d56..750af3922 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -730,7 +730,8 @@ reads the newest `review-fix-check` row from `gstack-review-read`, chained back through earlier rows whose commit is the next row's `range_base` ([chained fix-checks](ship-policy.md#fix-check-chain), GST-1627). When every link is a completed clean read on committed content (freshness `CURRENT` against its own -tree) whose commit is on the branch, the oldest link's `range_base` is a branch commit +tree; a fix-check that stopped on findings the owner has since ruled notes reads +clean, [ruled notes](ship-policy.md#ruled-notes)) whose commit is on the branch, the oldest link's `range_base` is a branch commit holding the reviewed tree, and for each link an approved `review_loop_grant` in `gstack-approvals` names a stop head inside its `range_base..commit`, each code or test path inside a link's `range_base..commit` that is unchanged since, or whose change against the base is still the one the diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 453696a36..3f7c46755 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1683,13 +1683,22 @@ row is ignored. Step 16's `finish` reads the branch's rulings (`lib/ruled-notes.ts`) and prints each as `RULED_NOTE: at ruled by at