A conflicting PR branch has no CI verdict at all, and nothing says so #517

Activity

1. blooop commented on Aug 29, 2026

   

   blooop

   on Aug 29, 2026

   OwnerAuthor

   More actions

   Second sighting from the other end, and it sharpens the fix. A parallel session working map #444 reports PRs #516, #519 and #515 each showing 14 green checks while CONFLICTING — the results were real, but for an older head SHA, so the PR page read as green to anyone scanning it. Same root as this ticket: nothing ties a displayed verdict to the commit that would actually merge.

   Note the repo has already learned this once. AGENTS.md carries "verify checks against the full 40-char head SHA" as guidance, which is the manual version of the check. Today it was followed by hand on both sides and caught the staleness both times, but guidance only works when someone remembers to apply it.

   So the durable fix is probably narrower than "make gate un-skippable": refuse a merge when no completed run exists for the exact head SHA, which covers all three shapes seen today in one rule — the absent run (a conflicting branch has no merge ref), the dropped webhook (PR #506, where three re-fires produced nothing until a merge commit cured it), and the stale-but-green display (#516/#519/#515).

   Practical check that found it, usable before any merge:

   sha=$(gh pr view <n> --json headRefOid --jq .headRefOid)
   gh api "repos/blooop/devlaunch/commits/$sha/check-runs" --jq '[.check_runs[] | "\(.name)=\(.conclusion // .status)"]'
   
2. blooop commented on Aug 29, 2026

   

   blooop

   on Aug 29, 2026

   OwnerAuthor

   More actions

   Third sighting, and the subtle case: a low count reads as green

   The other session merging in parallel today hit this repeatedly and independently.
   Its report, in its own terms: gh pr checks and the PR's statusCheckRollup
   "lied to me repeatedly" — showing 14 green on a conflicting branch, and showing
   zero pending while commits/$SHA/check-runs still had three jobs in_progress.
   It ended up merging all three of its PRs only on the raw
   gh api repos/blooop/devlaunch/commits/$SHA/check-runs answer.

   That adds a case sharper than the one this issue opened with, and it is the one
   worth writing the guard against:

   A conflicting PR does not report zero runs. It reports a small number of green
   ones. A scanner whose rule is "no failures and nothing pending" therefore reads
   green on it — the answer is not wrong, it is partial, and partial is
   indistinguishable from good news under that rule. The same shape appears while a
   run is still being scheduled: the checks that exist are green, the ones that
   matter have not been created yet.

   So the verdict needs a floor on the total, not just an empty failure list:

   sha=$(gh pr view "$N" --repo blooop/devlaunch --json headRefOid -q .headRefOid)
   gh api "repos/blooop/devlaunch/commits/$sha/check-runs" --paginate --jq \
     '[.check_runs[]] | {
        bad:     map(select(.conclusion=="failure" or .conclusion=="cancelled" or .conclusion=="timed_out"))|length,
        running: map(select(.status!="completed"))|length,
        total:   length }'

   Merge only on bad == 0 && running == 0 && total >= 10. This repo's full set is
   15 runs (ci, rust, e2e, gate, prek, public-api, packaging, review,
   rust-coverage, four codecov contexts, GitGuardian, and publish-job which
   correctly reports skipped). Ten is a deliberately loose floor that still
   excludes every partial answer either of us actually saw.

   The one-line version, for whatever ends up enforcing this: absence of a failure
   is not a pass. The count is part of the verdict.

   Both of today's sessions converged on the same workaround by hand, on different
   PRs, without coordinating — which is the argument for putting it somewhere a
   human or an agent cannot forget it.
