diff --git a/packages/loopover-miner/lib/worktree-allocator.ts b/packages/loopover-miner/lib/worktree-allocator.ts index 95b7c7a6f6..4ab0375a65 100644 --- a/packages/loopover-miner/lib/worktree-allocator.ts +++ b/packages/loopover-miner/lib/worktree-allocator.ts @@ -273,12 +273,16 @@ export function openWorktreeAllocator(options: { const maxLeaseMs = normalizeMaxLeaseMs(options.maxLeaseMs); const hostId = normalizeHostId(options.hostId); const processPid = Number.isInteger(options.processPid) ? options.processPid as number : process.pid; - const nowMs = Number.isFinite(options.nowMs) ? options.nowMs as number : Date.now(); + // A function, not a captured value: production callers (no injected nowMs) get a genuinely fresh + // Date.now() on every reclaim call -- including each acquire() below (#8859) -- while a test's injected + // nowMs stays frozen at that same simulated instant across the initial open-time reclaim AND every + // later acquire() in that test, matching the deterministic clock the rest of this file's tests assume. + const getNowMs = Number.isFinite(options.nowMs) ? () => options.nowMs as number : () => Date.now(); const db = openLocalStoreDb(resolvedPath); ensureSlotTable(db); ensureSlots(db, worktreeBaseDir, maxConcurrency); - reclaimOrphanedAllocations(db, nowMs, maxLeaseMs, hostId); + reclaimOrphanedAllocations(db, getNowMs(), maxLeaseMs, hostId); const getByAttempt = db.prepare( "SELECT slot_index, worktree_path, attempt_id, repo_full_name, status, owner_pid, owner_host, allocated_at FROM worktree_slots WHERE attempt_id = ?", @@ -326,6 +330,10 @@ export function openWorktreeAllocator(options: { processPid, hostId, acquire(attemptId, repoFullName) { + // Reclaim orphaned slots first, so a peer's crashed/killed allocation frees up promptly instead of + // only at the next process-restart's openWorktreeAllocator() call (#8859) -- same per-call sweep + // pattern as portfolio-queue-manager.ts's claimNextBatch(). + reclaimOrphanedAllocations(db, getNowMs(), maxLeaseMs, hostId); const normalizedAttempt = normalizeAttemptId(attemptId); const normalizedRepo = normalizeRepoFullName(repoFullName); const existing = getByAttempt.get(normalizedAttempt) as WorktreeSlotRow | undefined; diff --git a/test/unit/miner-worktree-allocator-lease-expiry.test.ts b/test/unit/miner-worktree-allocator-lease-expiry.test.ts index b67ed82dad..d22580393a 100644 --- a/test/unit/miner-worktree-allocator-lease-expiry.test.ts +++ b/test/unit/miner-worktree-allocator-lease-expiry.test.ts @@ -224,3 +224,36 @@ describe("loopover-miner worktree allocator age-based orphan reclaim (#7085)", ( expect(allocator.release("attempt-a")?.ownerHost).toBeNull(); }); }); + +describe("loopover-miner worktree allocator per-acquire orphan reclaim (#8859)", () => { + it("reclaims a peer's allocation orphaned AFTER the allocator was opened, on the very next acquire() -- no reopen needed", () => { + const paths = tempPaths(); + const allocator = reopen(paths, { hostId: "host-A" }); + allocator.acquire("attempt-1", "acme/widgets"); + expect(activeCount(allocator)).toBe(1); + + // Simulate a PEER crashing on this same host well AFTER openWorktreeAllocator() already ran its + // one-time startup reclaim: a second connection to the same store file overwrites the active row as + // a dead pid with a recent (well within-lease) allocated_at, so only the same-host dead-pid fast path + // -- not the age guard -- can explain a reclaim here. + const peer = new DatabaseSync(paths.dbPath); + try { + peer + .prepare(` + UPDATE worktree_slots + SET owner_pid = ?, owner_host = ?, allocated_at = ? + WHERE attempt_id = 'attempt-1' + `) + .run(DEAD_PID, "host-A", new Date(Date.now() - 60_000).toISOString()); + } finally { + peer.close(); + } + + // Without #8859's fix, this allocator's reclaim only ever ran once, at the reopen() call above -- + // this acquire() would see the slot as still "active" and throw worktree_capacity_exceeded. No fresh + // openWorktreeAllocator() call happens here: the SAME long-lived instance must reclaim it itself. + const allocation = allocator.acquire("attempt-2", "acme/other"); + expect(allocation.status).toBe("active"); + expect(activeCount(allocator)).toBe(1); + }); +});