From 4c7cf68b4d2662eaf53cfdba137d34f38c7fa21a Mon Sep 17 00:00:00 2001 From: galuis116 Date: Sun, 12 Jul 2026 17:43:36 -0400 Subject: [PATCH] test(miner): add real crash-recovery test for portfolio-queue stuck items (#4868) The existing suite only exercised sweepStuckItems/findStuckItems in-process against fake timers. Spawns a real Node child process that claims the only queued item, then idles forever with no cleanup handler; the test SIGKILLs it and verifies the item stays genuinely stuck until its lease expires, then is swept back to queued and re-claimable. --- .../claim-and-hold-child.mjs | 26 ++++ ...ner-portfolio-queue-crash-recovery.test.ts | 119 ++++++++++++++++++ 2 files changed, 145 insertions(+) create mode 100644 test/fixtures/miner-concurrent-stores/claim-and-hold-child.mjs create mode 100644 test/unit/miner-portfolio-queue-crash-recovery.test.ts diff --git a/test/fixtures/miner-concurrent-stores/claim-and-hold-child.mjs b/test/fixtures/miner-concurrent-stores/claim-and-hold-child.mjs new file mode 100644 index 0000000000..4535ec379d --- /dev/null +++ b/test/fixtures/miner-concurrent-stores/claim-and-hold-child.mjs @@ -0,0 +1,26 @@ +#!/usr/bin/env node +// Cross-process crash-recovery helper for portfolio-queue's stuck-lease sweep (#4868). +// Opens the shared queue, claims the next item via dequeueNext() (stamping a real lease), reports the +// claimed entry, then idles forever without ever marking it done -- simulating a process that claims work +// and then crashes mid-attempt. The test kills this process (SIGKILL) and asserts the item is left +// genuinely stuck 'in_progress' until swept, then reclaimable. +import { initPortfolioQueueStore } from "../../../packages/gittensory-miner/lib/portfolio-queue.js"; + +const [dbPath] = process.argv.slice(2); +if (!dbPath) { + process.stderr.write("usage: claim-and-hold-child.mjs \n"); + process.exit(2); +} + +const store = initPortfolioQueueStore(dbPath); +const entry = store.dequeueNext(); +// dequeueNext()'s own return shape carries no leasedAt (only listInProgress()'s lease-annotated projection +// does), and the test needs the real stamped lease time to compute expiry windows against. +const lease = entry + ? store.listInProgress().find((row) => row.repoFullName === entry.repoFullName && row.identifier === entry.identifier) + : null; +process.stdout.write(`CLAIMED ${JSON.stringify({ ...entry, leasedAt: lease?.leasedAt ?? null })}\n`); + +// Idle forever -- never mark done, never close the store, never exit on its own. The test's SIGKILL is +// the only thing that ends this process, mirroring a real crash (no cleanup handler runs). +setInterval(() => {}, 1_000_000); diff --git a/test/unit/miner-portfolio-queue-crash-recovery.test.ts b/test/unit/miner-portfolio-queue-crash-recovery.test.ts new file mode 100644 index 0000000000..cbabebe564 --- /dev/null +++ b/test/unit/miner-portfolio-queue-crash-recovery.test.ts @@ -0,0 +1,119 @@ +import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { afterEach, describe, expect, it } from "vitest"; +import { initPortfolioQueueStore } from "../../packages/gittensory-miner/lib/portfolio-queue.js"; +import { sweepStuckItems } from "../../packages/gittensory-miner/lib/portfolio-queue-expiry.js"; + +// Real crash-recovery coverage for portfolio-queue's stuck-item lease/reclaim mechanism (#4868). The +// existing unit suite (test/unit/miner-portfolio-queue-expiry.test.ts) only exercises sweepStuckItems +// in-process against fake timers -- it never actually kills a process mid-claim. This spawns a real Node +// child process that claims the only queued item (stamping a real on-disk lease) and then idles forever +// (never marks done, never closes cleanly), SIGKILLs it to simulate a crash, and verifies the item is left +// genuinely stuck 'in_progress' until its lease expires, at which point the sweep reclaims it and it +// becomes claimable again -- the full crash -> detect -> reclaim -> re-claim cycle. +// +// Scope note (per the issue): this only tests portfolio-queue's own single-miner crash/reclaim behavior. It +// does not touch, and must not be extended into, cross-miner claim-conflict resolution. + +const holdChildScript = join( + dirname(fileURLToPath(import.meta.url)), + "../fixtures/miner-concurrent-stores/claim-and-hold-child.mjs", +); + +const roots: string[] = []; +const stores: Array<{ close(): void }> = []; + +function tempRoot(): { root: string; dbPath: string } { + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-crash-recovery-")); + roots.push(root); + return { root, dbPath: join(root, "portfolio-queue.sqlite3") }; +} + +function tempStore(dbPath: string) { + const store = initPortfolioQueueStore(dbPath); + stores.push(store); + return store; +} + +afterEach(() => { + for (const store of stores.splice(0)) store.close(); + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +type ClaimedEntry = { repoFullName: string; identifier: string; status: string; leasedAt: string }; + +async function spawnAndWaitForClaim(dbPath: string): Promise<{ child: ChildProcessWithoutNullStreams; claimed: ClaimedEntry }> { + const child = spawn(process.execPath, [holdChildScript, dbPath], { stdio: ["pipe", "pipe", "pipe"] }); + const claimed = await new Promise((resolve, reject) => { + let buffer = ""; + const onData = (chunk: Buffer | string) => { + buffer += chunk.toString(); + const line = buffer.split("\n").find((entry) => entry.startsWith("CLAIMED ")); + if (line) { + child.stdout.off("data", onData); + resolve(JSON.parse(line.slice("CLAIMED ".length)) as ClaimedEntry); + } + }; + child.stdout.on("data", onData); + child.once("error", reject); + child.once("exit", (code) => { + if (code !== 0 && code !== null) reject(new Error(`child exited before claiming (${code})`)); + }); + }); + return { child, claimed }; +} + +async function killAndWaitForExit(child: ChildProcessWithoutNullStreams): Promise { + await new Promise((resolve) => { + child.once("exit", () => resolve()); + child.kill("SIGKILL"); + }); +} + +describe("portfolio-queue crash recovery (#4868)", () => { + it("a process killed mid-claim leaves the item stuck in_progress until the lease expires, then it is swept back to queued and re-claimable", async () => { + const { dbPath } = tempRoot(); + const bootstrap = tempStore(dbPath); + bootstrap.enqueue({ repoFullName: "acme/widgets", identifier: "pr:1" }); + + const { child, claimed } = await spawnAndWaitForClaim(dbPath); + expect(claimed).toMatchObject({ repoFullName: "acme/widgets", identifier: "pr:1", status: "in_progress" }); + const leasedAtMs = Date.parse(claimed.leasedAt); + expect(Number.isFinite(leasedAtMs)).toBe(true); + + // Simulate a real crash: SIGKILL, no cleanup handler runs, no markDone/close. + await killAndWaitForExit(child); + + // The crash alone does not un-stick the row -- it is still genuinely 'in_progress' on disk. + expect(bootstrap.listInProgress()).toEqual([ + { repoFullName: "acme/widgets", identifier: "pr:1", status: "in_progress", leasedAt: claimed.leasedAt }, + ]); + + // Sweeping before the lease bound elapses must NOT reclaim it (still within the grace window). + const tooSoon = sweepStuckItems(bootstrap, leasedAtMs + 500, 1000); + expect(tooSoon).toEqual([]); + expect(bootstrap.listInProgress()).toHaveLength(1); + + // Sweeping once the lease bound has elapsed reclaims the crashed process's item back to 'queued'. + const reclaimed = sweepStuckItems(bootstrap, leasedAtMs + 1001, 1000); + expect(reclaimed).toHaveLength(1); + expect(reclaimed[0]).toMatchObject({ repoFullName: "acme/widgets", identifier: "pr:1", status: "queued" }); + expect(bootstrap.listInProgress()).toEqual([]); + + // Full recovery: the reclaimed item is claimable again, exactly as if it had never been claimed. + const reclaimedThenReclaimed = bootstrap.dequeueNext(); + expect(reclaimedThenReclaimed).toMatchObject({ repoFullName: "acme/widgets", identifier: "pr:1", status: "in_progress" }); + }); + + it("rejects the claim-and-hold-child helper when required args are missing", async () => { + const child = spawn(process.execPath, [holdChildScript], { stdio: ["ignore", "pipe", "pipe"] }); + const exitCode = await new Promise((resolve, reject) => { + child.once("error", reject); + child.once("exit", resolve); + }); + expect(exitCode).toBe(2); + }); +});