Skip to content

ci(conflict-gate): a conflicted PR reads red, not empty-green (backend#2637) - #359

Merged
LukasWodka merged 6 commits into
developfrom
ci/2637-conflict-gate
Aug 27, 2026
Merged

ci(conflict-gate): a conflicted PR reads red, not empty-green (backend#2637)#359
LukasWodka merged 6 commits into
developfrom
ci/2637-conflict-gate

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

A PR with a merge conflict against its base dispatches none of its pull_request workflows. GitHub cannot compute the merge ref those runs are keyed on, so every drift / source-of-truth guard is silently inactive on exactly the PRs most likely to need one — and the rollup shows nothing red, because nothing ran.

This is platform behaviour, not a bug in our YAML: there is no merge commit for those jobs to check out, so no trigger change can fix it. The only signal that can reach a conflicted PR is one written onto its head sha from outside. This adds that: an org-wide sweep that writes a conflict-gate / mergeable commit status onto every open PR.

The hole is real and open — measured, not assumed

Two live cases on 2026-08-27, and they are different shapes:

1. model-zoo#206 — nothing ran.

mergeableCONFLICTING
actions/runs?head_sha=e7465eatotal_count: 0
rollupone entry: Cursor Bugbot = SUCCESS
model-zoo/develop requires7 contexts — none present
PRs #204 / #205 either sidefull 3–4 workflow matrix each

The conflict was the only difference. Bugbot reviews the diff rather than via a pull_request trigger, so it is the one voice left — and it says green.

2. backend#2257 — worse: unanimously green while conflicted.

mergeableCONFLICTING
workflow runs on head sha8
rollup16 entries
backend/develop's 11 required contextsall present, all SUCCESS

Its head sha never changed, so it kept every check it had already earned — all computed against a merge base that no longer exists. This is the client#847 shape from the ticket: checks that reported before the conflict appeared, with a version-bump-gate failure invisible for ~3 hours.

Case 2 is why bricked-prs.py is not already the answer. That watcher infers a problem from a required context being absent (missing = required - present), and its conflicted cause only ever attaches to a missing finding. Here nothing is missing, so it cannot see this PR at all. Its necessary 60-minute grace window also means it says nothing about a fresh conflict. The two files answer different questions; neither subsumes the other.

Approach chosen — option 2 from the ticket, as visibility

A commit status written from outside the PR, on a scheduled org-wide sweep:

  • conflictedfailure — the PR finally has something red to point at
  • clearsuccess — the context clears itself, so it can never brick a healthy PR
  • undeterminedpending — "cannot tell" is neither, and says so

Rejected, and why

  • A pull_request-triggered job — that is the defect. There is nothing for it to run against.
  • pull_request_target — needs no merge ref either, but nothing in this org uses it: measured 2026-08-27, actions/runs?event=pull_request_target returns total_count: 0 fleet-wide, so its behaviour on a conflicted PR is unverified here. Building a fail-closed guard on an unmeasured platform claim is CLAUDE.md rule 8's mistake.
  • push to the base branch — the causally exact trigger (a conflict is created when base moves), but a push here fires only for this repo. Covering all 17 would need a per-repo caller; that is a latency improvement, not a correctness one, and is the natural follow-up.
  • Option 3, requiring the drift workflows by namealready done. Measured across the fleet: every train repo's develop already carries 6–12 required contexts (backend 11, client 10, model-zoo 7). That is why the merge path is not the exposure; with dismiss_stale_reviews: true fleet-wide, resolving a conflict also re-triggers the guards and dismisses the approval. What was left was visibility, which is what this ships.
  • Extending bricked-prs.py — it is a read-only 4-hourly audit with a deliberate 60-minute grace window; all three are wrong for a per-PR signal, and mixing them would degrade the watcher.

Not required anywhere yet — deliberately, and this is a decision for @LukasWodka

Landing this as advice on purpose, per "arm while green". Requiring the context needs one more thing first: a trigger that gives every PR a status promptly. On a 30-minute sweep a PR opened at minute one would sit at Expected — waiting for status until the next run — precisely the brick bricked-prs.py exists to hunt. The per-repo push caller above is what would make it requireable. No branch protection was touched.

Evidence

$ python3 scripts/conflict-gate.py --dry-run --repo model-zoo --retries 0
CONFLICTED model-zoo#206 -> develop (would mark failure)
mergeable=CONFLICTING
no pull_request check can run on this head until the conflict is resolved
EXIT=1
$ python3 scripts/conflict-gate.py --dry-run --retries 1 # whole fleet
CONFLICTED backend#2257 -> develop (would mark failure)
mergeable=CONFLICTING
EXIT=1 # 0 COULD NOT EVALUATE
$ python3 scripts/tests/conflict-gate-selftest.py
conflict-gate-selftest: 92 assertions, all passed
$ python3 scripts/tests/conflict-gate-mutations.py
17 mutation(s): 0 stale, 0 uncaught
$ make selftests-cover
selftests-cover: all 20 selftests and 10 mutation runner(s) are wired to a target, and CI runs both tiers
$ make check
EXIT=0 # ruff, shellcheck, house-rules, action-pins, mint-scope (0 findings), actionlint (0 findings)

The input domain is derived, not hand-written

MERGEABLE_STATES and MERGE_STATE_STATUSES come from GitHub's own GraphQL schema introspection (the command is in the file, reproduce rather than edit). The selftest walks the full 3 × 7 cross product and asserts the verdict for every pair — mutation coverage cannot see a vocabulary gap. The first draft carried a DRAFT member of MergeStateStatus that the schema does not have; deriving it caught that.

The load-bearing case is mergeable=CONFLICTING + mergeStateStatus=UNKNOWN — the shape a conflicted PR presents moments after its base moves, and the one an "any UNKNOWN wins" rule silently drops while passing everything else. It has its own mutation.

Mutation-proven

All 17 mutations call the real scripts/conflict-gate.py and re-run the real suite — no inline copy of the rule. Two mutations initially reported UNCAUGHT (harness broke, not detected): they emptied a list the suite then indexed, so it died with an IndexError instead of reporting a FAIL. The harness correctly refused to score a traceback as coverage; fixed in caefcaf and both are now genuinely caught.

Fail-closed throughout: an unreadable PR list, a PR list at the truncation cap, a status that failed to write, a missing head sha, and a mergeability GitHub will not state are each an error with exit 2 — never a clean sweep. Each has a mutation.

Closes tracebloc/backend#2637


Note

Medium Risk
Fleet-wide GitHub status writes with a scoped app token affect every open PR’s check rollup; misclassification or token scope mistakes could show false reds or fail silently on private repos, though the change is not yet a required merge gate.

Overview
Adds an org-wide conflict gate because merge-conflicted PRs dispatch nopull_request workflows—so drift and quality checks never run and the rollup can look green (including stale successes on an unchanged head sha).

scripts/conflict-gate.py sweeps open PRs from repo-inventory.yml, classifies mergeability (mergeable / mergeStateStatus), and writes an external commit status conflict-gate / mergeable on each head: failure when conflicted, success when clear, pending when GitHub cannot answer. It dedupes writes via the REST combined-status endpoint (narrow token: pull-requests: read, statuses: write), retries lazy UNKNOWN mergeability, and fails closed on unreadable lists or failed writes.

.github/workflows/conflict-gate.yml runs that sweep on a 30-minute schedule and workflow_dispatch from tracebloc/.github, minting the release-train app token—not on pull_request, which cannot fire on conflicted PRs.

Makefile wires selftest-conflict-gate and mutation-conflict-gate into the existing selftest/mutation tiers. Hermetic selftests plus mutations cover classification, workflow triggers, token scopes, and status dedup. The context is visibility only until branch protection requires it.

Reviewed by Cursor Bugbot for commit b04f603. Bugbot is set up for automated code reviews on this repo. Configure here.

LukasWodkaand others added 3 commits August 27, 2026 17:06
…d#2637)
A PR with a merge conflict against its base dispatches NONE of its
`pull_request` workflows: GitHub cannot compute the merge ref those runs are
keyed on. Every drift and source-of-truth guard is therefore silently inactive
on exactly the PRs most likely to need it, and the rollup shows nothing red
because nothing ran.
Measured on model-zoo#206 while it was DIRTY against develop: 0 workflow runs
on its head sha, one rollup entry (Cursor Bugbot, SUCCESS), and all seven of
model-zoo/develop's required contexts absent. PRs #204 and #205 either side of
it got their full matrix.
Adds an org-wide sweep that writes a commit status onto each open PR's head sha
- failure when conflicted, success when clear, pending when GitHub will not say
- which is the only signal that can reach a PR with no merge ref.
Not required anywhere yet, deliberately: a 30-minute sweep would leave a
freshly-opened PR at "Expected - waiting" until the next run.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ackend#2637)
Two mutations -- drafts skipped in plan(), and an unreadable PR list returning
no error -- emptied a list the suite then indexed, so it died with an IndexError
instead of reporting a FAIL. The harness correctly scored both as 'broke the
harness' rather than caught: a traceback proves nothing about coverage.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…r cannot see (backend#2637)
backend#2257, measured live and CONFLICTING, carries 8 workflow runs and all
eleven of backend/develop's required contexts present and SUCCESS -- computed
against a merge base that no longer exists. bricked-prs.py reasons from an
ABSENT required context, so with nothing missing it cannot see this PR at all.
That is the case that justifies asking about mergeability directly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Aug 27, 2026
…ething (backend#2637)
GitHub caps statuses at 1000 per sha AND context. A 30-minute sweep is 48
writes a day onto an unchanged head, so a PR left open three weeks would
exhaust the cap and every later write would 422 -- the gate going silent on
exactly the stalest PRs, which are the ones most likely to have conflicted.
The current state is read out of statusCheckRollup, which the PR list already
returns, so this costs no extra API call. A truncated rollup omits our context,
which reads as 'no status yet' and produces a write -- the safe direction.
The case fold is load-bearing: GraphQL reports SUCCESS, the Statuses API takes
success, and comparing unfolded would make every status look changed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Pushed c3b8279 — a defect I found in my own design while reasoning about scale, before it could ship.

GitHub caps statuses at 1000 per sha and context. A 30-minute sweep is 48 writes a day onto an unchanged head, so a PR left open ~3 weeks would exhaust the cap and every write after that would 422. The gate would go silent on exactly the stalest PRs — the ones most likely to have conflicted. That is this ticket's own failure mode, reintroduced by the fix for it.

A status is now written only when it would change something. The current state comes out of statusCheckRollup, which the PR list already returns, so it costs no extra API call. Two details worth flagging:

  • The case fold is load-bearing. GraphQL reports a legacy status's state as SUCCESS; the Statuses API takes success. Comparing them unfolded makes every status look changed and the dedup silently does nothing — a mutation pins it.
  • When in doubt, write. A rollup truncated by pagination omits our context, which reads as "no status yet" and produces a write. The unsafe direction would be skipping a write we needed.

The direction that matters most is clearing: a stale failure left on a since-resolved PR would block it for no reason, so resolved conflict → overwrite with success has its own case and its own mutation (the skip inverted).

Updated numbers:

conflict-gate-selftest 102 assertions, all passed (was 92)
conflict-gate-mutations 21 mutations, 0 stale, 0 uncaught (was 17)
make check EXIT=0
make selftests-cover 20 selftests, 10 mutation runners

Fleet dry-run on the current head, writing nothing:

$ python3 scripts/conflict-gate.py --dry-run --retries 1 --json
PRs swept: 49
errors: []
conflicted: backend#2257 (failure), e2e-test-agent#292 (failure)

e2e-test-agent#292 is a third live instance, appeared since this PR was opened — model-zoo#206 from the description has since been resolved. The churn rate is the point: this is not a rare state.

… (backend#2637)
Everything the suite asserted proved the SCRIPT was right; none of it proved
anything RUNS it. Adds nine assertions parsing conflict-gate.yml as YAML (a
"run:" line under a comment or an "if: false" greps identically to a live one)
and six workflow mutations, so the harness now rewrites both files.
The regression most likely to be made in good faith has its own mutation:
someone asks why this does not run on PRs, adds a "pull_request:" trigger, and
the gate becomes inert on exactly the conflicted PRs it exists for while looking
more thorough. Rule 5 does not exempt a guarantee for being written in YAML.
The baseline guard now covers both targets: a mutation left in the workflow by a
killed run would become the next run's premise just as silently as one left in
the script.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c3b8279. Configure here.

Comment thread.github/workflows/conflict-gate.yml
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Pushed 0ec1518. Everything the suite asserted proved the script was right; none of it proved anything runs it. That is the org's own rule — assert a caller is ARMED, not merely present (backend#1977) — and it was missing here.

Adds nine assertions that parse conflict-gate.ymlas YAML rather than grepping it (a run: line inside a commented block, or under an if: false, greps identically to a live one), and six workflow mutations. The harness now rewrites both files, and the baseline guard covers both — a mutation left in the workflow by a killed run would become the next run's premise just as silently as one left in the script.

The regression most likely to be made in good faith has its own mutation: someone asks why doesn't this run on PRs?, adds pull_request:, and the gate becomes inert on exactly the conflicted PRs it exists for — while looking more thorough. Rule 5 does not exempt a guarantee for being written in YAML.

conflict-gate-selftest 111 assertions, all passed (was 102)
conflict-gate-mutations 27 mutations across 2 files, 0 stale, 0 uncaught (was 21)
make check EXIT=0

Also pinned there: the mint asks for exactly pull-requests: read + statuses: write and nothing more — one mutation adds a contents: write this job has no use for and the suite reddens.

…(Bugbot, #359)
Bugbot raised this as high on the previous head, and it was right.
"gh pr list --json statusCheckRollup" resolves "commit.status" underneath, and
GraphQL REFUSES that subfield on a PRIVATE repo unless the token also holds
"actions: read" -- measured under backend#2157 and documented in
bricked-prs.yml, which declares "permission-actions: read" for exactly this
reason. This gate's mint deliberately holds only pull-requests:read and
statuses:write, so the dedup added in c3b8279 would have made open_prs raise on
every private repo in the org: each becomes COULD NOT EVALUATE and the sweep
exits 2 having judged almost nothing.
The trap was documented in a file I read while writing this one, which is the
argument for measuring a narrow token rather than reasoning about it.
Reads "GET /repos/{o}/{r}/commits/{sha}/status" instead. It reads commit statuses
and nothing else, so the statuses permission already held covers it; it cannot be
refused for a scope this job has no other use for; and it has no pagination cap to
straddle. One GET per open PR, ~50 per sweep. The mint stays at two permissions.
Two new mutations pin it: switching back to the rollup, and treating an
unreadable current state as agreement (which would silently stop reporting
whenever the status read flakes).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka
LukasWodka requested review from aptracebloc and removed request for saadqbalAugust 27, 2026 15:53

@aptraceblocaptracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. This closes a genuinely nasty blind spot and the test suite is among the most rigorous I've reviewed.

The "empty-green" bug is real and well-diagnosed: a PR with a merge conflict dispatches none of its pull_request workflows (GitHub can't compute the merge ref they're keyed on), so every drift/source-of-truth guard silently goes inactive on exactly the PRs that most need it, and the rollup shows nothing red because nothing ran. Reaching those PRs from outside via a scheduled org-wide sweep that writes a conflict-gate / mergeable commit status onto the head sha — failure on conflict, success when clear (self-clearing, so it can never brick a healthy PR), pending when GitHub won't answer — is the right shape.

The UNKNOWN handling is the load-bearing part and it's correctly fail-closed: classify rejects any non-enum value to undetermined, lets affirmative conflict evidence win over an UNKNOWN in the other field (so CONFLICTING + UNKNOWN reads CONFLICTED, not dropped), and keys clear only on mergeable == MERGEABLE — never on a bare UNKNOWN. Every residual edge (missing head sha, failed write, unreadable list, truncation cap) fails closed to exit 2, never a silent clean sweep. I ran the mutation harness: 115/115 self-test assertions pass and 29/29 mutations are caught, including the "any UNKNOWN wins" and "treat absent as CLEAR" mutations; the workflow is verified as parsed YAML (token scope pinned to exactly statuses:write + pull-requests:read), not grepped. No wedge risk — it's deliberately not required anywhere yet ("arm while green"), and every swept PR gets a status so nothing parks at "Expected."

Three low, non-blocking fast-follows:

  • scripts/conflict-gate.py:290existing_state reads the combined-status endpoint (statuses paginated 30/page), but the comment claims "no pagination cap to straddle." On a sha with >30 status contexts the gate's own context can land on page 2 and be re-written every sweep. Safe direction (extra writes, never a wrong verdict), but the comment is factually wrong.
  • Makefile:167mutation-conflict-gate omits the guard-pyyaml dep its selftest sibling has; confusing standalone failure without pyyaml.
  • .github/workflows/conflict-gate.yml:122 — exit 1 on any conflict with no continue-on-error makes the cron red on the common case, so a real exit-2 operational failure looks the same as "found conflicts" at the run-list level. Mirrors bricked-prs.py, so it's the house convention — flagging, not blocking.

(Minor: the PR body's evidence cites 92 assertions / 17 mutations — stale; HEAD actually has 115 / 29, i.e. more coverage than advertised.)

— drafted with Claude Code

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

@aptracebloc thank you for the review — and apologies, the approval cannot unblock this PR and that is our config's fault, not yours.

Measured just now:

repos/tracebloc/.github/collaborators/aptracebloc/permission -> read

GitHub only counts approving reviews from collaborators with write access toward required_approving_review_count. So your APPROVED review is recorded on the current head (b04f60372, not dismissed) and shows in latestReviews, while reviewDecision stays REVIEW_REQUIRED and the PR reads BLOCKED. Nothing you did is wrong and none of your review is wasted — it just cannot satisfy the gate.

Ruled out, so nobody re-diagnoses this: require_code_owner_reviews is false on .github/develop, and the promotion-branches-merge-commit-only ruleset targets only refs/heads/main and refs/heads/staging, so neither is the cause.

@saadqbal — requesting you, since you hold write here. Arturo's review above is substantive and stands; this is only about whose approval the gate counts.

The wider problem, worth fixing once rather than per-PR. On tracebloc/.github specifically, aptracebloc and shujaatTracebloc both have read; saadqbal and saqlainsyed007 have write. Everyone has write on e2e-test-agent, tracebloc-engine and client. So assigning either of the first two as reviewer on .github produces a PR that can never go green no matter how good the review is — and this has already happened once before, on .github#237 (backend#1945), where a reviewer diagnosed the problem correctly and could not clear the check because he had read on that repo. @LukasWodka that is either a permissions fix or a reviewer-routing fix, and it needs deciding rather than working around.

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

@aptracebloc — thank you for this review, and I owe you a heads-up: your approval cannot unblock this PR, through no fault of yours.

You hold read on tracebloc/.github, and GitHub does not count an approval from a user without write access. So the PR shows your ✅ in the review list while:

decision=REVIEW_REQUIRED mergeable=MERGEABLE state=BLOCKED
all 8 required contexts: pass unresolved threads: 0

Nothing in the PR view, the protection API or the checks list says why — the only way to find it is to query collaborator permission per user per repo.

It is not just this repo or just you. Measured today:

reposaadqbalsaqlainsyed007aptraceblocwaqaskhanroghanishujaatTracebloc
.githubwritewritereadreadread
release-trainwritewritewritereadread
rfcswritewritewritereadread
all 15 otherswritewritewritewritewrite

Filed as backend#2734 — it is the same class this repo keeps writing guards against: a mechanism that renders as doing the thing and is not connected to what it gates. Sorry it cost you a careful read on a PR it could never merge.

@saadqbal is requested and has write, so this merges on his approval.

— drafted with Claude Code

@saadqbalsaadqbal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the deepest version of a failure this org has been finding all week, and the one that could not be fixed from inside: a conflicted PR dispatches nopull_request workflows, so every drift and source-of-truth guard is inactive on exactly the PRs most likely to need one — and the rollup shows nothing red because nothing ran. Naming it as platform behaviour rather than a YAML bug is what makes the architecture right: no trigger change can produce a merge ref that does not exist, so the only signal that can reach a conflicted PR is one written onto its head sha from outside.

The two cases being different shapes is the part that makes the measurement worth having.model-zoo#206 with total_count: 0, a rollup containing exactly one entry — Cursor Bugbot = SUCCESS — and seven required contexts simply absent, with the neighbouring PRs either side carrying full matrices so the conflict is the only variable. And backend#2257 worse: eight runs on the head sha, unanimously green, while conflicted. One is an empty rollup, the other a stale-but-affirmative one; a fix that only addressed the first would have left the second reading clean.

Everything I would have gone looking for is here, and reasoned rather than merely present:

Three states, and pending for "cannot tell"."undetermined -> pending — 'cannot tell' is neither, and says so", with the note that pending blocks a merge exactly as failure does, so honesty about the unknown costs no safety. Choosing not to assert a conflict it cannot see is the right call in both directions.

success mattering as much as failure. Writing the context on clear PRs is what lets it clear itself, so a resolved conflict can't leave a permanent red — and it makes the context's absence meaningful, which a failure-only writer would destroy.

Two fields of conflict evidence, without letting one field's UNKNOWN suppress the other's affirmative answer. That's the trap in reading mergeable and mergeStateStatus together, and it's closed explicitly rather than by luck.

Knowing that the read is what schedules the computation, so a first-read UNKNOWN on both fields is expected rather than a real unknown — "answer, not a workaround. Without this every sweep would paint pending over" everything.

And the recursive one, which is the mark of the thing:"WHEN IN DOUBT, WRITE. An unreadable current state returns None, which equals no status … empty-green, which is the failure this whole file exists to remove." A tool whose own read failure reproduces the bug it fixes is the commonest way this kind of guard dies, and it's handled.

One correction of mine: I reported this merged two passes ago. It never was — it dropped out of my filtered view and I read the absence as a merge, which is the second time I've done that this session and a fitting error to make on a PR about absence rendering as green.

Green, no threads. 👍

@LukasWodka
LukasWodka merged commit 33944d5 into developAug 27, 2026
12 checks passed
@LukasWodka
LukasWodka deleted the ci/2637-conflict-gate branch August 27, 2026 18:31
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@LukasWodka@saadqbal@aptracebloc