feat(reviewer): publish reviewer-verdict as a commit status (required-check gate) - #333
Merged
Merged
Conversation
…-check gate)
The reviewer's LGTM/CONCERNS verdict was a bot issue-comment, not a status
check — so a manual `gh pr merge` (operator/admin) merged straight over an
unresolved CONCERNS verdict, and fast manual merges raced past the review loop
before it even ran. This makes the verdict first-class so it can be marked a
*required* status check:
- GitHubPRClient.set_commit_status() — POST /statuses/{sha}.
- _publish_reviewer_verdict() (best-effort) publishes the `reviewer-verdict`
context on the PR head: success on LGTM, failure on CONCERNS. _merge_and_done
re-blesses success right before merging, so the fleet's own merge and the
non-LGTM merge paths (e.g. ci_validated_after_retraction) clear the gate.
Before any review the context is absent → fail-closed (merge blocked) for the
fleet and humans alike. Adding `reviewer-verdict` to OC main's required checks
(with enforce_admins) is a follow-up step done AFTER the fleet runs this code.
Tests: set_commit_status (adapter) + _publish_reviewer_verdict success/no-op/
best-effort. Reviewer suite 116 pass; adapter 80 pass; audit clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 18, 2026
ProtocolWarden
added a commit
that referenced
this pull request
Jun 18, 2026
…ix-push (#335) A CONCERNS PR whose concerns are unsatisfiable in-diff (e.g. a doc summarizing out-of-diff facts) looped forever: each auto-fix pushed a new head, which _phase1's "head changed after concerns → reset fix state" treated as new work and zeroed fix_attempts — so the budget never reached max_fix_attempts and the PR never terminated (observed on #334: 7 self-pushes, fix_attempts stuck at 1, accumulating evidence-file cruft). Fix: record the head each fix pass produces (last_fix_push_sha) and reset the budget ONLY on an EXTERNAL push (current head ≠ our last fix-push). Self-pushes now accumulate toward max_fix_attempts, so a non-converging PR terminates (close+requeue) instead of churning. External (human) pushes still reset for a fresh review, unchanged. Surfaced now because Part B (#333) made reviewer-verdict a required check, so the loop became a hard merge blocker rather than advisory churn. Tests: self-pushed fix preserves the budget (→2); external push resets (→1). Reviewer suite 118 pass; audit clean. Co-authored-by: ProtocolWarden <ProtocolWarden@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
ProtocolWarden
added a commit
that referenced
this pull request
Jun 18, 2026
…336) The "Backbone notes" section still described B2 as red, the audit gate as advisory, and the fleet venv as behind-pin — all resolved (#330/#331/#333). Replace it with a terse claim-free pointer. Intentionally assertion-free: a pointer has nothing for the reviewer to demand in-diff proof of. Co-authored-by: ProtocolWarden <ProtocolWarden@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The reviewer's LGTM/CONCERNS verdict is a bot issue-comment, not a status
check. So a manual
gh pr merge(operator/admin) merges straight over anunresolved CONCERNS verdict, and fast manual merges race past the review loop
before it runs. Today that produced #330 and #328 merging with standing
concerns, and #328's auto-fix becoming the orphan #329 that reintroduced a leak.
What
Makes the verdict first-class so it can be a required status check:
GitHubPRClient.set_commit_status()—POST /repos/{o}/{r}/statuses/{sha}._publish_reviewer_verdict()(best-effort) publishes thereviewer-verdictcontext on the PR head: success on LGTM, failure on CONCERNS.
_merge_and_donere-blessessuccessright before merging, so the fleet's ownmerge and the non-LGTM merge paths (e.g.
ci_validated_after_retraction) clearthe gate.
Before any review the context is absent → fail-closed (merge blocked) for
the fleet and humans alike — closing the manual-merge bypass.
Rollout (follow-up, not in this PR)
Add
reviewer-verdictto OCmainrequired status checks after the fleet isrunning this code (else PRs deadlock waiting for a status the old fleet never
posts). Enforce on admins (else admin merges still bypass). Outage recovery: lift
protection to merge manually.
Tests
set_commit_status(adapter) +_publish_reviewer_verdictsuccess/no-op/best-effort. Reviewer suite 116 pass, adapter 80 pass, audit clean.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
🤖 Generated with Claude Code