perf(test): eliminate SQLite fsync and real rate-limit backoff wait in tests (#8553)

Profiling queue-lifecycle-guards.test.ts (33.7s) and backfill.test.ts (19.2s)
found two more real per-test costs, confirmed via a CPU profile showing wall
time going to idle (real I/O wait), not compute.

SQLite fsyncs on every autocommit write by default; TestD1Database's clones
are real temp files (needed for the template-clone trick from the earlier
fix), so every app .prepare(...).run() paid a real fsync. journal_mode=MEMORY
+ synchronous=OFF are safe for a throwaway clone unlinked as soon as it's
open -- crash-consistency durability is meaningless for data nothing outlives
the test to read back. Benchmarked: 50 inserts, 13.9ms default vs 0.6ms tuned.

fetchWithGitHubRetry's exponential backoff (500/1000/2000ms real setTimeout)
is legitimate production behavior for a genuine 403/429, but several tests
deliberately drive a fixture through the full retry loop to assert the
resulting sync state, not the delay -- paying the full 3.5s every time.
client.ts gains setGithubRateLimitRetrySleepCapMsForTest(), capping only what
is actually awaited, not rateLimitRetryMs's return value -- its dedicated
pure-function backoff-math test stays untouched and still exercises the real
numbers.

Both wired once, suite-wide, via a new test/helpers/vitest-setup.ts
(vitest.config.ts setupFiles) -- same ...ForTest convention as
clearInstallationTokenCacheForTest, applied automatically so every current
AND future test with this shape benefits without opting in.

queue-lifecycle-guards.test.ts: 33.7s -> 7.2s. backfill.test.ts: 19.2s -> 1.7s.

Author: JSONbored

src/github/backfill.ts

@@ -2424,6 +2424,20 @@ function toPullRequestFileRecordFromGitHub(repoFullName: string, pullNumber: num
 // manualReview alone — it might be a human's own hold, not just a stale bot one) — so a guardrail-configured
 // repo's PR could otherwise sit "held for manual review" indefinitely over nothing but a fetch timing gap.
 const REVIEW_FILES_EMPTY_RETRY_DELAY_MS = 500;
+// Test-only override (#test-hotspots): the dedicated retry tests (backfill-2.test.ts) pin retry BEHAVIOR
+// (attempt count, which response wins), never the wall-clock delay itself. Any OTHER test whose fetch stub
+// happens to return an empty files array (a common, often-incidental stub shape) pays this real 500ms sleep
+// unknowingly — confirmed as a major contributor to queue-lifecycle-guards.test.ts's slowest tests. Same
+// `...ForTest` convention as clearInstallationTokenCacheForTest / setMaxChunksPerRepoForTest; the whole
+// suite defaults it near-zero via test/helpers/vitest-setup.ts (vitest.config.ts's setupFiles), so this
+// fires for every current AND future test with this shape, not just ones an author remembers to opt in.
+let reviewFilesEmptyRetryDelayMsOverride: number | null = null;
+export function reviewFilesEmptyRetryDelayMs(): number {
+  return reviewFilesEmptyRetryDelayMsOverride ?? REVIEW_FILES_EMPTY_RETRY_DELAY_MS;
+}
+export function setReviewFilesEmptyRetryDelayMsForTest(value: number | null): void {
+  reviewFilesEmptyRetryDelayMsOverride = value;
+}
 
 const sleep = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms));
 
