Menu ▾ ▴

#140 fix(automations,pulls): re-read review state from disk before gating

closed
nobody
bug (36)
2026-08-04
2026-08-04
Anonymous
No

Originally created by: theBGuy
Originally owned by: theBGuy

AI review automations gate on persisted review history and dismissal watermarks, but the Tauri plugin-store caches its contents per process and only loads at launch. A second GitDesktop window/instance — or a restarted dev session — therefore gated against a launch-time snapshot, concluded "never reviewed", and re-ran a review another instance had already delivered. This adds an opt-in fresh read that reloads the store from disk at the gates, and flushes dismissal writes immediately so a fresh read can't race past them.

Persistence layer

  • Adds a read(repo, opts) helper in src/lib/pulls/reviews-history.ts that reloads the store from disk before merging records when fresh is set, running the reload inside the existing serialize queue so it's ordered against in-flight mutations. getLatestReview and listReviews both take the new opts?: { fresh?: boolean } and route through it.
  • Adds reloadRaw() to src/lib/automations/dismissals.ts, guarding the case where reload() rejects with a raw io error on a store file that doesn't exist yet (unlike load()), and falling back to in-memory state on any failure — mirroring the guard already in reviews-history.ts. getDismissedHead gains the same fresh option.
  • Makes setDismissedHead and clearDismissedHead in dismissals.ts await store.save() instead of relying on autoSave's ~100ms debounce; without the explicit flush, a fresh gate read inside that window would discard a just-written dismissal (or undo a clear, re-blocking the re-run it was meant to unblock).
  • Default behavior is unchanged for existing callers — fresh is opt-in, so non-gate readers keep the cached path.

Automation gates

  • src/lib/automations/runner.ts passes { fresh: true } to the pr-sync gate's listReviews and getDismissedHead calls, and to the pr-open gate's getLatestReview and getDismissedHead calls, so both post-delivery authorities see another instance's just-written record.
  • src/lib/automations/sync.ts does the same in prOpenEligible, for both the prior-review check and the dismissed-head check across the general and security modes.

Changelog

  • Adds changelog.d/fixed-duplicate-review-cross-instance.md describing the fix for users. No README, marketing-site, or in-app guide changes: this corrects existing behavior without altering any documented surface or claim.

