Repository navigation
fix(hub): keep a row written after a reconcile pass read the state - #707
Merged
TheGreatAxios merged 1 commit intoOct 28, 2026
Conversation
A reconcile pass that lost the save race merged its plan over rows by comparing timestamps, so a head the coalescer queued moments earlier was overwritten with the pass's own row. The coalescer's failed send then found no queued row, left the head without `unrouted`, and it waited for the new-head grace and the next ordinary pass. The merge now keeps any row written after the pass read the state, and the pass mails none of those heads. A chain timeout in `settled` also prints the pull request's triage rows.
TheGreatAxios
commented
Oct 28, 2026
TheGreatAxios
left a comment
Contributor
Author
There was a problem hiding this comment.
When a reconcile pass loses the save race and reloads, it now keeps every row whose updatedAt changed since it read the state, and it mails none of those heads. It no longer compares the two clocks.
- Repro: the new interleave test fails 3/3 against 27729d5 (head 9 is never mailed) and passes 3/3 on 548cb91. With the coalescer's clock 5 ms later than the pass's, it passes on both, so the rule no longer depends on clock order.
- No legitimate pass write is lost: a pass's row loses only to a row someone else wrote after the pass read the state, and only on the save after a conflict. If the pass moved a head to queued or running while the coalescer touched it too, the coalescer's row wins and the pass leaves that head unmailed, because the coalescer is already mailing it. Anything else the pass observed is derived again on the next pass.
restoreHeadputs back the row as it was before, with its oldupdatedAt, so a send that was undone still counts as unchanged and the pass's row wins. A head the pass never read (read.getis undefined) always goes to theirs, which is the reported race. Passes on one loop never overlap, so the other writers are the coalescer, the request routes andmarkBaseMoved, and each of them stamps a freshupdatedAt. settled():artifacts.artifacthastenant_id,title,contentandarchived_at, andtriageStateName(repo)matches the GitHub state key the chain repositories use. A missing artifact parses to[], so the timeout path does not throw on it.- There are no CI-wait leftovers, and the diff is clean.
hubandtests/chaintypecheck. The interleave, triage-reconciler, reconcile-plan and coalescer suites pass 88/88.
No findings.
TheGreatAxios
marked this pull request as ready for review
October 28, 2026 08:43
TheGreatAxios
deleted the
cl-11646-hold-a-head-as-unrouted-when-a-reconcile-pass-overwrote-its
branch
October 28, 2026 08:45
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
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.
A webhook's head could miss the quick retry after its mail reached no listener. A reconcile pass that lost the race to save the triage state merged its plan over the stored rows by comparing timestamps, so it could overwrite a head the coalescer had queued a few milliseconds earlier. When the coalescer's send then failed, it found no queued row to mark
unrouted, and the head waited for the new-head grace period and the next five-minute pass.What changed:
failDelivery, so the coalescer stays the only writer of the rows it queues. Marking the headunroutedwhatever its status would have left the pass free to overwrite a queued row and relied on a second write to repair it.settlednow prints the pull request's rows from the triage state.How it was verified:
settledoutput was not exercised.Closes CL-11646