diff --git a/src/queue/processors.ts b/src/queue/processors.ts index b03350e751..4ddd253196 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -2826,6 +2826,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. @@ -2836,7 +2842,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