diff --git a/apps/server/src/pullRequest/PullRequestProvider.ts b/apps/server/src/pullRequest/PullRequestProvider.ts index f1f8bdb8..d8bd9f0d 100644 --- a/apps/server/src/pullRequest/PullRequestProvider.ts +++ b/apps/server/src/pullRequest/PullRequestProvider.ts @@ -17,6 +17,7 @@ import { type PullRequestLabel, type PullRequestListState, type PullRequestMergeability, + type PullRequestMergeGate, type PullRequestMergeMethod, type PullRequestReactionContent, type PullRequestReviewCommentDraft, @@ -116,6 +117,8 @@ export interface ProviderChangeRequestDetail extends ProviderChangeRequest { readonly closedAt: string | null; readonly reviewers: ReadonlyArray; readonly checks: ReadonlyArray; + /** Absent where the host does not say whether its rules would take a merge. */ + readonly mergeGate?: PullRequestMergeGate; readonly baseComparison: PullRequestBaseComparison; /** Null where the host could not compare the branch with its base. */ readonly behindBy: number | null; diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 8461c150..0f2b3920 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -464,6 +464,7 @@ function toDetail(input: { labels: row.labels, checks: row.checks, ...(row.checksState === undefined ? {} : { checksState: row.checksState }), + ...(row.mergeGate === undefined ? {} : { mergeGate: row.mergeGate }), viewer: { canWrite: input.repository.canWrite, canReview: viewerKnown && !viewerIsAuthor, diff --git a/apps/server/src/pullRequest/gitHubPullRequestDetail.test.ts b/apps/server/src/pullRequest/gitHubPullRequestDetail.test.ts index 63e28544..8a043a9e 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestDetail.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestDetail.test.ts @@ -58,6 +58,14 @@ function decodeActivity(payload: Record) { } describe("decodeGitHubPullRequestDetailJson", () => { + it("reads the host's merge gate the way gh does, and says nothing while it is undecided", () => { + assert.strictEqual(decodeDetail({ mergeStateStatus: "BLOCKED" }).mergeGate, "blocked"); + assert.strictEqual(decodeDetail({ mergeStateStatus: "BEHIND" }).mergeGate, "behind"); + assert.strictEqual(decodeDetail({ mergeStateStatus: "UNSTABLE" }).mergeGate, "clear"); + assert.strictEqual(decodeDetail({ mergeStateStatus: "UNKNOWN" }).mergeGate, undefined); + assert.strictEqual(decodeDetail({}).mergeGate, undefined); + }); + it("lists a re-requested reviewer as pending, keeps a verdict over a later comment, and never the author", () => { const detail = decodeDetail({ author: { login: "octocat", is_bot: false }, diff --git a/apps/server/src/pullRequest/gitHubPullRequestDetail.ts b/apps/server/src/pullRequest/gitHubPullRequestDetail.ts index 81e120ef..e0b6dcb2 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestDetail.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestDetail.ts @@ -8,6 +8,7 @@ import { type PullRequestComment, type PullRequestCommit, type PullRequestMergeability, + type PullRequestMergeGate, type PullRequestMergeMethod, type PullRequestReviewer, type PullRequestReviewState, @@ -40,6 +41,9 @@ export const GITHUB_PULL_REQUEST_DETAIL_FIELDS = [ "closedAt", "reviews", "autoMergeRequest", + // What `gh pr merge` itself consults before it tries: the host's verdict on + // whether its rules would take the merge. + "mergeStateStatus", // Qualifies the head branch as `owner:branch`, which is the only name a // branch on a fork has in the base repository. "headRepositoryOwner", @@ -87,6 +91,8 @@ export interface GitHubPullRequestDetailRow extends GitHubPullRequestListRow { readonly closedAt: string | null; readonly reviewers: ReadonlyArray; readonly checks: ReadonlyArray; + /** Absent while the host is still deciding after a push, or on a host too old to say. */ + readonly mergeGate?: PullRequestMergeGate; /** Null on a host too old to report an auto-merge instruction at all. */ readonly autoMergeEnabled: boolean | null; /** Qualifies the head branch when it lives on a fork. */ @@ -137,6 +143,7 @@ const GitHubPullRequestDetailRowSchema = Schema.Struct({ reviews: Schema.optional(Schema.NullOr(Schema.Array(GitHubReviewSchema))), /** An object while auto-merge is armed, null once it is not, absent on an older CLI. */ autoMergeRequest: Schema.optional(Schema.NullOr(Schema.Struct({}))), + mergeStateStatus: Schema.optional(Schema.NullOr(Schema.String)), headRepositoryOwner: Schema.optional( Schema.NullOr(Schema.Struct({ login: Schema.optional(Schema.NullOr(Schema.String)) })), ), @@ -223,6 +230,27 @@ function normalizeReviewers(input: { } /** One row per check. A repeated name is a re-run, so the last one wins. */ +/** + * GitHub's `mergeStateStatus` as the gate the client renders. `BLOCKED` and + * `BEHIND` are the two `gh pr merge` refuses on without `--admin`; `DIRTY` is a + * conflict and already told through `mergeability`, and `UNKNOWN` is a host + * that has not finished deciding, so both say nothing here. + */ +function normalizeMergeGate(value: string | null | undefined): PullRequestMergeGate | undefined { + switch (value?.trim().toUpperCase()) { + case "BLOCKED": + return "blocked"; + case "BEHIND": + return "behind"; + case "CLEAN": + case "HAS_HOOKS": + case "UNSTABLE": + return "clear"; + default: + return undefined; + } +} + function normalizeChecks( checks: ReadonlyArray> | null | undefined, ): ReadonlyArray { @@ -384,6 +412,7 @@ export function decodeGitHubPullRequestDetailJson( const row = payload.success; const base = normalizeGitHubPullRequestListRow(row); + const mergeGate = normalizeMergeGate(row.mergeStateStatus); return Result.succeed({ ...base, body: row.body ?? "", @@ -399,6 +428,7 @@ export function decodeGitHubPullRequestDetailJson( reviews: row.reviews ?? [], }), checks: normalizeChecks(row.statusCheckRollup), + ...(mergeGate === undefined ? {} : { mergeGate }), // A CLI too old for the field leaves it absent, which is "the host did not // say" rather than "auto-merge is off". autoMergeEnabled: row.autoMergeRequest === undefined ? null : row.autoMergeRequest !== null, diff --git a/apps/web/src/components/pull-requests/PullRequestDetailPanel.browser.tsx b/apps/web/src/components/pull-requests/PullRequestDetailPanel.browser.tsx index 627528bd..52a5808f 100644 --- a/apps/web/src/components/pull-requests/PullRequestDetailPanel.browser.tsx +++ b/apps/web/src/components/pull-requests/PullRequestDetailPanel.browser.tsx @@ -570,6 +570,28 @@ describe("PullRequestDetailPanel", () => { await behind.cleanup(); }); + it("keeps Merge on screen but off while the host's rules refuse it, and says why", async () => { + const rendered = await renderPanel({ + detail: { + viewer: { canWrite: true, canReview: false, canManage: true }, + mergeGate: "blocked", + checks: [ + { name: "build", status: "success", description: null, url: null }, + { name: "lint", status: "pending", description: null, url: null }, + { name: "test", status: "pending", description: null, url: null }, + ], + checksState: "pending", + }, + }); + + await expect.element(page.getByTestId("pull-request-merge")).toBeDisabled(); + await expect + .element(page.getByTestId("pull-request-merge-block")) + .toHaveTextContent("Waiting on 2 checks"); + + await rendered.cleanup(); + }); + it("asks someone for a review from the reviewers row", async () => { const rendered = await renderPanel({ detail: { viewer: { canWrite: true, canReview: false, canManage: true } }, diff --git a/apps/web/src/components/pull-requests/PullRequestDetailPanel.tsx b/apps/web/src/components/pull-requests/PullRequestDetailPanel.tsx index d78e8b9d..c3706210 100644 --- a/apps/web/src/components/pull-requests/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pull-requests/PullRequestDetailPanel.tsx @@ -1413,7 +1413,12 @@ function usePullRequestActions({ {/* A disabled button cannot be focused, so the reason it is disabled is written out as well as tucked in its tooltip. */} {primary === "merge" && mergeBlock !== null ? ( -

{mergeBlock}

+

+ {mergeBlock} +

) : null} {mutation.isError ? (

diff --git a/apps/web/src/components/pull-requests/pullRequests.logic.test.ts b/apps/web/src/components/pull-requests/pullRequests.logic.test.ts index d6df2f26..a1f48a8f 100644 --- a/apps/web/src/components/pull-requests/pullRequests.logic.test.ts +++ b/apps/web/src/components/pull-requests/pullRequests.logic.test.ts @@ -2,6 +2,7 @@ import { EnvironmentId, ProjectId, ThreadId, + type PullRequestCheck, type PullRequestComment, type PullRequestListEntry, type PullRequestListResult, @@ -44,6 +45,7 @@ import { sortPullRequests, summarizePullRequestChecks, resolveThreadPullRequest, + shouldPollPullRequestDetail, type PullRequestDiffFile, type PullRequestEntry, type PullRequestFilters, @@ -968,6 +970,90 @@ describe("resolvePullRequestMergeBlock", () => { ); expect(resolvePullRequestMergeBlock({ mergeability: "unknown", isDraft: false })).toBeNull(); }); + + it("says what the host's own rules are waiting on", () => { + const check = (status: PullRequestCheck["status"]) => ({ + name: status, + status, + description: null, + url: null, + }); + const blocked = { + mergeability: "mergeable" as const, + isDraft: false, + mergeGate: "blocked" as const, + }; + + expect( + resolvePullRequestMergeBlock({ + ...blocked, + checks: [check("pending"), check("pending"), check("failure")], + }), + ).toBe("Waiting on 2 checks"); + expect(resolvePullRequestMergeBlock({ ...blocked, checks: [check("pending")] })).toBe( + "Waiting on 1 check", + ); + expect( + resolvePullRequestMergeBlock({ ...blocked, checks: [check("failure"), check("success")] }), + ).toBe("A check failed"); + expect( + resolvePullRequestMergeBlock({ + ...blocked, + checks: [check("success")], + reviewDecision: "review-required", + }), + ).toBe("Needs an approving review"); + expect(resolvePullRequestMergeBlock({ ...blocked, checks: [] })).toBe( + "Blocked by branch rules", + ); + expect(resolvePullRequestMergeBlock({ ...blocked, mergeGate: "behind" })).toBe( + "Update the branch first", + ); + // A running check on its own is not a block: without a rule requiring it, + // the host merges anyway. + expect( + resolvePullRequestMergeBlock({ ...blocked, mergeGate: "clear", checks: [check("pending")] }), + ).toBeNull(); + }); +}); + +describe("shouldPollPullRequestDetail", () => { + const now = Date.parse("2026-09-04T12:00:00.000Z"); + const check = (status: PullRequestCheck["status"]) => ({ + name: status, + status, + description: null, + url: null, + }); + const settled = { + state: "open" as const, + mergeability: "mergeable" as const, + updatedAt: "2026-09-04T11:00:00.000Z", + }; + + it("polls while a check runs, and for a while after a push before any check exists", () => { + expect(shouldPollPullRequestDetail({ ...settled, checks: [check("pending")] }, now)).toBe(true); + expect(shouldPollPullRequestDetail({ ...settled, checks: [check("success")] }, now)).toBe( + false, + ); + expect(shouldPollPullRequestDetail({ ...settled, checks: [] }, now)).toBe(false); + + const justPushed = { ...settled, updatedAt: "2026-09-04T11:59:30.000Z" }; + expect(shouldPollPullRequestDetail({ ...justPushed, checks: [] }, now)).toBe(true); + expect( + shouldPollPullRequestDetail( + { ...justPushed, mergeability: "unknown", checks: [check("success")] }, + now, + ), + ).toBe(true); + // Checks that have arrived and settled end the watch early. + expect(shouldPollPullRequestDetail({ ...justPushed, checks: [check("success")] }, now)).toBe( + false, + ); + expect( + shouldPollPullRequestDetail({ ...justPushed, state: "merged" as const, checks: [] }, now), + ).toBe(false); + }); }); describe("handing a review comment to a thread", () => { diff --git a/apps/web/src/components/pull-requests/pullRequests.logic.ts b/apps/web/src/components/pull-requests/pullRequests.logic.ts index a576758f..9f942e7a 100644 --- a/apps/web/src/components/pull-requests/pullRequests.logic.ts +++ b/apps/web/src/components/pull-requests/pullRequests.logic.ts @@ -1354,12 +1354,15 @@ export function resolveDefaultMergeMethod( } /** - * Why merging is off the table right now, or null when it is available. Both - * answers are things the user fixes elsewhere, so the button stays visible and - * says what is in the way rather than disappearing. + * Why merging is off the table right now, or null when it is available. Every + * answer is something the user fixes elsewhere or waits out, so the button + * stays visible and says what is in the way rather than disappearing. The + * host's own gate comes last: a draft or a conflict is the more useful thing + * to say when both hold. */ export function resolvePullRequestMergeBlock( - detail: Pick, + detail: Pick & + Partial>, ): string | null { if (detail.isDraft) { return "Mark as ready first"; @@ -1367,9 +1370,57 @@ export function resolvePullRequestMergeBlock( if (detail.mergeability === "conflicting") { return "Resolve the conflicts first"; } + if (detail.mergeGate === "behind") { + return "Update the branch first"; + } + if (detail.mergeGate === "blocked") { + // The host does not say which rule refused, so this reads the most likely + // one off what it did say: running checks, then failed ones, then reviews. + const { pending, failing } = summarizePullRequestChecks(detail.checks ?? []); + if (pending > 0) { + return pending === 1 ? "Waiting on 1 check" : `Waiting on ${pending} checks`; + } + if (failing > 0) { + return failing === 1 ? "A check failed" : "Checks failed"; + } + if ( + detail.reviewDecision === "review-required" || + detail.reviewDecision === "changes-requested" + ) { + return "Needs an approving review"; + } + return "Blocked by branch rules"; + } return null; } +/** How long after a push the header waits for the host to register the new commit's checks. */ +export const PULL_REQUEST_FRESH_PUSH_WATCH_MS = 120_000; + +/** + * Whether the header should keep re-reading itself. A check still running is + * the plain case. The other is the quiet moment right after a push (an update + * from the base, a new commit) when the host has the commit but has not queued + * its checks or decided whether it merges: a read then shows no checks at all, + * and would otherwise sit on that answer until the user hit Refresh. + */ +export function shouldPollPullRequestDetail( + detail: Pick, + now: number, +): boolean { + if (detail.checks.some((check) => check.status === "pending")) { + return true; + } + if (detail.state !== "open") { + return false; + } + const unsettled = detail.checks.length === 0 || detail.mergeability === "unknown"; + const updatedAt = Date.parse(detail.updatedAt); + return ( + unsettled && Number.isFinite(updatedAt) && now - updatedAt < PULL_REQUEST_FRESH_PUSH_WATCH_MS + ); +} + /** * The prompt a review comment becomes when it is handed to the thread working * the branch. The comment is quoted rather than restated, so the agent reads diff --git a/apps/web/src/lib/pullRequestsReactQuery.ts b/apps/web/src/lib/pullRequestsReactQuery.ts index 4b519115..b06a8219 100644 --- a/apps/web/src/lib/pullRequestsReactQuery.ts +++ b/apps/web/src/lib/pullRequestsReactQuery.ts @@ -24,6 +24,7 @@ import { useMemo } from "react"; import { resolveEnvironmentOptionLabel } from "~/components/BranchToolbar.logic"; import { mergePullRequestListResults, + shouldPollPullRequestDetail, type PullRequestEntry, type PullRequestProjectFailure, } from "~/components/pull-requests/pullRequests.logic"; @@ -84,10 +85,11 @@ function readPayload(input: PullRequestReadInput) { } /** - * The detail reads never poll: a pull request is a document the user reads, - * and the panel's own Refresh is the one thing that re-runs `gh`. + * The pace the header keeps itself current at while there is something to wait + * for: a running check, or a fresh push whose checks the host has not queued + * yet. The rest of the time it is a document the user reads, and the panel's + * own Refresh is the one thing that re-runs `gh`. */ -/** While a check is still running the header keeps itself current at this pace. */ export const PULL_REQUEST_CHECKS_POLL_INTERVAL_MS = 20_000; export function pullRequestDetailQueryOptions(input: PullRequestReadInput) { @@ -101,10 +103,8 @@ export function pullRequestDetailQueryOptions(input: PullRequestReadInput) { ensureEnvironmentApi(input.environmentId).pullRequests.detail(readPayload(input)), staleTime: PULL_REQUEST_READ_STALE_TIME_MS, refetchOnWindowFocus: false, - // A run in progress is the one moment the reader is watching the checks, - // so the header polls until every check has settled, then goes quiet. refetchInterval: (query) => - query.state.data?.checks.some((check) => check.status === "pending") + query.state.data !== undefined && shouldPollPullRequestDetail(query.state.data, Date.now()) ? PULL_REQUEST_CHECKS_POLL_INTERVAL_MS : false, refetchIntervalInBackground: false, diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index 7ba2f5ab..17aa8732 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -29,6 +29,16 @@ export type PullRequestChecksState = typeof PullRequestChecksState.Type; export const PullRequestMergeability = Schema.Literals(["mergeable", "conflicting", "unknown"]); export type PullRequestMergeability = typeof PullRequestMergeability.Type; +/** + * Whether the host's own rules would take a merge right now. `blocked` is a + * protection rule in the way: required checks still running or failed, a + * review still owed. `behind` is a host that insists the branch be current + * first. `clear` is a merge the host would accept. Conflicts are `mergeability`, + * not this. + */ +export const PullRequestMergeGate = Schema.Literals(["clear", "blocked", "behind"]); +export type PullRequestMergeGate = typeof PullRequestMergeGate.Type; + export const PullRequestActor = Schema.Struct({ login: TrimmedNonEmptyString, isBot: Schema.Boolean, @@ -407,6 +417,8 @@ export const PullRequestDetail = Schema.Struct({ labels: Schema.Array(PullRequestLabel), checks: Schema.Array(PullRequestCheck), checksState: Schema.optionalKey(PullRequestChecksState), + /** Absent where the host does not say, or has not decided yet after a push. */ + mergeGate: Schema.optionalKey(PullRequestMergeGate), viewer: PullRequestViewerPermissions, /** The methods the repository allows, in the order merge, squash, rebase. */ mergeMethods: Schema.Array(PullRequestMergeMethod),