From 1c27491d8703a101fd07c83e4a0fb413e21bbf0c Mon Sep 17 00:00:00 2001
From: Felix Stubner <felix.stubner@gmail.com>
Date: Sun, 6 Sep 2026 02:36:34 +0100
Subject: [PATCH] test(index): assert the scan-budget ordering with a gate
 instead of a stopwatch
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The test means "the read returns while the scan is still going". It asserted
that by racing a 400ms sleep against a 300ms wall-clock ceiling, which is a
question about how fast the machine is, not about what the code did. On CI it
came up 320ms on ubuntu while macos and windows passed, and a re-run went
green.

The scraper now blocks on a promise only the test can resolve, so the scan
provably has not finished when the assertions run — nothing but the test can
finish it. The wall clock is gone from the assertions entirely.

Two things the old version could not say and this one does: that the scan had
actually started (otherwise an empty answer and `isScanning()` would also hold
for a scan that never began), and that the empty answer means "not yet" rather
than "never" — the gate opens, the scan settles, and the session is there.

Honest about the evidence: I could not reproduce the failure locally. Eight
runs before the fix and ten after passed, and ten runs of the *old* test under
eight saturating CPU loops also passed, so the load I could generate is not
the condition CI hit. The justification is the observed failure itself — 320ms
against a 300ms bound is a timing assertion losing a race — plus the fact that
the new assertions cannot fail that way whatever the cause was.

The two other SlowScraper tests are left alone. They assert on settled state
rather than on elapsed time, so neither races anything.
---
 tests/handoff/sqlite-index.test.ts | 70 +++++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 10 deletions(-)

diff --git a/tests/handoff/sqlite-index.test.ts b/tests/handoff/sqlite-index.test.ts
index 2797beb..e6862df 100644
--- a/tests/handoff/sqlite-index.test.ts
+++ b/tests/handoff/sqlite-index.test.ts
@@ -652,21 +652,71 @@ describe("scan time budget", () => {
     }
   }
 
+  /**
+   * A scraper that starts, then waits for the test to let it finish.
+   *
+   * The point of the test below is ordering — the read returns while the scan
+   * is still going — and it used to assert that by racing a 400ms sleep
+   * against a 300ms wall-clock ceiling. That is a coin toss on a loaded CI
+   * runner, and it came up tails: 320ms on ubuntu, green on the two other
+   * platforms and on eight consecutive local runs. A test that fails on
+   * machine speed rather than on behaviour teaches everyone to press re-run,
+   * which is how a real intermittent failure gets waved through.
+   *
+   * With a gate the ordering is structural: the scan cannot have finished,
+   * because nothing but this test can finish it.
+   */
+  class GatedScraper extends FixtureScraper {
+    private releaseGate!: () => void;
+    private markStarted!: () => void;
+    /** Resolves once `fullSync` has actually been entered. */
+    readonly started = new Promise<void>((resolve) => {
+      this.markStarted = resolve;
+    });
+    private readonly gate = new Promise<void>((resolve) => {
+      this.releaseGate = resolve;
+    });
+
+    override async *fullSync(): AsyncIterable<ConversationChunk> {
+      this.markStarted();
+      await this.gate;
+      yield* super.fullSync();
+    }
+
+    /** Let the scan run to completion. */
+    finish(): void {
+      this.releaseGate();
+    }
+  }
+
   it("returns before a slow scan finishes, and says the index is still filling", async () => {
-    const scraper = new SlowScraper([chunk("slow-session", 0, "user", "eventually indexed")], 400);
+    const scraper = new GatedScraper([chunk("slow-session", 0, "user", "eventually indexed")]);
     const index = new SqliteHandoffIndex(join(tempDir, "xtctx.db"), tempDir, [
       { tool: "codex", scraper },
     ], { refreshBudgetMs: 30 });
 
-    const startedAt = Date.now();
-    const recent = await index.listRecentSessions(5);
-    const elapsed = Date.now() - startedAt;
-
-    expect(elapsed).toBeLessThan(300);
-    expect(recent).toEqual([]);
-    expect(index.isScanning()).toBe(true);
-
-    await index.close();
+    try {
+      const recent = await index.listRecentSessions(5);
+
+      // The scan is genuinely under way — without this the assertions below
+      // would also pass for a scan that never started at all.
+      await scraper.started;
+      // ...and genuinely unfinished, because the gate is still closed.
+      expect(recent).toEqual([]);
+      expect(index.isScanning()).toBe(true);
+
+      // The other half: once it is allowed to finish, the data is there. This
+      // is what makes the empty answer above "not yet" rather than "never".
+      scraper.finish();
+      await index.whenScanSettled();
+      expect(index.isScanning()).toBe(false);
+      expect(await index.listRecentSessions(5)).toHaveLength(1);
+    } finally {
+      // `close()` waits for the scan to settle, so a failed assertion above
+      // would otherwise hang the suite on a gate nobody opened.
+      scraper.finish();
+      await index.close();
+    }
   });
 
   it("has the data once the scan it started has finished", async () => {