fix(tooling): a merged PR is not green, and a skipped shim does not supersede a failure

[mrveiss]

Closes #16044.

Single-issue rationale: this fixes the merge-decision tool every session on this box uses to decide what to land. It is in flight while other PRs are being merged against its verdicts, so it goes alone rather than sharing a review pass with unrelated work.

Thinking Path

Another session found the fourth blind spot the hard way: it told a third session a merge window was open on three PRs that had merged hours earlier. Its gate said CONTEXTS-GREEN, and that was true — a merged PR's required contexts are green, because they completed before it landed. True and useless, and read as clearance.

That is the fourth:

#	blind spot	reported
1	pending contributed to no bucket	green, eight shards still running
2	a conflicted branch produces zero required contexts	green, nothing having run
3	a superseded cancelled outranked its replacement	red, on two clean PRs
4	a MERGED PR	green, read as clearance

Not four bugs. One property: a check that enumerates conditions is blind to the conditions it does not enumerate, and every blind spot reads as success. Every previous fix added a case to the list; none changed that.

Which predicted a fifth, and it turned up while fixing the fourth. On #16027 code-quality concluded failure at 06:43 and a path-filtered shim concluded skipped at 06:45 — same commit, same context name. Newest-wins took the skip.

What Changed

• _pr_head_and_state() reads state and head together, so neither is used without the other. A non-open PR returns "MERGED — not open; required contexts read green because it already landed" with exit 1.
• _supersedes() — a skipped run never supersedes one that actually ran. Everything else stays newest-wins.
• The property is now in the module docstring, where a reader meets the tool rather than buried at the line that implements one case.

Verification

Both against the live API, on real PRs:

PR	before	after
#16023 (merged)	CONTEXTS-GREEN	MERGED — not open; …already landed
#15978 (open, failing)	GREEN-BUT-OTHERS-FAILING	unchanged
#16027 (open, running)	BLOCKED	unchanged

Four tests, and two of them are contrasts — which is the half that matters here, because both fixes are of the form "stop trusting X" and the cheap wrong version of each is to stop trusting everything:

• an ordinary failure → re-run success must still supersede, or every fixed PR reads red forever;
• a context that is only ever skipped — the normal path-filtered case — must stay green rather than becoming unreadable.

What this does not fix

The design still assumes its list of conditions is complete, and each fix so far has been "add the case that just bit us". #16044 carries that as the open work: the tool says nothing about what it did not examine — review threads, base-freshness, conflicts — and that silence is what turns a blind spot into a green. It already names app-pinned contexts it cannot verify; that pattern needs to cover the rest.

Model Used

Opus 5

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: fd6e85dc-7f96-4009-a867-877968b518d8

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mrveiss]

Closing as superseded by #16047 — 46 and I fixed the same two blind spots in parallel, and theirs carries a test mine does not.

Two PRs on scripts/pr_required_gate.py for the same defects guarantees a conflict and spends two CI runs on a queue that is already starving four PRs (#16015). The duplicated work is spent either way; the only question is which artifact survives, and it should be the one with the better coverage.

That is theirs, on one specific test. They mutated their own fix with "ignore skips entirely" and it passed every test they had written — because all of them placed the conclusive run later in the list. The check-runs API returns runs unordered, so a newer skipped can arrive first with the older failure second, and that ordering is the only one reaching the promotion branch.

My implementation handles both orderings — verified directly:

skipped-first : failure
failure-first : failure

But my test only covered one of them. Same gap, caught by their mutation and not by mine. Their framing is the part worth keeping: when a mutation survives, the missing test is usually one nobody thought to write, not one that exists and is weak.

Nothing here is lost. The verification that mattered — running the fixed gate against #16023 (merged), #15978 (open, failing) and #16027 (open, running) — is recorded on this PR and on #16044.

What I am NOT closing: #16044. #16046 covers the same two defects; #16044 additionally carries the criterion neither PR implements — that the tool enumerate the conditions it did not examine. It already does this for app-pinned contexts ("matched by name only"); review threads, base-freshness and conflicts are checked by nobody and named by nothing, and that silence is what turns each blind spot into a green. Five instances in, adding one case per incident is the pattern that guarantees a sixth.

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.