Skip to content

fix(hub): keep a row written after a reconcile pass read the state - #707

Merged
TheGreatAxios merged 1 commit into
mainfrom
cl-11646-hold-a-head-as-unrouted-when-a-reconcile-pass-overwrote-its
Oct 28, 2026
Merged

TheGreatAxios merged 1 commit into
mainfrom
cl-11646-hold-a-head-as-unrouted-when-a-reconcile-pass-overwrote-its

Conversation

@TheGreatAxios

Copy link
Copy Markdown
Contributor

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:

  • When a pass merges after a conflict, it keeps every row someone wrote after the pass read the state, and it mails none of those heads. Before, it went by whichever timestamp was later.
  • The fix is in the merge rather than in failDelivery, so the coalescer stays the only writer of the rows it queues. Marking the head unrouted whatever its status would have left the pass free to overwrite a queued row and relied on a second write to repair it.
  • In the chain tests, a timeout in settled now prints the pull request's rows from the triage state.

How it was verified:

  • New Postgres test: a reconcile pass plans while a webhook's head is queued, the head's mail then reaches no listener, and the next pass mails it. It fails before the fix, because the head is not sent on the next pass, and passes after it.
  • The interleave, reconciler, plan and coalescer suites pass (88 tests). Hub and tests/chain typecheck clean, and oxlint is clean.
  • The disconnected chain scenario passed once locally (108 s). It does not hit the timeout path, so the new settled output was not exercised.

Closes CL-11646

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.
@linear-code

linear-code Bot commented Oct 28, 2026

Copy link
Copy Markdown

CL-11646

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. restoreHead puts back the row as it was before, with its old updatedAt, so a send that was undone still counts as unchanged and the pass's row wins. A head the pass never read (read.get is 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 and markBaseMoved, and each of them stamps a fresh updatedAt.
  • settled(): artifacts.artifact has tenant_id, title, content and archived_at, and triageStateName(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.
  • hub and tests/chain typecheck. The interleave, triage-reconciler, reconcile-plan and coalescer suites pass 88/88.

No findings.

@TheGreatAxios
TheGreatAxios marked this pull request as ready for review October 28, 2026 08:43
@TheGreatAxios
TheGreatAxios merged commit f07a33d into main Oct 28, 2026
17 checks passed
@TheGreatAxios
TheGreatAxios deleted the cl-11646-hold-a-head-as-unrouted-when-a-reconcile-pass-overwrote-its branch October 28, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant