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
12 changes: 11 additions & 1 deletion packages/loopover-miner/lib/submission-freshness-check.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,14 @@ export type SubmissionFreshnessDeps = {

export type SubmissionFreshnessResult = { fresh: true } | { fresh: false; reason: FreshnessAbortReason };

export function checkSubmissionFreshness(candidate: SubmissionFreshnessCandidate, deps: SubmissionFreshnessDeps): Promise<SubmissionFreshnessResult>;
export type SubmissionFreshnessRetryOptions = {
maxAttempts?: number;
sleepFn?: (ms: number) => Promise<unknown>;
backoffMs?: (attempt: number) => number;
};

export function checkSubmissionFreshness(
candidate: SubmissionFreshnessCandidate,
deps: SubmissionFreshnessDeps,
options?: SubmissionFreshnessRetryOptions,
): Promise<SubmissionFreshnessResult>;
49 changes: 42 additions & 7 deletions packages/loopover-miner/lib/submission-freshness-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,27 @@
// FAIL CLOSED: an unreachable/failed live-state fetch is treated as stale (aborts), never as "no evidence of
// staleness, so proceed" -- mirrors this package's fail-closed convention elsewhere (harness-submission-
// trigger.js's predicted_gate_unavailable/slop_assessment_unavailable, iterate-loop.ts's ambiguous-on-error).
// That fail-closed OUTCOME is unchanged; a single transient blip just gets a bounded retry-with-backoff to
// resolve itself FIRST (#7089) -- the same window claim-conflict-resolver.js's resolveClaimConflict (#6058)
// already gives its own call to this identical fetchLiveIssueSnapshot, reusing http-retry.js's shared backoff.
// Aborting here discards a fully-completed create/iterate loop's local work, so riding out a brief 5xx /
// GraphQL-index propagation lag before failing closed matters more here than in the post-submission case.
//
// NOT a rejection outcome: staleness is caught BEFORE any PR exists, so it is not the same lifecycle event as
// rejection-state-machine.js's DISENGAGED_OUTCOME (which handles an EXISTING PR a maintainer closed). "No PR,
// no noisy failure" here just means: return a quiet not-fresh result, same shape as any other blocked gate
// decision in this package -- never throw, never surface anything to the target repo.

import { defaultRetryBackoffMs } from "./http-retry.js";

export const SUBMISSION_FRESHNESS_ABORT_EVENT = "submission_freshness_abort";

// Bounded retry for the pre-submission live-state fetch (#7089), mirroring claim-conflict-resolver.js's
// resolveClaimConflict (#6058): a few attempts with exponential backoff let a transient GitHub blur (a brief
// 5xx, or GraphQL-index propagation lag) resolve itself before we fail closed, without an unbounded loop.
const DEFAULT_SNAPSHOT_MAX_ATTEMPTS = 3;
const defaultSnapshotSleep = (delayMs) => new Promise((resolve) => setTimeout(resolve, delayMs));

/**
* Evaluate whether a submission candidate's live repo state is still fresh enough to proceed toward open_pr.
* Checks the miner's own claim-ledger status first (local, free) before spending a network round-trip on the
Expand All @@ -34,8 +47,14 @@ export const SUBMISSION_FRESHNESS_ABORT_EVENT = "submission_freshness_abort";
* fetchLiveIssueSnapshot: (repoFullName: string, issueNumber: number) => Promise<{ state: "open"|"closed", referencingPrs: Array<{ number: number, state: "open"|"closed"|"merged", authorLogin: string }> } | null>,
* eventLedger: { appendEvent(event: { type: string, repoFullName?: string, payload: Record<string, unknown> }): unknown },
* }} deps
* @param {{ maxAttempts?: number, sleepFn?: (ms: number) => Promise<unknown>, backoffMs?: (attempt: number) => number }} [options]
* Bounded retry for the live-state snapshot fetch (#7089): up to `maxAttempts` (default 3) attempts with
* `backoffMs(attempt)` backoff between them, returning as soon as a real (non-null, well-formed) snapshot is
* obtained. Optional -- every existing caller works unchanged. Pure over the injected `sleepFn`/`backoffMs`
* -- no real timers in tests. Only the fetch itself is retried; a well-formed snapshot's own signals
* (issue_closed / already_addressed) are decided once, never retried.
*/
export async function checkSubmissionFreshness(candidate, deps) {
export async function checkSubmissionFreshness(candidate, deps, options = {}) {
if (!candidate || typeof candidate !== "object") throw new Error("invalid_freshness_candidate");
const repoFullName = typeof candidate.repoFullName === "string" ? candidate.repoFullName.trim() : "";
if (!repoFullName) throw new Error("invalid_repo_full_name");
Expand All @@ -49,18 +68,34 @@ export async function checkSubmissionFreshness(candidate, deps) {
if (typeof fetchLiveIssueSnapshot !== "function") throw new Error("invalid_live_state_fetcher");
if (!eventLedger || typeof eventLedger.appendEvent !== "function") throw new Error("invalid_event_ledger");

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;

const claim = claimLedger.listClaims({ repoFullName }).find((c) => c.issueNumber === candidate.issueNumber);
if (!claim || claim.status !== "active") {
return abort(eventLedger, repoFullName, candidate.issueNumber, "claim_superseded");
}

let snapshot;
try {
snapshot = await fetchLiveIssueSnapshot(repoFullName, candidate.issueNumber);
} catch {
snapshot = null;
let snapshot = null;
for (let attempt = 1; attempt <= maxAttempts; attempt += 1) {
let current;
try {
current = await fetchLiveIssueSnapshot(repoFullName, candidate.issueNumber);
} catch {
current = null;
}
if (current && typeof current === "object") {
// A real, well-formed snapshot resolves the transient window: stop retrying and decide on it now.
snapshot = current;
break;
}
// Back off before the next attempt (transient 5xx / index-propagation lag); never after the last one.
if (attempt < maxAttempts) await sleepFn(backoffMs(attempt));
}
if (!snapshot || typeof snapshot !== "object") {
if (!snapshot) {
// Retry budget exhausted with no usable snapshot -- fail closed exactly as before (#7089 only widens the window).
return abort(eventLedger, repoFullName, candidate.issueNumber, "live_state_unavailable");
}

Expand Down
153 changes: 152 additions & 1 deletion test/unit/miner-submission-freshness-check.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,14 +194,15 @@ describe("checkSubmissionFreshness (#3007)", () => {
expect(result).toEqual({ fresh: true });
});

it("live-state-unavailable abort when the fetch returns null", async () => {
it("live-state-unavailable abort when the fetch returns null on every attempt", async () => {
const { claimLedger } = stubClaimLedger([activeClaim]);
const { eventLedger } = stubEventLedger();
const fetchLiveIssueSnapshot = vi.fn(async () => null);

const result = await checkSubmissionFreshness(
{ repoFullName: "acme/widgets", issueNumber: 42, minerLogin: "miner-bot" },
{ claimLedger, fetchLiveIssueSnapshot, eventLedger },
{ sleepFn: async () => {}, backoffMs: () => 0 },
);

expect(result).toEqual({ fresh: false, reason: "live_state_unavailable" });
Expand All @@ -217,6 +218,7 @@ describe("checkSubmissionFreshness (#3007)", () => {
const result = await checkSubmissionFreshness(
{ repoFullName: "acme/widgets", issueNumber: 42, minerLogin: "miner-bot" },
{ claimLedger, fetchLiveIssueSnapshot, eventLedger },
{ sleepFn: async () => {}, backoffMs: () => 0 },
);

expect(result).toEqual({ fresh: false, reason: "live_state_unavailable" });
Expand Down Expand Up @@ -260,3 +262,152 @@ describe("checkSubmissionFreshness (#3007)", () => {
await expect(checkSubmissionFreshness(candidate, { claimLedger, fetchLiveIssueSnapshot } as never)).rejects.toThrow("invalid_event_ledger");
});
});

describe("checkSubmissionFreshness live-state retry (#7089)", () => {
it("rides out a thrown transient failure and proceeds on a later attempt instead of aborting", async () => {
const { claimLedger } = stubClaimLedger([activeClaim]);
const { eventLedger, appendEvent } = stubEventLedger();
// First call: transient GitHub blip (a 5xx surfaces as a thrown fetch); second call: real snapshot.
const fetchLiveIssueSnapshot = vi
.fn()
.mockRejectedValueOnce(new Error("transient 5xx"))
.mockResolvedValueOnce({ state: "open" as const, referencingPrs: [] });
const sleeps: number[] = [];

const result = await checkSubmissionFreshness(
{ repoFullName: "acme/widgets", issueNumber: 42, minerLogin: "miner-bot" },
{ claimLedger, fetchLiveIssueSnapshot, eventLedger },
{ maxAttempts: 3, sleepFn: async (ms: number) => { sleeps.push(ms); }, backoffMs: (attempt: number) => attempt * 100 },
);

expect(result).toEqual({ fresh: true });
expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(2);
expect(sleeps).toEqual([100]); // backed off once (after attempt 1) with backoffMs(1); no abort logged
expect(appendEvent).not.toHaveBeenCalled();
});

it("rides out a null (non-object) snapshot and proceeds on a later attempt", async () => {
const { claimLedger } = stubClaimLedger([activeClaim]);
const { eventLedger, appendEvent } = stubEventLedger();
const fetchLiveIssueSnapshot = vi
.fn()
.mockResolvedValueOnce(null)
.mockResolvedValueOnce({ state: "open" as const, referencingPrs: [] });

const result = await checkSubmissionFreshness(
{ repoFullName: "acme/widgets", issueNumber: 42, minerLogin: "miner-bot" },
{ claimLedger, fetchLiveIssueSnapshot, eventLedger },
{ maxAttempts: 3, sleepFn: async () => {}, backoffMs: () => 0 },
);

expect(result).toEqual({ fresh: true });
expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(2);
expect(appendEvent).not.toHaveBeenCalled();
});

it("stops the instant a real snapshot is obtained: does not burn remaining attempts or sleep", async () => {
const { claimLedger } = stubClaimLedger([activeClaim]);
const { eventLedger } = stubEventLedger();
const fetchLiveIssueSnapshot = vi.fn(async () => ({ state: "open" as const, referencingPrs: [] }));
const sleepFn = vi.fn(async () => {});

const result = await checkSubmissionFreshness(
{ repoFullName: "acme/widgets", issueNumber: 42, minerLogin: "miner-bot" },
{ claimLedger, fetchLiveIssueSnapshot, eventLedger },
{ maxAttempts: 3, sleepFn, backoffMs: (attempt: number) => attempt * 100 },
);

expect(result).toEqual({ fresh: true });
expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(1);
expect(sleepFn).not.toHaveBeenCalled();
});

it("still fails closed after the bounded retries are exhausted, appending the abort event exactly once", async () => {
const { claimLedger } = stubClaimLedger([activeClaim]);
const { eventLedger, appendEvent } = stubEventLedger();
const fetchLiveIssueSnapshot = vi.fn(async () => null); // never recovers
const sleeps: number[] = [];

const result = await checkSubmissionFreshness(
{ repoFullName: "acme/widgets", issueNumber: 42, minerLogin: "miner-bot" },
{ claimLedger, fetchLiveIssueSnapshot, eventLedger },
{ maxAttempts: 3, sleepFn: async (ms: number) => { sleeps.push(ms); }, backoffMs: (attempt: number) => attempt * 100 },
);

expect(result).toEqual({ fresh: false, reason: "live_state_unavailable" });
expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(3);
expect(sleeps).toEqual([100, 200]); // backed off between attempts, never after the last
expect(appendEvent).toHaveBeenCalledTimes(1); // logged once for the whole check, not once per failed attempt
expect(appendEvent).toHaveBeenCalledWith({
type: SUBMISSION_FRESHNESS_ABORT_EVENT,
repoFullName: "acme/widgets",
payload: { issueNumber: 42, reason: "live_state_unavailable" },
});
});

it("does not retry a real staleness signal: a closed issue aborts on the first snapshot", async () => {
const { claimLedger } = stubClaimLedger([activeClaim]);
const { eventLedger } = stubEventLedger();
const fetchLiveIssueSnapshot = vi.fn(async () => ({ state: "closed" as const, referencingPrs: [] }));
const sleepFn = vi.fn(async () => {});

const result = await checkSubmissionFreshness(
{ repoFullName: "acme/widgets", issueNumber: 42, minerLogin: "miner-bot" },
{ claimLedger, fetchLiveIssueSnapshot, eventLedger },
{ maxAttempts: 3, sleepFn, backoffMs: () => 0 },
);

expect(result).toEqual({ fresh: false, reason: "issue_closed" });
expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(1); // a well-formed snapshot ends the retry loop
expect(sleepFn).not.toHaveBeenCalled();
});

it("defaults maxAttempts/sleepFn/backoffMs when no options are passed (existing callers unchanged)", async () => {
const { claimLedger } = stubClaimLedger([activeClaim]);
const { eventLedger, appendEvent } = stubEventLedger();
const fetchLiveIssueSnapshot = vi.fn(async () => ({ state: "open" as const, referencingPrs: [] }));

const result = await checkSubmissionFreshness(
{ repoFullName: "acme/widgets", issueNumber: 42, minerLogin: "miner-bot" },
{ claimLedger, fetchLiveIssueSnapshot, eventLedger },
);

expect(result).toEqual({ fresh: true });
expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(1); // succeeds first attempt, so no default backoff/sleep runs
expect(appendEvent).not.toHaveBeenCalled();
});

it("uses the default (real) sleep between retries when no sleepFn is injected", async () => {
const { claimLedger } = stubClaimLedger([activeClaim]);
const { eventLedger } = stubEventLedger();
// No sleepFn → exercises the default setTimeout-based sleep; backoffMs 0 keeps it instant.
const fetchLiveIssueSnapshot = vi
.fn()
.mockResolvedValueOnce(null)
.mockResolvedValueOnce({ state: "open" as const, referencingPrs: [] });

const result = await checkSubmissionFreshness(
{ repoFullName: "acme/widgets", issueNumber: 42, minerLogin: "miner-bot" },
{ claimLedger, fetchLiveIssueSnapshot, eventLedger },
{ maxAttempts: 2, backoffMs: () => 0 },
);

expect(result).toEqual({ fresh: true });
expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(2);
});

it("clamps a maxAttempts below 1 back to the default rather than skipping every attempt", async () => {
const { claimLedger } = stubClaimLedger([activeClaim]);
const { eventLedger } = stubEventLedger();
const fetchLiveIssueSnapshot = vi.fn(async () => null); // never recovers, so it runs the full default budget

const result = await checkSubmissionFreshness(
{ repoFullName: "acme/widgets", issueNumber: 42, minerLogin: "miner-bot" },
{ claimLedger, fetchLiveIssueSnapshot, eventLedger },
{ maxAttempts: 0, sleepFn: async () => {}, backoffMs: () => 0 },
);

expect(result).toEqual({ fresh: false, reason: "live_state_unavailable" });
expect(fetchLiveIssueSnapshot).toHaveBeenCalledTimes(3); // DEFAULT_SNAPSHOT_MAX_ATTEMPTS, not 0
});
});