@@ -2457,7 +2471,7 @@ export async function fetchAndStorePullRequestFilesForReview(
   const fetchOnce = () => fetchPullRequestFiles(env, repoFullName, pullNumber, token, warnings, admissionKey, "live_review").catch(() => [] as GitHubFilePayload[]);
   let files = await fetchOnce();
   if (files.length === 0) {
-    await sleep(REVIEW_FILES_EMPTY_RETRY_DELAY_MS);
+    await sleep(reviewFilesEmptyRetryDelayMs());
     files = await fetchOnce();
   }
   if (warnings.length > 0) {

src/github/client.ts

@@ -390,7 +390,20 @@ export function isGitHubResponseCacheReplay(response: Response): boolean {
 const GITHUB_RATE_LIMIT_MAX_RETRIES = 3;
 const GITHUB_RATE_LIMIT_MAX_DELAY_MS = 8_000;
 
-const sleep = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms));
+// Test-only override (#test-hotspots): several tests deliberately drive fetchWithGitHubRetry through its
+// FULL rate-limit backoff (up to 500+1000+2000=3500ms real wall time) to assert the resulting sync state
+// (rate_limited status, capped segments) -- not the delay itself. This caps what's actually AWAITED
+// without touching rateLimitRetryMs's own return value, so its dedicated pure-function correctness test
+// (github-app.test.ts, asserting exact backoff math) stays exercising the real numbers unmodified. Same
+// `...ForTest` convention as reviewFilesEmptyRetryDelayMs; the suite defaults it near-zero suite-wide via
+// test/helpers/vitest-setup.ts.
+let githubRateLimitRetrySleepCapMsOverride: number | null = null;
+export function setGithubRateLimitRetrySleepCapMsForTest(value: number | null): void {
+  githubRateLimitRetrySleepCapMsOverride = value;
+}
+
+const sleep = (ms: number): Promise<void> =>
+  new Promise((resolve) => setTimeout(resolve, githubRateLimitRetrySleepCapMsOverride ?? ms));
 
 /** Does this GitHub response signal a rate limit (primary or secondary)? 403/429 with a Retry-After header, an
  *  exhausted x-ratelimit-remaining, or a secondary-limit/abuse body. A 403 with NONE of these is a real

test/helpers/d1.ts

@@ -86,6 +86,14 @@ export class TestD1Database {
     );
     copyFileSync(getMigratedTemplatePath(), clonePath);
     this.db = new DatabaseSync(clonePath);
+    // #test-hotspots: SQLite's defaults (journal_mode=DELETE, synchronous=FULL) fsync on every
+    // autocommit write -- each of the app's `.prepare(...).run()` calls is its own implicit
+    // transaction, so a write-heavy test (audit events, PR upserts, gate results) pays a real disk
+    // fsync per statement. Under concurrent CI/local load this dominated wall time far more than a
+    // quiet-machine benchmark suggests (queue-lifecycle-guards.test.ts: ~1000ms on several tests).
+    // Both are safe to disable for a throwaway per-test clone that's unlinked as soon as it's open:
+    // crash-consistency durability is meaningless for data nothing outlives the test to read back.
+    this.db.exec("PRAGMA journal_mode = MEMORY; PRAGMA synchronous = OFF;");
     state.clonePaths.push(clonePath);
     if (!state.exitSweepRegistered) {
       state.exitSweepRegistered = true;

test/helpers/vitest-setup.ts

@@ -0,0 +1,13 @@
+// Runs once per test file (Vitest's setupFiles, distinct from globalSetup which runs once for the
+// whole run in the main process — this needs to touch each file's own module registry). Defaults
+// production retry/backoff delays that exist for real network-timing reasons to near-zero so a test
+// whose stub incidentally triggers one (e.g. an empty-files fetch stub tripping
+// fetchAndStorePullRequestFilesForReview's empty-retry) doesn't pay real wall-clock time for it
+// (#test-hotspots). The delay CONSTANT and its production default are untouched — this only sets the
+// `...ForTest` override every such helper already exposes; a dedicated test asserting the retry's own
+// behavior (attempt count, precedence) still exercises the identical code path, just without the sleep.
+import { setReviewFilesEmptyRetryDelayMsForTest } from "../../src/github/backfill";
+import { setGithubRateLimitRetrySleepCapMsForTest } from "../../src/github/client";
+
+setReviewFilesEmptyRetryDelayMsForTest(0);
+setGithubRateLimitRetrySleepCapMsForTest(0);

vitest.config.ts

@@ -17,6 +17,7 @@ export default defineConfig({
     globals: true,
     testTimeout: 15000,
     globalSetup: ["./test/helpers/vitest-global-setup-node-version.ts"],
+    setupFiles: ["./test/helpers/vitest-setup.ts"],
     // Retry a failed test once before failing the run. The loopover gate auto-CLOSES a contributor PR
     // on a red required CI, so a single transient flake must not kill an honest PR; a deterministic
     // failure still fails both attempts (and vitest flags the retried test as flaky so it stays visible).