merge_pr.py's explain()/--why learns presubmit, and reads the published merge-authority check (design 006 piece 6) nf-yn91 ← Beads

closed priority 2 task unassigned activesession:explain-presubmit

Twinned from https://github.com/cfsh/ninefold/issues/825 by tools/beads/import_github.py's reconcile pass.

Design 006 (design_docs/006-the-coders-front-door.md) piece 6 (S4a). Standalone — touches merge_pr.py only, no dependency on any other piece. Landing this alongside [#824](https://github.com/cfsh/ninefold/issues/824) and a third sibling issue (filed separately) clears every remaining prerequisite for piece 8 (dev.py status/review/merge), the highest-value remaining piece in the doc.

The gap, measured

explain() in tools/pr/merge_pr.py (roughly lines 478-559 as of #787 — grep to confirm, other pieces have touched this file since) is --why's engine. It checks, in order: PR state, CHANGES_REQUESTED, verdict freshness, Tier-1 authority (paused, perimeter, verdict result, verdict-certifies-head), mergeable, base branch, the feedback ledger. Good "what's left" answer, wrong on two counts:

  1. It never checks presubmit. run_presubmit() is only called from merge(), never from explain(). So --why can say "would merge" and then fail the gate minutes later, mid-merge — the opposite of #733's ask for "clear test errors on pre-flight checks".
  2. It cannot see the published merge-authority GitHub check run, which is what GitHub's own required-status-check ruleset (20540766, enforcement: active) actually blocks the merge button on. explain() only *recomputes* authority locally. The check is SHA-scoped and refreshed by review_now/perimeter_check.py (CHECK_NAME = "merge-authority", published via POST repos/{repo}/check-runs, see tools/pr/perimeter_check.py:119,250) — so it has its own staleness mode, and local-recompute vs. published-check can disagree exactly once per approval-after-verdict. That's #763, confirmed live on #759's merge: --why said "would merge" while GitHub said no, fixed in the moment by review_now.py --reconcile-only republishing the check.
What to build

Add two checks to explain()'s pipeline, in whatever order makes the output read naturally alongside the existing ones:

  1. Presubmit awareness. Some read on current presubmit state for this PR's head — reuse whatever dev.py check/presubmit.py already exposes rather than inventing a second gate; the point is --why should not say "would merge" when presubmit would fail.
  2. Read the published check, don't just recompute. GET the merge-authority check run for the PR's head sha (same API family perimeter_check.py posts to — GET repos/{repo}/commits/{sha}/check-runs, filter by name == "merge-authority") and report its actual state alongside the local computation. If they disagree, say so explicitly — that disagreement *is* #763, and surfacing it is more valuable than silently trusting either side.
Constraints
Reference (context, not required reading)

design_docs/006-the-coders-front-door.md §2.5 (the gap, in full), §4 "Option F" (why the published check is the merge-time source of truth and its known staleness mode), §8.2 row 6.

Dependencies

None.

Comments

No comments.

Add a comment