From 7153b7f5f68680cd416be5dd57bc3b7000866438 Mon Sep 17 00:00:00 2001 From: badcuban <108198679+badcuban@users.noreply.github.com> Date: Sat, 5 Sep 2026 02:22:38 -0400 Subject: [PATCH] feat(pull-requests): list rows say auto-merge, and arming never merges outright Only the detail page said whether a pull request was armed to merge on its own, so the list gave no hint which branches would land without anyone coming back. Worse, "Enable auto-merge" ran `gh pr merge --auto`, which merges a ready pull request on the spot instead of arming it, with no confirmation. List rows now carry the auto-merge flag from every host that reports one (GitHub, including the authored search; GitLab; Azure DevOps) and the page shows it as a mono "auto-merge" word on the meta line of an open, non-draft row. The GitHub provider reads the pull request's merge readiness before arming and refuses a ready one with "This pull request can merge right now. Use Merge instead." The repository read also picks up `allow_auto_merge`, and the service drops the enable action from the detail's capabilities where it is off, so the menu item never appears on a repository that would refuse it. --- .../AzureDevOpsPullRequestProvider.ts | 2 +- .../BitbucketPullRequestProvider.ts | 2 - .../GitHubPullRequestProvider.test.ts | 61 +++++++++++++++++-- .../pullRequest/GitHubPullRequestProvider.ts | 57 +++++++++++++++-- .../src/pullRequest/PullRequestProvider.ts | 9 ++- .../pullRequest/PullRequestService.test.ts | 8 ++- .../src/pullRequest/PullRequestService.ts | 15 ++++- .../gitHubPullRequestDetail.test.ts | 8 ++- .../pullRequest/gitHubPullRequestDetail.ts | 42 +++++++++++-- .../gitHubPullRequestGraphql.test.ts | 2 + .../pullRequest/gitHubPullRequestGraphql.ts | 3 + .../pullRequest/gitHubPullRequestList.test.ts | 17 ++++++ .../src/pullRequest/gitHubPullRequestList.ts | 10 +++ .../src/pullRequest/gitLabMergeRequest.ts | 14 ++--- .../PullRequestsView.browser.tsx | 3 + .../pull-requests/PullRequestsView.tsx | 21 +++++++ .../pull-requests/pullRequests.logic.ts | 13 ++++ packages/contracts/src/pullRequest.ts | 2 + 18 files changed, 256 insertions(+), 33 deletions(-) diff --git a/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts b/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts index eb69a920..83cc047e 100644 --- a/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts +++ b/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts @@ -296,6 +296,7 @@ export const make = Effect.fn("makeAzureDevOpsPullRequestProvider")(function* () reviewRequestedLogins: row.reviewRequestedLogins, // Azure keeps labels on work items rather than on the pull request. labels: [], + autoMergeEnabled: row.autoMergeEnabled, }); const provider: PullRequestProviderApi = { @@ -342,7 +343,6 @@ export const make = Effect.fn("makeAzureDevOpsPullRequestProvider")(function* () checks: [], baseComparison: "unknown" as const, behindBy: null, - autoMergeEnabled: row.autoMergeEnabled, })), ), diff --git a/apps/server/src/pullRequest/BitbucketPullRequestProvider.ts b/apps/server/src/pullRequest/BitbucketPullRequestProvider.ts index 41ca00bf..2003158d 100644 --- a/apps/server/src/pullRequest/BitbucketPullRequestProvider.ts +++ b/apps/server/src/pullRequest/BitbucketPullRequestProvider.ts @@ -332,8 +332,6 @@ export const make = Effect.fn("makeBitbucketPullRequestProvider")(function* () { // Bitbucket compares no branch with its base. baseComparison: "unknown" as const, behindBy: null, - // Bitbucket has nothing that arms a merge to run on its own. - autoMergeEnabled: null, })), ); }), diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts index bf258c04..06d85442 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts @@ -39,11 +39,6 @@ describe("GitHubPullRequestProvider.runAction", () => { input: { action: "update-branch", updateMethod: "rebase" }, args: ["pr", "update-branch", "12", "--repo", "octocat/example-app", "--rebase"], }, - { - name: "arms auto-merge with the strategy the host stores alongside it", - input: { action: "enable-auto-merge", mergeMethod: "squash" }, - args: ["pr", "merge", "12", "--repo", "octocat/example-app", "--auto", "--squash"], - }, { name: "disarms auto-merge", input: { action: "disable-auto-merge" }, @@ -68,6 +63,62 @@ describe("GitHubPullRequestProvider.runAction", () => { }).pipe(Effect.provide(layer)), ); } + + /** The host answers the readiness read with one status and every write with nothing. */ + const hostReports = (status: string) => { + mockExecute.mockImplementation((input) => + Effect.succeed( + processOutput(input.args[1] === "view" ? JSON.stringify({ mergeStateStatus: status }) : ""), + ), + ); + }; + const prCalls = () => + calls() + .map((call) => call.args) + .filter((args) => args[0] === "pr"); + + it.effect("arms auto-merge only once the host says the merge would wait", () => + Effect.gen(function* () { + hostReports("BLOCKED"); + const provider = yield* GitHubPullRequestProvider.make(); + + yield* provider.runAction({ + ...repository, + number: 12, + action: "enable-auto-merge", + mergeMethod: "squash", + }); + + assert.deepStrictEqual(prCalls(), [ + ["pr", "view", "12", "--repo", "octocat/example-app", "--json", "mergeStateStatus"], + // The strategy travels with the standing instruction. + ["pr", "merge", "12", "--repo", "octocat/example-app", "--auto", "--squash"], + ]); + }).pipe(Effect.provide(layer)), + ); + + it.effect("refuses to arm a pull request gh would merge on the spot", () => + Effect.gen(function* () { + hostReports("CLEAN"); + const provider = yield* GitHubPullRequestProvider.make(); + + const error = yield* provider + .runAction({ + ...repository, + number: 12, + action: "enable-auto-merge", + mergeMethod: "squash", + }) + .pipe(Effect.flip); + + assert.equal(error.detail, "This pull request can merge right now. Use Merge instead."); + // Nothing ran but the read: the merge never reached the host. + assert.deepStrictEqual( + prCalls().map((args) => args[1]), + ["view"], + ); + }).pipe(Effect.provide(layer)), + ); }); describe("GitHubPullRequestProvider.submitReview", () => { diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts index 9d34a183..bcd8f379 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts @@ -32,6 +32,7 @@ import { import { decodeGitHubPullRequestActivityJson, decodeGitHubPullRequestDetailJson, + decodeGitHubImmediatelyMergeableJson, decodeGitHubRepositoryJson, GITHUB_PULL_REQUEST_ACTIVITY_FIELDS, GITHUB_PULL_REQUEST_DETAIL_FIELDS, @@ -259,6 +260,43 @@ export const make = Effect.fn("makeGitHubPullRequestProvider")(function* () { stdin: encodeGraphQlRequestJson({ query: input.query, variables: input.variables }), }); + /** + * Fails where GitHub would merge the pull request the moment it is armed. The + * refusal is this app's own sentence, since the host never gets asked. + */ + const refuseIfImmediatelyMergeable = ( + input: ProviderRepositoryRef & { readonly number: number }, + ) => + run({ + operation: "runAction", + cwd: input.cwd, + args: [ + "pr", + "view", + String(input.number), + ...repositoryArgs(input), + "--json", + "mergeStateStatus", + ], + }).pipe( + Effect.flatMap((output) => { + const decoded = decodeGitHubImmediatelyMergeableJson(output.stdout.trim()); + if (!Result.isSuccess(decoded)) { + return Effect.fail(decodeError("runAction", "merge state", decoded.failure)); + } + return decoded.success + ? Effect.fail( + new PullRequestProviderError({ + provider: PROVIDER_KIND, + operation: "runAction", + reason: "failed", + detail: "This pull request can merge right now. Use Merge instead.", + }), + ) + : Effect.void; + }), + ); + const graphqlRead = (input: { readonly operation: string; readonly cwd: string; @@ -626,12 +664,19 @@ export const make = Effect.fn("makeGitHubPullRequestProvider")(function* () { updateMethod: input.updateMethod, deleteBranch: input.deleteBranch, }); - return run({ - operation: "runAction", - cwd: input.cwd, - args: ["pr", subcommand, String(input.number), ...repositoryArgs(input), ...flags], - ...(input.action === "merge" ? { timeoutMs: MERGE_TIMEOUT_MS } : {}), - }).pipe(Effect.asVoid); + const action = () => + run({ + operation: "runAction", + cwd: input.cwd, + args: ["pr", subcommand, String(input.number), ...repositoryArgs(input), ...flags], + ...(input.action === "merge" ? { timeoutMs: MERGE_TIMEOUT_MS } : {}), + }).pipe(Effect.asVoid); + // `gh pr merge --auto` merges outright when nothing is pending, which is + // not what someone arming a merge asked for. The readiness is read first + // and a ready pull request is sent back to the Merge button instead. + return input.action === "enable-auto-merge" + ? refuseIfImmediatelyMergeable(input).pipe(Effect.flatMap(action)) + : action(); }, comment: (input) => diff --git a/apps/server/src/pullRequest/PullRequestProvider.ts b/apps/server/src/pullRequest/PullRequestProvider.ts index f1f8bdb8..310e5bc0 100644 --- a/apps/server/src/pullRequest/PullRequestProvider.ts +++ b/apps/server/src/pullRequest/PullRequestProvider.ts @@ -90,6 +90,8 @@ export interface ProviderChangeRequest { readonly mergeability?: PullRequestMergeability; /** Absent where there are no checks, or where the listing did not ask for them. */ readonly checksState?: PullRequestChecksState; + /** Whether the host is armed to merge it on its own; absent where the host does not say. */ + readonly autoMergeEnabled?: boolean; } /** @@ -119,8 +121,6 @@ export interface ProviderChangeRequestDetail extends ProviderChangeRequest { readonly baseComparison: PullRequestBaseComparison; /** Null where the host could not compare the branch with its base. */ readonly behindBy: number | null; - /** Null where the host does not say whether it is armed to merge on its own. */ - readonly autoMergeEnabled: boolean | null; } /** @@ -132,6 +132,11 @@ export interface ProviderRepositoryAccess { readonly mergeMethods: ReadonlyArray; /** What a pull request has to target not to be stacked on other work. */ readonly defaultBranch: string | null; + /** + * Whether the repository lets a pull request be armed to merge on its own. + * Absent where the host has no such switch, or did not say. + */ + readonly autoMergeAllowed?: boolean; } /** diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 2d535e81..fc179333 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -214,6 +214,7 @@ const repositoryJson = (input?: { readonly merge?: boolean; readonly squash?: boolean; readonly rebase?: boolean; + readonly autoMerge?: boolean; }) => JSON.stringify({ name: "example-app", @@ -221,6 +222,7 @@ const repositoryJson = (input?: { allow_merge_commit: input?.merge ?? true, allow_squash_merge: input?.squash ?? true, allow_rebase_merge: input?.rebase ?? true, + allow_auto_merge: input?.autoMerge ?? true, }); const detailJson = (input?: { readonly author?: string; readonly isDraft?: boolean }) => @@ -846,7 +848,7 @@ describe("PullRequestService pull request reads", () => { onlyGitHubProject(); let author = "hubot"; hostAnswers({ - repository: repositoryJson({ push: false, merge: false }), + repository: repositoryJson({ push: false, merge: false, autoMerge: false }), detail: () => detailJson({ author }), }); @@ -859,6 +861,10 @@ describe("PullRequestService pull request reads", () => { canManage: false, }); assert.deepStrictEqual(theirs.mergeMethods, ["squash", "rebase"]); + // A repository with auto-merge switched off is never offered the switch; + // everything else the host can do stays. + assert.equal(theirs.capabilities.actions.includes("enable-auto-merge"), false); + assert.equal(theirs.capabilities.actions.includes("merge"), true); author = "OctoCat"; const mine = yield* service.detail({ ...reference, force: true }); diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 8461c150..2b8c6c1a 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -401,6 +401,7 @@ function toEntry(input: { ...(row.mergeability === undefined || row.mergeability === "unknown" ? {} : { mergeability: row.mergeability }), + ...(row.autoMergeEnabled === undefined ? {} : { autoMergeEnabled: row.autoMergeEnabled }), labels: row.labels, origin: input.origin, }; @@ -473,11 +474,19 @@ function toDetail(input: { }, mergeMethods: input.repository.mergeMethods, // What the host supports in general, narrowed to what this repository - // actually allows, so the client never offers a merge the host refuses. - capabilities: { ...input.capabilities, mergeMethods: input.repository.mergeMethods }, + // actually allows, so the client never offers a merge the host refuses, + // nor an auto-merge switch the repository has turned off. + capabilities: { + ...input.capabilities, + mergeMethods: input.repository.mergeMethods, + actions: + input.repository.autoMergeAllowed === false + ? input.capabilities.actions.filter((action) => action !== "enable-auto-merge") + : input.capabilities.actions, + }, baseComparison: row.baseComparison, behindBy: row.behindBy, - autoMergeEnabled: row.autoMergeEnabled, + autoMergeEnabled: row.autoMergeEnabled ?? null, isStacked: defaultBranch !== null && row.baseBranch !== defaultBranch, defaultBranch, }; diff --git a/apps/server/src/pullRequest/gitHubPullRequestDetail.test.ts b/apps/server/src/pullRequest/gitHubPullRequestDetail.test.ts index 63e28544..46807e0d 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestDetail.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestDetail.test.ts @@ -192,9 +192,15 @@ describe("decodeGitHubRepositoryJson", () => { allow_merge_commit: false, allow_squash_merge: true, allow_rebase_merge: true, + allow_auto_merge: false, default_branch: "main", }), - { canWrite: true, mergeMethods: ["squash", "rebase"], defaultBranch: "main" }, + { + canWrite: true, + mergeMethods: ["squash", "rebase"], + defaultBranch: "main", + autoMergeAllowed: false, + }, ); }); diff --git a/apps/server/src/pullRequest/gitHubPullRequestDetail.ts b/apps/server/src/pullRequest/gitHubPullRequestDetail.ts index 81e120ef..e4a6d5ec 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestDetail.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestDetail.ts @@ -54,6 +54,8 @@ export interface GitHubRepositoryAccess { readonly mergeMethods: ReadonlyArray; /** What a pull request has to target not to be stacked on other work. */ readonly defaultBranch: string | null; + /** Absent on a host too old to report the auto-merge switch. */ + readonly autoMergeAllowed?: boolean; } /** The order the detail surface offers the allowed merge methods in. */ @@ -75,9 +77,24 @@ const GitHubRepositorySchema = Schema.Struct({ allow_merge_commit: Schema.optional(Schema.NullOr(Schema.Boolean)), allow_squash_merge: Schema.optional(Schema.NullOr(Schema.Boolean)), allow_rebase_merge: Schema.optional(Schema.NullOr(Schema.Boolean)), + allow_auto_merge: Schema.optional(Schema.NullOr(Schema.Boolean)), default_branch: Schema.optional(Schema.NullOr(Schema.String)), }); +/** + * `gh pr view --json mergeStateStatus`: the one word GitHub sums a pull + * request's readiness into. + */ +const GitHubMergeStateSchema = Schema.Struct({ + mergeStateStatus: Schema.optional(Schema.NullOr(Schema.String)), +}); + +/** + * The states `gh pr merge --auto` merges outright instead of arming, copied + * from the CLI: nothing is pending, so there is nothing to wait for. + */ +const IMMEDIATELY_MERGEABLE_STATES = new Set(["CLEAN", "HAS_HOOKS", "UNSTABLE"]); + /** A `gh pr view` row: everything a list row carries, plus the detail fields. */ export interface GitHubPullRequestDetailRow extends GitHubPullRequestListRow { readonly body: string; @@ -87,8 +104,6 @@ export interface GitHubPullRequestDetailRow extends GitHubPullRequestListRow { readonly closedAt: string | null; readonly reviewers: ReadonlyArray; readonly checks: ReadonlyArray; - /** 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. */ readonly headRepositoryOwnerLogin: string | null; } @@ -345,6 +360,7 @@ function normalizeCommits( const decodeDetailPayload = decodeJsonResult(GitHubPullRequestDetailRowSchema); const decodeActivityPayload = decodeJsonResult(GitHubPullRequestActivitySchema); const decodeRepositoryPayload = decodeJsonResult(GitHubRepositorySchema); +const decodeMergeStatePayload = decodeJsonResult(GitHubMergeStateSchema); /** * Decodes `gh api repos//` into the viewer's access and the merge @@ -370,9 +386,28 @@ export function decodeGitHubRepositoryJson( (_method, index) => !reported || switches[index] === true, ), defaultBranch: nonEmptyText(row.default_branch), + ...(typeof row.allow_auto_merge === "boolean" + ? { autoMergeAllowed: row.allow_auto_merge } + : {}), }); } +/** + * Whether GitHub would merge the pull request this instant, read from + * `gh pr view --json mergeStateStatus`. A status the host did not name is not + * ready: arming then waits, which is the safe way to be wrong. + */ +export function decodeGitHubImmediatelyMergeableJson( + raw: string, +): Result.Result> { + const payload = decodeMergeStatePayload(raw); + if (!Result.isSuccess(payload)) { + return Result.fail(payload.failure); + } + const status = nonEmptyText(payload.success.mergeStateStatus); + return Result.succeed(status !== null && IMMEDIATELY_MERGEABLE_STATES.has(status)); +} + /** Decodes `gh pr view --json ` into the header the panel renders. */ export function decodeGitHubPullRequestDetailJson( raw: string, @@ -399,9 +434,6 @@ export function decodeGitHubPullRequestDetailJson( reviews: row.reviews ?? [], }), checks: normalizeChecks(row.statusCheckRollup), - // 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, headRepositoryOwnerLogin: nonEmptyText(row.headRepositoryOwner?.login), }); } diff --git a/apps/server/src/pullRequest/gitHubPullRequestGraphql.test.ts b/apps/server/src/pullRequest/gitHubPullRequestGraphql.test.ts index c0ea3c02..da15b568 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestGraphql.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestGraphql.test.ts @@ -272,6 +272,7 @@ describe("decodeGitHubAuthoredPullRequestsJson", () => { labels: { nodes: [{ name: "bug", color: "d73a4a" }] }, reviewRequests: { nodes: [{ requestedReviewer: { login: "hubot" } }] }, commits: { nodes: [{ commit: { statusCheckRollup: { state: "FAILURE" } } }] }, + autoMergeRequest: { enabledAt: "2026-08-31T09:00:00Z" }, ...overrides, }); @@ -303,6 +304,7 @@ describe("decodeGitHubAuthoredPullRequestsJson", () => { reviewDecision: "changes-requested", checksState: "failure", mergeability: "conflicting", + autoMergeEnabled: true, labels: [{ name: "bug", color: "d73a4a" }], repository: "openai/codex", }, diff --git a/apps/server/src/pullRequest/gitHubPullRequestGraphql.ts b/apps/server/src/pullRequest/gitHubPullRequestGraphql.ts index 8eb08102..42e4c317 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestGraphql.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestGraphql.ts @@ -127,6 +127,7 @@ export const AUTHORED_PULL_REQUESTS_GRAPHQL_QUERY = `query($q: String!, $first: deletions mergeable reviewDecision + autoMergeRequest { enabledAt } author { login avatarUrl } repository { nameWithOwner viewerPermission } labels(first: ${AUTHORED_CONNECTION_PAGE_SIZE}) { nodes { name color } } @@ -541,6 +542,7 @@ const RawAuthoredNodeSchema = Schema.Struct({ deletions: Schema.optional(Schema.NullOr(Schema.Number)), mergeable: Schema.optional(Schema.NullOr(Schema.String)), reviewDecision: Schema.optional(Schema.NullOr(Schema.String)), + autoMergeRequest: Schema.optional(Schema.NullOr(Schema.Struct({}))), author: Schema.optional(Schema.NullOr(GitHubAuthorSchema)), repository: Schema.optional( Schema.NullOr( @@ -669,6 +671,7 @@ function toGitHubListRowShape(node: RawAuthoredNode): unknown { updatedAt: node.updatedAt, mergeable: node.mergeable, reviewDecision: node.reviewDecision, + autoMergeRequest: node.autoMergeRequest, reviewRequests: (node.reviewRequests?.nodes ?? []).map((request) => ({ login: request?.requestedReviewer?.login ?? null, })), diff --git a/apps/server/src/pullRequest/gitHubPullRequestList.test.ts b/apps/server/src/pullRequest/gitHubPullRequestList.test.ts index 4c2518ea..ab886281 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestList.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestList.test.ts @@ -118,6 +118,23 @@ describe("decodeGitHubPullRequestListJson", () => { }); }); + it("reads the auto-merge instruction, and says nothing where the CLI did not carry it", () => { + const rows = decodeRows([ + { ...baseRow, number: 1, autoMergeRequest: { enabledAt: "2026-08-31T10:00:00Z" } }, + { ...baseRow, number: 2, autoMergeRequest: null }, + { ...baseRow, number: 3 }, + ]); + + assert.deepStrictEqual( + rows.map((row) => [row.number, row.autoMergeEnabled]), + [ + [1, true], + [2, false], + [3, undefined], + ], + ); + }); + it("skips a malformed row and keeps the rest", () => { const rows = decodeRows([ { ...baseRow, number: 0 }, diff --git a/apps/server/src/pullRequest/gitHubPullRequestList.ts b/apps/server/src/pullRequest/gitHubPullRequestList.ts index e1343919..84b4dd55 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestList.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestList.ts @@ -39,6 +39,7 @@ export const GITHUB_PULL_REQUEST_LIST_FIELDS = [ "reviewDecision", "reviewRequests", "labels", + "autoMergeRequest", ] as const; export const GITHUB_PULL_REQUEST_LIST_CHECKS_FIELD = "statusCheckRollup"; @@ -69,6 +70,8 @@ export interface GitHubPullRequestListRow { readonly checksState?: PullRequestChecksState; /** Absent where the host said nothing about whether the branch still merges. */ readonly mergeability?: PullRequestMergeability; + /** Absent on a CLI too old to report an auto-merge instruction at all. */ + readonly autoMergeEnabled?: boolean; readonly labels: ReadonlyArray<{ readonly name: string; readonly color: string | null }>; } @@ -127,6 +130,8 @@ export const GitHubPullRequestListRowSchema = Schema.Struct({ reviewRequests: Schema.optional(Schema.NullOr(Schema.Array(GitHubReviewRequestSchema))), labels: Schema.optional(Schema.NullOr(Schema.Array(GitHubLabelSchema))), statusCheckRollup: Schema.optional(Schema.NullOr(Schema.Array(GitHubStatusCheckSchema))), + /** An object while auto-merge is armed, null once it is not, absent on an older CLI. */ + autoMergeRequest: Schema.optional(Schema.NullOr(Schema.Struct({}))), }); const FAILING_CHECK_CONCLUSIONS = new Set([ @@ -298,6 +303,11 @@ export function normalizeGitHubPullRequestListRow( ...(reviewDecision === undefined ? {} : { reviewDecision }), ...(checksState === undefined ? {} : { checksState }), ...(mergeability === undefined ? {} : { mergeability }), + // A CLI too old for the field leaves it absent, which is "the host did not + // say" rather than "auto-merge is off". + ...(raw.autoMergeRequest === undefined + ? {} + : { autoMergeEnabled: raw.autoMergeRequest !== null }), labels: (raw.labels ?? []).flatMap((label) => { const name = nonEmptyText(label.name); return name === null ? [] : [{ name, color: nonEmptyText(label.color) }]; diff --git a/apps/server/src/pullRequest/gitLabMergeRequest.ts b/apps/server/src/pullRequest/gitLabMergeRequest.ts index 8a0fe5e3..2f87c8d5 100644 --- a/apps/server/src/pullRequest/gitLabMergeRequest.ts +++ b/apps/server/src/pullRequest/gitLabMergeRequest.ts @@ -214,6 +214,8 @@ export interface GitLabMergeRequestRow { readonly reviewRequestedLogins: ReadonlyArray; readonly labels: ReadonlyArray; readonly checksState?: PullRequestChecksState; + /** Absent where GitLab named neither auto-merge field, which is not "off". */ + readonly autoMergeEnabled?: boolean; } /** The three revisions a positioned comment is written against. */ @@ -235,8 +237,6 @@ export interface GitLabMergeRequestDetailRow extends GitLabMergeRequestRow { readonly avatarUrl: string | null; }>; readonly checks: ReadonlyArray; - /** Null where GitLab named neither auto-merge field, which is not "off". */ - readonly autoMergeEnabled: boolean | null; /** Null where GitLab did not count, which is not the same as "up to date". */ readonly behindBy: number | null; /** Null on a merge request with no revisions to place a comment against. */ @@ -363,6 +363,10 @@ function toChecksState( function toRow(raw: Schema.Schema.Type): GitLabMergeRequestRow { const checksState = toChecksState(raw); + const autoMergeEnabled = + raw.merge_when_pipeline_succeeds == null && raw.auto_merge_enabled == null + ? undefined + : raw.merge_when_pipeline_succeeds === true || raw.auto_merge_enabled === true; return { number: raw.iid, title: raw.title, @@ -381,6 +385,7 @@ function toRow(raw: Schema.Schema.Type): GitLab }), labels: toLabels(raw.labels), ...(checksState === undefined ? {} : { checksState }), + ...(autoMergeEnabled === undefined ? {} : { autoMergeEnabled }), }; } @@ -398,10 +403,6 @@ function toDiffRefs( function toDetailRow( raw: Schema.Schema.Type, ): GitLabMergeRequestDetailRow { - const autoMerge = - raw.merge_when_pipeline_succeeds == null && raw.auto_merge_enabled == null - ? null - : raw.merge_when_pipeline_succeeds === true || raw.auto_merge_enabled === true; return { ...toRow(raw), body: raw.description ?? "", @@ -415,7 +416,6 @@ function toDetailRow( : [{ id: String(reviewer.id), login: actor.login, avatarUrl: actor.avatarUrl }]; }), checks: toChecks(raw), - autoMergeEnabled: autoMerge, behindBy: raw.diverged_commits_count ?? null, diffRefs: toDiffRefs(raw), }; diff --git a/apps/web/src/components/pull-requests/PullRequestsView.browser.tsx b/apps/web/src/components/pull-requests/PullRequestsView.browser.tsx index 1cc977f3..3fee047c 100644 --- a/apps/web/src/components/pull-requests/PullRequestsView.browser.tsx +++ b/apps/web/src/components/pull-requests/PullRequestsView.browser.tsx @@ -537,6 +537,7 @@ describe("PullRequestsView", () => { mergeability: "conflicting", reviewDecision: "approved", checksState: "failure", + autoMergeEnabled: true, labels: [{ name: "dependencies", color: "0366d6" }], }), ], @@ -546,6 +547,8 @@ describe("PullRequestsView", () => { await expect.element(page.getByText("Bump the runner")).toBeVisible(); // The triangle stands in for the open glyph, and says so in words. expect(page.getByText("Conflicts with main").elements()).toHaveLength(1); + // The standing instruction is a word on the meta line, not a glyph. + expect(page.getByText("auto-merge").elements()).toHaveLength(1); // The reviews and the checks are both glyphs, and each carries the words // the row no longer spends its meta line on. expect(page.getByText("Approved").elements()).toHaveLength(1); diff --git a/apps/web/src/components/pull-requests/PullRequestsView.tsx b/apps/web/src/components/pull-requests/PullRequestsView.tsx index 7540c747..3b2c1cea 100644 --- a/apps/web/src/components/pull-requests/PullRequestsView.tsx +++ b/apps/web/src/components/pull-requests/PullRequestsView.tsx @@ -79,6 +79,7 @@ import { narrowPullRequests, projectRepository, pullRequestBadgeTone, + pullRequestAutoMergeLabel, pullRequestConflictLabel, pullRequestEntryKey, pullRequestFilterChips, @@ -927,9 +928,13 @@ function PullRequestRow({ // opens it, since a glyph in a sibling is not part of that name: the state // word, then the conflict, then how the checks went. const checksLabel = pullRequestChecksTone(entry.checksState)?.label ?? null; + // Armed to land on its own is worth a word on an open row: it is the one that + // needs nobody to come back for it. + const autoMergeLabel = pullRequestAutoMergeLabel(entry); const rowLabel = `${[ `${glyphLabel} pull request #${entry.number}`, ...(conflictLabel ? [lowerFirst(conflictLabel)] : []), + ...(autoMergeLabel ? [lowerFirst(autoMergeLabel)] : []), ...(checksLabel ? [lowerFirst(checksLabel)] : []), ].join(", ")}: ${entry.title}`; const reviewTone = pullRequestReviewTone({ @@ -1014,6 +1019,22 @@ function PullRequestRow({ }, ] : []), + // The standing merge instruction, as a mono meta word like the number: it + // is a fact the host holds, not a state the glyphs beside it already say. + ...(autoMergeLabel === null + ? [] + : [ + { + key: "auto-merge", + fit: "whole" as const, + className: "font-mono", + content: ( + + {autoMergeLabel.toLowerCase()} + + ), + }, + ]), // Where the reviews stand and how the checks went, as two glyphs at the end // of the line: the same pair on every row, in every group, so the eye finds // them in the same place rather than reading a coloured word out of the diff --git a/apps/web/src/components/pull-requests/pullRequests.logic.ts b/apps/web/src/components/pull-requests/pullRequests.logic.ts index 00c07ab4..bc672067 100644 --- a/apps/web/src/components/pull-requests/pullRequests.logic.ts +++ b/apps/web/src/components/pull-requests/pullRequests.logic.ts @@ -1019,6 +1019,19 @@ export function pullRequestConflictLabel(entry: PullRequestEntry): string | null : null; } +/** + * The word for a row the host will merge on its own, or null where it will + * not. Only an open row can still be armed: the host drops the instruction the + * moment the pull request settles, and a draft cannot be armed at all. + */ +export function pullRequestAutoMergeLabel( + entry: Pick, +): string | null { + return entry.state === "open" && !entry.isDraft && entry.autoMergeEnabled === true + ? "Auto-merge" + : null; +} + /** One author the loaded rows have seen, and how many of them they wrote. */ export interface PullRequestAuthorFacet { readonly login: string; diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index 7ba2f5ab..fda22da6 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -93,6 +93,8 @@ export const PullRequestListEntry = Schema.Struct({ checksState: Schema.optionalKey(PullRequestChecksState), /** Absent where the host does not say whether the branch still merges. */ mergeability: Schema.optionalKey(PullRequestMergeability), + /** Armed to merge on its own once its requirements pass; absent where the host does not say. */ + autoMergeEnabled: Schema.optionalKey(Schema.Boolean), labels: Schema.Array(PullRequestLabel), origin: PullRequestListEntryOrigin, });