Repository navigation
ci(guards): the required-context complement guard covers 1 of 5 shim pairs — two workflows already publish conflicting conclusions for one context #17391
Description
What
A required status context published by two workflows — a real job and a "required context" shim — is only safe while exactly one of them can conclude. That invariant is stated plainly in .github/filters/python-paths.yml:
Two workflows load this file with dorny/paths-filter, and between them they publish exactly one conclusion for the
python-suitestatus context... The twoif:conditions are exact complements.
repo_tests/required_context_complements_test.py enforces it. Its docstring says "Guards for every required-context complement split". It is hardcoded to one:
:28 CI = REPO_ROOT / ".github" / "workflows" / "ci.yml"
:29 SHIM = REPO_ROOT / ".github" / "workflows" / "python-required-context.yml"
:30 FILTER = REPO_ROOT / ".github" / "filters" / "python-paths.yml"
:32 REQUIRED_CONTEXT = "python-suite"
There are five shim workflows:
backend-required-context.yml
code-quality-required-context.yml
docker-smoke-required-context.yml
frontend-required-context.yml
python-required-context.yml <- the only one guarded
So four required contexts carry the same double-publisher structure with nothing asserting their if: conditions are complements.
This is not hypothetical — it was observed today
While triaging PR #17379 I read its code-quality context as not passing, and found two entries for one context name on one SHA:
code-quality conclusion=SUCCESS started 07:17:07Z (the real job, 11m34s)
code-quality conclusion=SKIPPED started 07:17:50Z (the shim)
Both workflows concluded. It was benign only because one concluded SKIPPED rather than FAILURE, and I misread it as a defect in my own query rather than as evidence that the complement does not hold for code-quality.
Why it matters: a reachable conflicting-conclusion path
Review of #17388 (the docker-smoke gate, #17126) found the sharp case. On needs.changes.result != 'success' the shim refuses — reports failure — while the real build runs and passes if the build succeeds. Same smoke-test context, two runs, opposite conclusions, one SHA.
How GitHub's branch protection resolves that is not documented and was not verified — correctly left unguessed by the reviewer. It cuts both ways:
- if the later conclusion wins, a real build finishing after the shim turns a deliberate refusal green;
- if the failure sticks, "build when we cannot tell" can never satisfy the gate it was written for.
The repo's own answer to this question is that it must not arise. That is what "exactly one conclusion" means. The fix is not to discover the resolution order; it is to restore the invariant and guard it.
Acceptance criteria
-
required_context_complements_test.pycovers every required context that has a shim, not onlypython-suite. Parametrising over the five pairs is the obvious route. - For each pair, the real job's
if:and the shim'sif:are asserted to be exact complements over the same computed value — and asserted to read that value from the same filter file, since two filters computing "the same" thing is how they drift apart. - Proved by mutation on a path it reaches: weaken one pair's shim condition so both can conclude, confirm the new assertion reddens by name, revert. A parametrised test proved only on the python pair proves the pair that already worked — the same trap as the guard this issue is about.
- The
code-qualitypair specifically is checked, since it is observed double-concluding today. - Where a pair cannot be made complementary (ci(docker-smoke): smoke-test passes with no Docker run when path detection fails — skipped satisfies the required check #17126's "build when we cannot tell" is a deliberate non-complement), that is recorded in the workflow as an accepted exception naming what it assumes about conclusion resolution — so the assumption is visible rather than inherited.
Related
Same shape as #17386 — a guard written for a class, scoped to one instance of it, whose docstring claims the class. That one is path filters; this one is context complements. Both were found by asking "what is this guard's domain, and how big is the set it claims?"
Activity
fix(ci): claim the invariant the smoke gate has, not a stronger one (#…
mrveiss commentedon Sep 24, 2026
The complement assumption fails in BOTH directions — derived from the two live conditions
Evidence from #17388, where two independent reviews and two sessions converged on this. Writing the
walk-through here because it is not fixable inside either workflow.
The
smoke-testrequired context is published by two jobs in two workflow files, each with itsown
changesjob. Call them S (the shim's detector) and R (the real build's). Theconditions as they stand on that branch:
On a pull request where both detectors succeed, that reduces to:
Row 3 is #17126's own hole by another door. That issue was filed because a skipped check
satisfies a required status check. A check that is never created satisfies it the same way — and
here nothing failed, nothing was cancelled, and both detectors answered successfully.
Row 4 is worse than conservative. The shim reports green because its detector said no Docker
paths changed; the real build runs because its detector said they did. Two check runs, one context,
and which one decides it depends on completion order. So the shim's green does not imply a build
ran, and can mask a failed one.
Neither row is a defect in either condition. Each is individually correct for the value its own
detector computed. The defect is that one question is asked of two detectors and the design assumes
they agree — and two
dorny/paths-filterruns, in separate workflow runs, resolve the diff against abase that can move between them.
Why this is the single-publisher problem
Any fix inside one file is a fix to one side of a disagreement it cannot see. The shapes that
actually close it:
report the no-op, so there is one detector and one conclusion by construction; or
workflow, or an artifact/output the second job reads rather than recomputes).
The first is smaller and removes the complement entirely rather than making two complements agree.
What #17388 does in the meantime
It closes the failure-driven half — a detector that fails no longer skips both jobs into a green —
and now states this table in both workflows' comments, so the remaining assumption is visible where
a reader would otherwise trust the invariant. I corrected my own comment twice getting there: it
first claimed "exactly one conclusion" (false), then claimed the shim's green "can never admit a PR
no build covered" (also false, row 4).
Proposed acceptance criteria for this issue
smoke-testcontext has exactly one publisher, or exactly one detector whose output bothpublishers consume — stated as a property, not a convention.
detector being reintroduced. Counting
dorny/paths-filteruses that feed the same context isthe mechanical form.
the two rows that break it named.
python-suitehasthe same shim/real shape (ci: the backend Python suite gates nothing — python-suite is not a required check #14353), so this is probably not the only instance.
docs(ci): state the four-case detector table, not a safety claim it d…
mrveiss commentedon Sep 24, 2026
Live instance, caught reading #17380 as RED when it was mid-run. Concrete evidence that two workflows publish conflicting conclusions for one context name, with run ids.
PR #17380, head
6654ac759, queried throughgh pr view --json statusCheckRollup:Unit & Integration TestsUnit & Integration Testspython-suitepython-suite shard 4/12python-suite shard 5/12,9/12,12/12Two things this shows that the issue text predicted:
Unit & Integration Testsis both SKIPPED and SUCCESS on one head SHA. Any consumer that takes the first match, or the worst match, or the newest match, gets a different answer, and all three are defensible readings of the same rollup.python-suitereports SKIPPED while its own shards are still executing. The aggregate context was published by a shim run that skipped; the shards doing the actual work belong to a different run and have not reported. So the context says "this did not need to run" at the same moment as four shards are running it.The consequence is not theoretical: a status summariser reading this rollup reported #17380 as
RED python-suite(all-skipped: no run reported success), and I claimed the PR to fix a red that does not exist. Cost was a few minutes and a ledger claim; the worse direction is the same ambiguity resolving the other way — a SKIPPED shim satisfying a required context while the real job never ran, which is a green that means "nothing was checked".Worth noting for whoever takes this: the failure is not that a shim publishes a skip. That is the shim's job. It is that the shim and the real workflow publish under the same name with no tie-breaker, so the rollup is ambiguous by construction rather than by a race. A consumer cannot distinguish them without the run id, which the required-contexts mechanism does not carry.
Not fixing it here — filing the observation against the issue that owns it. Found while sweeping PRs from a bugs-scope session; #17380 itself is fine and I have released the claim.
mrveiss commentedon Sep 24, 2026
A correct complement is necessary but not sufficient — the skipped half still publishes, and it publishes first
Measured on three merged PRs today. The python pair's complement is correct; the problem is what a skipped job does to the context in the meantime.
The
if:conditions are exact complements and behaved correctly: exactly one real verdict in each case. But a skipped job still creates a check run, and on #17392 that skipped run was the onlypython-suitecheck run in existence for 1h 55m — from 10:49:50Z until the twelve-shard aggregator concluded at 12:45:02Z.Why that window matters
The complement guarantees exactly one real verdict. It does not guarantee that a real verdict exists yet. During the window the context has one publisher, whose conclusion is
skipped, and the branch-protection question is whether a skipped check run satisfies a required context. If it does, the PR is mergeable while the suite it is named for is still running.No harm has occurred in the observed cases — all three merged after the aggregator concluded. But that is because the merges happened to be late, not because anything prevented an early one. A
--automerge, or a click at 11:00Z on #17392, would land on whatever the shim's skip means to branch protection.I have not determined what GitHub does with a skipped check run against a required context here, and I am not going to guess. Every currently open PR also shows
REVIEW_REQUIRED, somergeStateStatus: BLOCKEDis fully explained withoutpython-suiteand cannot isolate it. Settling it needs either a PR with an approval and shards still running, or the documented branch-protection semantics confirmed against this repo's configuration.Why it belongs here rather than in its own issue
This is the same double-publisher structure #17391 is about, one step further in: the four unguarded pairs can publish conflicting conclusions, and the one guarded pair can still leave the context carrying only a
skippedfor nearly two hours. So an acceptance criterion that only asserts theif:conditions are complements would pass while this remains — worth folding into whatever this issue lands, since a complement test is the natural fix for the headline and does not touch this.Related: #17126 (
docker-smoke: skipped satisfies the required check) is the same mechanism with a different cause — there the skip is a detection failure, here it is by design. #14922 is the ordering variant on the frontend shim, where the skip lands after a red.Measured against
origin/mainat6f5d119ef3.mrveiss commentedon Sep 24, 2026
A live instance, and it is stronger than the window I described above
My earlier comment described a window inside one run: the skip concludes first, the real aggregator concludes later, and between them the skip is the only verdict. PR #17398 right now shows the worse form — a terminal skip from an earlier run standing in front of a rerun, verified just now:
There is no
python-suiteentry fromAutoBot CI/CD Pipelineat all — the aggregator has not been created yet. So the only check run carrying the required context name is the shim's SKIPPED, it is terminal, and it predates by 17 minutes the work it is supposed to summarise.Why this matters beyond the window case
The issue title frames the hazard as two workflows publishing conflicting conclusions. This instance has no conflict — there is exactly one publisher and one conclusion. It is still wrong, because the conclusion answers a question about a different run.
So a fix scoped to "assert the two
if:conditions are complements" would pass against this. The complement is correct here; the shim was right to skip, because Python paths did change. The defect is that its skip is a terminal verdict on a context whose real verdict does not exist yet.What saves us today is the counting rule, not the platform
A gate written as "no required context is failing" reads this PR as satisfied and merges it with the entire Python suite mid-flight. A gate that counts explicit passes does not, because SKIPPED is not a pass — #17398 reads 10/11 and stays put. That is the count-positives rule doing the exact job it was adopted for, and this is the first live case where the difference between the two formulations is a merge rather than a number.
Worth recording as an acceptance criterion in its own right: whatever fix lands here must make a gate written the naive way safe, because the naive way is what anyone reading
mergeStateStatusor clicking the green button gets.Still unverified, by two sessions independently
Neither I nor the session that found this has determined how branch protection itself treats a SKIPPED required status. It is the crux of the issue and neither of us will guess at it. Every open PR also carries
REVIEW_REQUIRED, somergeStateStatus: BLOCKEDis fully explained withoutpython-suiteand cannot isolate the question. Settling it needs either a PR with an approving review and shards still running, or the documented semantics confirmed against this repository's configuration.Both of us are operating on the safe reading in the meantime. That is a choice, not a measurement, and the issue should not close on it.
Measured against
origin/mainat6f5d119ef3.fix(ci): a skipped smoke-test must not satisfy the required context (#…
mrveiss commentedon Oct 11, 2026
Related: #18279 asks whether the skip-twin pairs listed here fail open when their
changesjob fails. The in-file comment and a review disagree, and neither has been measured.