fix(extension): guard overlay refresh against out-of-order pull-context responses (#7489)

mountOverlay's refresh button called load() with no request-ordering
guard, so two rapid clicks issued two independent pull-context round-trips
and whichever resolved last won. Under network jitter the older click's
response could resolve after the newer one, silently leaving the overlay
showing stale data.

Move load() into a per-overlay-instance createOverlayLoader closure that
stamps each invocation with a monotonically incrementing ticket and
discards any resolved response whose ticket is no longer the latest, so
the rendered state always reflects the most recently initiated request.
Single-click behavior (loading text, error and success rendering) is
unchanged.

Closes #7463

Co-authored-by: bitfathers94 <237535319+bitfathers94@users.noreply.github.com>

Author: bitfathers94

apps/loopover-extension/content.js

@@ -39,9 +39,10 @@ function mountOverlay(target) {
   } else {
     document.body.appendChild(container);
   }
+  const load = createOverlayLoader(container, target);
   const refresh = container.querySelector(".loopover-overlay__refresh");
-  refresh?.addEventListener("click", () => load(container, target));
-  void load(container, target);
+  refresh?.addEventListener("click", () => load());
+  void load();
 }
 
 function findPullRequestSidebar() {
@@ -53,17 +54,23 @@ function findPullRequestSidebar() {
   );
 }
 
-async function load(container, target) {
-  const body = container.querySelector(".loopover-overlay__body");
-  if (!body) return;
-  body.textContent = "Loading private context...";
-  const response = await chrome.runtime.sendMessage({ type: "loopover:pull-context", ...target });
-  if (!response?.ok) {
-    body.innerHTML = `<div class="loopover-overlay__error">${escapeHtml(response?.error || "Context unavailable")}</div>`;
-    return;
-  }
-  body.innerHTML = renderPullContext(response.payload);
-  renderActions(body, response.payload?.actions);
+function createOverlayLoader(container, target) {
+  let latestTicket = 0;
+  return async function load() {
+    const body = container.querySelector(".loopover-overlay__body");
+    if (!body) return;
+    const ticket = ++latestTicket;
+    body.textContent = "Loading private context...";
+    const response = await chrome.runtime.sendMessage({ type: "loopover:pull-context", ...target });
+    // A newer load() has started while this request was in flight; discard the stale response.
+    if (ticket !== latestTicket) return;
+    if (!response?.ok) {
+      body.innerHTML = `<div class="loopover-overlay__error">${escapeHtml(response?.error || "Context unavailable")}</div>`;
+      return;
+    }
+    body.innerHTML = renderPullContext(response.payload);
+    renderActions(body, response.payload?.actions);
+  };
 }
 
 function renderPullContext(payload) {
@@ -186,6 +193,7 @@ if (globalThis.__LOOPOVER_EXTENSION_TEST__) {
   globalThis.__loopoverContentInternals = {
     matchGitHubPageTarget,
     matchPullRequestTarget,
+    createOverlayLoader,
     renderPullContext,
     renderSection,
     renderLegacyPanels,

test/unit/extension-content.test.ts

@@ -74,9 +74,46 @@ describe("extension content script", () => {
     expect(html).toContain("public");
     expect(html).toContain("no");
   });
+
+  it("discards an out-of-order refresh response so the overlay keeps the newest request's payload", async () => {
+    const resolvers: Array<(response: unknown) => void> = [];
+    const sendMessage = vi.fn(() => new Promise((resolve) => resolvers.push(resolve)));
+    const internals = loadContentInternals({
+      chrome: { runtime: { sendMessage } },
+    });
+
+    const body = { innerHTML: "", textContent: "" };
+    const container = { querySelector: vi.fn(() => body) };
+    const load = internals.createOverlayLoader(container, {
+      kind: "pull_request",
+      owner: "JSONbored",
+      repo: "loopover",
+      pullNumber: 42,
+    });
+
+    // Two rapid refresh clicks: the second (newer) request is issued while the first is still in flight.
+    const first = load();
+    const second = load();
+    expect(sendMessage).toHaveBeenCalledTimes(2);
+    const [resolveFirst, resolveSecond] = resolvers;
+    if (!resolveFirst || !resolveSecond) throw new Error("expected two in-flight pull-context requests");
+
+    // The newer request resolves and renders first...
+    resolveSecond({ ok: true, payload: { sections: [{ label: "Newer context" }] } });
+    await second;
+    expect(body.innerHTML).toContain("Newer context");
+
+    // ...then the older, first-issued request resolves last — the out-of-order case. Its stale
+    // payload must be discarded rather than clobbering the fresher render already on screen.
+    resolveFirst({ ok: true, payload: { sections: [{ label: "Stale context" }] } });
+    await first;
+
+    expect(body.innerHTML).toContain("Newer context");
+    expect(body.innerHTML).not.toContain("Stale context");
+  });
 });
 
-function loadContentInternals() {
+function loadContentInternals(overrides: Record<string, unknown> = {}) {
   const context: Record<string, unknown> = {
     __LOOPOVER_EXTENSION_TEST__: true,
     location: { pathname: "/JSONbored/loopover/issues/146" },
@@ -88,6 +125,7 @@ function loadContentInternals() {
       body: { appendChild: vi.fn() },
     },
     chrome: { runtime: { sendMessage: vi.fn() } },
+    ...overrides,
   };
   context.globalThis = context;
   const vmContext = createContext(context);
@@ -97,6 +135,7 @@ function loadContentInternals() {
       pathname: string,
     ) => { kind: "pull_request"; owner: string; repo: string; pullNumber: number } | null;
     matchPullRequestTarget: (pathname: string) => { owner: string; repo: string; pullNumber: number } | null;
+    createOverlayLoader: (container: { querySelector: (selector: string) => unknown }, target: unknown) => () => Promise<void>;
     renderPullContext: (payload: unknown) => string;
   };
 }