fix(queue): stop rich merge-queue row from freezing the page#535
Merged
Conversation
On PRs with an engine merge-queue payload comment, the page froze and every button in the Mergify row (Remove from merge queue, Refresh, Rebase, Update, brand link) stopped responding to clicks. Root cause: updateAllMergifyRows() runs on every tryInject tick, which fires from the body MutationObserver. The rich-row path in updateMergifyRow called row.replaceChildren(...) unconditionally — and that swap is itself a tree mutation in the observed subtree, so it re-triggered the observer in a self-sustaining requestAnimationFrame loop. The loop pegged the main thread and rebuilt the row's buttons ~60x/sec, so clicks never survived mousedown -> mouseup on the same node. Only "some PRs" were affected because the legacy row path is already idempotent and the context panel / stack-nav pill both dedup via data-mergify-hash; the rich row (added in #522) was the one surface missing that guard. Fix: give the rich row the same data-mergify-hash dedup. updateMergifyRow now skips the children swap when the payload-derived fingerprint is unchanged. Live ETA text is excluded from the hash since the ETA ticker already owns it on a 1s cadence. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: Ic45d555b676693cf784cad6b526500272c6f419d
Contributor
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Required ReviewsWonderful, this rule succeeded.
🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 🔎 ReviewsWonderful, this rule succeeded.
🟢 📕 PR descriptionWonderful, this rule succeeded.
|
There was a problem hiding this comment.
Pull request overview
Prevents the “rich” merge-queue row from continuously rebuilding itself (and thereby freezing the page / eating clicks) by making rich-row updates idempotent when the underlying payload hasn’t changed.
Changes:
- Add a
data-mergify-hashfingerprint to rich rows and skipreplaceChildren(...)when the fingerprint matches. - Introduce
_richRowHash(...)to compute a stable payload-derived hash (excluding live ETA text). - Add regression tests asserting that no-op rich-row updates produce zero DOM mutations while real state changes still rebuild.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/queue.js | Adds rich-row hash + idempotency guard to avoid self-triggered MutationObserver loops. |
| src/tests/queue.test.js | Adds regression coverage for the rich-row freeze/idempotency behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The queue/logs links and the merged-message easter egg are all keyed on the PR number, but _richRowHash() omitted it. Two same-state queued PRs in one org/repo hashed identically, so a rich row reused across an SPA navigation (resetForNavigation doesn't remove the merge-box row) would skip the swap and keep the previous PR's links/message. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: I3f097c5f46b1e505a41cc434d04dfa2126e1f990
jd
approved these changes
Jun 9, 2026
remyduthu
approved these changes
Jun 9, 2026
Contributor
Merge Queue Status
This pull request spent 1 minute 43 seconds in the queue, including 1 minute 2 seconds running CI. Required conditions to merge
|
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.
Problem
On PRs with an engine merge-queue payload comment (e.g. queued PRs like monorepo#34031), the page froze: clicking anything in the Mergify row — Remove from merge queue, Refresh, Rebase, Update, the Mergify brand — did nothing, as if the click never registered.
Root cause
updateAllMergifyRows()runs on everytryInjecttick, which fires from the bodyMutationObserver. The rich-row path inupdateMergifyRowcalledrow.replaceChildren(...)unconditionally. That swap is itself a tree mutation in the observed subtree, so it re-triggered the observer in a self-sustainingrequestAnimationFrameloop:The loop pegged the main thread and rebuilt the row's buttons ~60×/sec, so a click could never survive
mousedown → mouseupon the same node.Why only "some PRs": the legacy row path is already idempotent, and the context panel + stack-nav pill both dedup via
data-mergify-hash. The rich row (added in #522) was the one surface missing that guard.Fix
Give the rich row the same
data-mergify-hashdedup used elsewhere in the codebase.updateMergifyRownow skips the children swap when the payload-derived fingerprint is unchanged. Live ETA text is intentionally excluded from the hash, since the ETA ticker already refreshes it on a 1s cadence by touching only[data-mergify-eta].Tests
MutationObserver.takeRecords(): a no-op rich-row update emitted 2 mutation records before the fix, 0 after.checking → frozen) still rebuilds, so the guard isn't over-aggressive.🤖 Generated with Claude Code