diff --git a/README.md b/README.md index e920bb1..232c218 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,9 @@ It ignores closed pull requests, drafts, and pull requests without an automerge latest commit status is `pending`. Once all reported work is terminal, it requests a SHA-pinned merge. A failed optional check does not prevent the request, but GitHub rejects it when any required condition is unsatisfied. +The App also waits up to 10 seconds for GitHub to calculate whether the current head can merge cleanly with the latest base branch. This calculation +can be pending after another pull request merges. Pull requests with merge conflicts are left open. + Before merging, the App waits until the active automerge label has been present for 10 seconds, then checks the pull request again. Removing the label during this grace period cancels the merge. If GitHub has not exposed the matching label event yet, the grace period starts when the App first observes the label instead of waiting repeatedly for event history. diff --git a/src/automerge.ts b/src/automerge.ts index 2f7b4fc..d67cff5 100644 --- a/src/automerge.ts +++ b/src/automerge.ts @@ -2,6 +2,8 @@ import type { CheckRun, CommitStatus, IssueEvent, MergeMethod, PullRequest, Repo export const AUTOMERGE_LABELS = new Set(["automerge", "tag: automerge"]); export const AUTOMERGE_GRACE_PERIOD_MS = 10_000; +export const MERGEABILITY_POLL_INTERVAL_MS = 1_000; +export const MERGEABILITY_POLL_LIMIT = 10; export interface AutomergeGitHub { pullRequest(number: number): Promise; @@ -65,8 +67,27 @@ export async function evaluatePullRequest( let pullRequest = await github.pullRequest(number); const labelObservedAt = now(); if (pullRequest.state !== "open" || pullRequest.draft || !hasAutomergeLabel(pullRequest)) return false; + let mergeabilityPolls = 0; for (;;) { + if (pullRequest.mergeable === null) { + if (mergeabilityPolls >= MERGEABILITY_POLL_LIMIT) { + console.log(`GitHub did not calculate mergeability for ${repository}#${number}`); + return false; + } + mergeabilityPolls += 1; + console.log(`Waiting for GitHub to calculate mergeability for ${repository}#${number}`); + await waitFor(MERGEABILITY_POLL_INTERVAL_MS); + pullRequest = await github.pullRequest(number); + if (pullRequest.state !== "open" || pullRequest.draft || !hasAutomergeLabel(pullRequest)) return false; + continue; + } + if (!pullRequest.mergeable) { + console.log(`GitHub reports merge conflicts for ${repository}#${number}`); + return false; + } + mergeabilityPolls = 0; + const [checks, statuses] = await Promise.all([ github.checkRuns(pullRequest.head.sha), github.statuses(pullRequest.head.sha), diff --git a/src/github.ts b/src/github.ts index 73d4463..5622d3c 100644 --- a/src/github.ts +++ b/src/github.ts @@ -6,6 +6,7 @@ export interface PullRequest { number: number; state: "open" | "closed"; draft: boolean; + mergeable: boolean | null; labels: Array<{ name: string }>; head: { sha: string }; } diff --git a/tests/automerge.test.ts b/tests/automerge.test.ts index 57c216e..abfa34c 100644 --- a/tests/automerge.test.ts +++ b/tests/automerge.test.ts @@ -5,6 +5,8 @@ import { gracePeriodRemaining, hasAutomergeLabel, mergeMethod, + MERGEABILITY_POLL_INTERVAL_MS, + MERGEABILITY_POLL_LIMIT, type AutomergeGitHub, } from "../src/automerge"; import type { CheckRun, CommitStatus, IssueEvent, MergeMethod, PullRequest, RepositorySettings } from "../src/github"; @@ -14,6 +16,7 @@ class FakeGitHub implements AutomergeGitHub { number: 12, state: "open", draft: false, + mergeable: true, labels: [{ name: "automerge" }], head: { sha: "abc123" }, }; @@ -133,6 +136,50 @@ test("merges when the label event is not yet visible", async () => { expect(github.merges).toEqual([{ number: 12, sha: "abc123", method: "squash" }]); }); +test("waits for GitHub to calculate mergeability", async () => { + const github = new FakeGitHub(); + const waits: number[] = []; + let requests = 0; + github.pullRequest = async () => { + requests += 1; + return { ...github.pull, mergeable: requests >= 3 ? true : null }; + }; + + expect( + await evaluatePullRequest(github, 12, "owner/repository", { + wait: async (milliseconds) => { + waits.push(milliseconds); + }, + }), + ).toBe(true); + expect(waits).toEqual([MERGEABILITY_POLL_INTERVAL_MS, MERGEABILITY_POLL_INTERVAL_MS]); + expect(github.merges).toEqual([{ number: 12, sha: "abc123", method: "squash" }]); +}); + +test("stops polling when GitHub does not calculate mergeability", async () => { + const github = new FakeGitHub(); + github.pull.mergeable = null; + const waits: number[] = []; + + expect( + await evaluatePullRequest(github, 12, "owner/repository", { + wait: async (milliseconds) => { + waits.push(milliseconds); + }, + }), + ).toBe(false); + expect(waits).toEqual(Array(MERGEABILITY_POLL_LIMIT).fill(MERGEABILITY_POLL_INTERVAL_MS)); + expect(github.merges).toEqual([]); +}); + +test("does not attempt to merge a conflicting pull request", async () => { + const github = new FakeGitHub(); + github.pull.mergeable = false; + + expect(await evaluatePullRequest(github, 12, "owner/repository")).toBe(false); + expect(github.merges).toEqual([]); +}); + test("waits until check runs and commit statuses finish", async () => { const github = new FakeGitHub(); github.checks = [{ status: "completed" }, { status: "in_progress" }]; diff --git a/tests/events.test.ts b/tests/events.test.ts index 5e99152..7a83449 100644 --- a/tests/events.test.ts +++ b/tests/events.test.ts @@ -8,7 +8,7 @@ function payload(values: Partial): WebhookPayload { } function pullRequest(number: number, state: "open" | "closed" = "open"): PullRequest { - return { number, state, draft: false, labels: [], head: { sha: "abc123" } }; + return { number, state, draft: false, mergeable: true, labels: [], head: { sha: "abc123" } }; } describe("webhook event routing", () => {