From 26f445898ac36530955428db152f643b05221ebd Mon Sep 17 00:00:00 2001
From: bitfathers94 <237535319+bitfathers94@users.noreply.github.com>
Date: Mon, 20 Jul 2026 12:28:47 +0000
Subject: [PATCH] fix(extension): guard overlay refresh against out-of-order
pull-context responses
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
---
apps/loopover-extension/content.js | 34 +++++++++++++++---------
test/unit/extension-content.test.ts | 41 ++++++++++++++++++++++++++++-
2 files changed, 61 insertions(+), 14 deletions(-)
diff --git a/apps/loopover-extension/content.js b/apps/loopover-extension/content.js
index 5cef4120a7..44536bff4b 100644
--- a/apps/loopover-extension/content.js
+++ b/apps/loopover-extension/content.js
@@ -38,9 +38,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() {
@@ -52,17 +53,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 = `${escapeHtml(response?.error || "Context unavailable")}
`;
- 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 = `${escapeHtml(response?.error || "Context unavailable")}
`;
+ return;
+ }
+ body.innerHTML = renderPullContext(response.payload);
+ renderActions(body, response.payload?.actions);
+ };
}
function renderPullContext(payload) {
@@ -185,6 +192,7 @@ if (globalThis.__LOOPOVER_EXTENSION_TEST__) {
globalThis.__loopoverContentInternals = {
matchGitHubPageTarget,
matchPullRequestTarget,
+ createOverlayLoader,
renderPullContext,
renderSection,
renderLegacyPanels,
diff --git a/test/unit/extension-content.test.ts b/test/unit/extension-content.test.ts
index b6634bf430..089227fba0 100644
--- a/test/unit/extension-content.test.ts
+++ b/test/unit/extension-content.test.ts
@@ -70,9 +70,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 = {}) {
const context: Record = {
__LOOPOVER_EXTENSION_TEST__: true,
location: { pathname: "/JSONbored/loopover/issues/146" },
@@ -84,6 +121,7 @@ function loadContentInternals() {
body: { appendChild: vi.fn() },
},
chrome: { runtime: { sendMessage: vi.fn() } },
+ ...overrides,
};
context.globalThis = context;
const vmContext = createContext(context);
@@ -93,6 +131,7 @@ function loadContentInternals() {
pathname: string,
) => { kind: "pull_request"; owner: string; repo: string; pullNumber: number } | { kind: "issue"; owner: string; repo: string; issueNumber: number } | null;
matchPullRequestTarget: (pathname: string) => { owner: string; repo: string; pullNumber: number } | null;
+ createOverlayLoader: (container: { querySelector: (selector: string) => unknown }, target: unknown) => () => Promise;
renderPullContext: (payload: unknown) => string;
};
}