3. blooop commented on Aug 29, 2026

   

   blooop

   on Aug 29, 2026

   OwnerAuthor

   More actions

   Correction to the comment above: do not use a count. Name the checks.

   I recommended total >= 10 one comment up. That threshold is wrong, and it fails
   in exactly the direction this issue is about — it passes a partial answer. Three
   measurements, taken while the PRs in this batch were building:

   • A PR mid-run reports exactly 10 check-runs, all green, nothing marked
     pending, because the four codecov contexts are not created until the coverage
     job finishes. #504 0f/6r/10t and #505 0f/4r/10t were live readings. A floor
     of 10 admits both.
   • A PR at rest reports 15. That was Sweep dangling citations to retired Python tests #511's merged head: ci, rust, e2e,
     gate, prek, public-api, packaging, review, rust-coverage, four
     codecov contexts, GitGuardian, and publish-job skipped.
   • main's head reports 12, and it is a different set — bench, Build conda package, Publish to prefix.dev, Decide whether there is a release to build
     are there; gate and prek are not. Push events and pull_request events run
     different workflows, so a guard calibrated against main would reject every PR
     and a guard calibrated against a PR would misread main.

   The general lesson is that the size of the expected set is not a constant —
   it depends on the event, and on how far along the run is. Any threshold is either
   too low to catch a partial answer or too high to ever pass. My previous comment
   picked "too low".

   So the verdict should enumerate, not count:

   required="ci rust e2e gate prek public-api packaging review rust-coverage"
   sha=$(gh pr view "$N" --repo blooop/devlaunch --json headRefOid -q .headRefOid)
   runs=$(gh api "repos/blooop/devlaunch/commits/$sha/check-runs" --paginate \
           --jq '.check_runs[] | "\(.name)\t\(.status)\t\(.conclusion // "")"')
   for c in $required; do
     line=$(grep -P "^\Q$c\E\t" <<<"$runs") || { echo "MISSING: $c"; exit 1; }
     grep -q $'\tcompleted\t' <<<"$line" || { echo "PENDING: $c"; exit 1; }
     case "$line" in *$'\t'success|*$'\t'skipped|*$'\t'neutral) ;; *) echo "BAD: $line"; exit 1;; esac
   done

   A named check that is absent is a failure, which is the whole point of the issue:
   a conflicting PR's missing runs are missing, not passing. Codecov is deliberately
   out of the required list, since it is informational and arrives late.

   Keeping the one-line version from the previous comment, which survives the
   correction intact: absence of a failure is not a pass. The fix is just that
   "and there are enough of them" has to be "and each one I require is here", because
   "enough" has no fixed value.
4. blooop

   added a commit that references this issue on Aug 29, 2026

   ci: a released changelog section is not a place to file a new entry

   ...

   a18f48c
5. blooop commented on Aug 29, 2026

   

   blooop

   on Aug 29, 2026

   OwnerAuthor

   More actions

   A fourth member of this family, from code review rather than CI

   This issue is about a verdict pinned to the wrong object: green checks belonging
   to a superseded SHA, or to a partial set. #522 produced the same shape somewhere
   else entirely, and it is worth recording because the check involved was careful
   and still confirmed the wrong thing.

   The claim. A reviewer proposed adding a write_f64 override to
   PythonPrettyFormatter and stated it "moves no byte" — i.e. the change is
   provably inert against existing output.

   The check. Trace every document that reaches the formatter. There are two:
   dl --ls --json and metadata.json. Neither carries an f64 anywhere: the
   listing's numbers are u64 byte counts inside objects, and metadata's only bare
   number is version: i64. So the override is unreachable, moves no byte, and
   every existing byte pin passes unchanged. All of that is true.

   Why it confirmed the wrong thing. as_python_writes_it_indented is pub and
   takes any serde_json::Value. A float can reach it through the public API even
   though no float reaches it through either document. Without the override such a
   float is spelled ryu's way (1e-5) where CPython writes 1e-05 — a real
   divergence on the public surface, in a crate whose entire purpose here is that
   the two spellings agree.

   So "moves no byte" was true of the documents on the wire and false of the
   function, and a reviewer checking the wire would have come away satisfied. The
   override is not cosmetic; it closes a live divergence. It is now covered by a
   test, on the principle that an override with nothing measuring it is how the two
   formatters drift apart again.

   The generalisation, which is this issue's: a check can be correct, carefully
   executed, and still be about a narrower object than the claim it appears to
   support. The failure is not in the check but in the silent substitution of what
   it ranges over — wire documents for a public function, one SHA for another, a
   partial check set for a complete one.

   The mechanical tell in all four instances so far is that the claim quantifies over
   something ("no byte", "all checks", "nothing pending") and the check quantifies
   over a subset that is easier to enumerate. Worth asking, of any green: what did
   this actually range over, and is that the same set the claim names?

   Siblings: #527 (a clean merge that is wrong by the changelog's rule), #529
   (a review guard whose red is routine on exactly the largest diffs).