From 671b20922e9bc69ba34c48cae5ac135717de1362 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 25 Jun 2026 07:07:54 -0700 Subject: [PATCH] fix(queue): render the unified comment from the LIVE merge-state, matching the disposition (#4220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The public review comment built mergeReadiness from the STORED pr.mergeableState while the auto-close disposition (planAgentMaintenanceActions) reads the LIVE merge-state. GitHub recomputes mergeable_state asynchronously, so the stored value lags: a base-conflicting PR could read `clean` in the comment ("✅ safe to merge") while the disposition reads the live `dirty` and AUTO-CLOSES it — the exact #4220 contradiction (comment says merge-safe, action closes the PR). Fetch the live merge-state in the comment path (the same fetchLivePullRequestMergeState + token the disposition uses) and feed it to deriveUnifiedStatus, fail-safe to the stored value. Now `dirty` → "blocked" and `behind` → "held" in the comment, matching the action. --- src/queue/processors.ts | 8 +++++++- test/unit/queue.test.ts | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 431dd8982a..4f3bc196d4 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -2814,6 +2814,12 @@ async function maybePublishPrPublicSurface( // RC2: only branch-protection-required checks gate the PR; a red codecov/* is surfaced but never blocks. const requiredContexts = await fetchRequiredStatusContexts(env, repoFullName, pr.baseRef ?? repo?.defaultBranch, ciToken ?? env.GITHUB_PUBLIC_TOKEN); const liveCi = await fetchLiveCiAggregate(env, repoFullName, pr.headSha, ciToken ?? env.GITHUB_PUBLIC_TOKEN, requiredContexts); + // Live merge-state too — the SAME source the disposition uses (planAgentMaintenanceActions reads liveMergeState). + // The stored pr.mergeableState lags GitHub's async recompute, so a base-conflicting PR could read `clean` here + // ("✅ safe to merge") while the disposition reads the live `dirty` and auto-CLOSES it — the exact #4220 + // contradiction. (#review-audit / #ready-needs-mergeable) + const liveMergeState = await fetchLivePullRequestMergeState(env, repoFullName, pr.number, ciToken ?? env.GITHUB_PUBLIC_TOKEN).catch(() => undefined); + const mergeStateLabel = liveMergeState ?? pr.mergeableState; // fail-safe to the stored value const ciState: MergeReadiness["ciState"] = liveCi.ciState === "passed" ? "passed" : liveCi.ciState === "failed" ? "failed" : "unverified"; // Per-failed-check WHY (codecov %/test/lint reason) from each check-run output or commit-status // description — capped + public-safe (name + short reason only). The renderer lists these under the CI chip. @@ -2824,7 +2830,7 @@ async function maybePublishPrPublicSurface( })); const mergeReadiness: MergeReadiness = { ciState, - ...(pr.mergeableState ? { mergeStateLabel: pr.mergeableState } : {}), + ...(mergeStateLabel ? { mergeStateLabel } : {}), ...(failingDetails.length > 0 ? { failingChecks: failingDetails.map((detail) => detail.name) } : {}), ...(failingDetails.length > 0 ? { failingDetails } : {}), }; diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index d74cb08f38..6514b32be2 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -3579,6 +3579,9 @@ describe("queue processors", () => { if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); // PR files — the unified branch (re)fetches them to count changed files for the readiness chip. if (url.includes("/pulls/3/files")) return Response.json([{ filename: "src/cache.ts", additions: 5, deletions: 1, status: "modified" }]); + // #review-audit: the LIVE merge-state the comment now reads — the base just advanced with a conflict, so the + // live state is `dirty` even though the stored mergeableState (unset on this payload) would not say so. + if (/\/pulls\/3(?:\?|$)/.test(url)) return Response.json({ number: 3, mergeable_state: "dirty" }); // Gate check-run — must succeed so `gateEvaluation` is produced and the flag-ON branch runs. // The pending check is POSTed (in_progress), then PATCHed to its completed conclusion. if (url.includes("/check-runs") && method === "GET") return Response.json({ total_count: 0, check_runs: [] }); @@ -3634,6 +3637,9 @@ describe("queue processors", () => { expect(postedBody).toContain("**Code review**"); // Public-safe by construction — no internal trust/economics fields leak through the unified renderer. expect(postedBody).not.toMatch(/wallet|hotkey|reward|trust score/i); + // #review-audit (#4220): the comment reads the LIVE `dirty` merge-state (not the stale stored one), so it must + // NOT headline "safe to merge" while the disposition would auto-close the base-conflicting PR. + expect(postedBody).not.toMatch(/safe to merge/i); }); // FIX B + FIX D3 at the processor call site: a unified comment for a PR whose CI has a FAILED check, with the