From f3c85a470149c095a9a18a64a1e2b98cd3298af9 Mon Sep 17 00:00:00 2001 From: jaytbarimbao-collab <300663773+jaytbarimbao-collab@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:34:01 -0400 Subject: [PATCH] feat(miner): add bounded retry/backoff to claim-conflict-resolver's post-submission live-state check Fixes #6058. resolveClaimConflict fetched the live competing-claims snapshot exactly once post-submission, so a genuine competing PR that hadn't yet propagated through GitHub's search/GraphQL index in that instant was invisible (the module's own header documented this gap). Wraps the snapshot fetch in a bounded retry-with-backoff following http-retry.js's convention: up to maxAttempts (default 3) attempts with exponential backoff between them, returning as soon as a competing claim is observed and otherwise giving a late-propagating competitor time to surface. Pure over injected sleepFn/backoffMs (no real timers in tests); the maintainer-gated write boundary (#4833) is unchanged. --- .../lib/claim-conflict-resolver.d.ts | 12 ++- .../lib/claim-conflict-resolver.js | 51 ++++++++--- .../miner-claim-conflict-resolver.test.ts | 88 +++++++++++++++++++ 3 files changed, 139 insertions(+), 12 deletions(-) diff --git a/packages/loopover-miner/lib/claim-conflict-resolver.d.ts b/packages/loopover-miner/lib/claim-conflict-resolver.d.ts index 3f71336503..7377c70ee1 100644 --- a/packages/loopover-miner/lib/claim-conflict-resolver.d.ts +++ b/packages/loopover-miner/lib/claim-conflict-resolver.d.ts @@ -26,4 +26,14 @@ export type ClaimConflictResult = | { checked: true; isWinner: true; winnerNumber: number | null; competingCount: number } | { checked: true; isWinner: false; winnerNumber: number | null; competingCount: number; closeResult: unknown }; -export function resolveClaimConflict(input: ClaimConflictInput, deps: ClaimConflictDeps): Promise; +export type ClaimConflictRetryOptions = { + maxAttempts?: number; + sleepFn?: (ms: number) => Promise; + backoffMs?: (attempt: number) => number; +}; + +export function resolveClaimConflict( + input: ClaimConflictInput, + deps: ClaimConflictDeps, + options?: ClaimConflictRetryOptions, +): Promise; diff --git a/packages/loopover-miner/lib/claim-conflict-resolver.js b/packages/loopover-miner/lib/claim-conflict-resolver.js index aa19991402..351427ec52 100644 --- a/packages/loopover-miner/lib/claim-conflict-resolver.js +++ b/packages/loopover-miner/lib/claim-conflict-resolver.js @@ -18,12 +18,21 @@ // the best real, publicly-observable proxy available for someone else's PR -- live-issue-snapshot.js's own // comment on `createdAt` explains this in more detail. // -// EVENTUAL CONSISTENCY: this checks GitHub's live state immediately after submission. A competing PR that -// exists but hasn't yet propagated through GitHub's own search/GraphQL indexing in that instant would be -// invisible to this one-shot check -- there is no retry/backoff here, which would be its own separate scope. +// EVENTUAL CONSISTENCY: this checks GitHub's live state after submission. A competing PR that exists but +// hasn't yet propagated through GitHub's own search/GraphQL indexing in the first instant would be invisible +// to a single check, so the live-state snapshot fetch is wrapped in a bounded retry-with-backoff (#6058): +// a few attempts with exponential backoff (following http-retry.js's convention), returning as soon as a +// competing claim is observed, and otherwise giving a late-propagating competitor time to surface before +// this miner is declared the winner. The write-authorization boundary (#4833) is unchanged. import { adjudicateSoftClaim } from "./claim-adjudication.js"; import { buildClosePrSpec } from "@loopover/engine"; +import { defaultRetryBackoffMs } from "./http-retry.js"; + +// Bounded retry for the post-submission live-state check (#6058): a few attempts give a competing PR that +// hasn't propagated through GitHub's search/GraphQL index yet time to surface, without an unbounded loop. +const DEFAULT_SNAPSHOT_MAX_ATTEMPTS = 3; +const defaultSnapshotSleep = (delayMs) => new Promise((resolve) => setTimeout(resolve, delayMs)); /** * Assemble the real competing-claims set from a fetched LiveIssueSnapshot: every OTHER open PR referencing @@ -58,6 +67,10 @@ export function assembleCompetingClaims(snapshot, selfPrNumber, minerLogin) { * fetchLiveIssueSnapshot: (repoFullName: string, issueNumber: number) => Promise, * executeLocalWrite: (spec: import("@loopover/engine").LocalWriteActionSpec) => Promise, * }} deps + * @param {{ maxAttempts?: number, sleepFn?: (ms: number) => Promise, backoffMs?: (attempt: number) => number }} [options] + * Bounded retry for the live-state snapshot fetch (#6058): up to `maxAttempts` (default 3) attempts with + * `backoffMs(attempt)` backoff between them, returning as soon as a competing claim is observed. Pure over + * the injected `sleepFn`/`backoffMs` -- no real timers in tests. * @returns {Promise<{ * checked: boolean, * reason?: "live_state_unavailable", @@ -67,18 +80,34 @@ export function assembleCompetingClaims(snapshot, selfPrNumber, minerLogin) { * closeResult?: unknown, * }>} */ -export async function resolveClaimConflict(input, deps) { - let snapshot; - try { - snapshot = await deps.fetchLiveIssueSnapshot(input.repoFullName, input.issueNumber); - } catch { - snapshot = null; +export async function resolveClaimConflict(input, deps, options = {}) { + const maxAttempts = + Number.isFinite(options.maxAttempts) && options.maxAttempts >= 1 ? Math.floor(options.maxAttempts) : DEFAULT_SNAPSHOT_MAX_ATTEMPTS; + const sleepFn = typeof options.sleepFn === "function" ? options.sleepFn : defaultSnapshotSleep; + const backoffMs = typeof options.backoffMs === "function" ? options.backoffMs : defaultRetryBackoffMs; + + let snapshot = null; + let competing = []; + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + let current; + try { + current = await deps.fetchLiveIssueSnapshot(input.repoFullName, input.issueNumber); + } catch { + current = null; + } + if (current && typeof current === "object") { + snapshot = current; + competing = assembleCompetingClaims(current, input.selfPrNumber, input.minerLogin); + // A competing claim observed = GitHub's index has propagated it; stop retrying and act on it now. + if (competing.length > 0) break; + } + // Back off before the next attempt (index-propagation lag / a transient fetch failure); never after the last. + if (attempt < maxAttempts) await sleepFn(backoffMs(attempt)); } - if (!snapshot || typeof snapshot !== "object") { + if (!snapshot) { return { checked: false, reason: "live_state_unavailable" }; } - const competing = assembleCompetingClaims(snapshot, input.selfPrNumber, input.minerLogin); const adjudication = adjudicateSoftClaim({ number: input.selfPrNumber, claimedAt: input.selfClaimedAt }, competing); if (adjudication.isWinner) { diff --git a/test/unit/miner-claim-conflict-resolver.test.ts b/test/unit/miner-claim-conflict-resolver.test.ts index 7f100a19da..a7376f7a29 100644 --- a/test/unit/miner-claim-conflict-resolver.test.ts +++ b/test/unit/miner-claim-conflict-resolver.test.ts @@ -96,6 +96,7 @@ describe("resolveClaimConflict (#4848)", () => { const result = await resolveClaimConflict( { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 5, selfClaimedAt: "2026-01-01T00:00:00Z", minerLogin: "miner-bot" }, { fetchLiveIssueSnapshot, executeLocalWrite }, + { sleepFn: async () => {} }, ); expect(result).toEqual({ checked: true, isWinner: true, winnerNumber: 5, competingCount: 0 }); @@ -109,6 +110,7 @@ describe("resolveClaimConflict (#4848)", () => { const result = await resolveClaimConflict( { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 5, selfClaimedAt: "2026-01-01T00:00:00Z", minerLogin: "miner-bot" }, { fetchLiveIssueSnapshot, executeLocalWrite }, + { sleepFn: async () => {} }, ); expect(result).toEqual({ checked: false, reason: "live_state_unavailable" }); @@ -124,6 +126,7 @@ describe("resolveClaimConflict (#4848)", () => { const result = await resolveClaimConflict( { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 5, selfClaimedAt: "2026-01-01T00:00:00Z", minerLogin: "miner-bot" }, { fetchLiveIssueSnapshot, executeLocalWrite }, + { sleepFn: async () => {} }, ); expect(result).toEqual({ checked: false, reason: "live_state_unavailable" }); @@ -149,4 +152,89 @@ describe("resolveClaimConflict (#4848)", () => { expect(spec.command).not.toContain("#null"); expect(spec.command).toContain("another open pull request already claims this issue"); }); + + it("retries with backoff and detects a competitor that only propagates on a later attempt (#6058)", async () => { + const competitor = snapshot([{ number: 5, state: "open", authorLogin: "someone-else", createdAt: "2026-01-01T00:00:00Z" }]); + // First check: GitHub's index hasn't surfaced the competing PR yet; second check (after backoff): it has. + const fetchLiveIssueSnapshot = vi.fn().mockResolvedValueOnce(snapshot([])).mockResolvedValueOnce(competitor); + const executeLocalWrite = vi.fn(async (spec: { action: string; command: string }) => ({ action: spec.action, code: 0, stdout: "", stderr: "", timedOut: false })); + const sleeps: number[] = []; + + const result = await resolveClaimConflict( + { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 6, selfClaimedAt: "2026-01-02T00:00:00Z", minerLogin: "miner-bot" }, + { fetchLiveIssueSnapshot, executeLocalWrite }, + { sleepFn: async (ms: number) => { sleeps.push(ms); }, backoffMs: (attempt: number) => attempt * 100 }, + ); + + expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(2); + expect(sleeps).toEqual([100]); // backed off once (after attempt 1) with backoffMs(1) + expect(result.checked).toBe(true); + if (!result.checked) throw new Error("expected checked"); + expect(result.isWinner).toBe(false); + expect(result.competingCount).toBe(1); + expect(executeLocalWrite).toHaveBeenCalledTimes(1); + }); + + it("stops early (no extra fetch or sleep) once a competitor is observed on the first attempt (#6058)", async () => { + const fetchLiveIssueSnapshot = vi.fn(async () => snapshot([{ number: 5, state: "open", authorLogin: "someone-else", createdAt: "2026-01-01T00:00:00Z" }])); + const executeLocalWrite = vi.fn(async (spec: { action: string; command: string }) => ({ action: spec.action, code: 0, stdout: "", stderr: "", timedOut: false })); + const sleepFn = vi.fn(async () => {}); + + const result = await resolveClaimConflict( + { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 6, selfClaimedAt: "2026-01-02T00:00:00Z", minerLogin: "miner-bot" }, + { fetchLiveIssueSnapshot, executeLocalWrite }, + { sleepFn }, + ); + + expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(1); + expect(sleepFn).not.toHaveBeenCalled(); + expect(result.checked).toBe(true); + if (!result.checked) throw new Error("expected checked"); + expect(result.isWinner).toBe(false); + }); + + it("exhausts the configured maxAttempts (backoff between each, never after the last) before declaring a winner (#6058)", async () => { + const fetchLiveIssueSnapshot = vi.fn(async () => snapshot([])); // no competitor ever appears + const executeLocalWrite = vi.fn(); + const backoffAttempts: number[] = []; + + const result = await resolveClaimConflict( + { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 5, selfClaimedAt: "2026-01-01T00:00:00Z", minerLogin: "miner-bot" }, + { fetchLiveIssueSnapshot, executeLocalWrite }, + { maxAttempts: 4, sleepFn: async () => {}, backoffMs: (attempt: number) => { backoffAttempts.push(attempt); return 0; } }, + ); + + expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(4); + expect(backoffAttempts).toEqual([1, 2, 3]); // 3 gaps between 4 attempts, none after the last + expect(result).toEqual({ checked: true, isWinner: true, winnerNumber: 5, competingCount: 0 }); + expect(executeLocalWrite).not.toHaveBeenCalled(); + }); + + it("retries a transient fetch failure and uses the first snapshot that comes back (#6058)", async () => { + const fetchLiveIssueSnapshot = vi.fn().mockRejectedValueOnce(new Error("index lag")).mockResolvedValueOnce(snapshot([])); + const executeLocalWrite = vi.fn(); + + const result = await resolveClaimConflict( + { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 5, selfClaimedAt: "2026-01-01T00:00:00Z", minerLogin: "miner-bot" }, + { fetchLiveIssueSnapshot, executeLocalWrite }, + { maxAttempts: 2, sleepFn: async () => {} }, + ); + + expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(2); + expect(result).toEqual({ checked: true, isWinner: true, winnerNumber: 5, competingCount: 0 }); + }); + + it("uses the default (real) sleep between retries when no sleepFn is injected (#6058)", async () => { + const fetchLiveIssueSnapshot = vi.fn(async () => snapshot([])); + const executeLocalWrite = vi.fn(); + // No sleepFn → exercises the default setTimeout-based sleep; backoffMs 0 keeps it instant. + const result = await resolveClaimConflict( + { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 5, selfClaimedAt: "2026-01-01T00:00:00Z", minerLogin: "miner-bot" }, + { fetchLiveIssueSnapshot, executeLocalWrite }, + { maxAttempts: 2, backoffMs: () => 0 }, + ); + + expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(2); + expect(result).toEqual({ checked: true, isWinner: true, winnerNumber: 5, competingCount: 0 }); + }); });