Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/loopover-miner/lib/claim-ledger.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ export type ListClaimsFilter = {
export type ClaimLedger = {
dbPath: string;
recordClaim(claim: RecordClaimInput): ClaimEntry;
/** Claims the issue, expiring any claim orphaned by a dead process first (#6156). */
claimIssue(repoFullName: string, issueNumber: number, note?: string, apiBaseUrl?: string): ClaimEntry;
/** Expire claims orphaned by a crashed/killed process, returning the transitioned rows (#6156). */
reclaimExpiredClaims(maxAgeMs?: number): ClaimEntry[];
releaseClaim(repoFullName: string, issueNumber: number, apiBaseUrl?: string): ClaimEntry | null;
expireClaim(repoFullName: string, issueNumber: number, apiBaseUrl?: string): ClaimEntry | null;
listClaims(filter?: ListClaimsFilter): ClaimEntry[];
Expand Down
12 changes: 12 additions & 0 deletions packages/loopover-miner/lib/claim-ledger.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { DatabaseSync } from "node:sqlite";
import { DEFAULT_MAX_CLAIM_AGE_MS, sweepExpiredClaims } from "./claim-ledger-expiry.js";
import { DEFAULT_FORGE_CONFIG } from "./forge-config.js";
import { normalizeLocalStoreDbPath, openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js";
import { isValidRepoSegment } from "./repo-clone.js";
Expand Down Expand Up @@ -220,7 +221,18 @@ export function openClaimLedger(dbPath = resolveClaimLedgerDbPath()) {
}
return rows.map(rowToClaim);
},
/** Expire claims orphaned by a crashed/killed process, returning the transitioned rows (#6156). The explicit
* counterpart to the sweep claimIssue runs on its own, mirroring reclaimStuckItems (portfolio-queue-manager.js). */
reclaimExpiredClaims(maxAgeMs = DEFAULT_MAX_CLAIM_AGE_MS) {
return sweepExpiredClaims(ledger, Date.now(), maxAgeMs);
},
claimIssue(repoFullName, issueNumber, note, apiBaseUrl) {
// Expire orphaned claims first, so an issue stranded 'active' by a dead process becomes claimable again
// instead of blocking indefinitely (#6156). Without this, recordClaim's `WHERE status <> 'active'` guard
// makes re-claiming an active row a no-op, so a claim whose owning process died keeps winning forever --
// there is no other path to expireClaim. Mirrors claimNextBatch's sweep-then-claim
// (portfolio-queue-manager.js), where a lease stranded by a dead process would otherwise starve the queue.
sweepExpiredClaims(ledger, Date.now(), DEFAULT_MAX_CLAIM_AGE_MS);
return ledger.recordClaim({ repoFullName, issueNumber, note, apiBaseUrl });
},
listActiveClaims(repoFullName) {
Expand Down
117 changes: 117 additions & 0 deletions test/unit/miner-claim-ledger-sweep-wiring.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest";
import { DEFAULT_MAX_CLAIM_AGE_MS } from "../../packages/loopover-miner/lib/claim-ledger-expiry.js";
import { closeDefaultClaimLedger, openClaimLedger } from "../../packages/loopover-miner/lib/claim-ledger.js";

// #6156: sweepExpiredClaims existed and worked, but nothing ever called it -- so a claim left behind by a
// crashed/killed miner never expired. That matters because recordClaim's `WHERE status <> 'active'` guard makes
// re-claiming an already-active row a deliberate no-op: with no sweep, the dead process's claim keeps winning
// forever and there is no path that reaches expireClaim. claimIssue now sweeps first, mirroring
// claimNextBatch's sweep-then-claim (portfolio-queue-manager.js).
const roots: string[] = [];
const ledgers: Array<{ close(): void }> = [];

function tempLedger() {
const root = mkdtempSync(join(tmpdir(), "loopover-miner-claim-sweep-"));
roots.push(root);
const ledger = openClaimLedger(join(root, "claim-ledger.sqlite3"));
ledgers.push(ledger);
return ledger;
}

/** Move the clock past the default window, so a claim recorded before it is stale by exactly one hour. */
function advancePastDefaultWindow(fromIso: string) {
vi.setSystemTime(new Date(Date.parse(fromIso) + DEFAULT_MAX_CLAIM_AGE_MS + 60 * 60 * 1000));
}

afterEach(() => {
for (const ledger of ledgers.splice(0)) ledger.close();
closeDefaultClaimLedger();
vi.useRealTimers();
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true });
});

describe("loopover-miner claim ledger sweep wiring (#6156)", () => {
it("REGRESSION: a claim stranded by a dead process is expired, so the issue can be claimed again", () => {
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
const ledger = tempLedger();
ledger.claimIssue("acme/widgets", 7, "attempt:crashed-process");

advancePastDefaultWindow("2026-01-01T00:00:00.000Z");
const reclaimed = ledger.claimIssue("acme/widgets", 7, "attempt:new-process");

// The whole point: the new attempt now OWNS the claim. Before the sweep was wired, recordClaim's no-op guard
// left the dead process's row untouched -- same claimedAt, same note -- while reporting success.
expect(reclaimed.note).toBe("attempt:new-process");
expect(reclaimed.status).toBe("active");
expect(reclaimed.claimedAt).toBe(new Date().toISOString());
expect(ledger.listClaims({ status: "active" })).toEqual([expect.objectContaining({ issueNumber: 7, note: "attempt:new-process" })]);
});

it("expires a stale claim on a DIFFERENT issue too -- the sweep is not scoped to the issue being claimed", () => {
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
const ledger = tempLedger();
ledger.claimIssue("acme/widgets", 1, "attempt:crashed-process");

advancePastDefaultWindow("2026-01-01T00:00:00.000Z");
ledger.claimIssue("acme/widgets", 2, "attempt:new-process");

expect(ledger.listClaims({ status: "expired" }).map((entry) => entry.issueNumber)).toEqual([1]);
expect(ledger.listClaims({ status: "active" }).map((entry) => entry.issueNumber)).toEqual([2]);
});

it("leaves a claim inside the window alone -- claiming does not expire a live sibling", () => {
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
const ledger = tempLedger();
ledger.claimIssue("acme/widgets", 1, "attempt:still-running");

// One hour short of the window: still the live process's claim.
vi.setSystemTime(new Date(Date.parse("2026-01-01T00:00:00.000Z") + DEFAULT_MAX_CLAIM_AGE_MS - 60 * 60 * 1000));
ledger.claimIssue("acme/widgets", 2, "attempt:other");

expect(ledger.listClaims({ status: "expired" })).toEqual([]);
expect(ledger.listClaims({ status: "active" }).map((entry) => entry.issueNumber)).toEqual([1, 2]);
});

it("reclaimExpiredClaims() expires stale claims on demand and returns the transitioned rows", () => {
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
const ledger = tempLedger();
ledger.claimIssue("acme/widgets", 1, "attempt:crashed-process");
advancePastDefaultWindow("2026-01-01T00:00:00.000Z");

// Default window -- the no-argument branch.
expect(ledger.reclaimExpiredClaims()).toEqual([expect.objectContaining({ issueNumber: 1, status: "expired" })]);
expect(ledger.reclaimExpiredClaims()).toEqual([]);
});

it("reclaimExpiredClaims(maxAgeMs) honours an explicit window", () => {
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
const ledger = tempLedger();
ledger.claimIssue("acme/widgets", 1, "attempt:recent");

// Two hours on: far inside the 14-day default, but past an explicit one-hour window -- so the argument is
// what decides, not the default.
vi.setSystemTime(new Date(Date.parse("2026-01-01T02:00:00.000Z")));
expect(ledger.reclaimExpiredClaims(DEFAULT_MAX_CLAIM_AGE_MS)).toEqual([]);
expect(ledger.reclaimExpiredClaims(60 * 60 * 1000)).toEqual([expect.objectContaining({ issueNumber: 1, status: "expired" })]);
});

it("a released claim is re-claimable as before -- the sweep doesn't disturb the normal lifecycle", () => {
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
const ledger = tempLedger();
ledger.claimIssue("acme/widgets", 1, "attempt:first");
ledger.releaseClaim("acme/widgets", 1);

const again = ledger.claimIssue("acme/widgets", 1, "attempt:second");
expect(again).toEqual(expect.objectContaining({ status: "active", note: "attempt:second" }));
expect(ledger.listClaims({ status: "expired" })).toEqual([]);
});
});