diff --git a/packages/loopover-miner/lib/attempt-runner.ts b/packages/loopover-miner/lib/attempt-runner.ts index 50e9e3f484..76bec82313 100644 --- a/packages/loopover-miner/lib/attempt-runner.ts +++ b/packages/loopover-miner/lib/attempt-runner.ts @@ -57,6 +57,10 @@ export type AttemptInput = { maxConsecutiveGateBlocks?: number; draft?: boolean; governor: AttemptGovernorContext; + /** The forge host this attempt's own claim was recorded under (#5563 composite key). Omitted matches every + * pre-existing single-forge caller: checkSubmissionFreshness resolves that the same way claim-ledger.js's + * own normalizeApiBaseUrl does, to the github.com default. */ + apiBaseUrl?: string; }; export type AttemptDeps = { @@ -233,7 +237,14 @@ export async function runMinerAttempt(input: AttemptInput, deps: AttemptDeps): P } const freshness = await checkSubmissionFreshness( - { repoFullName: input.loopInput.repoFullName, issueNumber: input.issueNumber, minerLogin: input.minerLogin }, + { + repoFullName: input.loopInput.repoFullName, + issueNumber: input.issueNumber, + minerLogin: input.minerLogin, + // Spread-omit rather than pass `undefined` explicitly -- SubmissionFreshnessCandidate's `apiBaseUrl` is + // optional but not `| undefined` under exactOptionalPropertyTypes, same reasoning as maxConsecutiveGateBlocks below. + ...(input.apiBaseUrl !== undefined ? { apiBaseUrl: input.apiBaseUrl } : {}), + }, { claimLedger: deps.claimLedger, fetchLiveIssueSnapshot: deps.fetchLiveIssueSnapshot, eventLedger: deps.eventLedger }, ); if (!freshness.fresh) { diff --git a/packages/loopover-miner/lib/submission-freshness-check.ts b/packages/loopover-miner/lib/submission-freshness-check.ts index 78d19be5fb..0eee7165b0 100644 --- a/packages/loopover-miner/lib/submission-freshness-check.ts +++ b/packages/loopover-miner/lib/submission-freshness-check.ts @@ -26,6 +26,7 @@ // 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 { DEFAULT_FORGE_CONFIG } from "./forge-config.js"; import { defaultRetryBackoffMs } from "./http-retry.js"; export const SUBMISSION_FRESHNESS_ABORT_EVENT = "submission_freshness_abort" as const; @@ -36,6 +37,10 @@ export type SubmissionFreshnessCandidate = { repoFullName: string; issueNumber: number; minerLogin: string; + /** The forge host this attempt's claim was recorded under (#5563 composite key). Omitted/blank -> the + * github.com default, matching claim-ledger.js's own normalizeApiBaseUrl, so every existing single-forge + * caller resolves to the same host it always did. */ + apiBaseUrl?: string; }; export type LiveIssueSnapshot = { @@ -44,7 +49,9 @@ export type LiveIssueSnapshot = { }; export type SubmissionFreshnessClaimLedger = { - listClaims(filter: { repoFullName?: string; status?: string }): Array<{ repoFullName: string; issueNumber: number; status: string }>; + listClaims( + filter: { repoFullName?: string; status?: string }, + ): Array<{ repoFullName: string; issueNumber: number; status: string; apiBaseUrl?: string }>; }; export type SubmissionFreshnessEventLedger = { @@ -71,6 +78,14 @@ export type SubmissionFreshnessRetryOptions = { const DEFAULT_SNAPSHOT_MAX_ATTEMPTS = 3; const defaultSnapshotSleep = (delayMs: number): Promise => new Promise((resolve) => setTimeout(resolve, delayMs)); +/** The forge host a claim row or candidate belongs to. Mirrors manage-poll.js's resolveManagedRowApiBaseUrl (and + * every store's normalizeApiBaseUrl): omitted/blank -> the github.com default, so a single-forge caller/row is + * unaffected. Used only to COMPARE hosts here -- claim-ledger.js's own writer path still does the real + * normalization/validation before a row is ever persisted. */ +function resolveFreshnessApiBaseUrl(apiBaseUrl: unknown): string { + return typeof apiBaseUrl === "string" && apiBaseUrl.trim() ? apiBaseUrl.trim() : DEFAULT_FORGE_CONFIG.apiBaseUrl; +} + /** * 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 @@ -107,7 +122,13 @@ export async function checkSubmissionFreshness( 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); + // Host-scoped match (#5563 composite key): a row belonging to a DIFFERENT forge host is ignored entirely -- + // neither a pass nor a claim_superseded abort -- rather than winning on `.find`'s first-match-wins order + // regardless of which host recorded it. + const candidateApiBaseUrl = resolveFreshnessApiBaseUrl(candidate.apiBaseUrl); + const claim = claimLedger + .listClaims({ repoFullName }) + .find((c) => c.issueNumber === candidate.issueNumber && resolveFreshnessApiBaseUrl(c.apiBaseUrl) === candidateApiBaseUrl); if (!claim || claim.status !== "active") { return abort(eventLedger, repoFullName, candidate.issueNumber, "claim_superseded"); } diff --git a/test/unit/miner-attempt-runner.test.ts b/test/unit/miner-attempt-runner.test.ts index 37c70fb0a8..b5593629dd 100644 --- a/test/unit/miner-attempt-runner.test.ts +++ b/test/unit/miner-attempt-runner.test.ts @@ -281,6 +281,23 @@ describe("runMinerAttempt (#2337) — the real create->review->gate->submit pipe expect(result.reason).toBe("claim_superseded"); }); + it("REGRESSION (#10004): threads the attempt's own apiBaseUrl into the freshness candidate, so a forge.internal attempt matches its own host's active row instead of a released github.com row for the same issue", async () => { + const deps = baseDeps({ + claimLedger: { + listClaims: () => [ + // Lowest id, github.com, released -- this attempt is NOT on github.com, so an unthreaded (always + // github.com) candidate would wrongly match THIS row first and abort claim_superseded. + { repoFullName: "acme/widgets", issueNumber: 7, status: "released", apiBaseUrl: "https://api.github.com" }, + // This attempt's OWN host and row -- must be the one matched once apiBaseUrl is actually threaded through. + { repoFullName: "acme/widgets", issueNumber: 7, status: "active", apiBaseUrl: "https://forge.internal" }, + ], + }, + }); + const result = await runMinerAttempt(baseAttemptInput({ apiBaseUrl: "https://forge.internal" }), deps); + + expect(result.outcome).toBe("submitted"); + }); + it("blocked: the submission-gate itself declines (e.g. global kill-switch) before the governor ever runs", async () => { const deps = baseDeps(); const result = await runMinerAttempt(baseAttemptInput({ killSwitchScope: "global" }), deps); diff --git a/test/unit/miner-submission-freshness-check.test.ts b/test/unit/miner-submission-freshness-check.test.ts index f7db8e2c58..b8d9afd104 100644 --- a/test/unit/miner-submission-freshness-check.test.ts +++ b/test/unit/miner-submission-freshness-check.test.ts @@ -1,8 +1,9 @@ import { describe, expect, it, vi } from "vitest"; +import { DEFAULT_FORGE_CONFIG } from "../../packages/loopover-miner/lib/forge-config"; import { checkSubmissionFreshness, SUBMISSION_FRESHNESS_ABORT_EVENT } from "../../packages/loopover-miner/lib/submission-freshness-check"; -function stubClaimLedger(claims: Array<{ repoFullName: string; issueNumber: number; status: string }> = []) { +function stubClaimLedger(claims: Array<{ repoFullName: string; issueNumber: number; status: string; apiBaseUrl?: string }> = []) { const listClaims = vi.fn((filter: { repoFullName?: string; status?: string }) => claims.filter((c) => (filter.repoFullName === undefined || c.repoFullName === filter.repoFullName) && (filter.status === undefined || c.status === filter.status)), ); @@ -64,6 +65,85 @@ describe("checkSubmissionFreshness (#3007)", () => { expect(fetchLiveIssueSnapshot).not.toHaveBeenCalled(); }); + describe("host-scoped claim match (#10004 -- the same owner/repo#issue can have one row per forge host)", () => { + const forgeInternal = "https://forge.internal"; + + it("REGRESSION: a released claim on another forge host does not abort this host's submission -- a released forge.internal row (lowest id) is ignored, and the active github.com row (no candidate apiBaseUrl) is matched instead", async () => { + const { claimLedger } = stubClaimLedger([ + { repoFullName: "acme/widgets", issueNumber: 42, status: "released", apiBaseUrl: forgeInternal }, + { repoFullName: "acme/widgets", issueNumber: 42, status: "active", apiBaseUrl: DEFAULT_FORGE_CONFIG.apiBaseUrl }, + ]); + 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(appendEvent).not.toHaveBeenCalled(); + }); + + it("claim-superseded abort: only an active row on a DIFFERENT host exists -- the default-host candidate does not false-pass on it", async () => { + const { claimLedger } = stubClaimLedger([{ repoFullName: "acme/widgets", issueNumber: 42, status: "active", apiBaseUrl: forgeInternal }]); + 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: false, reason: "claim_superseded" }); + expect(fetchLiveIssueSnapshot).not.toHaveBeenCalled(); + expect(appendEvent).toHaveBeenCalledWith({ + type: SUBMISSION_FRESHNESS_ABORT_EVENT, + repoFullName: "acme/widgets", + payload: { issueNumber: 42, reason: "claim_superseded" }, + }); + }); + + it("a candidate with an explicit apiBaseUrl matches the row recorded under that same host", async () => { + const { claimLedger } = stubClaimLedger([{ repoFullName: "acme/widgets", issueNumber: 42, status: "active", apiBaseUrl: forgeInternal }]); + const { eventLedger } = stubEventLedger(); + const fetchLiveIssueSnapshot = vi.fn(async () => ({ state: "open" as const, referencingPrs: [] })); + + const result = await checkSubmissionFreshness( + { repoFullName: "acme/widgets", issueNumber: 42, minerLogin: "miner-bot", apiBaseUrl: forgeInternal }, + { claimLedger, fetchLiveIssueSnapshot, eventLedger }, + ); + + expect(result).toEqual({ fresh: true }); + }); + + it("a candidate on the default host is unaffected by an unrelated row whose apiBaseUrl is blank (normalizes to the same default, not a distinct host)", async () => { + const { claimLedger } = stubClaimLedger([{ repoFullName: "acme/widgets", issueNumber: 42, status: "active", apiBaseUrl: "" }]); + const { eventLedger } = 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 }); + }); + + it("a candidate with a blank apiBaseUrl normalizes to the default host, matching a row with no apiBaseUrl at all", async () => { + const { claimLedger } = stubClaimLedger([{ repoFullName: "acme/widgets", issueNumber: 42, status: "active" }]); + const { eventLedger } = stubEventLedger(); + const fetchLiveIssueSnapshot = vi.fn(async () => ({ state: "open" as const, referencingPrs: [] })); + + const result = await checkSubmissionFreshness( + { repoFullName: "acme/widgets", issueNumber: 42, minerLogin: "miner-bot", apiBaseUrl: " " }, + { claimLedger, fetchLiveIssueSnapshot, eventLedger }, + ); + + expect(result).toEqual({ fresh: true }); + }); + }); + it("issue-closed abort", async () => { const { claimLedger } = stubClaimLedger([activeClaim]); const { eventLedger, appendEvent } = stubEventLedger();