fix(review): avoid cancel-restarting an in-flight visual-capture fallback dispatch

The fallback workflow runs with concurrency.cancel-in-progress: true per head
SHA, so a recapture-poll retry firing before a slower build finishes would
dispatch a second run, cancel the first, and repeat forever. Check for an
already-queued/in-progress run for the same (PR, head SHA) before dispatching.

Author: JSONbored

src/review/visual/actions-fallback.ts

@@ -144,6 +144,45 @@ export function parseFallbackRunCorrelation(displayTitle: string | undefined | n
   return { prNumber, headSha: (match[2] as string).toLowerCase() };
 }
 
+/** True when a fallback run for this EXACT (prNumber, headSha) is already queued or in progress -- checked
+ *  by buildCapture before dispatching, so the existing recapture-poll retry (every 90s, up to 5 attempts,
+ *  see PREVIEW_POLL_SECONDS/MAX_PREVIEW_POLLS in processors.ts) doesn't repeatedly re-dispatch while waiting
+ *  for the SAME run's workflow_run completion. That matters because the workflow's own `concurrency: group:
+ *  visual-capture-fallback-${{ inputs.head_sha }}` + `cancel-in-progress: true` means a second dispatch for
+ *  the same head SHA CANCELS the first -- without this check, a poll firing before a slow build finishes
+ *  would cancel-and-restart it every 90s and the fallback could never complete. Queries GitHub's own run
+ *  list rather than persisting new dispatch-tracking state, mirroring this pipeline's existing
+ *  live-query-don't-persist pattern (getLatestDeploymentStatus, findPreviewUrlFromChecks). Fails OPEN (false,
+ *  "nothing in flight") on any error -- a transient list-runs failure should still let the existing
+ *  concurrency group be the backstop dedup, not silently stop the fallback from ever being tried. */
+export async function hasInFlightFallbackDispatch(params: {
+  token: string;
+  repo: GitHubRepo;
+  prNumber: number;
+  headSha: string;
+  rateLimitAdmissionKey?: GitHubRateLimitAdmissionKey | undefined;
+}): Promise<boolean> {
+  const base = `https://api.github.com/repos/${params.repo.owner}/${params.repo.repo}`;
+  try {
+    const response = await timeoutFetch(`${base}/actions/workflows/${FALLBACK_WORKFLOW_FILE}/runs?event=workflow_dispatch&per_page=20`, {
+      headers: githubApiHeaders(params.token),
+      signal: AbortSignal.timeout(DEFAULT_TIMEOUT_MS),
+      githubRateLimitAdmission: params.rateLimitAdmissionKey !== undefined,
+      ...(params.rateLimitAdmissionKey ? { githubRateLimitAdmissionKey: params.rateLimitAdmissionKey } : {}),
+    });
+    if (!response.ok) return false;
+    const payload = (await response.json().catch(() => null)) as { workflow_runs?: Array<{ status?: string; display_title?: string }> } | null;
+    const headSha = params.headSha.toLowerCase();
+    return (payload?.workflow_runs ?? []).some((run) => {
+      if (run.status !== "queued" && run.status !== "in_progress") return false;
+      const correlation = parseFallbackRunCorrelation(run.display_title);
+      return correlation !== null && correlation.prNumber === params.prNumber && correlation.headSha === headSha;
+    });
+  } catch {
+    return false;
+  }
+}
+
 // ---------------------------------------------------------------------------------------------------------
 // Minimal ZIP reader -- just enough to read a GitHub Actions artifact (STORED / DEFLATE entries only).
 // ---------------------------------------------------------------------------------------------------------

src/review/visual/capture.ts

@@ -12,7 +12,7 @@
 // default TanStack route convention; those hooks can return if a per-repo visual config is added.
 import { sha256Hex } from "../../utils/crypto";
 import type { GitHubRateLimitAdmissionKey } from "../../github/client";
