fix(hydration): preserve merge-poll gate write across concurrent wave republish (#84)

gbar/Sources/Store/AppStore.swift

@@ -43,6 +43,19 @@ final class AppStore {
     /// hydrated → the row stays optimistic (buttons show). Written only by the hydration wave
     /// and the reset sites here; views read only.
     var prGates: [PRCheckKey: PRGate] = [:]
+    /// Monotonic clock for `prGates` writes. Both writers — the hydration wave's fresh full-fetch
+    /// (`publishChecks`) and the merge-readiness poll's single-key write (`refreshPRState`) — take
+    /// a tick when they *issue* their detail fetch, so a batch republish can resolve each key by
+    /// which fetch read the newer server state instead of clobbering a fresher gate with a staler
+    /// one (#84). Issue-time, not commit-time: the wave's full fetch folds only after its slow
+    /// checks/reviews legs, so a fetch that observed an older gate can commit *after* a fresher
+    /// poll write — issue order is the honest recency signal. Bookkeeping, never read from a view.
+    @ObservationIgnored
+    var gateWriteClock = 0
+    /// The `gateWriteClock` issue tick of the fetch whose result currently sits in `prGates[key]`
+    /// (see `gateWriteClock`). Pruned alongside `prGates`.
+    @ObservationIgnored
+    var prGateSeq: [PRCheckKey: Int] = [:]
     /// What each PR looked like at its last successful hydration — the `updated_at` we hydrated
     /// against and whether its CI had settled. Lets the next wave skip the detail/reviews/check-runs
     /// refetch for a PR that hasn't changed (see `canSkipHydration`). Never read from a view, so
@@ -706,6 +719,7 @@ extension AppStore {
         notifications.removeAll { $0.account.id == id }
         prChecks = prChecks.filter { $0.key.accountID != id }
         prGates = prGates.filter { $0.key.accountID != id }
+        prGateSeq = prGateSeq.filter { $0.key.accountID != id }
         repoMergeInfo = repoMergeInfo.filter { !$0.key.hasPrefix("\(id)\n") }
         starredByAccount[id] = nil
         actionRuns.removeAll { $0.account.id == id }
@@ -755,6 +769,7 @@ extension AppStore {
         notifications = []
         prChecks = [:]
         prGates = [:]
+        prGateSeq = [:]
         prHydrationMark = [:]
         repoMergeInfo = [:]
         starredByAccount = [:]

gbar/Sources/Store/AppStoreHydration.swift

@@ -86,17 +86,31 @@ extension AppStore {
     func refreshPRState(for item: AccountItem, using api: GitHubAPI) async -> PRGate? {
         let key = PRCheckKey(accountID: item.account.id, prID: item.issue.id)
         let generation = checksGeneration
+        // Stamp the moment we *issue* the fetch, not when we commit it: a later-issued fetch read a
+        // newer server state, so it must win by recency even if an earlier-issued write commits after
+        // it. See `gateWriteClock`/`publishChecks` (#84).
+        let issueSeq = nextGateWriteSeq()
         let cacheKey = repoPermissionKey(accountID: item.account.id, slug: item.issue.repositorySlug)
         let mergeInfo = repoMergeInfo[cacheKey]
         let state = await Self.fetchPRState(
             for: item.issue, login: item.account.login, mergeInfo: mergeInfo, using: api, includeChecks: false
         )
         guard checksGeneration == generation else { return prGates[key] }
         guard let gate = state.gate else { return prGates[key] } // failed fetch — keep the old gate
+        // Defer to a concurrently-issued *newer* write (a wave fetch issued after us) rather than
+        // clobber it with our older observation.
+        guard issueSeq > (prGateSeq[key] ?? .min) else { return prGates[key] }
         prGates[key] = gate
+        prGateSeq[key] = issueSeq
         return gate
     }
 
+    /// Advance and return the next `prGates` write-clock tick.
+    func nextGateWriteSeq() -> Int {
+        gateWriteClock += 1
+        return gateWriteClock
+    }
+
     /// After an approval that didn't immediately unblock Merge, poll the PR's gate in the
     /// background until it reports mergeable (or the backoff schedule is exhausted — the PR is
     /// genuinely still blocked, e.g. by a second required approval or a failing check). GitHub
@@ -193,6 +207,7 @@ extension AppStore {
         let live = Set(prs.map(\.key))
         prChecks = prChecks.filter { live.contains($0.key) }
         prGates = prGates.filter { live.contains($0.key) }
+        prGateSeq = prGateSeq.filter { live.contains($0.key) }
         prHydrationMark = prHydrationMark.filter { live.contains($0.key) }
         lastCheckStatus = lastCheckStatus.filter { live.contains($0.key) }
         guard !prs.isEmpty else {
@@ -234,6 +249,24 @@ extension AppStore {
         // Decide each PR's plan on the actor (reading `prGates`/`prHydrationMark`), so the
         // nonisolated `schedule()` below just spawns from the precomputed list.
         let plans = fetchPlans(for: prs, mergeInfoByKey: mergeInfoByKey)
+        // Issue-time write-clock tick per full fetch (a checks-only plan carries the cached gate
+        // unchanged, so it never competes). Ticked *before* the fetches run, not when they fold: a
+        // full fetch folds only after its slow checks/reviews legs, so a fetch that observed an older
+        // gate can commit after a fresher poll write. Comparing issue ticks lets the observation that
+        // read newer state win regardless of commit order (#84). Granularity is the whole wave vs. a
+        // concurrent poll — the poll either issued before this drain (lower tick) or after (higher) —
+        // which is the level the poll-vs-wave decision turns on. (Assigned here on the actor because
+        // the group's `schedule()` runs nonisolated and can't tick the main-actor clock.)
+        // Note: because the whole wave shares one issue instant, a poll that issues mid-drain beats
+        // even a late-queued key whose own network fetch (capped concurrency) read newer state. That
+        // residual is narrow and self-heals: the next wave out-ticks the poll, and the poll keeps
+        // refetching while `!mergeable`. True per-key issue stamping would need a main-actor hop per
+        // fetch, which isn't worth it for a transient, self-correcting gate flip.
+        var seqBuilder: [PRCheckKey: Int] = [:]
+        for plan in plans {
+            if case let .full(pr, _) = plan { seqBuilder[pr.key] = nextGateWriteSeq() }
+        }
+        let gateIssueSeq = seqBuilder
         await withTaskGroup(of: (PRCheckKey, PRState).self) { group in
             var next = 0
             func schedule() {
@@ -250,7 +283,10 @@ extension AppStore {
             var sinceFlush = 0
             while let (key, state) = await group.next() {
                 guard checksGeneration == generation else { continue }
-                fold(key: key, state: state, issueByKey: issueByKey, into: &pending)
+                // A fresh gate is a successful full fetch (a checks-only carry-over or a failed fetch
+                // has no issue tick here, so it leaves the live gate untouched at publish).
+                let freshSeq = state.gate != nil ? gateIssueSeq[key] : nil
+                fold(key: key, state: state, issueByKey: issueByKey, gateIssueSeq: freshSeq, into: &pending)
                 sinceFlush += 1
                 if sinceFlush >= Self.checksFlushBatch {
                     publishChecks(pending, generation: generation)
@@ -304,6 +340,11 @@ extension AppStore {
         var checks: [PRCheckKey: PRChecks]
         var gates: [PRCheckKey: PRGate]
         var marks: [PRCheckKey: HydrationMark]
+        /// Issue-time write-clock ticks for the keys this wave *freshly* re-fetched a gate for (a
+        /// successful full fetch — not a checks-only carry-over or a failed fetch). Only these keys
+        /// are eligible to overwrite the live gate at publish, and only when their tick beats the
+        /// live `prGateSeq` (so a merge-poll write issued later is preserved, #84).
+        var freshGateSeq: [PRCheckKey: Int] = [:]
     }
 
     /// Fold one completed PR's resolved state into the pending hydration maps, firing a pass/fail
@@ -314,9 +355,16 @@ extension AppStore {
         key: PRCheckKey,
         state: PRState,
         issueByKey: [PRCheckKey: SearchIssue],
+        gateIssueSeq: Int?,
         into pending: inout PendingHydration
     ) {
         pending.gates[key] = state.gate
+        // Carry the fetch's issue tick so publish can compare its recency against a concurrent
+        // merge-poll write for the same key (#84). `nil` for a checks-only carry-over or a failed
+        // fetch — those leave the live gate (and its stamp) untouched at publish.
+        if let gateIssueSeq {
+            pending.freshGateSeq[key] = gateIssueSeq
+        }
         if let resolved = state.checks {
             if let issue = issueByKey[key] {
                 notifyCheckStatusChange(key: key, pr: issue, newStatus: resolved.status)
@@ -339,9 +387,26 @@ extension AppStore {
     /// Publish a hydration batch as single whole-map assignments (one view invalidation each),
     /// unless a newer wave has superseded this one. The marks are view-invisible but published here
     /// too so they stay consistent with the gates/checks they describe.
+    ///
+    /// Checks and marks are reassigned wholesale from the batch, but **gates merge by issue-time
+    /// recency** onto the live map: only keys this wave *freshly* re-fetched (`freshGateSeq`)
+    /// overwrite the live gate, and only when their issue tick beats the live `prGateSeq`. This
+    /// keeps a merge-readiness poll's write that read newer state from being clobbered by a wave
+    /// fetch that read older state but folded later (#84), while a wave fetch that genuinely read
+    /// newer state still wins. Keys the wave didn't freshly fetch (a checks-only carry-over, or a
+    /// failed fetch) keep their live gate rather than blanking it on a transient miss.
     private func publishChecks(_ pending: PendingHydration, generation: Int) {
         guard checksGeneration == generation else { return }
-        prGates = pending.gates
+        var gates = prGates
+        var seqs = prGateSeq
+        for (key, seq) in pending.freshGateSeq where seq > (seqs[key] ?? .min) {
+            if let gate = pending.gates[key] {
+                gates[key] = gate
+                seqs[key] = seq
+            }
+        }
+        prGates = gates
+        prGateSeq = seqs
         prChecks = pending.checks
         prHydrationMark = pending.marks
     }

gbarTests/Sources/AppStoreTests.swift

@@ -381,6 +381,146 @@ final class AppStoreTests: XCTestCase {
         XCTAssertNil(store.prChecks[key(300)])
     }
 
+    /// #84, checks-only variant: an unchanged PR's wave re-reads only its check-runs and carries the
+    /// cached "blocked" gate. If the merge-readiness poll writes the unblocked gate mid-flight, the
+    /// wave's batch republish must not clobber it back to "blocked" and re-hide Merge. A carried gate
+    /// never competes, so the poll's write survives.
+    func testMergePollGateWriteSurvivesChecksOnlyRepublish() async throws {
+        // Stable PR (unchanged `updated_at`) so the second wave takes the checks-only path.
+        let prIssue = SearchIssue.stub(id: 100, number: 7, updatedAt: Date(timeIntervalSince1970: 1_700_000_000))
+
+        var seed = FakeGitHubAPI()
+        seed.defaultResult = [prIssue]
+        seed.pullRequestResult = .stub(number: 7, mergeableState: "blocked")
+        seed.repositoryResult = .stub(push: true)
+        seed.reviewsResult = []
+        let store = try makeStore(api: seed)
+
+        await store.refresh()
+        let pr = try XCTUnwrap(store.prSections.flatMap(\.items).first { $0.issue.number == 7 })
+        try await waitUntil { store.gate(for: pr) != nil }
+        XCTAssertEqual(store.gate(for: pr)?.mergeable, false) // seeded blocked → checks-only next wave
+
+        // Second wave blocks inside `checkRuns`, parked before republishing.
+        let gated = GatedGitHubAPI(
+            search: [prIssue],
+            pullRequest: .stub(number: 7, mergeableState: "blocked"),
+            checkRuns: [.stub(id: 1, conclusion: "success")]
+        )
+        store.makeAPI = { _, _ in gated }
+        await store.refresh()
+        await gated.waitUntilBlocked()
+        let wave = store.checksHydrationTaskForTests
+
+        // While parked, the poll observes the recompute and writes the unblocked gate.
+        var cleanFake = FakeGitHubAPI()
+        cleanFake.pullRequestResult = .stub(number: 7, mergeableState: "clean")
+        cleanFake.repositoryResult = .stub(push: true)
+        cleanFake.reviewsResult = [.stub(login: "octocat", state: "APPROVED")]
+        let refreshed = await store.refreshPRState(for: pr, using: cleanFake)
+        XCTAssertEqual(refreshed?.mergeable, true) // poll saw the unblock
+
+        await gated.release()
+        await wave?.value
+
+        XCTAssertTrue(try XCTUnwrap(store.gate(for: pr)).mergeable) // Merge stays shown
+    }
+
+    /// #84, the primary full-fetch trigger: after an approval bumps `updated_at`, the wave does a
+    /// *full* re-fetch. It issues its detail early and reads the still-stale "blocked" (GitHub hasn't
+    /// recomputed), then folds it late — after its slow checks/reviews legs. The poll issues *later*,
+    /// reads the recomputed "clean", and writes it. Because the wave's fetch read older state (issued
+    /// first) it must NOT clobber the poll at publish, even though it commits last. A commit-time
+    /// clock would fail this (the wave folds last); issue-time recency preserves the poll's gate.
+    func testLaterIssuedMergePollGateBeatsEarlierIssuedWaveFullFetch() async throws {
+        let seeded = SearchIssue.stub(id: 100, number: 7, updatedAt: Date(timeIntervalSince1970: 1_700_000_000))
+        // The approval bumps `updated_at`, so the second wave can't reuse the mark → full re-fetch.
+        let approved = SearchIssue.stub(id: 100, number: 7, updatedAt: Date(timeIntervalSince1970: 1_700_000_500))
+
+        var seed = FakeGitHubAPI()
+        seed.defaultResult = [seeded]
+        seed.pullRequestResult = .stub(number: 7, mergeableState: "blocked")
+        seed.repositoryResult = .stub(push: true)
+        seed.reviewsResult = []
+        let store = try makeStore(api: seed)
+
+        await store.refresh()
+        let pr = try XCTUnwrap(store.prSections.flatMap(\.items).first { $0.issue.number == 7 })
+        try await waitUntil { store.gate(for: pr) != nil }
+        XCTAssertEqual(store.gate(for: pr)?.mergeable, false) // seeded blocked
+
+        // Second wave full-fetches the still-"blocked" detail (issued early) and blocks in checkRuns
+        // before folding.
+        let gated = GatedGitHubAPI(
+            search: [approved],
+            pullRequest: .stub(number: 7, mergeableState: "blocked"),
+            checkRuns: [.stub(id: 1, conclusion: "success")]
+        )
+        store.makeAPI = { _, _ in gated }
+        await store.refresh()
+        await gated.waitUntilBlocked()
+        let wave = store.checksHydrationTaskForTests
+
+        // Poll issues *after* the wave's detail (later ⇒ read newer state) and writes "clean".
+        var cleanFake = FakeGitHubAPI()
+        cleanFake.pullRequestResult = .stub(number: 7, mergeableState: "clean")
+        cleanFake.repositoryResult = .stub(push: true)
+        cleanFake.reviewsResult = [.stub(login: "octocat", state: "APPROVED")]
+        let refreshed = await store.refreshPRState(for: pr, using: cleanFake)
+        XCTAssertEqual(refreshed?.mergeable, true)
+
+        // Wave folds its stale "blocked" last — but it read older state, so the poll's gate must win.
+        await gated.release()
+        await wave?.value
+
+        XCTAssertTrue(try XCTUnwrap(store.gate(for: pr)).mergeable) // Merge stays shown
+    }
+
+    /// The symmetric case: a wave fetch that read *newer* state must win over an earlier poll write.
+    /// The poll confirmed "clean" first; then the base advanced and the wave (issued later) reads
+    /// "behind". The recency merge must publish the wave's not-mergeable gate rather than resurrect
+    /// the poll's stale "clean" and show a Merge button that would 405. (A "poll always wins" overlay
+    /// regresses here.)
+    func testLaterIssuedWaveFetchBeatsEarlierMergePollGate() async throws {
+        let prIssue = SearchIssue.stub(id: 100, number: 7, updatedAt: Date(timeIntervalSince1970: 1_700_000_000))
+
+        // Seed a *mergeable* gate so the second wave can't skip to checks-only → fresh full re-fetch.
+        var seed = FakeGitHubAPI()
+        seed.defaultResult = [prIssue]
+        seed.pullRequestResult = .stub(number: 7, mergeableState: "clean")
+        seed.repositoryResult = .stub(push: true)
+        seed.reviewsResult = []
+        let store = try makeStore(api: seed)
+
+        await store.refresh()
+        let pr = try XCTUnwrap(store.prSections.flatMap(\.items).first { $0.issue.number == 7 })
+        try await waitUntil { store.gate(for: pr) != nil }
+        XCTAssertEqual(store.gate(for: pr)?.mergeable, true)
+
+        // Poll writes "clean" *before* the second wave issues its fetch (earlier observation).
+        var cleanFake = FakeGitHubAPI()
+        cleanFake.pullRequestResult = .stub(number: 7, mergeableState: "clean")
+        cleanFake.repositoryResult = .stub(push: true)
+        cleanFake.reviewsResult = []
+        _ = await store.refreshPRState(for: pr, using: cleanFake)
+
+        // Second wave issues later and reads "behind" (base advanced); its fresher fetch must win.
+        let gated = GatedGitHubAPI(
+            search: [prIssue],
+            pullRequest: .stub(number: 7, mergeableState: "behind"),
+            checkRuns: [.stub(id: 1, conclusion: "success")]
+        )
+        store.makeAPI = { _, _ in gated }
+        await store.refresh()
+        await gated.waitUntilBlocked()
+        let wave = store.checksHydrationTaskForTests
+
+        await gated.release()
+        await wave?.value
+
+        XCTAssertFalse(try XCTUnwrap(store.gate(for: pr)).mergeable) // fresher "behind" wins → Merge hidden
+    }
+
     // MARK: - Action gate derivation
 
     func testDeriveGateMergeableStates() {