test(gate,ai,rate-limit): recover coverage work orphaned by the #9501/#9504/#9505 merges

src/queue/processors.ts

@@ -2859,7 +2859,7 @@ function buildAgentMaintenancePlanInput(args: {
       : {}),
     copycatGateMode: settings.copycatGateMode,
     ...(screenshotTableMatch !== undefined ? { screenshotTableMatch } : {}),
-    ...(screenshotTableEvidenceUnresolved !== undefined ? { screenshotTableEvidenceUnresolved } : {}),
+    screenshotTableEvidenceUnresolved,
     ...(contributorCapMatch !== undefined ? { contributorCapMatch } : {}),
     // Always threaded (the DB layer populates it, default "over-contributor-limit"); the planner applies its
     // own fallback.
@@ -9491,6 +9491,8 @@ async function scheduleVisualCaptureRetry(
     // forever (the sole other clear needs a successful capture, which by definition never came), which silently
     // and permanently disabled the screenshotTableGate's close for that head. Best-effort, matching the mark
     // write below: a failed clear only means the gate stays deferred until the head moves, never a crash.
+    /* v8 ignore next -- a recapture-preview job is only ever minted for a PR that had a head SHA, so the
+       falsy arm is defensive; the mark write below carries the identical guard for the same reason. */
     if (args.pr.headSha) {
       await clearPullRequestVisualCaptureRetryPending(env, args.repoFullName, args.pr.number, args.pr.headSha).catch((error) => {
         console.log(

src/selfhost/ai.ts

@@ -604,10 +604,14 @@ async function isolatedCliCwd(): Promise<string> {
  *  instructions on disk indefinitely. Best-effort by design: a cleanup failure must never turn a completed
  *  review into a thrown error, and the next container recreation still collects anything missed. */
 async function removeIsolatedCliCwd(cwd: string | undefined): Promise<void> {
+  /* v8 ignore next -- the finally runs with cwd unset only when the mkdtemp itself threw, i.e. the temp dir was
+     never created and there is nothing to remove; unreachable from a test that gets far enough to spawn. */
   if (!cwd) return;
   try {
     const { rm } = await import("node:fs/promises");
     await rm(cwd, { recursive: true, force: true });
+    /* v8 ignore next 3 -- best-effort cleanup: rm with force:true does not throw for a missing path, so this
+       arm needs a filesystem-level failure (permissions, EBUSY) that no unit test can portably induce. */
   } catch {
     // best-effort -- see the doc comment.
   }

test/unit/ai-review.test.ts

@@ -3540,6 +3540,21 @@ describe("pure helpers", () => {
     expect(run).toHaveBeenCalledTimes(2); // 1 primary (stalled) + 1 fallback (succeeded on its first try).
   });
 
+  it("REGRESSION (#9476): a non-Error throw is not mistaken for a stall and still uses the full budget", async () => {
+    // isStalledNoOutput must reject a non-Error value rather than throwing on `.message` -- a provider adapter
+    // can reject with a string, and misclassifying that as a deadline signal would silently skip retries.
+    let primaryAttempts = 0;
+    const run = vi.fn(async (model: string) => {
+      if (model === "fallback") return { response: reviewJson() };
+      primaryAttempts += 1;
+      throw "claude_stalled_no_output: not an Error instance";
+    });
+    const env = createTestEnv({ AI: { run } as unknown as Ai });
+    const diagnostics: Array<{ status: string; model: string }> = [];
+    await runWorkersOpinion(env, "primary", "fallback", "sys", "user", 256, diagnostics as never);
+    expect(primaryAttempts).toBe(3);
+  });
+
   it("REGRESSION (#9476): a genuinely transient error still gets the FULL retry budget (the break is narrow)", async () => {
     // Guards against over-broadening the break: only the non-transient deadline/rate-limit/config signals
     // short-circuit. A dropped connection must still be retried up to the budget.

test/unit/github-app.test.ts

@@ -3123,6 +3123,48 @@ describe("GitHub rate-limit handling (#ratelimit-resilience)", () => {
     await expect(createInstallationToken(env, 5252)).rejects.toThrow();
     expect(calls).toBe(4); // initial + GITHUB_RATE_LIMIT_MAX_RETRIES (3)
   });
+
+  // #9494 regression: a secondary-limit Retry-After is typically 60s against an 8s inline cap. The old code
+  // took Math.min(retryAfter, cap) and retried up to 3 times INSIDE the window GitHub explicitly asked us to
+  // stay out of -- which its own docs warn can extend or escalate a secondary block. Surface it instead: the
+  // queue honors the full Retry-After with jitter and defers sibling jobs for the same admission target.
+  it("timeoutFetch stops retrying inline when Retry-After exceeds the inline budget (#9494)", async () => {
+    const privateKey = await generatePrivateKeyPem();
+    clearInstallationTokenCacheForTest();
+    let calls = 0;
+    vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
+      const url = input.toString();
+      if (url.includes("/access_tokens")) {
+        calls += 1;
+        return new Response("secondary rate limit", {
+          status: 403,
+          headers: { "retry-after": "60" }, // far beyond GITHUB_RATE_LIMIT_MAX_DELAY_MS
+        });
+      }
+      return new Response("not found", { status: 404 });
+    });
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: privateKey });
+    await expect(createInstallationToken(env, 5353)).rejects.toThrow();
+    expect(calls).toBe(1); // ONE attempt -- not 4. The wait belongs to the queue, not this loop.
+  });
+
+  it("timeoutFetch still uses its full inline budget when Retry-After fits within it (#9494)", async () => {
+    // Guards against over-broadening: a short instructed wait is exactly what the inline retries are for.
+    const privateKey = await generatePrivateKeyPem();
+    clearInstallationTokenCacheForTest();
+    let calls = 0;
+    vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
+      const url = input.toString();
+      if (url.includes("/access_tokens")) {
+        calls += 1;
+        return new Response("secondary rate limit", { status: 403, headers: { "retry-after": "0" } });
+      }
+      return new Response("not found", { status: 404 });
+    });
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: privateKey });
+    await expect(createInstallationToken(env, 5454)).rejects.toThrow();
+    expect(calls).toBe(4);
+  });
 });
 
 describe("repoFullName segment-count + whitespace guard (#8311)", () => {

test/unit/queue-3.test.ts

@@ -2339,6 +2339,64 @@ describe("queue processors", () => {
     expect((await getPullRequest(env, "JSONbored/gittensory", 62))?.visualCaptureRetryPendingSha).toBeNull();
   });
 
+  // #9462: the clear is best-effort, mirroring its sibling mark write -- a failed clear means the gate stays
+  // deferred until the head moves, which is strictly better than letting a cleanup failure crash the pass.
+  it("screenshot-table gate (#9462): a failed marker clear is swallowed (fail-safe) -- the pass still completes", async () => {
+    const buildCaptureSpy = vi.spyOn(visualCaptureModule, "buildCapture").mockRejectedValue(new Error("browserless connection refused"));
+    const clearSpy = vi.spyOn(repositoriesModule, "clearPullRequestVisualCaptureRetryPending").mockRejectedValue(new Error("D1 write failed"));
+    const env = createTestEnv({
+      GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(),
+      LOOPOVER_REVIEW_SCREENSHOTS: "true",
+    });
+    env.JOBS = { async send() {} } as unknown as Queue;
+    await upsertInstallation(env, {
+      installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" }, target_type: "User", repository_selection: "all", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] },
+      repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }],
+    });
+    await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", autonomy: { close: "auto", label: "auto" } });
+    await upsertRepoFocusManifest(env, "JSONbored/gittensory", { settings: { commentMode: "all_prs", publicSurface: "comment_only", checkRunMode: "off", screenshotTableGate: { enabled: true, whenLabels: ["visual"] }, reviewCheckMode: "required" } }, "repo_file");
+    await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {
+      number: 63,
+      title: "Update the app index route",
+      state: "open",
+      user: { login: "visual-contributor" },
+      head: { sha: "vis63" },
+      labels: [{ name: "visual" }],
+      body: "Changed the route layout, no table here.",
+    });
+    vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
+      const url = input.toString();
+      const method = init?.method ?? "GET";
+      if (url === "https://api.gittensor.io/miners") return Response.json([]);
+      if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
+      if (url.includes("/pulls/63/files")) return Response.json([{ filename: "apps/loopover-ui/src/routes/app.index.tsx", status: "modified", additions: 5, deletions: 1, changes: 6, patch: "@@\n+const ok = true;" }]);
+      if (url.includes("/pulls/63/reviews")) return Response.json([]);
+      if (url.includes("/pulls/63/commits")) return Response.json([]);
+      if (url.endsWith("/pulls/63") && method === "PATCH") return Response.json({ number: 63, state: "closed" });
+      if (url.endsWith("/pulls/63")) return Response.json({ number: 63, state: "open", user: { login: "visual-contributor" }, head: { sha: "vis63" }, mergeable_state: "clean" });
+      if (url.includes("/commits/vis63/status")) return Response.json({ state: "success", statuses: [] });
+      if (url.includes("/commits/vis63/check-runs")) return Response.json({ total_count: 0, check_runs: [] });
+      if (url.includes("/commits/vis63/check-suites")) return Response.json({ check_suites: [] });
+      if (url.includes("/issues/63/labels") && method === "GET") return Response.json([]);
+      if (url.includes("/issues/63/comments")) return Response.json([]);
+      if (url.endsWith("/labels") && method === "POST") return Response.json([]);
+      if (url.endsWith("/check-runs") && method === "POST") return Response.json({ id: 903 }, { status: 201 });
+      if (url.includes("/check-runs/903") && method === "PATCH") return Response.json({ id: 903 });
+      return new Response("not found", { status: 404 });
+    });
+
+    try {
+      // Must not throw even though the clear rejects.
+      await expect(
+        processJob(env, { type: "recapture-preview", deliveryId: "screenshot-marker-clear-failed", repoFullName: "JSONbored/gittensory", prNumber: 63, installationId: 123, attempt: MAX_PREVIEW_POLL_ATTEMPTS }),
+      ).resolves.not.toThrow();
+      expect(clearSpy).toHaveBeenCalled();
+    } finally {
+      clearSpy.mockRestore();
+      buildCaptureSpy.mockRestore();
+    }
+  });
+
   describe("live migrations/** collision recheck (#2550)", () => {
     // Full merge-eligible stub set (clean + green + approved), reused across scenarios — a positive test proves
     // the collision hold actually suppresses what would otherwise merge; a negative test proves the check

test/unit/selfhost-ai.test.ts

@@ -1780,6 +1780,36 @@ describe("subscription CLI helpers + fail-safe", () => {
     }
   });
 
+  // #9479 REGRESSION: child.on("error") catches SPAWN failures only -- it never receives stdio stream errors.
+  // A CLI that exits before draining stdin (an unknown flag on an upgraded binary, an immediate auth abort, an
+  // OOM kill) made the ~250KB stdin write fail with EPIPE on an emitter with no "error" listener, which Node
+  // escalates to an UNCAUGHT exception -> installSelfHostCrashHandlers -> exit(1), taking down every in-flight
+  // queue job in the container. This drives the real subprocess so the listener is genuinely exercised: the
+  // fake exits immediately without reading stdin, and the write must be swallowed rather than crash the run.
+  it("REAL subprocess: a CLI that exits without draining stdin does not crash the worker (#9479)", async () => {
+    const dir = mkdtempSync(join(tmpdir(), "fakecli-"));
+    const fake = join(dir, "claude");
+    // Exits at once, never reading stdin -> the parent's write lands on a closed pipe (EPIPE).
+    writeFileSync(fake, "#!/usr/bin/env node\nprocess.exit(2);\n");
+    chmodSync(fake, 0o755);
+    const origPath = process.env.PATH;
+    const uncaught: unknown[] = [];
+    const onUncaught = (err: unknown) => uncaught.push(err);
+    process.on("uncaughtException", onUncaught);
+    try {
+      // A large input makes the write span multiple chunks, so it cannot complete before the child is gone.
+      await expect(
+        createClaudeCodeAi({ PATH: `${dir}:${origPath ?? ""}`, CLAUDE_CODE_OAUTH_TOKEN: "t" }).run("sonnet", {
+          prompt: "x".repeat(300_000),
+        }),
+      ).rejects.toThrow(); // surfaces as a normal provider error via the exit-code guard...
+    } finally {
+      process.off("uncaughtException", onUncaught);
+      process.env.PATH = origPath;
+    }
+    expect(uncaught).toEqual([]); // ...and never as an uncaught EPIPE, which is what killed the container
+  });
+
   // REGRESSION (GITTENSORY-K/M/8/Z, #4994): the real defaultSpawn fast-fail path against a genuinely-hung fake
   // `claude` that writes nothing to either stream and never exits — mirrors the identical codex real-subprocess
   // test below, proving createClaudeCodeAi's plumbing (not just a stubbed spawn) actually wires