From 549ddefb20435ff7d64685cbfc7741a1bfb66fa8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Paul=20L=C3=BCtzner?= <karatektus@gmail.com>
Date: Wed, 14 Oct 2026 15:19:08 +0200
Subject: [PATCH] Take a list refresh asked for during another, instead of
 dropping it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

bulk-select-all's "the pager agrees with the rows after a bulk action"
failed now and then under load. Not the job: it archived the 67
conversations in six seconds. A nudge from the job started a list
refresh while it was still writing, the job's "done" arrived while that
fetch was out, and _refreshList() returned early because one was
already in flight. The fetch came back without the archive, and nothing
was left to refresh the list — it showed the archived conversations
until something unrelated happened.

A request that arrives during a refresh is now remembered and taken
once the current fetch lands, keeping the most urgent of whatever asked
in the meantime. list-refresh-queue.spec.ts holds the first fetch at the
network so the second request always arrives during it; it fails
without this change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 .../controllers/mail/mail_pane_controller.js  | 26 +++++++++
 tests/e2e/list-refresh-queue.spec.ts          | 56 +++++++++++++++++++
 2 files changed, 82 insertions(+)
 create mode 100644 tests/e2e/list-refresh-queue.spec.ts

diff --git a/assets/controllers/mail/mail_pane_controller.js b/assets/controllers/mail/mail_pane_controller.js
index b1fa4086..8107b55b 100644
--- a/assets/controllers/mail/mail_pane_controller.js
+++ b/assets/controllers/mail/mail_pane_controller.js
@@ -289,6 +289,9 @@ export default class extends Controller {
             clearTimeout(this._refreshTimer);
             this._refreshTimer = null;
         }
+
+        // The same for a refresh queued behind one in flight.
+        this._refreshAgain = null;
     }
 
     /**
@@ -768,7 +771,22 @@ export default class extends Controller {
 
         // One at a time: a burst of sync events would otherwise have several
         // responses racing to write the same element.
+        //
+        // But queued, not dropped. The fetch in flight was asked for BEFORE
+        // whatever this request is about, and may well come back without it:
+        // a bulk job's per-chunk nudge starts a refresh, the job finishes while
+        // that fetch is out, and the job's "done" — the one event that is
+        // certain to describe the finished list — arrived here and was thrown
+        // away. The list then went on showing the archived conversations until
+        // something unrelated happened to refresh it. One follow-up is taken
+        // when the current fetch lands, carrying the most urgent of whatever
+        // asked in the meantime.
         if (this._refreshing === true) {
+            this._refreshAgain = {
+                immediate: immediate || (this._refreshAgain?.immediate ?? false),
+                url: url ?? this._refreshAgain?.url ?? null,
+            };
+
             return;
         }
 
@@ -849,6 +867,14 @@ export default class extends Controller {
             console.warn("[mail-pane] list refresh failed", error);
         } finally {
             this._refreshing = false;
+
+            const again = this._refreshAgain ?? null;
+
+            this._refreshAgain = null;
+
+            if (again !== null) {
+                this._refreshList(again);
+            }
         }
     }
 
diff --git a/tests/e2e/list-refresh-queue.spec.ts b/tests/e2e/list-refresh-queue.spec.ts
new file mode 100644
index 00000000..f7cf9048
--- /dev/null
+++ b/tests/e2e/list-refresh-queue.spec.ts
@@ -0,0 +1,56 @@
+import { test, expect } from "./support/test";
+
+/**
+ * A list refresh asked for while another is in flight is taken afterwards,
+ * not dropped.
+ *
+ * Found as an intermittent failure of bulk-select-all.spec.ts: a whole-view
+ * archive runs as a job, a nudge from the job starts a refresh while the job is
+ * still writing, the job's "done" arrives while that fetch is out — and was
+ * thrown away, so the list kept the archived conversations on screen with
+ * nothing left to refresh it. Whether it happened depended on how the two
+ * timings fell under load. This makes them fall that way every time: the first
+ * fetch is held at the network, and the second request arrives while it is.
+ */
+test("a refresh asked for during another is taken when the first lands", async ({ page }) => {
+    await page.goto("/mail/inbox");
+    await expect(page.locator("#message-list li").first()).toBeVisible();
+
+    let fetches = 0;
+    let releaseFirst: () => void = () => {};
+    const firstHeld = new Promise<void>((resolve) => {
+        releaseFirst = resolve;
+    });
+
+    await page.route("**/mail/inbox**", async (route) => {
+        if (undefined === route.request().headers()["x-list-fragment"]) {
+            await route.continue();
+
+            return;
+        }
+
+        fetches += 1;
+
+        if (1 === fetches) {
+            await firstHeld;
+        }
+
+        await route.continue();
+    });
+
+    const jobChanged = (state: string) =>
+        page.evaluate((detail) => {
+            document.body.dispatchEvent(new CustomEvent("core--mercure:jobs-changed", { detail, bubbles: true }));
+        }, { state });
+
+    // The first refresh starts and is held in flight…
+    await jobChanged("done");
+    await expect.poll(() => fetches).toBe(1);
+
+    // …the job's last word arrives while it is…
+    await jobChanged("done");
+
+    // …and is taken once the first lands, rather than lost.
+    releaseFirst();
+    await expect.poll(() => fetches, { timeout: 5_000 }).toBe(2);
+});