fix(hydration): preserve merge-poll gate write across concurrent wave republish (#84) - #85
Merged
Conversation
… republish (#84) The merge-readiness poll (`refreshPRState`) writes a freshly-unblocked gate directly to `prGates` under the current `checksGeneration`. A hydration wave that snapshotted `prGates` before that write republishes the whole map at publish time, clobbering the poll's write back to the stale "blocked" gate and re-hiding the Merge button — the exact UX the poll exists to fix. The `checksGeneration` guard only catches cross-generation clobbers; this is a same-generation interleave across `await` suspension points. Track keys the poll writes in `mergePollGateWrites` (cleared at each wave start) and have `publishChecks` overlay their live values onto the batch before the whole-map assign, so the poll's write survives. Skips keys the wave pruned. Adds a deterministic `GatedGitHubAPI`-driven regression test. Closes #84
…ays-wins (#84) Addresses code-review findings on the first cut. The Set-based overlay preferred the merge poll's gate for a key over the *whole* generation, which (1) let a stale poll "mergeable" resurrect over the wave's fresher full-fetch "not-mergeable" (Merge shown → 405 on click), and (2) conflated a failed concurrent fetch with a prune, dropping the poll's fresh gate. Replace it with a monotonic gate write-clock (`gateWriteClock`/`prGateSeq`): both the poll's single-key write and the wave's fresh full-fetch stamp their keys. `publishChecks` now merges gates onto the live map per key, applying a wave gate only when it was freshly re-fetched AND its tick beats the live one — so the most recent writer wins in both directions. A checks-only carry-over or a failed fetch is not "fresh", so it leaves the live gate untouched instead of clobbering or blanking it. Adds a second regression test for the symmetric case (fresher wave fetch beats a staler poll write).
…me (#84) Second-review follow-up. The commit-time write-clock still reproduced #84 in the primary post-approval case: the wave's full fetch issues its detail early (reads the still-stale "blocked"), then folds *after* its slow checks/reviews legs — so it commits after the poll's faster single-request "clean" write and, stamped at fold time, wrongly wins and re-hides Merge. Stamp the clock when each fetch is *issued* instead of when it folds/commits, for both writers: the wave ticks a per-full-fetch issue seq up front, and the poll ticks before its detail request and defers to any concurrently-issued newer write. Now the observation that read the newer server state wins regardless of commit order, in both clobber directions. Tests: replace the two prior cases with three that pin the interleavings by issue order — checks-only carry-over, later-issued poll beats earlier-issued wave full-fetch (the primary #84 trigger; verified it fails under commit-time stamping), and later-issued wave fetch beats earlier poll write.
… gate stamping (#84)
jaylann
added a commit
that referenced
this pull request
Jul 5, 2026
Merge of stage (#85) added a required gateIssueSeq to fold(); the GraphQL batch path now ticks nextGateWriteSeq() per PR before issuing the batch and passes it when folding, so a batch-hydrated gate obeys the same merge-poll write-preserve ordering as the REST drain (#84). Fixes the CI build/test break from the stale base.
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.
What
Fixes #84 — the post-approval merge-readiness poll's gate write being clobbered by a concurrent CI hydration wave's batch republish, which briefly hides (or wrongly shows) the Merge button after an approval.
Root cause
prGates(the per-PR Approve/Merge action gate) is written by two@MainActorflows that interleave acrossawait:drainChecks/fold/publishChecks), which refetches many PRs and republishes gates in batches;refreshPRState, a single-PR gate refetch driven by the post-approve poll.A wave that read a PR's gate before the poll's single-key write would republish the whole map and clobber it. The
checksGenerationguard only catches cross-generation clobbers; this is a same-generation interleave. #82's.checksOnlypath widened the window.Approach — issue-time write-recency
The naive fixes fail: "poll always wins" shows a Merge button that 405s when the base advanced; a commit-time clock still loses because the wave's full fetch reads its gate early (from the PR detail) but folds late (after its slow checks/reviews legs), so a wave that observed older state commits last and wrongly wins.
Fix: a monotonic issue-time clock (
gateWriteClock+prGateSeq). Both writers take a tick when they issue their fetch — the wave stamps a per-full-fetch issue seq up front; the poll ticks before its detail request and defers to any concurrently-issued newer write.publishChecksmerges gates onto the live map per key, applying a wave gate only when its issue tick beats the live one. So the observation that read the newer server state wins in both clobber directions, regardless of commit order. Checks-only carry-overs and failed fetches don't compete (they keep the live gate rather than blanking it).prGateSeqis pruned alongsideprGatesat every mutation/reset site. A documented, bounded, self-healing residual remains (a whole wave shares one issue instant, so a mid-wave poll beats a late-queued key) — noted in code; per-key stamping would need a main-actor hop per fetch and isn't worth it for a transient gate flip.Tests
Three deterministic
GatedGitHubAPIregression tests pinning the interleavings by issue order:testMergePollGateWriteSurvivesChecksOnlyRepublish— checks-only carry-over can't clobber the poll.testLaterIssuedMergePollGateBeatsEarlierIssuedWaveFullFetch— the primary Merge-readiness poll gate write can be clobbered by a concurrent hydration wave's whole-map republish #84 trigger; verified it fails under commit-time stamping.testLaterIssuedWaveFetchBeatsEarlierMergePollGate— a fresher wave fetch beats a staler poll write (guards "poll always wins").just checkclean, full suite (269 tests) green. Reviewed over three rounds (multi-agent + single-agent); final review passed with no blocking findings.Closes #84