fix(checks-green): order a name's runs by when they concluded - #936
Open
wenzowski wants to merge 1 commit into
Open
fix(checks-green): order a name's runs by when they concluded#936wenzowski wants to merge 1 commit into
wenzowski wants to merge 1 commit into
Conversation
|
Warning Review limit reachedNext included review available in 53 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
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. Comment |
wenzowski
force-pushed
the
claude/slow-session-start-n9ittp
branch
from
September 9, 2026 22:35
1f3d19c to
2ee918a
Compare
wenzowski
marked this pull request as ready for review
September 9, 2026 22:35
Contributor
Author
|
/fast-forward |
Contributor
|
Refusing to fast-forward #936 at |
Contributor
Author
|
/fast-forward |
Contributor
|
Refusing to fast-forward #936 at |
wenzowski
force-pushed
the
claude/slow-session-start-n9ittp
branch
from
September 10, 2026 03:23
b230e62 to
eedb0f0
Compare
`started_at` stops tracking event order once two events for one head are in flight together, and both directions of that have now been measured. `land`'s pipeline pushes, firing `synchronize` while the PR is still a draft and minting the skip set, then readies, firing `ready_for_review` and minting the real run. Processed concurrently, the skip's job can start LATER than the run it exists to be replaced by, and CLOUD-503's mechanism inverts: 9ea4da6 windows failure started 04:05:37, skipped twin 04:05:39 -> a definitively red head read as "no verdict", and land sat on it 30 minutes holding the landing lease 9ca2b3a one pull_request event minted two CI workflow runs at the identical run_started_at 16:10:20Z — 4986 skipped, 4987 ran and succeeded — jobs starting 16:10:30 and 16:10:26, with the check-run ids inverted against the workflow-run ids so the tie-break could not save it either CLOUD-1662 asked whether the ordering field costs a second read per poll or is already on the check-runs projection. Measured: `completed_at` is already there. A run that judged nothing concludes essentially when it is minted — the twin above concluded in one second — while a run that judged concludes when the work finished, at 16:31:39. Nothing here is a duration, so non-negotiable rule 3 is untouched: this is an absolute order over a fact each run carries, not a threshold on the gap. THAT IS ALSO WHAT BUYS BACK THE CASE CLOUD-1722 GAVE UP. A PR readied, graded green, then re-drafted re-runs the name as a skip which genuinely concludes AFTER the success it supersedes, so it still speaks for the name (CLOUD-247, CLOUD-327). By `started_at` alone that reading and the concurrent twin are identical, which is why that row could not tell them apart and reversed the draft economy; by conclusion they are opposite. CLOUD-1722's guard is KEPT and narrowed to readings carrying no conclusion stamp, where the key still falls back to `started_at` and the guard is what keeps a green head landable. A five-field reading is a live shape rather than a legacy one: `sonar-gate.sh` emits precisely that. The column is last in the TSV, so every case written before it orders exactly as it did — which leaves them the guard on this change rather than its casualties. A run that has not concluded sorts above every run that has. That is the reading, not a sentinel: not-yet-finished is later than any finish, so a head being re-judged holds the poll open instead of falling back to the success it is re-judging. Verified: 50/50 in `checks_green`, including the three arms this adds and every case CLOUD-1722 landed. The `skip-outranks-a-concurrent-failure` mutation — dropping the conclusion stamp back out of the key, which is "restore clock-ordering" — was applied and confirmed to redden `a_concurrent_skip_does_not_bury_the_failure_it_raced`. Refs: CLOUD-1662 BREAKING CHANGE: `checks_green::Run` gains a `completed_at` field, so a caller constructing it with a struct literal must name the new field. `constructible_struct_adds_field`, declared rather than dodged: the field is what carries the ordering, and a reading that cannot say when a run concluded is the defect this closes.
wenzowski
force-pushed
the
claude/slow-session-start-n9ittp
branch
from
September 10, 2026 03:48
eedb0f0 to
c2f79cf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes CLOUD-1662.
started_atstops tracking event order once two events for one head are inflight together.
land's pipeline pushes — firingsynchronizewhile the PR isstill a draft, minting the skip set — then readies, firing
ready_for_reviewandminting the real run. Processed concurrently, the skip's job can start later
than the run it exists to be replaced by, and CLOUD-503's mechanism inverts.
Measured in both directions this can fail:
9ea4da6awindowsfailure started 04:05:37,skippedtwin 04:05:39landsat on it 30 minutes holding the landing lease9ca2b3acpull_requestevent minted two CI workflow runs at the identicalrun_started_at16:10:20Z — 4986 skipped, 4987 ran and succeeded — jobs starting 16:10:30 and 16:10:26On
9ca2b3acthe check-run ids were inverted against the workflow-run ids aswell, so the tie-break could not save it either.
The field
CLOUD-1662 asked whether the ordering field costs a second read per poll or is
already on the check-runs projection. Measured:
completed_atis alreadythere. A run that judged nothing concludes essentially when it is minted — the
twin above concluded in one second — while a run that judged concludes when the
work finished, at 16:31:39.
Nothing here is a duration, so non-negotiable rule 3 is untouched: this is an
absolute order over a fact each run carries, not a threshold on the gap.
It buys back the case CLOUD-1722 gave up
A PR readied, graded green, then re-drafted re-runs the name as a skip which
genuinely concludes after the success it supersedes, so it still speaks for the
name (CLOUD-247, CLOUD-327). By
started_atalone that reading and theconcurrent twin are identical — which is why CLOUD-1722 could not tell them apart
and reversed the draft economy. By conclusion they are opposite.
That row's own reasoning records
check_suite, the workflow-run id and thecheck-run id as tested and rejected.
completed_atis not among them and is notan id.
What is kept
CLOUD-1722's completed-but-unanswered guard is kept, narrowed to readings
carrying no conclusion stamp — where the key still falls back to
started_atandthe guard is what keeps a green head landable. A five-field reading is a live
shape rather than a legacy one:
sonar-gate.shemits precisely that. The columnis last in the TSV, so every case written before it orders exactly as it did,
which leaves them the guard on this change rather than its casualties.
A run that has not concluded sorts above every run that has — the reading, not a
sentinel: not-yet-finished is later than any finish, so a head being re-judged
holds the poll open instead of falling back to the success it is re-judging.
Verification
checks_green, including the three arms this adds and every caseCLOUD-1722 landed.
skip-outranks-a-concurrent-failuremutation — dropping the conclusionstamp back out of the key, which is "restore clock-ordering" — was applied and
confirmed to redden
a_concurrent_skip_does_not_bury_the_failure_it_raced.Noted, not fixed here
git::patch_identitiesis bounded byWindow::DEFAULT= 1000 commits, so aduplicate that landed more than 1000 commits back is never dropped. Unrelated to
this row; wants its own.
Generated by Claude Code