From 2786755b948ffd7c1f1b0854e69605e9c71ffdf0 Mon Sep 17 00:00:00 2001 From: Roomote Date: Sun, 18 Oct 2026 04:38:09 +0000 Subject: [PATCH 1/2] [Fix] Review-state labels swap on unchanged PRs while GitHub computes mergeability (#2089) --- .github/workflows/label-pr-review-state.yml | 103 ++++++++++------ .../pr-review-state-workflow.test.ts | 115 +++++++++++++----- 2 files changed, 151 insertions(+), 67 deletions(-) diff --git a/.github/workflows/label-pr-review-state.yml b/.github/workflows/label-pr-review-state.yml index 8a86c18207..abd57766fc 100644 --- a/.github/workflows/label-pr-review-state.yml +++ b/.github/workflows/label-pr-review-state.yml @@ -364,15 +364,15 @@ jobs: } } - // Strips stateLabels from a PR, optionally keeping one desired label - // plus any labels in keepLabels (e.g. has-conflicts while mergeability - // is unknown — kept, never added, per the hoisted check below). + // Strips stateLabels from a PR, optionally keeping one desired label. + // Callers skip this entirely while mergeability is unknown (see the + // hoisted check below), so no keep-list is needed. // Stale-clock labels are owned by the stale step below and are never // touched here: stripping one on a transient CI or mergeability flap // would erase a live warning and restart its close grace (issue #2076). // Only skipped when this run's own token is read-only (see isReadOnlyRun) — // schedule/workflow_dispatch runs reconcile fork PRs normally. - async function reconcileLabels(pr, desiredLabel, keepLabels = []) { + async function reconcileLabels(pr, desiredLabel) { if (isReadOnlyRun && isForkPR(pr)) { core.info(`PR #${pr.number}: fork PR on a read-only run — skipping label mutation`); return; @@ -393,7 +393,7 @@ jobs: } } for (const label of stateLabels) { - if (label !== desiredLabel && !keepLabels.includes(label) && currentLabels.has(label)) { + if (label !== desiredLabel && currentLabels.has(label)) { try { await github.rest.issues.removeLabel({ owner, repo, issue_number: pr.number, name: label, @@ -737,8 +737,27 @@ jobs: // GET endpoint and are computed asynchronously by GitHub — this // shard's own pulls.get above provides them, but even that can // return `null`/"unknown" while the merge check is still running. - // Treat "unknown" as not-yet-computed rather than as conflicting. - const prDetail = pr; + // Treat "unknown" as not-yet-computed rather than as conflicting, + // and give GitHub a bounded chance to finish computing it: retry + // the read up to 3 times with a short wait between reads before + // applying the unknown handling below (issue #2089). The wait is + // env-overridable so the test suite can retry without real waits. + const mergeabilityRetryDelayMs = Number(process.env.MERGEABILITY_RETRY_DELAY_MS ?? 5000); + const sleep = (ms) => new Promise(resolve => setTimeout(resolve, ms)); + let prDetail = pr; + for ( + let attempt = 1; + attempt <= 3 && + (prDetail.mergeable === null || prDetail.mergeable_state === 'unknown'); + attempt++ + ) { + await sleep(mergeabilityRetryDelayMs); + core.info(`PR #${pr.number}: mergeability unknown — retrying read (${attempt}/3)`); + const { data: retryDetail } = await github.rest.pulls.get({ + owner, repo, pull_number: pr.number, + }); + prDetail = retryDetail; + } if (prDetail.mergeable === false && prDetail.mergeable_state === 'dirty') { core.info(`PR #${pr.number}: has merge conflicts — labeling has-conflicts`); @@ -751,24 +770,29 @@ jobs: } // Unknown mergeability is handled once here, ahead of every - // branch that reconciles labels (issue #2045). GitHub recomputes - // mergeability asynchronously after each push and transiently - // reports `mergeable: null` / `mergeable_state: "unknown"` while - // the background job runs, so unknown says nothing about - // conflicts: an existing has-conflicts label is kept through - // reconciliation — unknown is not evidence the conflicts went - // away — and it is never added, since unknown is not evidence - // of conflicts either. Known-dirty never reaches here (the - // conflict check above handles it first); known-clean keeps the - // strip. The awaiting-maintainer branch below still refetches - // mergeability before the maintainer handoff, because a push - // between this fetch and labeling can change the answer. - // A kept has-conflicts label can sit next to other state labels - // until mergeability is known. This is intended. - const mergeabilityUnknown = + // branch that reconciles labels (issues #2045 and #2089). If the + // reads above still report `mergeable: null` / + // `mergeable_state: "unknown"`, GitHub has not finished computing + // the answer, so it says nothing about conflicts and nothing about + // whose turn it is. This run then changes NO state label for the + // PR: every state-label reconcile below is skipped. Acting on + // unknown previously let the normal reconcile swap labels (for + // example has-conflicts to awaiting-author and back on the next + // run's `dirty` read), which flapped labels on unchanged PRs. + // Known-dirty never reaches here (the conflict check above + // handles it first); known-clean reconciles normally. The + // awaiting-maintainer branch below still refetches mergeability + // before the maintainer handoff, because a push between this + // fetch and labeling can change the answer; a known read there + // re-enables the final reconcile. + let mergeabilityUnknown = prDetail.mergeable === null || prDetail.mergeable_state === 'unknown'; - const keepHasConflicts = mergeabilityUnknown && - pr.labels.some(label => label.name === 'has-conflicts'); + if (mergeabilityUnknown) { + core.info( + `PR #${pr.number}: mergeability still unknown after retries — ` + + 'leaving state labels unchanged this run' + ); + } // Check CI status for required checks on the PR's head commit only. // Scoping to required checks avoids advisory checks (e.g. codecov/patch) @@ -866,20 +890,25 @@ jobs: // While CI is running or has failed, remove state labels and move on. // CI failure is its own signal; the label would add noise, not clarity. - // Exception (issue #2025): while GitHub is still computing - // mergeability, a missing required check can simply mean a - // conflicting PR got no merge-ref CI run — not that its - // conflicts went away. keepHasConflicts (computed once above) - // keeps an existing has-conflicts label through this branch - // instead of stripping it with the other state labels. + // Exception (issues #2025 and #2089): while GitHub is still + // computing mergeability, a missing required check can simply + // mean a conflicting PR got no merge-ref CI run — not that its + // conflicts went away. Unknown mergeability changes no state + // label in this run, so this branch keeps every existing state + // label (has-conflicts included) instead of stripping them; a + // later run with a known read reconciles them. if (ciPending || ciFailed) { core.info( - `PR #${pr.number}: CI ${ciPending ? 'pending' : 'failed'} — stripping state labels` + - (keepHasConflicts ? ' (keeping has-conflicts while mergeability is unknown)' : '') + `PR #${pr.number}: CI ${ciPending ? 'pending' : 'failed'} — ` + + (mergeabilityUnknown + ? 'mergeability unknown; leaving state labels unchanged' + : 'stripping state labels') ); await updateReviewGate(pr, ciPending ? 'ci-pending' : 'ci-failed', false); await setCodeRabbitReviewActive(pr, false); - await reconcileLabels(pr, keepHasConflicts ? 'has-conflicts' : null); + if (!mergeabilityUnknown) { + await reconcileLabels(pr, null); + } await reconcileCommunityApproval(pr, 'remove'); await updateReviewGuide( pr, @@ -1132,6 +1161,10 @@ jobs: ); continue; } + // The refetch above settled mergeability with a fresh known read, + // so the final state-label reconcile below may run even when the + // shard's first reads were unknown (issue #2089). + mergeabilityUnknown = false; communityDecision = headChanged ? 'remove' : communityApprovalFresh @@ -1166,7 +1199,9 @@ jobs: if (recyclingActiveLabel) { await updateReviewGuide(pr, phase, existingGuide); } - await reconcileLabels(pr, desiredLabel, keepHasConflicts ? ['has-conflicts'] : []); + if (!mergeabilityUnknown) { + await reconcileLabels(pr, desiredLabel); + } await reconcileCommunityApproval(pr, communityDecision); if (readyForMaintainer) { await updateReviewGate(pr, phase, true); diff --git a/src/services/__tests__/pr-review-state-workflow.test.ts b/src/services/__tests__/pr-review-state-workflow.test.ts index e258ce64ff..56910b8381 100644 --- a/src/services/__tests__/pr-review-state-workflow.test.ts +++ b/src/services/__tests__/pr-review-state-workflow.test.ts @@ -27,6 +27,7 @@ interface HarnessOptions { mergeable?: boolean | null mergeableState?: string mergeabilitySequence?: Array<{ mergeable: boolean | null; mergeableState: string }> + mergeabilityRetryDelayMs?: number headShaSequence?: string[] fork?: boolean eventName?: string @@ -490,15 +491,23 @@ async function runWorkflow(options: HarnessOptions = {}) { // The reconcile job runs one matrix shard per PR and receives the target // through the job's PR_NUMBER env mapping, plus the job's concurrency group // through CONCURRENCY_GROUP. - const previousEnv = { prNumber: process.env.PR_NUMBER, group: process.env.CONCURRENCY_GROUP } + const previousEnv = { + prNumber: process.env.PR_NUMBER, + group: process.env.CONCURRENCY_GROUP, + mergeabilityRetryDelayMs: process.env.MERGEABILITY_RETRY_DELAY_MS, + } process.env.PR_NUMBER = String(pr.number) process.env.CONCURRENCY_GROUP = `label-pr-review-state-reconcile-${pr.number}` + // The workflow waits 5s between mergeability retries in production; tests + // override the delay so the retries run without real waits. + process.env.MERGEABILITY_RETRY_DELAY_MS = String(options.mergeabilityRetryDelayMs ?? 0) try { await new AsyncFunction("github", "context", "core", workflowScript)(github, context, core) } finally { for (const [key, value] of [ ["PR_NUMBER", previousEnv.prNumber], ["CONCURRENCY_GROUP", previousEnv.group], + ["MERGEABILITY_RETRY_DELAY_MS", previousEnv.mergeabilityRetryDelayMs], ] as const) { if (value === undefined) { delete process.env[key] @@ -1011,10 +1020,11 @@ describe("PR review-state workflow", () => { expect(result.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["coderabbit-review-active"] })) }) - it("keeps has-conflicts while mergeability is unknown and required CI is missing (#2025)", async () => { + it("changes no state label while mergeability is unknown and required CI is missing (#2025, #2089)", async () => { // A conflicting PR gets no merge-ref CI run, so its required checks are - // missing. Unknown mergeability must not read that as a reason to strip - // an existing has-conflicts label; the other state labels still clear. + // missing. Unknown mergeability must not read that as a reason to change + // any state label: has-conflicts stays, and so does every other state + // label until a later run reads a known mergeability value. const result = await runWorkflow({ eventName: "push", mergeable: null, @@ -1023,13 +1033,12 @@ describe("PR review-state workflow", () => { omitRequiredRuns: true, }) - expect(result.removeLabel).not.toHaveBeenCalledWith(expect.objectContaining({ name: "has-conflicts" })) + expect(result.removeLabel).not.toHaveBeenCalled() expect(result.addLabels).not.toHaveBeenCalledWith(expect.objectContaining({ labels: ["has-conflicts"] })) - expect(result.removeLabel).toHaveBeenCalledWith(expect.objectContaining({ name: "awaiting-author" })) expect(latestGateStatus(result)?.description).toContain("required CI checks") }) - it("keeps has-conflicts while mergeability is unknown and required CI has failed (#2025)", async () => { + it("changes no state label while mergeability is unknown and required CI has failed (#2025, #2089)", async () => { const result = await runWorkflow({ mergeable: null, mergeableState: "unknown", @@ -1037,13 +1046,12 @@ describe("PR review-state workflow", () => { requiredConclusion: "failure", }) - expect(result.removeLabel).not.toHaveBeenCalledWith(expect.objectContaining({ name: "has-conflicts" })) + expect(result.removeLabel).not.toHaveBeenCalled() expect(result.addLabels).not.toHaveBeenCalledWith(expect.objectContaining({ labels: ["has-conflicts"] })) - expect(result.removeLabel).toHaveBeenCalledWith(expect.objectContaining({ name: "awaiting-author" })) expect(latestGateStatus(result)?.description).toContain("failing required CI checks") }) - it("does not add has-conflicts while mergeability is unknown and required CI is missing (#2025)", async () => { + it("adds no state label while mergeability is unknown and required CI is missing (#2025, #2089)", async () => { const result = await runWorkflow({ mergeable: null, mergeableState: "unknown", @@ -1052,7 +1060,7 @@ describe("PR review-state workflow", () => { }) expect(result.addLabels).not.toHaveBeenCalledWith(expect.objectContaining({ labels: ["has-conflicts"] })) - expect(result.removeLabel).toHaveBeenCalledWith(expect.objectContaining({ name: "awaiting-author" })) + expect(result.removeLabel).not.toHaveBeenCalledWith(expect.objectContaining({ name: "awaiting-author" })) expect(latestGateStatus(result)?.description).toContain("required CI checks") }) @@ -1260,8 +1268,10 @@ describe("PR review-state workflow", () => { const result = await runWorkflow({ eventName: "push", labels: ["awaiting-maintainer"], + // The first read is known-clean, so no retry runs; the maintainer-handoff + // recheck then catches the fresh dirty read before tagging. mergeabilitySequence: [ - { mergeable: null, mergeableState: "unknown" }, + { mergeable: true, mergeableState: "clean" }, { mergeable: false, mergeableState: "dirty" }, ], reviews: [ @@ -1278,14 +1288,54 @@ describe("PR review-state workflow", () => { expect(result.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["has-conflicts"] })) }) - it("preserves the current state while mergeability is unknown", async () => { + // Issue #2089: the first read reports unknown, one retry reads dirty, and the + // run must not change any state label because of the unknown read itself. + it("retries an unknown mergeability read and handles the dirty retry normally (#2089)", async () => { const result = await runWorkflow({ eventName: "push", - labels: ["awaiting-maintainer", "coderabbit-review-active"], + labels: ["awaiting-maintainer"], mergeabilitySequence: [ { mergeable: null, mergeableState: "unknown" }, - { mergeable: null, mergeableState: "unknown" }, + { mergeable: false, mergeableState: "dirty" }, ], + }) + + expect(result.getPullRequest).toHaveBeenCalledTimes(2) + expect(result.removeLabel).toHaveBeenCalledWith(expect.objectContaining({ name: "awaiting-maintainer" })) + expect(result.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["has-conflicts"] })) + }) + + // Issue #2089: unknown on the first read and on every retry means the run + // changes no state label at all. + it("changes no state label when mergeability stays unknown across all retries (#2089)", async () => { + const result = await runWorkflow({ + eventName: "push", + labels: ["has-conflicts", "awaiting-author"], + mergeable: null, + mergeableState: "unknown", + }) + + expect(result.getPullRequest).toHaveBeenCalledTimes(4) + expect(result.removeLabel).not.toHaveBeenCalled() + for (const label of [ + "awaiting-author", + "awaiting-coderabbit", + "awaiting-ready", + "awaiting-maintainer", + "has-conflicts", + ]) { + expect(result.addLabels).not.toHaveBeenCalledWith(expect.objectContaining({ labels: [label] })) + } + }) + + it("preserves the current state while mergeability is unknown", async () => { + const result = await runWorkflow({ + eventName: "push", + labels: ["awaiting-maintainer", "coderabbit-review-active"], + // Every read — the shard fetch, each retry, and the maintainer-handoff + // recheck — reports unknown. + mergeable: null, + mergeableState: "unknown", reviews: [ { login: "coderabbitai[bot]", @@ -1307,10 +1357,10 @@ describe("PR review-state workflow", () => { const result = await runWorkflow({ eventName: "push", labels: ["awaiting-maintainer", "coderabbit-review-active"], - mergeabilitySequence: [ - { mergeable: null, mergeableState: "unknown" }, - { mergeable: null, mergeableState: "unknown" }, - ], + // Every read reports unknown, so the run reaches the maintainer-handoff + // recheck and tries to publish the mergeability-pending metadata. + mergeable: null, + mergeableState: "unknown", removeLabelStatus: 500, reviews: [ { @@ -2973,10 +3023,9 @@ describe("PR review-state workflow", () => { const result = await runWorkflow({ eventName: "push", labels: ["awaiting-maintainer", "community-approved"], - mergeabilitySequence: [ - { mergeable: null, mergeableState: "unknown" }, - { mergeable: null, mergeableState: "unknown" }, - ], + // Every read reports unknown. + mergeable: null, + mergeableState: "unknown", reviews: [coderabbitApproval, communityApproval], }) @@ -2994,11 +3043,12 @@ describe("PR review-state workflow", () => { const result = await runWorkflow({ eventName: "push", labels: ["community-approved"], - mergeabilitySequence: [ - { mergeable: null, mergeableState: "unknown" }, - { mergeable: null, mergeableState: "unknown" }, - ], - headShaSequence: [SHA, NEW_SHA], + // Every read reports unknown; the head moves to a new SHA on the first + // retry and stays there, so the maintainer-handoff recheck still sees + // the new head. + mergeable: null, + mergeableState: "unknown", + headShaSequence: [SHA, NEW_SHA, NEW_SHA, NEW_SHA, NEW_SHA], reviews: [coderabbitApproval, communityApproval], }) @@ -3110,7 +3160,7 @@ describe("PR review-state workflow", () => { eventName: "push", labels: ["awaiting-maintainer", "community-approved"], mergeabilitySequence: [ - { mergeable: null, mergeableState: "unknown" }, + { mergeable: true, mergeableState: "clean" }, { mergeable: false, mergeableState: "dirty" }, ], }, @@ -3167,10 +3217,9 @@ describe("PR review-state workflow", () => { const result = await runWorkflow({ eventName: "push", labels: ["awaiting-maintainer", "community-approved"], - mergeabilitySequence: [ - { mergeable: null, mergeableState: "unknown" }, - { mergeable: null, mergeableState: "unknown" }, - ], + // Every read reports unknown. + mergeable: null, + mergeableState: "unknown", reviews: [ coderabbitApproval, { ...communityApproval, id: 1 }, From b5b572c21fca75a8b47f177030b4a7e19f60ce88 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Tue, 20 Oct 2026 00:04:03 +0000 Subject: [PATCH 2/2] fix(ci): preserve PR labels on mergeability retry failure --- .github/workflows/label-pr-review-state.yml | 20 ++- .../pr-review-state-workflow.test.ts | 121 +++++++++++++++++- 2 files changed, 131 insertions(+), 10 deletions(-) diff --git a/.github/workflows/label-pr-review-state.yml b/.github/workflows/label-pr-review-state.yml index abd57766fc..de57d915ea 100644 --- a/.github/workflows/label-pr-review-state.yml +++ b/.github/workflows/label-pr-review-state.yml @@ -753,10 +753,22 @@ jobs: ) { await sleep(mergeabilityRetryDelayMs); core.info(`PR #${pr.number}: mergeability unknown — retrying read (${attempt}/3)`); - const { data: retryDetail } = await github.rest.pulls.get({ - owner, repo, pull_number: pr.number, - }); - prDetail = retryDetail; + try { + const { data: retryDetail } = await github.rest.pulls.get({ + owner, repo, pull_number: pr.number, + }); + prDetail = retryDetail; + } catch (error) { + // The loop condition guarantees mergeability was still + // unknown when this retry started. A failed retry (for + // example HTTP 503) adds no new mergeability answer, so + // the outer cleanup must keep the current state labels + // exactly like the still-unknown path below (issue + // #2089). Gate invalidation and failure reporting still + // happen in the outer catch. + error.preserveStateLabels = true; + throw error; + } } if (prDetail.mergeable === false && prDetail.mergeable_state === 'dirty') { diff --git a/src/services/__tests__/pr-review-state-workflow.test.ts b/src/services/__tests__/pr-review-state-workflow.test.ts index 56910b8381..eaab566b23 100644 --- a/src/services/__tests__/pr-review-state-workflow.test.ts +++ b/src/services/__tests__/pr-review-state-workflow.test.ts @@ -28,6 +28,11 @@ interface HarnessOptions { mergeableState?: string mergeabilitySequence?: Array<{ mergeable: boolean | null; mergeableState: string }> mergeabilityRetryDelayMs?: number + // The timer regression test omits MERGEABILITY_RETRY_DELAY_MS so the + // script takes its production 5000 ms default instead of a test override. + productionMergeabilityDelay?: boolean + // Fails every pulls.get read from this 0-based call onward with the status. + pullsGetError?: { fromCall: number; status: number } headShaSequence?: string[] fork?: boolean eventName?: string @@ -200,8 +205,15 @@ function buildEventPayload(eventName: string, options: HarnessOptions) { } } -/** Executes the embedded github-script workflow against deterministic GitHub API doubles. */ -async function runWorkflow(options: HarnessOptions = {}) { +/** Runs pending microtask turns so a started script reaches its next await suspension point. */ +async function flushMicrotasks(turns = 50) { + for (let index = 0; index < turns; index++) { + await Promise.resolve() + } +} + +/** Builds the reconcile job's GitHub API doubles without starting the embedded script. */ +function buildReconcileHarness(options: HarnessOptions = {}) { const eventName = options.eventName ?? "pull_request_target" const pr = buildHarnessPr(options) const requiredContexts = options.requiredContexts ?? ["tests"] @@ -376,6 +388,9 @@ async function runWorkflow(options: HarnessOptions = {}) { let pullRequestGetIndex = 0 const getPullRequest = vi.fn(async () => { const index = pullRequestGetIndex++ + if (options.pullsGetError && index >= options.pullsGetError.fromCall) { + throw Object.assign(new Error("Pull request read failed"), { status: options.pullsGetError.status }) + } const mergeability = options.mergeabilitySequence?.[index] const headSha = options.headShaSequence?.[index] let data = pr @@ -500,10 +515,14 @@ async function runWorkflow(options: HarnessOptions = {}) { process.env.CONCURRENCY_GROUP = `label-pr-review-state-reconcile-${pr.number}` // The workflow waits 5s between mergeability retries in production; tests // override the delay so the retries run without real waits. - process.env.MERGEABILITY_RETRY_DELAY_MS = String(options.mergeabilityRetryDelayMs ?? 0) - try { - await new AsyncFunction("github", "context", "core", workflowScript)(github, context, core) - } finally { + if (options.productionMergeabilityDelay) { + delete process.env.MERGEABILITY_RETRY_DELAY_MS + } else { + process.env.MERGEABILITY_RETRY_DELAY_MS = String(options.mergeabilityRetryDelayMs ?? 0) + } + + const execute = () => new AsyncFunction("github", "context", "core", workflowScript)(github, context, core) + const restoreEnv = () => { for (const [key, value] of [ ["PR_NUMBER", previousEnv.prNumber], ["CONCURRENCY_GROUP", previousEnv.group], @@ -518,6 +537,8 @@ async function runWorkflow(options: HarnessOptions = {}) { } return { + execute, + restoreEnv, addLabels, removeLabel, createComment, @@ -537,6 +558,17 @@ async function runWorkflow(options: HarnessOptions = {}) { } } +/** Executes the reconcile job's embedded script against deterministic GitHub API doubles. */ +async function runWorkflow(options: HarnessOptions = {}) { + const { execute, restoreEnv, ...result } = buildReconcileHarness(options) + try { + await execute() + } finally { + restoreEnv() + } + return result +} + /** Executes the resolve job's embedded script against deterministic GitHub API doubles. */ async function runResolve(options: HarnessOptions = {}) { const eventName = options.eventName ?? "pull_request_target" @@ -1328,6 +1360,83 @@ describe("PR review-state workflow", () => { } }) + // Issue #2091: a failed mergeability retry adds no mergeability answer, so + // the run keeps every state label exactly like the still-unknown path, + // while gate invalidation and failure reporting still happen. + it("keeps state labels when the mergeability retry read fails (#2091)", async () => { + const result = await runWorkflow({ + eventName: "push", + labels: ["awaiting-author", "has-conflicts", "community-approved"], + mergeabilitySequence: [{ mergeable: null, mergeableState: "unknown" }], + pullsGetError: { fromCall: 1, status: 503 }, + }) + + // The shard read plus one failed retry; the 503 stops further retries. + expect(result.getPullRequest).toHaveBeenCalledTimes(2) + + // No label mutation at all: exclusive state labels and the advisory + // community signal survive the failed retry. + expect(result.removeLabel).not.toHaveBeenCalled() + expect(result.addLabels).not.toHaveBeenCalled() + + // Gate invalidation is retained. + expect(result.createCommitStatus).toHaveBeenCalledTimes(1) + expect(result.createCommitStatus.mock.calls[0][0]).toMatchObject({ + state: "pending", + context: "Zoo Code / PR review gate", + }) + + // Failure reporting is retained. + expect(result.setFailed).toHaveBeenCalledTimes(1) + expect(result.setFailed.mock.calls[0][0]).toMatch(/#1437/) + expect(result.setFailed.mock.calls[0][0]).toMatch(/503/) + }) + + // Issue #2091: the retry read must stay gated behind the full production + // wait. A controlled fake timer holds the sleep pending, so the test can + // prove no retry request leaves before 5000 ms elapse. Removing the await + // on the sleep alone fails here: the retry read runs while the timer is + // still pending, and deleting the sleep leaves no timer at all. + it("waits the full five second default before retrying an unknown mergeability read (#2091)", async () => { + const harness = buildReconcileHarness({ + eventName: "push", + labels: ["awaiting-author"], + mergeabilitySequence: [ + { mergeable: null, mergeableState: "unknown" }, + { mergeable: true, mergeableState: "clean" }, + ], + productionMergeabilityDelay: true, + }) + vi.useFakeTimers() + try { + const runPromise = harness.execute() + await flushMicrotasks() + + // The script is suspended on its production wait: exactly one pending + // timer and no retry read yet. + expect(vi.getTimerCount()).toBe(1) + expect(harness.getPullRequest).toHaveBeenCalledTimes(1) + + // A shorter default would already have fired the timer and issued the + // retry read before the full interval. + await vi.advanceTimersByTimeAsync(4999) + expect(harness.getPullRequest).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(1) + expect(vi.getTimerCount()).toBe(0) + await runPromise + + // The wait gated the retry: shard read plus one clean retry read, and + // the reconcile continued normally. + expect(harness.getPullRequest).toHaveBeenCalledTimes(2) + expect(harness.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["awaiting-coderabbit"] })) + expect(harness.setFailed).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + harness.restoreEnv() + } + }) + it("preserves the current state while mergeability is unknown", async () => { const result = await runWorkflow({ eventName: "push",