Repository navigation
ci(parity): gate that a required check can be superseded, not only that it can exist - #387
Conversation
CLOUD-503 A required check whose workflow never subscribes to `ready_for_review` can never be superseded, so a fully green PR wedges `land` and holds the lease
Why Measured 2026-08-12 ~23:40 UTC on PR #382, head
The cause is
Isolated on head Every Why it is intermittent rather than constant, which is why it has survived. Both are path-filtered, so it needs a PR touching Why the existing sensor missed it. Distinct from the two issues it looks like. CLOUD-436's latest-run-per-name cannot help — there is exactly one run to select. Its proposed weaker condition ("a definitive red anywhere is terminal") cannot help either — there was no red at all. Both are about reading a set; this is about a set that can never become readable. Refinement — Ready (a required check's verdict stops depending on which event fired) Refinement gate: Definition of Ready & Done. This body carries only specializations.
The fix costs no CI minutes, which is worth stating because it looks like it should Adding Acceptance
Reported first as a comment on CLOUD-494 and split out here rather than folded into it, on that issue's own precedent — it was itself split out of CLOUD-240's property set. Folding a third property into an issue whose PR is already readied would leave its acceptance unmet the moment that PR merges. |
…at it can exist CLOUD-503's gate half. The workflow fix landed in 49b89dd; this is the property that stops it regressing, and the reason it was split has gone — CLOUD-494's PR landed, so `ci-local-parity` and its suite are free. Property 11: every `pull_request` workflow producing a name in `$CI_REQUIRED_CHECKS` must declare `ready_for_review` in its `types:`. It sits on the seam properties 5 and 6 already own. Those couple `CI_REQUIRED_CHECKS` to job names in both directions — a required name that is no job fails, a `pull_request` job missing from the list fails. Both ask whether a run can EXIST. This asks whether it can be REFRESHED, which is the half that was missing: omitting `types:` defaults to `[opened, synchronize, reopened]`, every job is draft-gated because property 1 makes that universal, so a PR created as a draft mints a `skipped` run on `opened` and no event remains that could replace it. `land` readies before pushing and its push moves nothing whenever HEAD is already on the remote, so the ready event is the only one that can arrive. `checks-green` correctly refuses to read a skip as an answer, and the poll never terminates while the branch holds the fleet lease. SCOPED TO WORKFLOWS PRODUCING A REQUIRED NAME, deliberately. A workflow whose checks nothing waits on may miss the ready event with no consequence, and demanding the key of every `pull_request` workflow would assert more than the failure needs. That scoping is why the check sits beside the job-name collection inside the loop rather than in the `want_names` comparison at the end, which has the names but no longer knows which file they came from. The `pull_request:` sub-block is read on its own, never the whole `on:` block: a `types:` belonging to another trigger says nothing about this one, and reading them together would let one satisfy the other. SHOWN ABLE TO FAIL, BOTH DIRECTIONS (CLOUD-418). A fixture naming a required check and omitting `ready_for_review` is refused; one producing no required name is not asked for the key. Mutation-checked rather than argued: replacing the comparison with `true` reddens case 3 and only case 3. The `workflow` fixture helper hardcoded `types: [opened, synchronize]`, which is non-compliant under this property — so it gains a `types` argument defaulting to the full set, and the `fanin` fixture's own `final.yml` is corrected the same way. Without that both would have failed for the new reason rather than their own, which is a suite that no longer tests what its names say. Refs: CLOUD-503
2e87d16 to
e783ae9
Compare
|
|
/fast-forward |
2 participants
CLOUD-503's gate half. The workflow fix landed in
49b89dd; this is the property that stops it regressing.The property
Every
pull_requestworkflow producing a name in$CI_REQUIRED_CHECKSmust declareready_for_reviewin itstypes:.It sits on the seam properties 5 and 6 already own. Those couple
CI_REQUIRED_CHECKSto job names in both directions — a required name that is no job fails, apull_requestjob missing from the list fails. Both ask whether a run can exist. This asks whether it can be refreshed, which is the half that was missing:types:defaults the trigger to[opened, synchronize, reopened]skippedrun onopened, and no event remains that could replace itlandreadies before pushing, and its push moves nothing whenever HEAD is already on the remote, so the ready event is the only one that can arrivechecks-greencorrectly refuses to read a skip as an answer, and the poll never terminates while the branch holds the fleet leaseMeasured as a deadlock across #382 and #385 at once, both fully green but for one such name, with
mainfrozen so neither could rebase out of it.Scoping, deliberately
A workflow whose checks nothing waits on may miss the ready event with no consequence. Demanding the key of every
pull_requestworkflow would assert more than the failure needs, so the property is scoped to workflows producing a required name — which is why it sits beside the job-name collection inside the loop rather than in thewant_namescomparison at the end, which has the names but no longer knows which file they came from.The
pull_request:sub-block is read on its own, never the wholeon:block: atypes:belonging to another trigger says nothing about this one, and reading them together would let one satisfy the other.Shown able to fail, both directions
ready_for_reviewMutation-checked rather than argued (CLOUD-418): replacing the comparison with
truereddens case 3 and only case 3.A fixture correction that had to ride along
The
workflowhelper hardcodedtypes: [opened, synchronize], andfanin'sfinal.ymldid the same — both non-compliant under this property. Left alone they would have failed for the new reason rather than their own, which is a suite that no longer tests what its names say. The helper gains atypesargument defaulting to the full set, so the refusing case overrides exactly one thing.Why this is here and not on the earlier PR
It was split out because PR #385 was readied and rewriting these same lines. That PR has landed (CLOUD-494 is Done), so the reason is gone.
Refs: CLOUD-503