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
49 changes: 48 additions & 1 deletion packages/loopover-miner/lib/attempt-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import type { GovernorLedger } from "./governor-ledger.js";
import { openWorktreeAllocator } from "./worktree-allocator.js";
import type { WorktreeAllocation, WorktreeAllocator } from "./worktree-allocator.js";
import { isValidRepoSegment } from "./repo-clone.js";
import { REJECTION_REASON_AI_USAGE_POLICY_BAN, REJECTION_REASON_OWN_SUBMISSION_REJECTED, resolveRejectionSignaled } from "./rejection-signal.js";
import { REJECTION_REASON_AI_USAGE_POLICY_BAN, REJECTION_REASON_OWN_SUBMISSION_REJECTED, resolveOwnOpenPrForIssue, resolveRejectionSignaled } from "./rejection-signal.js";
import type { resolveRejectionSignaled as ResolveRejectionSignaledFn } from "./rejection-signal.js";
import { cleanupAttemptWorktree, prepareAttemptWorktree } from "./attempt-worktree.js";
import type {
Expand Down Expand Up @@ -141,6 +141,8 @@ export type RunAttemptOptions = {
initSignalTrackingStore?: () => SignalStore;
buildAttemptDeps?: typeof buildAttemptDeps;
resolveRejectionSignaled?: typeof ResolveRejectionSignaledFn;
// #8808: injection seam for the own-open-PR idempotency guard, mirroring resolveRejectionSignaled above.
resolveOwnOpenPrForIssue?: typeof resolveOwnOpenPrForIssue;
fetchImpl?: SelfReviewContextFetch;
prepareAttemptWorktree?: typeof PrepareAttemptWorktreeFn;
cleanupAttemptWorktree?: typeof CleanupAttemptWorktreeFn;
Expand Down Expand Up @@ -450,6 +452,51 @@ export async function runAttempt(args: string[], options: RunAttemptOptions = {}
return 5;
}

// #8808: idempotency against this miner's OWN already-open PR for this exact issue — the crash-retry
// double-open guard. Checked before a worktree slot is consumed, mirroring the rejection-signal block
// above. Fail-open by construction (resolveOwnOpenPrForIssue returns null on any read/fetch failure), so
// a hiccup never blocks a legitimate attempt; refusing (not adopting) is the safe disposition — the
// still-open PR is already doing this issue's job.
const ownOpenPr = await (options.resolveOwnOpenPrForIssue ?? resolveOwnOpenPrForIssue)(parsed.repoFullName, parsed.issueNumber, {
fetchImpl: options.fetchImpl,
} as Parameters<typeof resolveOwnOpenPrForIssue>[2]);
if (ownOpenPr !== null) {
const reason = "own_open_pr_for_issue";
attemptLog.appendAttemptLogEvent({
eventType: "attempt_aborted",
attemptId,
actionClass: "open_pr",
mode,
reason,
payload: { repoFullName: parsed.repoFullName, issueNumber: parsed.issueNumber, existingPullRequestNumber: ownOpenPr },
});
eventLedger.appendEvent({
type: "attempt_blocked",
repoFullName: parsed.repoFullName,
payload: { issueNumber: parsed.issueNumber, reason, existingPullRequestNumber: ownOpenPr },
});
const duplicateResult = {
outcome: "blocked_own_open_pr",
reason,
repoFullName: parsed.repoFullName,
issueNumber: parsed.issueNumber,
existingPullRequestNumber: ownOpenPr,
minerLogin: parsed.minerLogin,
base: parsed.base,
mode,
attemptId,
};
if (parsed.json) {
console.log(JSON.stringify(duplicateResult, null, 2));
} else {
console.error(
`Attempt for ${parsed.repoFullName}#${parsed.issueNumber} is blocked: this miner already has open PR #${ownOpenPr} for this issue (a crash-retry duplicate would double-open). Close it or wait for its outcome first.`,
);
}
options.onResult?.(duplicateResult as AttemptCliResult);
return 5;
}

allocation = allocator.acquire(attemptId, parsed.repoFullName);

// #7858: only when the operator has configured Neon (dbForkConfig !== null) -- otherwise a complete
Expand Down
67 changes: 61 additions & 6 deletions packages/loopover-miner/lib/rejection-signal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export type RejectionSignalFetch = (

type RejectionSignalResponse = Awaited<ReturnType<RejectionSignalFetch>>;

type OwnRejectionHistorySubmission = { pullRequestNumber?: number | null | undefined };
type OwnRejectionHistorySubmission = { pullRequestNumber?: number | null | undefined; issueNumber?: number | null | undefined };

type ListOwnSubmissions = (filter: { repoFullName?: string }) => OwnRejectionHistorySubmission[];

Expand Down Expand Up @@ -152,11 +152,13 @@ async function fetchPullRequestPayload(
* fetch/parse failure is skipped so it never blocks the others. Consumes both upstream modules without modifying
* either. Every dependency is injectable for testing.
*/
export async function resolveOwnRejectionHistory(repoFullName: string, options: OwnRejectionHistoryOptions = {}): Promise<boolean> {
const target = parseRepoFullName(repoFullName);
if (!target) return false;
const listSubmissions = options.listSubmissions ?? listRecentOwnSubmissions;
const resolved = {
function resolveHistoryOptions(options: OwnRejectionHistoryOptions): {
fetchImpl: RejectionSignalFetch;
githubToken: string;
githubApiBaseUrl: string;
maxChecks: number;
} {
return {
fetchImpl: options.fetchImpl ?? (fetch as unknown as RejectionSignalFetch),
githubToken: typeof options.githubToken === "string" ? options.githubToken.trim() : (process.env.GITHUB_TOKEN ?? ""),
githubApiBaseUrl:
Expand All @@ -166,6 +168,13 @@ export async function resolveOwnRejectionHistory(repoFullName: string, options:
? (options.maxRejectionHistoryChecks as number)
: DEFAULT_MAX_REJECTION_HISTORY_CHECKS,
};
}

export async function resolveOwnRejectionHistory(repoFullName: string, options: OwnRejectionHistoryOptions = {}): Promise<boolean> {
const target = parseRepoFullName(repoFullName);
if (!target) return false;
const listSubmissions = options.listSubmissions ?? listRecentOwnSubmissions;
const resolved = resolveHistoryOptions(options);

let submissions: OwnRejectionHistorySubmission[];
try {
Expand Down Expand Up @@ -193,6 +202,52 @@ export async function resolveOwnRejectionHistory(repoFullName: string, options:
return false;
}

/**
* #8808: does THIS miner already have an OPEN PR for THIS exact issue on THIS repo? The crash-retry
* double-open guard: claim-conflict resolution deliberately never treats the miner's own sibling PR as a
* competing claim, and freshness excludes same-author PRs -- so nothing downstream refuses a duplicate
* attempt. Reads the miner's own recorded submissions (issue-tagged since #8172-era rows), live-checks the
* most recent candidates' PR state (bounded by the same maxRejectionHistoryChecks fan-out cap), and returns
* the first still-OPEN PR number -- or null. FULLY FAIL-OPEN: any read/fetch failure returns null (an
* idempotency guard must never block a legitimate attempt on a hiccup; submission-freshness + the claim
* ledger remain the downstream backstops).
*/
export async function resolveOwnOpenPrForIssue(
repoFullName: string,
issueNumber: number,
options: OwnRejectionHistoryOptions = {},
): Promise<number | null> {
const target = parseRepoFullName(repoFullName);
if (!target || !Number.isInteger(issueNumber) || issueNumber <= 0) return null;
const listSubmissions = options.listSubmissions ?? listRecentOwnSubmissions;
const resolved = resolveHistoryOptions(options);

let submissions: OwnRejectionHistorySubmission[];
try {
submissions = listSubmissions({ repoFullName });
} catch {
return null; // wholesale read failure -- fail open, never block an attempt on it
}
const candidates = (Array.isArray(submissions) ? submissions : [])
.filter(
(submission) =>
submission &&
submission.issueNumber === issueNumber &&
Number.isInteger(submission.pullRequestNumber) &&
(submission.pullRequestNumber as number) > 0,
)
.slice(0, resolved.maxChecks);
for (const candidate of candidates) {
try {
const payload = (await fetchPullRequestPayload(target, candidate.pullRequestNumber as number, resolved)) as { state?: unknown } | null;
if (payload && payload.state === "open") return candidate.pullRequestNumber as number;
} catch {
// Individual fetch failure -- skip (fail open), keep checking the rest.
}
}
return null;
}

/**
* Resolve whether the target repo has signaled it does not want automated/AI-authored contributions --
* either trigger documented above. Returns `false` (never throws) on any fetch/parse failure for the policy
Expand Down
64 changes: 64 additions & 0 deletions test/unit/miner-attempt-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1571,6 +1571,70 @@ describe("runAttempt (#5132)", () => {
expect(error).toHaveBeenCalledWith(expect.stringContaining("AI-usage policy bans automated/AI-authored contributions"));
});

it("#8808: refuses a duplicate attempt when this miner already has an open PR for the exact issue (crash-retry double-open guard)", async () => {
const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers();
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
const appendAttemptLogEventSpy = vi.spyOn(attemptLog, "appendAttemptLogEvent");
const acquireSpy = vi.spyOn(allocator, "acquire");

const exitCode = await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], {
env: { MINER_CODING_AGENT_PROVIDER: "noop" },
openWorktreeAllocator: () => allocator,
openClaimLedger: () => claimLedger,
initEventLedger: () => eventLedger,
initAttemptLog: () => attemptLog,
initGovernorLedger: () => governorLedger,
resolveRejectionSignaled: async () => false,
resolveOwnOpenPrForIssue: async () => 42,
});

expect(exitCode).toBe(5);
expect(acquireSpy).not.toHaveBeenCalled(); // refused BEFORE consuming a worktree slot
expect(appendAttemptLogEventSpy).toHaveBeenCalledWith(
expect.objectContaining({ eventType: "attempt_aborted", reason: "own_open_pr_for_issue" }),
);
const payload = JSON.parse(String(log.mock.calls.at(-1)?.[0]));
expect(payload).toMatchObject({ outcome: "blocked_own_open_pr", reason: "own_open_pr_for_issue", existingPullRequestNumber: 42 });
});

it("#8808: the non-json refusal names the existing PR on stderr", async () => {
const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers();
const error = vi.spyOn(console, "error").mockImplementation(() => undefined);

const exitCode = await runAttempt(["acme/widgets", "7", "--miner-login", "alice"], {
env: { MINER_CODING_AGENT_PROVIDER: "noop" },
openWorktreeAllocator: () => allocator,
openClaimLedger: () => claimLedger,
initEventLedger: () => eventLedger,
initAttemptLog: () => attemptLog,
initGovernorLedger: () => governorLedger,
resolveRejectionSignaled: async () => false,
resolveOwnOpenPrForIssue: async () => 42,
});

expect(exitCode).toBe(5);
expect(error).toHaveBeenCalledWith(expect.stringContaining("already has open PR #42"));
});

it("#8808: a null resolution (no open PR / fail-open hiccup) proceeds to the worktree slot exactly as before", async () => {
const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers();
vi.spyOn(console, "log").mockImplementation(() => undefined);
const acquireSpy = vi.spyOn(allocator, "acquire");

await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], {
env: { MINER_CODING_AGENT_PROVIDER: "noop" },
openWorktreeAllocator: () => allocator,
openClaimLedger: () => claimLedger,
initEventLedger: () => eventLedger,
initAttemptLog: () => attemptLog,
initGovernorLedger: () => governorLedger,
resolveRejectionSignaled: async () => false,
resolveOwnOpenPrForIssue: async () => null,
});

expect(acquireSpy).toHaveBeenCalled(); // the guard let the attempt through
});

it("REGRESSION (#6055): labels own-rejection-history aborts as own_submission_rejected in --json output", async () => {
const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers();
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
Expand Down
62 changes: 62 additions & 0 deletions test/unit/miner-rejection-signal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ vi.mock("@loopover/engine", async () => {
import {
REJECTION_REASON_AI_USAGE_POLICY_BAN,
REJECTION_REASON_OWN_SUBMISSION_REJECTED,
resolveOwnOpenPrForIssue,
resolveOwnRejectionHistory,
resolveRejectionSignaled,
} from "../../packages/loopover-miner/lib/rejection-signal.js";
Expand Down Expand Up @@ -463,3 +464,64 @@ describe("resolveRejectionSignaled combines both triggers (#5655)", () => {
expect(result).toBe(false);
});
});

describe("resolveOwnOpenPrForIssue (#8808)", () => {
it("returns the still-open PR number when this miner already has one for the exact issue", async () => {
const fetchImpl = vi.fn(async (_url: string, _init?: unknown) => jsonResponse({ state: "open" }));
const result = await resolveOwnOpenPrForIssue("acme/widgets", 12, {
listSubmissions: () => [{ pullRequestNumber: 42, issueNumber: 12 }],
fetchImpl,
});
expect(result).toBe(42);
expect(String(fetchImpl.mock.calls[0]?.[0])).toContain("/repos/acme/widgets/pulls/42");
});

it("returns null (no fetch) when no recorded submission matches the issue — other issues' PRs never block", async () => {
const fetchImpl = vi.fn();
const result = await resolveOwnOpenPrForIssue("acme/widgets", 12, {
listSubmissions: () => [{ pullRequestNumber: 41, issueNumber: 11 }, { pullRequestNumber: null, issueNumber: 12 }, {}],
fetchImpl,
});
expect(result).toBeNull();
expect(fetchImpl).not.toHaveBeenCalled();
});

it("a CLOSED/merged prior PR for the issue does not block a fresh attempt", async () => {
const fetchImpl = vi.fn(async () => jsonResponse({ state: "closed" }));
const result = await resolveOwnOpenPrForIssue("acme/widgets", 12, {
listSubmissions: () => [{ pullRequestNumber: 42, issueNumber: 12 }],
fetchImpl,
});
expect(result).toBeNull();
});

it("FAIL-OPEN: a submissions read failure or a fetch rejection returns null — never blocks an attempt on a hiccup", async () => {
expect(
await resolveOwnOpenPrForIssue("acme/widgets", 12, {
listSubmissions: () => {
throw new Error("store down");
},
fetchImpl: vi.fn(),
}),
).toBeNull();
expect(
await resolveOwnOpenPrForIssue("acme/widgets", 12, {
listSubmissions: () => [{ pullRequestNumber: 42, issueNumber: 12 }],
fetchImpl: vi.fn(async () => {
throw new Error("network");
}),
}),
).toBeNull();
});

it("bounds the live checks to maxRejectionHistoryChecks and rejects degenerate inputs", async () => {
const fetchImpl = vi.fn(async () => jsonResponse({ state: "closed" }));
const submissions = Array.from({ length: 15 }, (_, i) => ({ pullRequestNumber: i + 1, issueNumber: 12 }));
expect(await resolveOwnOpenPrForIssue("acme/widgets", 12, { listSubmissions: () => submissions, fetchImpl, maxRejectionHistoryChecks: 3 })).toBeNull();
expect(fetchImpl).toHaveBeenCalledTimes(3);
expect(await resolveOwnOpenPrForIssue("not-a-repo", 12, { listSubmissions: () => submissions, fetchImpl })).toBeNull();
expect(await resolveOwnOpenPrForIssue("acme/widgets", 0, { listSubmissions: () => submissions, fetchImpl })).toBeNull();
// Defensive non-array return (mirrors the sibling resolver's own guard).
expect(await resolveOwnOpenPrForIssue("acme/widgets", 12, { listSubmissions: (() => null) as never, fetchImpl })).toBeNull();
});
});