-import { dispatchVisualCaptureFallback, fallbackShotR2Key } from "./actions-fallback";
+import { dispatchVisualCaptureFallback, fallbackShotR2Key, hasInFlightFallbackDispatch } from "./actions-fallback";
 import {
   findPreviewUrlFromChecks,
   findPreviewUrlFromPrComments,
@@ -405,15 +405,23 @@ export async function buildCapture(env: Env, token: string, target: CaptureTarge
   const actionsFallbackEnabled = visualConfig?.actionsFallback === true;
   const routes = resolveVisualRoutes(visualFiles, visualConfig?.routes);
   if (!previewBase && !previewFailed && !previewPending && actionsFallbackEnabled && target.headSha && target.defaultBranchRef) {
-    const dispatched = await dispatchVisualCaptureFallback({
-      token,
-      repo,
-      ref: target.defaultBranchRef,
-      prNumber: target.prNumber,
-      headSha: target.headSha,
-      routes,
-      rateLimitAdmissionKey,
-    });
+    // Never re-dispatch onto an already in-flight run (#4112 review fix): the workflow's own `concurrency:
+    // cancel-in-progress: true` group would CANCEL that run the instant a second dispatch for the same head
+    // SHA lands, so a recapture-poll retry (every 90s -- see PREVIEW_POLL_SECONDS in processors.ts) firing
+    // before a slower build finishes could cancel-and-restart it forever and never complete. See
+    // hasInFlightFallbackDispatch's own doc comment for the full rationale.
+    const alreadyInFlight = await hasInFlightFallbackDispatch({ token, repo, prNumber: target.prNumber, headSha: target.headSha, rateLimitAdmissionKey });
+    const dispatched =
+      alreadyInFlight ||
+      (await dispatchVisualCaptureFallback({
+        token,
+        repo,
+        ref: target.defaultBranchRef,
+        prNumber: target.prNumber,
+        headSha: target.headSha,
+        routes,
+        rateLimitAdmissionKey,
+      }));
     if (dispatched) previewPending = true;
   }
 

test/unit/actions-fallback.test.ts

@@ -7,6 +7,7 @@ import {
   fallbackShotR2Key,
   fetchFallbackArtifactShots,
   FALLBACK_ARTIFACT_NAME,
+  hasInFlightFallbackDispatch,
   isGithubArtifactStorageUrl,
   parseFallbackRunCorrelation,
   parseZipEntries,
@@ -349,6 +350,82 @@ describe("dispatchVisualCaptureFallback", () => {
   });
 });
 
+describe("hasInFlightFallbackDispatch (#4112 review fix -- avoid cancel-in-progress re-dispatch)", () => {
+  const HEAD_SHA = "cafebabecafebabecafebabecafebabecafebabe";
+
+  function runsResponse(runs: Array<{ status?: string; display_title?: string }>): Response {
+    return Response.json({ workflow_runs: runs });
+  }
+
+  it("true when a QUEUED run matches this exact pr+headSha", async () => {
+    vi.stubGlobal("fetch", async () => runsResponse([{ status: "queued", display_title: `gittensory-visual-fallback pr=7 sha=${HEAD_SHA}` }]));
+    const inFlight = await hasInFlightFallbackDispatch({ token: "tok", repo: { owner: "acme", repo: "widgets" }, prNumber: 7, headSha: HEAD_SHA });
+    expect(inFlight).toBe(true);
+  });
+
+  it("true when an IN_PROGRESS run matches (case-insensitive headSha)", async () => {
+    vi.stubGlobal("fetch", async () => runsResponse([{ status: "in_progress", display_title: `gittensory-visual-fallback pr=7 sha=${HEAD_SHA}` }]));
+    const inFlight = await hasInFlightFallbackDispatch({ token: "tok", repo: { owner: "acme", repo: "widgets" }, prNumber: 7, headSha: HEAD_SHA.toUpperCase() });
+    expect(inFlight).toBe(true);
+  });
+
+  it("false for an empty run list", async () => {
+    vi.stubGlobal("fetch", async () => runsResponse([]));
+    const inFlight = await hasInFlightFallbackDispatch({ token: "tok", repo: { owner: "acme", repo: "widgets" }, prNumber: 7, headSha: HEAD_SHA });
+    expect(inFlight).toBe(false);
+  });
+
+  it("false when the matching run has already COMPLETED (not queued/in_progress)", async () => {
+    vi.stubGlobal("fetch", async () => runsResponse([{ status: "completed", display_title: `gittensory-visual-fallback pr=7 sha=${HEAD_SHA}` }]));
+    const inFlight = await hasInFlightFallbackDispatch({ token: "tok", repo: { owner: "acme", repo: "widgets" }, prNumber: 7, headSha: HEAD_SHA });
+    expect(inFlight).toBe(false);
+  });
+
+  it("false when an in-progress run exists for a DIFFERENT PR", async () => {
+    vi.stubGlobal("fetch", async () => runsResponse([{ status: "in_progress", display_title: `gittensory-visual-fallback pr=99 sha=${HEAD_SHA}` }]));
+    const inFlight = await hasInFlightFallbackDispatch({ token: "tok", repo: { owner: "acme", repo: "widgets" }, prNumber: 7, headSha: HEAD_SHA });
+    expect(inFlight).toBe(false);
+  });
+
+  it("false when an in-progress run exists for the same PR but a DIFFERENT headSha (new push)", async () => {
+    vi.stubGlobal("fetch", async () => runsResponse([{ status: "in_progress", display_title: `gittensory-visual-fallback pr=7 sha=${"f".repeat(40)}` }]));
+    const inFlight = await hasInFlightFallbackDispatch({ token: "tok", repo: { owner: "acme", repo: "widgets" }, prNumber: 7, headSha: HEAD_SHA });
+    expect(inFlight).toBe(false);
+  });
+
+  it("false when the run's display_title doesn't match the expected correlation shape at all", async () => {
+    vi.stubGlobal("fetch", async () => runsResponse([{ status: "in_progress", display_title: "Manually triggered run" }]));
+    const inFlight = await hasInFlightFallbackDispatch({ token: "tok", repo: { owner: "acme", repo: "widgets" }, prNumber: 7, headSha: HEAD_SHA });
+    expect(inFlight).toBe(false);
+  });
+
+  it("false on a non-ok response", async () => {
+    vi.stubGlobal("fetch", async () => new Response("nope", { status: 500 }));
+    const inFlight = await hasInFlightFallbackDispatch({ token: "tok", repo: { owner: "acme", repo: "widgets" }, prNumber: 7, headSha: HEAD_SHA });
+    expect(inFlight).toBe(false);
+  });
+
+  it("false (never throws) on a network failure", async () => {
+    vi.stubGlobal("fetch", async () => {
+      throw new Error("network down");
+    });
+    const inFlight = await hasInFlightFallbackDispatch({ token: "tok", repo: { owner: "acme", repo: "widgets" }, prNumber: 7, headSha: HEAD_SHA });
+    expect(inFlight).toBe(false);
+  });
+
+  it("true when a rateLimitAdmissionKey is supplied and a match is found", async () => {
+    vi.stubGlobal("fetch", async () => runsResponse([{ status: "queued", display_title: `gittensory-visual-fallback pr=7 sha=${HEAD_SHA}` }]));
+    const inFlight = await hasInFlightFallbackDispatch({
+      token: "tok",
+      repo: { owner: "acme", repo: "widgets" },
+      prNumber: 7,
+      headSha: HEAD_SHA,
+      rateLimitAdmissionKey: "installation:1",
+    });
+    expect(inFlight).toBe(true);
+  });
+});
+
 describe("fetchFallbackArtifactShots", () => {
   function stubSequence(handlers: Array<(input: RequestInfo | URL, init?: RequestInit) => Response | Promise<Response>>): void {
     let call = 0;

test/unit/visual-capture.test.ts

@@ -1289,6 +1289,64 @@ describe("review.visual.actions_fallback (#4112 GitHub-Actions build-and-serve f
     expect(result.previewPending).toBe(true);
   });
 
+  it("skips dispatching a NEW run when one is already queued/in-progress for this exact pr+headSha, but still marks the capture pending (#4112 review fix)", async () => {
+    let dispatchCalled = false;
+    vi.stubGlobal(
+      "fetch",
+      stubNoPreviewFound((url) => {
+        if (url.includes("/actions/workflows/visual-capture-fallback.yml/runs")) {
+          return Response.json({ workflow_runs: [{ status: "in_progress", display_title: "gittensory-visual-fallback pr=20 sha=cafebabecafebabecafebabecafebabecafebabe" }] });
+        }
+        if (url.includes("/dispatches")) {
+          dispatchCalled = true;
+          return new Response(null, { status: 204 });
+        }
+        return null;
+      }),
+    );
+
+    const result = await buildCapture(
+      createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "" }),
+      "installation-token",
+      { repoFullName: "owner/repo", prNumber: 20, headSha: "cafebabecafebabecafebabecafebabecafebabe", previewFromChecks: true, defaultBranchRef: "main" },
+      ["apps/gittensory-ui/src/routes/app.index.tsx"],
+      undefined,
+      { actionsFallback: true },
+    );
+
+    expect(dispatchCalled).toBe(false);
+    expect(result.previewPending).toBe(true);
+  });
+
+  it("dispatches a NEW run when the only in-flight run found is for a DIFFERENT headSha (a later push)", async () => {
+    let dispatchCalled = false;
+    vi.stubGlobal(
+      "fetch",
+      stubNoPreviewFound((url) => {
+        if (url.includes("/actions/workflows/visual-capture-fallback.yml/runs")) {
+          return Response.json({ workflow_runs: [{ status: "in_progress", display_title: "gittensory-visual-fallback pr=20 sha=ffffffffffffffffffffffffffffffffffffffff" }] });
+        }
+        if (url.includes("/dispatches")) {
+          dispatchCalled = true;
+          return new Response(null, { status: 204 });
+        }
+        return null;
+      }),
+    );
+
+    const result = await buildCapture(
+      createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "" }),
+      "installation-token",
+      { repoFullName: "owner/repo", prNumber: 20, headSha: "cafebabecafebabecafebabecafebabecafebabe", previewFromChecks: true, defaultBranchRef: "main" },
+      ["apps/gittensory-ui/src/routes/app.index.tsx"],
+      undefined,
+      { actionsFallback: true },
+    );
+
+    expect(dispatchCalled).toBe(true);
+    expect(result.previewPending).toBe(true);
+  });
+
   it("leaves the capture non-pending when the dispatch call itself fails", async () => {
     vi.stubGlobal(
       "fetch",