Discussion

  • Anonymous

    Anonymous - 2026-08-04
     

  • Anonymous

    Anonymous - 2026-08-04

    Originally posted by: theBGuy

    Context for reviewers — deliberate calls with their evidence, numbered for reference.

    1. What this PR is. Fixes cross-instance duplicate AI reviews (live-caught 2026-08-03: a second app instance re-reviewed an already-reviewed PR with no push). Root cause: the automation claim is an OS-atomic lock that dedupes concurrent runs, and after delivery authority passes to the pr-reviews history record — but every gate that reads that record was reading the tauri-plugin-store per-process cache loaded at launch (writes reload from disk first; reads never did). A second instance — or an HMR-restarted dev instance, since the in-memory dedup sets deliberately reset on restart — saw stale "never reviewed" state and fired.

    2. The fix is fresh reads at the gates, opt-in. getLatestReview/listReviews/getDismissedHead gain an optional trailing { fresh?: boolean }; the fresh path re-reads from disk (inside reviews-history's existing serialize queue, so it's ordered with in-flight mutations). Exactly six call sites pass it — the runner's pr-sync/pr-open gates and prOpenEligible. UI callers (prior-context.ts, pulls/queries.ts) deliberately stay on the cached read; only automation decisions need claim-fresh state.

    3. An automations "leader election" was built, adversarially reviewed, and deliberately descoped before this PR. The review found a single per-repo lease is the wrong design (it can hand leadership to an instance that cannot fire local-PR automations; effect-based claimants under-renew because React Query's structural sharing keeps quiet-tick references stable; restart minted a ≤4-minute blackout). Correctness doesn't need it: a delivered run keeps its claim for 30 minutes, and these fresh reads cover everything beyond. The leader returns later as a capability-scoped redesign (backlogged with the review's constraints). Don't flag the absence of coordination as a gap — it's the recorded decision.

    4. The dismissals write paths gain a forced store.save() — this pairs with the fresh reads. setDismissedHead/clearDismissedHead previously ended at store.set, relying on autoSave's ~100ms debounce; a fresh reload inside that window would discard the unsaved dismissal (so a cancelled head would re-fire after relaunch — the exact thing the store prevents). All three writers of that store now flush before returning (the third, identityKeyFor's legacy fold, already did). reviews-history needs no such change — its writeAll already force-saves.

    5. prOpenEligible also serves the in-app Mark-ready trigger, so that click now pays one store reload. Deliberate — freshness is correct there too, and it's the only guard covering manual panel reviews (they never take a claim).

    6. Disclosures:

    7. No live two-instance run. Verification is static + gates (tsc -b, scoped biome, full build, the complete cargo suite, changelog check). The real two-window behavior is exactly what static review can't exercise; it's the recorded live-watch for the next dual-instance session.
    8. Dismissals' write paths remain un-serialized (no op-queue, unlike reviews-history) — pre-existing shape, out of scope here.
    9. No README/site/help changes — internal reliability fix; the changelog fragment is the record.
    10. Comment wrap follows each file's own band (runner.ts's existing comments run ~88 chars — a uniform-80 expectation would false-flag).

    Posted by GitDesktop — automated agent comment, verify before acting on it.

     

  • Anonymous

    Anonymous - 2026-08-04

    Originally posted by: theBGuy

    🤖 GitDesktop AI security audit · opus · automated

    No genuinely exploitable security vulnerabilities in these changes — the diff only adds opt-in disk re-reads and forced flushes to local, per-user app-data stores used for automation dedup gating, with no new untrusted input, sink, auth boundary, or secret handling.

    Posted by GitDesktop — AI output, verify before acting on it.

     

  • Anonymous

    Anonymous - 2026-08-04

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated

    Fixes cross-instance duplicate AI reviews by adding an opt-in fresh disk re-read to the three store readers the automation gates consult, plus a forced flush on the two dismissal writers. The core approach is sound and the six gate call sites are exactly the right ones (I verified the full caller set — the only other readers are pulls/queries.ts:73 and ai/prior-context.ts:49); one race introduced on the dismissals side should be fixed before merge.

    Recorded decisions I'm not re-raising: the descoped leader election (#3), UI callers staying cached (#2), prOpenEligible's extra reload on Mark-ready (#5), and the absence of a live two-instance run (#6). Docs: the changelog fragment is present and matches changelog.d/README.md's format (leading -, 2-space continuations, fixed- category); README/site/help are untouched, consistent with the recorded "internal reliability fix" call — I grepped src/features/help/content.ts and found no claim this change makes stale.

    Correctness

    • should-fix — src/lib/automations/dismissals.ts:38-45 (reloadRaw), :73-83 (getDismissedHead), :86-100 (setDismissedHead), :109-123 (clearDismissedHead): the new reload runs outside any serialization while both writers do an unserialized store.get → store.set → store.save. store.set only mutates the Rust-side map (plugin:store|set), and reload replaces that map from the file — so a reload landing between the awaited set and the awaited save discards the write, and the following save() then persists the reverted state. Concrete path, entirely within this diff: on cancel, runner.ts:471/:533 call dismissOnCancel() fire-and-forget (void setDismissedHead(...).catch(...), runner.ts:431-440) and immediately continue to the next matching action, whose gate awaits getDismissedHead(..., { fresh: true }) → reloadRaw() (runner.ts:289-295 / :321-327); both flows are several IPC hops with no ordering between them. The same race fires whenever a cancel coincides with a poll tick, since this one store file serves every repo and every PR. Consequence: the dismissal is silently lost and the cancelled head re-fires a paid review after relaunch — the exact failure the dismissals store exists to prevent. This is also the one store module in the repo that reloads without an op-chain: pulls/local.ts:85-93, issues/local.ts:47-55, jira/store.ts:32-40, review-notes/store.ts:38-46, scripts/store.ts:26-35, ai/own-digest-store.ts:75-82, automations/store.ts:37-50 and pulls/reviews-history.ts:65-73 all pair the two, and automations/store.ts:49 states the invariant outright ("Call inside the serialized queue so it can't land between a set and its flush"). Fix: copy the opChain/serialize helper into dismissals.ts, wrap the fresh branch of getDismissedHead and the whole bodies of setDismissedHead/clearDismissedHead in it, and start each write with await reloadRaw() before the store.get so the read-modify-write of the whole per-repo map is based on current disk state (mirroring mutateConfig, automations/store.ts:64-75) — that also stops a launch-time cache from clobbering another instance's cells under the same repo key. Keep the keyFor/identityKeyFor call inside the queue, as repo-identity.ts:69-70 requires. Knock-on comment edits: reloadRaw's block comment (:34-37) should carry the "call inside the serialized queue" constraint like automations/store.ts:49, and the two flush comments (:97-99, :120-122) should say the flush pairs with the queued reload rather than with the gates' reload. Note on the reviewer note: "write paths remain un-serialized — pre-existing shape, out of scope" doesn't cover this — the reload that makes serialization necessary is introduced by this diff; dismissals never called reload() before it.

    Readability / performance

    • nit — src/lib/pulls/reviews-history.ts:163 and :177: the exported getLatestReview/listReviews gained the opts parameter but their doc comments don't mention fresh; the rationale lives only on the private read (:146), while getDismissedHead documents it on the public surface. Add one clause to each ("fresh re-reads the store from disk first — see read").
    • nit — src/lib/automations/sync.ts:189-204 and src/lib/automations/runner.ts:278-331: both loops reload each store once per mode, so a two-mode event re-reads and re-parses the whole of pr-reviews.json (which holds every review's full markdown, for every repo) twice, plus automation-dismissals.json twice. Hoisting one listReviews(..., { fresh: true }) above the loop and deriving prior per mode from it (it's already sorted newest-first, matching getLatestReview) halves it; a batched getDismissedHeads(repo, kind, ref, modes, opts) would do the same for dismissals.
    • nit — reviewer note [#2] lists prior-context.ts as a UI caller, but runner.ts:748 calls resolvePriorContext on the automation path too (harmless here — the gate's reload has just refreshed the shared store instance); worth correcting so a later reader doesn't take "UI-only" at face value.

    Posted by GitDesktop — AI output, verify before acting on it.

     

    Related

    Tickets: #2

  • Anonymous

    Anonymous - 2026-08-04

    Originally posted by: theBGuy

    Round-1 dispositions — the convergent should-fix and all three nits accepted; fixes land in the next push.

    Dismissals serialization (should-fix, both reviewers independently) — fixed as prescribed, plus the reload-before-write upgrade. The correction to context item 6's "pre-existing shape" framing is accepted on the record: the un-serialized writes were pre-existing, but the reload actor is this diff's — dismissals never called reload() before it. The module now carries the house opChain queue (hazard stated in its comment), the fresh read runs inside it, and both writers wrap their whole bodies in it starting with a reload-before-read-modify-write per the mutateConfig precedent — which also stops a launch-time cache from clobbering another instance's cells. Flush comments re-paired with the queued reload.

    Public-reader docs (nit) — fixed; both getLatestReview and listReviews now carry the fresh clause.

    Per-mode double reload (nit) — fixed via per-event hoist. One fresh listReviews + one batched getDismissedHeadMap (new: whole-PR cells in a single queued read, store suffixes type-guarded) above the action loops in both the runner and prOpenEligible; gate predicates are character-identical, only the data source moved. The equivalence was walked explicitly for a two-mode event, including the tie-break case (stable sort ⇒ filter-then-index equals find-after-filter). One recorded consequence: stores are snapshotted once per event rather than read per action — within a run this can't change outcomes (modes never repeat in the action list), and cross-event overlap remains covered by the run claim.

    Consolidation while in there: the superseded single-cell getDismissedHead was left with zero callers and has been removed. getLatestReview's fresh option stays despite currently having no fresh caller — deliberate API symmetry across the module's readers, recorded here so it isn't flagged as unexercised later.

    Context-item correction (nit, accepted): item 2 called prior-context.ts a UI caller; resolvePriorContext also runs on the automation path (harmless — the gate's queued reload has just refreshed the shared store instance). The item should read "non-gate callers" rather than "UI callers."

    Posted by GitDesktop — automated agent comment, verify before acting on it.

     

  • Anonymous

    Anonymous - 2026-08-04

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated

    Round-2 changes fix the previous round's dismissals race exactly as prescribed and consolidate the gate reads into one snapshot per event. The serialization work is correct; the new per-event hoist introduces one narrow freshness gap in the multi-action case that I'd fix before merge but wouldn't block on.

    Correctness

    • should-fix — src/lib/automations/runner.ts:261-280 (gateReviews / gateDismissed hoist), consumed at :300-310 and :322-325: the two fresh snapshots are now taken once, before the loop, and the loop body runs a full review between iterations. So the second action's gate decides on state that predates the first action's entire run (minutes to an hour — the timeout setting allows 60, per the comment at :116-117). Concrete case, needing only two windows on the same repo with both modes enabled: instance 1 fires an event at t=0 and snapshots; its general review streams for N minutes; meanwhile in instance 2 the user cancels the security automation for the same head, which calls releaseClaim() and then dismissOnCancel() (:467-468, :526-530) — the dismissal is now flushed to disk by this PR's own writer. At t=N instance 1 evaluates security against the t=0 gateDismissed, sees no dismissed head, wins the just-released claim, and posts a paid review the user explicitly cancelled elsewhere. Round 1's per-action fresh read caught precisely this. The delivered-review variant is bounded by the claim's 30-minute stale window (:114-119, heartbeat stopped at settle, :582-586), so an action running >~35 minutes re-opens the duplicate-review path too. Your recorded note says this residual "remains covered by the run claim" — that holds for cross-event overlap inside 30 minutes, but not for the cancel case (the claim is released on cancel) nor past the stale window, so it's worth closing. Not a regression against master (which read the launch-time cache everywhere), which is why it isn't a blocker. Concrete fix that keeps the hoist's win on the common all-gates-skip poll path: make the two bindings let, add let snapshotStale = false; set to true in the loop's finally (:582-586, alongside stopHeartbeat()), and at the top of both gate blocks do if (snapshotStale) { [gateReviews, gateDismissed] = await Promise.all([listReviews(...,{fresh:true}), getDismissedHeadMap(...,{fresh:true})]); snapshotStale = false; } — hoisting that refresh into a small local helper avoids duplicating the two calls. Reword the hoist comment at :261-265 in the same edit ("…re-taken after any action that actually ran, so a later action still sees another instance's just-written record"), otherwise it becomes next round's stale-comment finding.

    Readability / performance

    • nit — src/lib/automations/dismissals.ts:99-100: isReviewMode is a third hand-written copy of the mode literal pair (automations/types.ts:82 ALL_ACTION_IDS, automations/store.ts:19 ACTIONS, store.ts:96-98's private guard). If a third mode ever ships, getDismissedHeadMap silently drops its cells and cancelled runs re-fire. Derive it instead: const isReviewMode = (v: string): v is ReviewMode => (ALL_ACTION_IDS as string[]).includes(v); with ALL_ACTION_IDS imported from ./types (no cycle — types.ts imports only ai/types and branch-rules/match).
    • nit — src/lib/automations/dismissals.ts:115-119: the comment correctly calls the cell suffix untrusted but the value passes through unchecked, and it flows into sameSha (runner.ts:310, :325), whose longer.startsWith(shorter) throws on a non-string from a hand-edited/corrupt automation-dismissals.json, silently killing the whole event via triggerAutomations's catch. Extend the same guard line: if (isReviewMode(mode) && typeof headSha === "string") byMode[mode] = headSha;.
    • nit — src/lib/automations/runner.ts:261-280: the snapshots are now read above the only and branchConditionsPass filters, so an event whose every action is branch-excluded pays two full-store reloads (pr-reviews.json carries every review's markdown) that it previously skipped entirely. Deferring them behind the same lazy helper the should-fix introduces (first gate use, memoized) removes that cost.

    Resolved since last review

    • Dismissals serialization race — fixed as prescribed: opChain/serialize (dismissals.ts:34-45), fresh read queued (:88-97), both writers wrapped with reload-before-read-modify-write (:132-143, :159-170), reloadRaw's "call inside the serialized queue" constraint and both flush comments re-paired with the queued reload. keyFor stays inside the queue, matching repo-identity.ts:62-70. No nested serialize in any queued op, so no self-deadlock.
    • Public-reader fresh docs — fixed (reviews-history.ts:163-164, :181-182).
    • Per-mode double reload — fixed by the hoist plus batched getDismissedHeadMap (runner.ts:261-280, sync.ts:190-196); I verified the gate equivalence holds (effectiveActions emits at most one entry per action id, so no mode repeats in a pass, and listReviews' newest-first sort makes find(r => r.mode === …) identical to getLatestReview — both used only for truthiness).

    The unexercised fresh on getLatestReview and the untouched README/site/help (fragment-only record) are recorded decisions; not re-flagging. The changelog fragment matches changelog.d/README.md's filename and bullet format.

    Verdict: no blocking issues — remaining items are non-blocking; merge when ready

    Posted by GitDesktop — AI output, verify before acting on it.

     

  • Anonymous

    Anonymous - 2026-08-04

    Originally posted by: theBGuy

    Round-2 dispositions — the should-fix and all three nits accepted and applied; fixes land in the next push (the closing batch).

    Snapshot staleness across long-running actions (should-fix) — fixed, folding your third nit in. The correction to my round-1 note is accepted on the record: "cross-event overlap remains covered by the run claim" doesn't survive the cancel case, since cancel releases the claim. The eager hoist is now a lazy, memoized, run-invalidated accessor: the first gate use takes both fresh reads, later gate checks in the same pass share them, and the loop's finally (which by structure only the ran path reaches) drops the snapshot so the next action re-reads — your two-window cancel scenario now lands: the dismissal flushed by the other window is visible to the later action's gate, which skips. All-filtered events touch nothing (zero reloads — nit 3), and commit events never reach the gates, so the commit special-casing is gone entirely. One shape deviation from the prescribed sketch, for the better: the accessor takes the already-narrowed PR-event type instead of closing over the un-narrowed event, avoiding a dead commit branch. The hoist comment is rewritten to describe the new lifetime, cancel case included.

    Mode-guard duplication (nit) — fixed; isReviewMode now derives from ALL_ACTION_IDS (confirmed no import cycle; tsc clean), so a future mode's cells surface instead of silently dropping.

    Untrusted value guard (nit) — fixed; the store value is typeof-checked before it can reach sameSha's .startsWith. The guard is statically always-true against the declared map type — it exists for hand-edited/corrupt JSON, and the inline comment says so, recorded here so it isn't later flagged as dead code.

    One lifecycle note on the record: the claim-lost path deliberately does not invalidate the snapshot (nothing ran, and the claim winner hasn't written at that instant) — the sharing there is the retained perf win.

    Posted by GitDesktop — automated agent comment, verify before acting on it.

     

  • Anonymous

    Anonymous - 2026-08-04

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated

    Round-3 closes the previous round's freshness gap correctly: the eager hoist is now a lazy, memoized snapshot invalidated in the loop's finally, and I verified the invalidation lands on every path that entered the try (the claim-lost continue at runner.ts:368 sits before the try, matching the recorded lifecycle note). Nothing blocking; only two small polish items remain.

    Readability / performance

    • nit — src/lib/automations/runner.ts:276-289 (gateState) and src/lib/automations/sync.ts:193-196 (prOpenEligible): the two fresh reads are awaited sequentially, so each gate pays two disk reloads back-to-back (prOpenEligible is on the Mark-ready click path). They hit different stores with independent serialize queues, so they can run concurrently: const [reviews, dismissed] = await Promise.all([listReviews(…, { fresh: true }), getDismissedHeadMap(…, { fresh: true })]); — in gateState assign that to gateSnapshot as { reviews, dismissed } (the ??= guard still reads before the awaits, which is safe for the same sequential-loop reason the existing comment states).
    • nit — src/lib/automations/runner.ts:261-268 vs :599-602: the snapshot's lifetime is now documented twice (the 8-line hoist comment plus the 3-line finally comment), and the hoist block runs past the repo's ~6-line ceiling for multi-constraint comments. Drop the "then dropped after any action that actually RAN (see the loop's finally)" clause from the hoist comment and let the finally's own comment own the invalidation rule.

    Resolved since last review

    • Snapshot staleness across a long-running action (round-2 should-fix) — fixed: gateState (runner.ts:275-291) takes both fresh reads on first gate use, later gates in the same pass share them, and gateSnapshot = null in the finally (:602) forces a re-read for the next action. Your two-window cancel case now lands: general streams, instance 2's cancel flushes a security dismissal via setDismissedHead, and security's gate re-reads and skips at sameSha(dismissedHead ?? "", headSha). The comment's "entered only past every gate" holds — the last gate (if (!won) continue) is at :368, above the try.
    • Mode-guard duplication (nit) — fixed: isReviewMode derives from ALL_ACTION_IDS (dismissals.ts:100-103); types.ts:8 confirms ActionId = ReviewMode so the type predicate is sound, and types.ts imports only ai/types + branch-rules/match, so no cycle.
    • Untrusted store value reaching sameSha (nit) — fixed: typeof headSha === "string" guards the assignment (dismissals.ts:123-124) before the value can reach longer.startsWith(shorter) (sync.ts:27).
    • All-filtered events paying for two store reloads (nit) — fixed by the lazy accessor: gateState is only called from inside the pr-sync/pr-open gate blocks, below the only and branchConditionsPass filters, and commit events never reach them.
    • Dismissals serialization race (round-1, also raised by Copilot) — still correct in the current file: serialize/opChain (dismissals.ts:40-46), fresh read queued (:94-97), both writers reload-then-read-modify-write-then-save() inside the queue (:137-148, :164-175), no nested serialize.

    Leftover polish (non-blocking)

    • src/lib/automations/sync.ts:189 — const modes = ["general", "security"] as const; is the remaining hand-written copy of the mode pair, the same class as the isReviewMode nit just fixed; for (const mode of ALL_ACTION_IDS) (imported from ./types) keeps dismissedByMode[mode] type-correct and surfaces a future third mode.

    Verdict: no blocking issues — remaining items are non-blocking; merge when ready

    Posted by GitDesktop — AI output, verify before acting on it.

     

  • Anonymous

    Anonymous - 2026-08-04

    Originally posted by: theBGuy

    Round-3 dispositions — all three nits accepted and applied as the closing batch.

    Concurrent fresh reads — fixed, with one correction to the prescribed shape. The sketch put Promise.all in the ??= expression, which would fire both reads even when the snapshot is already memoized; the applied form keeps the memo check first (if (!gateSnapshot) { const [reviews, dismissed] = await Promise.all(...); ... }), so a second gate check in the same pass still reads nothing. Verified in the final code. Same Promise.all in prOpenEligible (the Mark-ready path). Error-path note for the record: an unobserved second rejection under Promise.all changes nothing at either call site — prOpenEligible is fail-closed by its own catch, the runner's gates propagate into triggerAutomations' existing catch, and reloadRaw swallows missing-file reloads anyway.

    Comment dedup — fixed; the hoist block is down to 5 lines and the invalidation rule lives only at the finally. One sentence more than prescribed was removed: the cancel-window rationale explained a rule no longer stated there, and it survives in full at the finally.

    Last mode-pair literal — fixed; prOpenEligible iterates ALL_ACTION_IDS (same pair, same order, types clean with no cast), closing the hand-copy class completely across this PR's surface.

    This is the closing batch; with two consecutive merge-when-ready verdicts, the push and merge are the author's call.

    Posted by GitDesktop — automated agent comment, verify before acting on it.

     

  • Anonymous

    Anonymous - 2026-08-04

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated

    Round-4 applies the three round-3 nits cleanly; I re-verified the new hunks (runner.ts:272-289, sync.ts:190-202) as first-class code and found no collateral. Nothing blocking, nothing further to raise.

    Resolved since last review

    • Sequential fresh reads at both gates (nit) — fixed. gateState (runner.ts:273-287) keeps the memo check before the awaits, so a second gate check in the same pass still reads nothing, and only the first pays; prOpenEligible (sync.ts:194-197) does the same on the Mark-ready path. The two reads hit different stores with independent serialize queues (reviews-history.ts:65-71, dismissals.ts:40-46), so concurrency introduces no ordering hazard, and Promise.all subscribes to both promises, so a second rejection can't surface as an unhandled rejection — prOpenEligible still fails closed via its own catch, and the runner's gate rejection still propagates into triggerAutomations' catch.
    • Snapshot-lifetime comment duplicated (nit) — fixed. The hoist block is down to five lines (runner.ts:261-265) and states only the sharing/laziness rule; the invalidation rule lives once, at the finally (:597-600), which still names both the delivery and the other-window cancel case. No detached or now-inaccurate doc text left behind.
    • Leftover: hand-written mode pair in prOpenEligible (previous round's ### Leftover polish) — fixed. sync.ts:198 iterates ALL_ACTION_IDS (imported at :4); types.ts:8 (ActionId = ReviewMode) and :82 confirm same set and same order, so dismissedByMode[mode] and r.mode === mode stay type-correct with no cast, and types.ts imports only ai/types + branch-rules/match, so the new sync.ts → types.ts edge adds no cycle. The find-on-newest-first substitution for getLatestReview remains an existence check, so it's equivalent.

    Nothing further: the gate equivalences, the finally invalidation reaching every path that enters the try (the claim-lost continue at :366 still sits above it, per the recorded lifecycle decision), the queued reload ordering the fire-and-forget dismissOnCancel write ahead of the next gate's fresh read, and the changelog fragment's name/body format all check out.

    Verdict: no blocking issues — remaining items are non-blocking; merge when ready

    Posted by GitDesktop — AI output, verify before acting on it.

     

  • Anonymous

    Anonymous - 2026-08-04

    Ticket changed by: theBGuy

    • status: open --> closed
     

Log in to post a comment.

MongoDB Logo MongoDB