diff --git a/PRIVACY.md b/PRIVACY.md index 93b18463a2..695bff42d4 100644 --- a/PRIVACY.md +++ b/PRIVACY.md @@ -436,7 +436,10 @@ nothing of the file is stored. pull request as GitHub holds it, in two views. Each time a view first opens, or you refresh it, our service reads it from GitHub with a token the Luke GitHub App mints for that one repository on the workspace's installation. The overview -reads its title, branches, and whether it would merge; its description and up +reads its title, branches, and whether it would merge, with how it may be +merged: the merge methods and auto-merge its repository allows, whether a merge +queue merges its base and its place in that queue, whether auto-merge is on, +its head commit, what its required reviews say, and its checks as one word; its description and up to its 100 newest comments and reviews, each whole with its author, as GitHub renders them; the name, App, state, and times of up to 100 checks on its head; its reviewers; and up to its 100 newest review threads on lines, each with the @@ -450,7 +453,8 @@ Files changed reads each changed file's name and patch, for up to 300 files, leaving out a patch over 100,000 characters. It stores none of it, and Luke holds it in memory while the tab is open, never on disk, except that downloading a patch writes it where you save it. A button in the tab that -changes the pull request, such as Ready for review or Convert to draft, is +changes the pull request, such as Ready for review, Convert to draft, Merge, +or Enable auto-merge, is sent to GitHub by our service as you, on the token of your own GitHub sign-in, whoever started the agent, so it can do only what your GitHub account may; it is never sent as the Luke GitHub App, and where you hold no GitHub sign-in diff --git a/apps/desktop/src/renderer/styles/tasks.css b/apps/desktop/src/renderer/styles/tasks.css index 6f5e199722..521417dbaa 100644 --- a/apps/desktop/src/renderer/styles/tasks.css +++ b/apps/desktop/src/renderer/styles/tasks.css @@ -2036,6 +2036,63 @@ color: var(--merge-tone); } +.pull-request-merge-words { + min-width: 0; + flex: 1 1 auto; +} + +/* The merge box's presses at the line's end (`merge-box.tsx`): what stands + of a merge already asked for, quiet, then the one press, a method's + press joined to the chevron that chooses the method. */ +.merge-box-actions { + display: inline-flex; + flex: 0 0 auto; + flex-wrap: wrap; + align-items: center; + justify-content: flex-end; + gap: var(--luke-space-8); + margin-inline-start: auto; +} + +.merge-box-note { + color: var(--luke-color-text-muted); +} + +.merge-box-split { + display: inline-flex; + align-items: stretch; +} + +.merge-box-press { + height: 26px; + padding: 0 var(--luke-space-8); + gap: var(--luke-space-4); + font-size: 12px; +} + +.merge-box-press svg { + width: 13px; + height: 13px; +} + +.merge-box-split > .merge-box-press:not(:last-child) { + border-start-end-radius: 0; + border-end-end-radius: 0; +} + +.merge-box-picker { + height: 26px; + padding: 0 var(--luke-space-4); + border-start-start-radius: 0; + border-end-start-radius: 0; + margin-inline-start: -1px; +} + +.merge-box-picker svg { + width: 13px; + height: 13px; +} + [data-merge-tone="ready"] { --merge-tone: var(--luke-color-complete); } diff --git a/apps/desktop/src/renderer/tasks/agent-pull-request-tab.test.ts b/apps/desktop/src/renderer/tasks/agent-pull-request-tab.test.ts index 49416fd6a1..44c0b48bff 100644 --- a/apps/desktop/src/renderer/tasks/agent-pull-request-tab.test.ts +++ b/apps/desktop/src/renderer/tasks/agent-pull-request-tab.test.ts @@ -22,13 +22,17 @@ import { DEPLOYMENT_STATE, DIFF_SIDE, MERGE_STATE, + type MergeState, PULL_REQUEST_STATE, + type PullRequestMerging, type PullRequestState, + REVIEW_DECISION, REVIEW_STATE, type ReviewThread, } from "@luke/features/coding-agent-wire"; import { LINK_PLACE, type ReadLink } from "@luke/features/luke-link"; import { + MERGE_METHOD, PULL_REQUEST_ACTION, PULL_REQUEST_WRITE_OUTCOME, type PullRequestActionRequest, @@ -1576,6 +1580,196 @@ test("a change GitHub refused says so in GitHub's own words until dismissed, and assert.equal(tab.querySelector(".pull-request-write-note"), null); }); +/** How the pull request may be merged where a test says nothing else: squash or rebase, nothing asked for yet, checks passing. */ +const MERGING: PullRequestMerging = { + methods: [MERGE_METHOD.SQUASH, MERGE_METHOD.REBASE], + autoMergeAllowed: true, + queue: false, + queued: null, + autoMerge: null, + headSha: "c0ffee1", + review: null, + checks: CHECK_SUMMARY.PASSING, +}; + +/** The plan's agents, its pull request standing as given and merged as given. */ +function merging(merge: MergeState, merged: Partial = {}): CodingAgentsControl { + return controlAnswering({ + pullRequest: () => + Promise.resolve({ + pullRequest: { ...PULL_REQUEST, merge, merging: { ...MERGING, ...merged } }, + }), + }); +} + +/** The merge box's presses, by their words. */ +function boxPresses(tab: HTMLElement): string[] { + return words(tab, ".merge-box-press"); +} + +/** The merge box's press of the words given, pressed. */ +async function pressBox(tab: HTMLElement, label: string): Promise { + const press = + [...tab.querySelectorAll(".merge-box-press")].find( + (each) => each.textContent === label, + ) ?? assert.fail(`the merge box offers ${label}`); + act(() => press.click()); + await settle(); +} + +test("a pull request ready to merge offers Merge by the first method the repository allows, keeps the method chosen for the repository, and asks before merging against the head it read", async () => { + const first = await draw(merging(MERGE_STATE.CLEAN)); + assert.deepEqual(boxPresses(first), ["Squash and merge"]); + act(() => + ( + first.querySelector('[aria-label="Choose how to merge"]') ?? + assert.fail("the box offers the repository's other methods") + ).click(), + ); + const rebase = + [ + ...document.querySelectorAll( + '[role="menuitemcheckbox"], [role="menuitem"]', + ), + ].find((each) => each.textContent?.startsWith("Rebase and merge")) ?? + assert.fail("the methods are offered"); + act(() => rebase.click()); + assert.deepEqual(boxPresses(first), ["Rebase and merge"]); + act(() => root?.unmount()); + root = undefined; + + const again = await draw(merging(MERGE_STATE.CLEAN)); + assert.deepEqual(boxPresses(again), ["Rebase and merge"], "the method chosen is kept"); + await pressBox(again, "Rebase and merge"); + assert.deepEqual(changes, [], "a merge asks first"); + const dialog = + document.querySelector('[role="dialog"], [role="alertdialog"]') ?? + assert.fail("the merge is asked about"); + const merge = + [...dialog.querySelectorAll("button")].find( + (each) => each.textContent === "Rebase and merge", + ) ?? assert.fail("the dialog offers the merge"); + act(() => merge.click()); + await settle(); + assert.deepEqual(changes, [ + [ + AGENT.id, + { action: PULL_REQUEST_ACTION.MERGE, method: MERGE_METHOD.REBASE, headSha: "c0ffee1" }, + ], + ]); +}); + +test("auto-merge is offered while checks run, and turned off once on; a merge queue's base offers Merge when ready, and a queued pull request Remove from queue; a conflict offers nothing", async () => { + const cases: readonly (readonly [CodingAgentsControl, string, PullRequestActionRequest])[] = [ + [ + merging(MERGE_STATE.BLOCKED, { checks: CHECK_SUMMARY.PENDING }), + "Enable auto-merge", + { + action: PULL_REQUEST_ACTION.ENABLE_AUTO_MERGE, + method: MERGE_METHOD.SQUASH, + headSha: "c0ffee1", + }, + ], + [ + merging(MERGE_STATE.BLOCKED, { autoMerge: { method: MERGE_METHOD.SQUASH } }), + "Disable auto-merge", + { action: PULL_REQUEST_ACTION.DISABLE_AUTO_MERGE }, + ], + [ + merging(MERGE_STATE.CLEAN, { queue: true }), + "Merge when ready", + { action: PULL_REQUEST_ACTION.ENABLE_AUTO_MERGE, headSha: "c0ffee1" }, + ], + [ + merging(MERGE_STATE.CLEAN, { + queue: true, + queued: { position: 2 }, + autoMerge: { method: null }, + }), + "Remove from queue", + { action: PULL_REQUEST_ACTION.DEQUEUE }, + ], + ]; + for (const [control, label, request] of cases) { + changes = []; + const tab = await draw(control); + assert.deepEqual(boxPresses(tab), [label]); + await pressBox(tab, label); + assert.deepEqual(changes, [[AGENT.id, request]]); + act(() => root?.unmount()); + root = undefined; + } + + const blocked = await draw( + merging(MERGE_STATE.BLOCKED, { + review: REVIEW_DECISION.REVIEW_REQUIRED, + checks: CHECK_SUMMARY.PENDING, + }), + ); + assert.match( + blocked.querySelector(".pull-request-merge")?.textContent ?? "", + /A required review is missing/u, + ); + act(() => root?.unmount()); + root = undefined; + const conflicted = await draw(merging(MERGE_STATE.CONFLICTS)); + assert.deepEqual(boxPresses(conflicted), []); +}); + +test("a merge asked about merges the head it was asked about, though a read landing under the question names a newer one", async () => { + let reads = 0; + const tab = await draw( + controlAnswering({ + pullRequest: () => { + reads += 1; + const headSha = reads === 1 ? "c0ffee1" : "beef002"; + return Promise.resolve({ + pullRequest: { + ...PULL_REQUEST, + merge: MERGE_STATE.CLEAN, + merging: { ...MERGING, headSha }, + }, + }); + }, + }), + ); + await pressBox(tab, "Squash and merge"); + const dialog = + document.querySelector('[role="dialog"], [role="alertdialog"]') ?? + assert.fail("the merge is asked about"); + assert.match(dialog.textContent ?? "", /at commit c0ffee1/u); + + act(() => + ( + tab.querySelector('[aria-label="Pull request actions"]') ?? + assert.fail("the bar has a ⋯") + ).click(), + ); + const refresh = + [...document.querySelectorAll('[role="menuitem"]')].find( + (each) => each.textContent === "Refresh pull request", + ) ?? assert.fail("the ⋯ offers Refresh"); + await act(async () => refresh.click()); + await settle(); + assert.ok(reads > 1, "a newer head was read under the question"); + + const merge = + [ + ...document.querySelectorAll( + '[role="dialog"] button, [role="alertdialog"] button', + ), + ].find((each) => each.textContent === "Squash and merge") ?? + assert.fail("the dialog still offers the merge"); + act(() => merge.click()); + await settle(); + assert.deepEqual(changes, [ + [ + AGENT.id, + { action: PULL_REQUEST_ACTION.MERGE, method: MERGE_METHOD.SQUASH, headSha: "c0ffee1" }, + ], + ]); +}); + /** The bar's Review press. */ function reviewPress(tab: HTMLElement): HTMLButtonElement { return ( diff --git a/apps/desktop/src/renderer/tasks/agent-pull-request-tab.tsx b/apps/desktop/src/renderer/tasks/agent-pull-request-tab.tsx index 03a5097490..7f9192b3ec 100644 --- a/apps/desktop/src/renderer/tasks/agent-pull-request-tab.tsx +++ b/apps/desktop/src/renderer/tasks/agent-pull-request-tab.tsx @@ -87,6 +87,7 @@ import { } from "./conversation-filters"; import { PierreDiff, useDiffLook } from "./file-diff"; import { useFileViewSettings } from "./file-view-settings"; +import { blockedWords, MergeActions } from "./merge-box"; import { PullRequestLabels } from "./pull-request-labels"; import { HeaderReviewers, OpenedBy, PersonRef, ReviewStanding } from "./pull-request-people"; import { PullRequestReview } from "./pull-request-review"; @@ -705,17 +706,23 @@ function Checks({ /** Whether the pull request would merge as it stands, in its tone's colour; drawn only while it is open. */ function MergeLine({ pullRequest, + actions, }: { pullRequest: CodingAgentOverviewPullRequest; + /** The merge box's presses at the line's end (`merge-box.tsx`). */ + actions: ReactNode; }): React.JSX.Element { const { line, tone } = mergeWords(pullRequest.merge, pullRequest.base); + const blocked = + pullRequest.merge === MERGE_STATE.BLOCKED ? blockedWords(pullRequest.merging) : undefined; return ( -

+

{tone === MERGE_TONE.READY ?
); } @@ -1002,7 +1009,18 @@ interface OverviewParts { } /** Under the title: its labels, the branches and the stack it stands in, who opened it and when, its reviewers, and, while it is open, whether it merges; a skeleton while its read is out, and its failure with Retry. */ -function PullRequestHead({ parts, now }: { parts: OverviewParts; now: number }): React.JSX.Element { +function PullRequestHead({ + parts, + now, + repository, + writes, +}: { + parts: OverviewParts; + now: number; + /** The agent's repository, which the merge box keeps the method chosen by. */ + repository: string | null; + writes: PullRequestWrites; +}): React.JSX.Element { const { pullRequest: part, conversation } = parts; if (part.kind === PART.PENDING) { return ( @@ -1047,7 +1065,14 @@ function PullRequestHead({ parts, now }: { parts: OverviewParts; now: number }): {reviewers.length === 0 ? null : } - {open ? : null} + {open ? ( + + } + /> + ) : null} ); } @@ -1621,7 +1646,7 @@ export function AgentPullRequestTab({ <>
- +
label="Pull request" diff --git a/apps/desktop/src/renderer/tasks/merge-box.tsx b/apps/desktop/src/renderer/tasks/merge-box.tsx new file mode 100644 index 0000000000..f0d5f084b2 --- /dev/null +++ b/apps/desktop/src/renderer/tasks/merge-box.tsx @@ -0,0 +1,288 @@ +import { + CHECK_SUMMARY, + type CodingAgentOverviewPullRequest, + MERGE_STATE, + type MergeState, + PULL_REQUEST_STATE, + type PullRequestMerging, + REVIEW_DECISION, +} from "@luke/features/coding-agent-wire"; +import { + MERGE_METHOD, + type MergeMethod, + PULL_REQUEST_ACTION, +} from "@luke/features/pull-request-write-wire"; +import { ConfirmDialog, DIALOG_TONE, useConfirmDialog, useMenu } from "@luke/ui"; +import { ACTION_RESULT_STATUS } from "@luke/wire"; +import { Schema } from "effect"; +import { ChevronDownIcon, GitMergeIcon } from "lucide-react"; +import { useState } from "react"; +import { ActionMenu, type MenuAction } from "../desktop/action-menu"; +import { readKept, writeKept } from "../desktop/folded-groups"; +import type { PullRequestWrites } from "./pull-request-writes"; + +/** + * merge-box.tsx -- the merge box's presses, at the end of the line saying whether an open pull request would merge, as stage-cli's merge status offers them. + * + * The box offers the one thing that may be done from where the pull request + * stands (`mergeActionOf`), each a change the service makes on GitHub as the + * member pressing it (`pull-request-writes.tsx`), and draws nothing where + * the overview did not say how it may be merged: + * + * - in the merge queue, Remove from queue; + * - with auto-merge on, Disable auto-merge; + * - on a base merged through a merge queue, Merge when ready, which turns + * auto-merge on with no method, so GitHub adds it to the queue once it is + * ready, as `gh pr merge --auto` does; + * - ready to merge, Merge by the method chosen among those the repository + * allows, asked first since a merge cannot be undone from here; + * - with checks still running and auto-merge allowed, Enable auto-merge by + * the method chosen. + * + * The method chosen is kept per repository across launches, in this window's + * own storage as the sidebar's folds are (`folded-groups.ts`), and the first + * the repository allows stands until one is chosen. A merge and auto-merge + * name the head the overview read, so GitHub refuses either against a head + * pushed since. The line's own words say what a merge waits on where GitHub + * says it is blocked (`blockedWords`). + */ + +/** What the box offers from where the pull request stands. */ +const MERGE_ACTION = { + MERGE: "merge", + MERGE_WHEN_READY: "merge-when-ready", + ENABLE_AUTO_MERGE: "enable-auto-merge", + DISABLE_AUTO_MERGE: "disable-auto-merge", + DEQUEUE: "dequeue", +} as const; + +type MergeAction = (typeof MERGE_ACTION)[keyof typeof MERGE_ACTION]; + +/** Each method's press, as GitHub words it. */ +const METHOD_WORDS = { + [MERGE_METHOD.MERGE]: "Create a merge commit", + [MERGE_METHOD.SQUASH]: "Squash and merge", + [MERGE_METHOD.REBASE]: "Rebase and merge", +} as const satisfies Record; + +/** Each method's word where it says how auto-merge will merge. */ +const METHOD_NOUNS = { + [MERGE_METHOD.MERGE]: "merge commit", + [MERGE_METHOD.SQUASH]: "squash", + [MERGE_METHOD.REBASE]: "rebase", +} as const satisfies Record; + +/** The box's own words. */ +const BOX_WORDS = { + MERGE_WHEN_READY: "Merge when ready", + ENABLE_AUTO_MERGE: "Enable auto-merge", + DISABLE_AUTO_MERGE: "Disable auto-merge", + DEQUEUE: "Remove from queue", + CHOOSE_METHOD: "Choose how to merge", + queued: (position: number) => `#${position} in the merge queue`, + autoMerge: (method: MergeMethod | null) => + method === null ? "Auto-merge is on" : `Auto-merge is on (${METHOD_NOUNS[method]})`, +} as const; + +/** The states GitHub would merge in now: ready, or with checks nothing requires short of passing. */ +const MERGEABLE: ReadonlySet = new Set([MERGE_STATE.CLEAN, MERGE_STATE.UNSTABLE]); + +/** The one thing the box offers from where the pull request stands, or nothing. */ +function mergeActionOf(merge: MergeState, merging: PullRequestMerging): MergeAction | undefined { + if (merging.queued !== null) return MERGE_ACTION.DEQUEUE; + if (merging.autoMerge !== null) return MERGE_ACTION.DISABLE_AUTO_MERGE; + if (merge === MERGE_STATE.CONFLICTS || merge === MERGE_STATE.DRAFT) return undefined; + if (merging.queue) return MERGE_ACTION.MERGE_WHEN_READY; + if (merging.methods.length === 0) return undefined; + if (MERGEABLE.has(merge)) return MERGE_ACTION.MERGE; + const waiting = merging.checks === CHECK_SUMMARY.PENDING; + return merging.autoMergeAllowed && waiting ? MERGE_ACTION.ENABLE_AUTO_MERGE : undefined; +} + +/** What a merge GitHub says is blocked waits on, where the reviews or checks say; none where neither does. */ +export function blockedWords(merging: PullRequestMerging | undefined): string | undefined { + if (merging === undefined) return undefined; + if (merging.review === REVIEW_DECISION.CHANGES_REQUESTED) return "Changes were requested"; + if (merging.review === REVIEW_DECISION.REVIEW_REQUIRED) return "A required review is missing"; + if (merging.checks === CHECK_SUMMARY.PENDING) return "Waiting on required checks"; + if (merging.checks === CHECK_SUMMARY.FAILING) return "Required checks are failing"; + return undefined; +} + +/** Where the method chosen is kept, by repository. */ +const METHODS_KEY = "luke.pullRequestMergeMethods"; + +const storedMethods = Schema.fromJsonString( + Schema.Record(Schema.String, Schema.Literals(Object.values(MERGE_METHOD))), +); + +/** The method chosen for the repository, kept across launches: the one last chosen where the repository still allows it, else the first it allows. */ +function useMergeMethod( + repository: string | null, + allowed: readonly MergeMethod[], +): readonly [MergeMethod | undefined, (method: MergeMethod) => void] { + const [kept, setKept] = useState(() => readKept(METHODS_KEY, storedMethods, {})); + const chosen = repository === null ? undefined : kept[repository]; + const method = chosen !== undefined && allowed.includes(chosen) ? chosen : allowed[0]; + const choose = (next: MergeMethod) => { + if (repository === null) return; + const now = { ...readKept(METHODS_KEY, storedMethods, {}), [repository]: next }; + writeKept(METHODS_KEY, storedMethods, now); + setKept(now); + }; + return [method, choose]; +} + +/** The chevron beside a press that names a method, opening the methods the repository allows. */ +function MethodPicker({ + methods, + method, + disabled, + onChoose, +}: { + methods: readonly MergeMethod[]; + method: MergeMethod; + disabled: boolean; + onChoose: (method: MergeMethod) => void; +}): React.JSX.Element { + const menu = useMenu(true); + const items: MenuAction[] = methods.map((each) => ({ + label: METHOD_WORDS[each], + checked: each === method, + onSelect: () => onChoose(each), + })); + return ( + <> + + {menu.drawn === undefined ? null : ( + + )} + + ); +} + +/** + * The box's presses for an open pull request, with what stands of a merge + * already asked for: its place in the queue, or the auto-merge on. + */ +export function MergeActions({ + pullRequest, + repository, + writes, +}: { + pullRequest: CodingAgentOverviewPullRequest; + /** The agent's repository, which the method chosen is kept by. */ + repository: string | null; + writes: PullRequestWrites; +}): React.JSX.Element | null { + const { merging } = pullRequest; + const [method, choose] = useMergeMethod(repository, merging?.methods ?? []); + // The merge asked about, as it stood when Merge was pressed: a read landing under the question never changes what it merges. + const [asked, setAsked] = useState<{ method: MergeMethod; headSha: string } | undefined>(); + const confirm = useConfirmDialog( + { subject: asked !== undefined, surfaceOpen: true }, + async () => { + if (asked !== undefined) await writes.write({ action: PULL_REQUEST_ACTION.MERGE, ...asked }); + return { status: ACTION_RESULT_STATUS.ACCEPTED }; + }, + ); + if (merging === undefined || pullRequest.state !== PULL_REQUEST_STATE.OPEN) return null; + const action = mergeActionOf(pullRequest.merge, merging); + const busy = writes.pending !== undefined; + const { headSha } = merging; + const picker = + method !== undefined && merging.methods.length > 1 ? ( + + ) : null; + const press = (label: string, onPress: () => void, primary: boolean) => ( + + ); + return ( + + {merging.queued === null ? null : ( + {BOX_WORDS.queued(merging.queued.position)} + )} + {merging.autoMerge === null || merging.queued !== null ? null : ( + {BOX_WORDS.autoMerge(merging.autoMerge.method)} + )} + {action === MERGE_ACTION.DEQUEUE + ? press( + BOX_WORDS.DEQUEUE, + () => void writes.write({ action: PULL_REQUEST_ACTION.DEQUEUE }), + false, + ) + : null} + {action === MERGE_ACTION.DISABLE_AUTO_MERGE + ? press( + BOX_WORDS.DISABLE_AUTO_MERGE, + () => void writes.write({ action: PULL_REQUEST_ACTION.DISABLE_AUTO_MERGE }), + false, + ) + : null} + {action === MERGE_ACTION.MERGE_WHEN_READY + ? press( + BOX_WORDS.MERGE_WHEN_READY, + () => void writes.write({ action: PULL_REQUEST_ACTION.ENABLE_AUTO_MERGE, headSha }), + true, + ) + : null} + {action === MERGE_ACTION.MERGE && method !== undefined ? ( + + {press( + METHOD_WORDS[method], + () => { + setAsked({ method, headSha }); + confirm.ask(); + }, + true, + )} + {picker} + + ) : null} + {action === MERGE_ACTION.ENABLE_AUTO_MERGE && method !== undefined ? ( + + {press( + BOX_WORDS.ENABLE_AUTO_MERGE, + () => + void writes.write({ action: PULL_REQUEST_ACTION.ENABLE_AUTO_MERGE, method, headSha }), + false, + )} + {picker} + + ) : null} + {asked === undefined ? null : ( + + )} + + ); +} diff --git a/apps/desktop/src/renderer/tasks/pull-request-writes.tsx b/apps/desktop/src/renderer/tasks/pull-request-writes.tsx index ec595a02bb..c72677c151 100644 --- a/apps/desktop/src/renderer/tasks/pull-request-writes.tsx +++ b/apps/desktop/src/renderer/tasks/pull-request-writes.tsx @@ -55,6 +55,10 @@ const ACTION_WORDS = { [PULL_REQUEST_ACTION.REPLY_TO_THREAD]: "Reply", [PULL_REQUEST_ACTION.RESOLVE_THREAD]: "Resolve conversation", [PULL_REQUEST_ACTION.UNRESOLVE_THREAD]: "Unresolve conversation", + [PULL_REQUEST_ACTION.MERGE]: "Merge", + [PULL_REQUEST_ACTION.ENABLE_AUTO_MERGE]: "Enable auto-merge", + [PULL_REQUEST_ACTION.DISABLE_AUTO_MERGE]: "Disable auto-merge", + [PULL_REQUEST_ACTION.DEQUEUE]: "Remove from queue", } as const satisfies Record; /** The word for where the pull request stands, which the pill says. */ @@ -101,8 +105,12 @@ export interface PullRequestWrites { /** The change out now; none while nothing is. */ readonly pending: PullRequestAction | undefined; readonly note: WriteNote | undefined; - /** Sends the change; `onDone` hears once GitHub made it, so the press that sent it can let go of what it held. */ - readonly write: (request: PullRequestActionRequest, onDone?: () => void) => void; + /** + * Sends the change; `onDone` hears once GitHub made it, so the press that + * sent it can let go of what it held, and what it answers settles once + * GitHub answered and what it said is drawn, as a merge's question waits. + */ + readonly write: (request: PullRequestActionRequest, onDone?: () => void) => Promise; readonly dismiss: () => void; } @@ -133,11 +141,11 @@ export function usePullRequestWrites(agentId: string, onWritten: () => void): Pu const out = useRef(false); const write = useCallback( (request: PullRequestActionRequest, onDone?: () => void) => { - if (out.current) return; + if (out.current) return Promise.resolve(); out.current = true; setPending(request.action); setNote(undefined); - void act(ACT_KIND.CODING_AGENTS_PULL_REQUEST_ACTION, { agentId, request }) + return act(ACT_KIND.CODING_AGENTS_PULL_REQUEST_ACTION, { agentId, request }) .catch( (): CodingAgentPullRequestWriteAnswerView => ({ failure: CODING_AGENT_CALL_FAILURE.UNANSWERED, diff --git a/packages/features/fixtures/json-schema/coding-agent-view-codingAgentOverviewPullRequestAnswerViewSchema.json b/packages/features/fixtures/json-schema/coding-agent-view-codingAgentOverviewPullRequestAnswerViewSchema.json index c9d091faa4..6f6302c995 100644 --- a/packages/features/fixtures/json-schema/coding-agent-view-codingAgentOverviewPullRequestAnswerViewSchema.json +++ b/packages/features/fixtures/json-schema/coding-agent-view-codingAgentOverviewPullRequestAnswerViewSchema.json @@ -123,6 +123,118 @@ }, "maxItems": 50 }, + "merging": { + "type": "object", + "properties": { + "methods": { + "type": "array", + "items": { + "type": "string", + "enum": [ + "merge", + "squash", + "rebase" + ] + } + }, + "autoMergeAllowed": { + "type": "boolean" + }, + "queue": { + "type": "boolean" + }, + "queued": { + "anyOf": [ + { + "type": "object", + "properties": { + "position": { + "type": "number", + "minimum": 0 + } + }, + "required": [ + "position" + ], + "additionalProperties": false + }, + { + "type": "null" + } + ] + }, + "autoMerge": { + "anyOf": [ + { + "type": "object", + "properties": { + "method": { + "anyOf": [ + { + "type": "string", + "enum": [ + "merge", + "squash", + "rebase" + ] + }, + { + "type": "null" + } + ] + } + }, + "required": [ + "method" + ], + "additionalProperties": false + }, + { + "type": "null" + } + ] + }, + "headSha": { + "type": "string", + "maxLength": 64 + }, + "review": { + "anyOf": [ + { + "type": "string", + "enum": [ + "approved", + "changes-requested", + "review-required" + ] + }, + { + "type": "null" + } + ] + }, + "checks": { + "type": "string", + "enum": [ + "pending", + "passing", + "failing", + "none" + ] + } + }, + "required": [ + "methods", + "autoMergeAllowed", + "queue", + "queued", + "autoMerge", + "headSha", + "review", + "checks" + ], + "additionalProperties": false + }, "labels": { "type": "array", "items": { diff --git a/packages/features/fixtures/json-schema/coding-agent-view-codingAgentPullRequestActionParamsSchema.json b/packages/features/fixtures/json-schema/coding-agent-view-codingAgentPullRequestActionParamsSchema.json index 6d86ca7214..066854ea4d 100644 --- a/packages/features/fixtures/json-schema/coding-agent-view-codingAgentPullRequestActionParamsSchema.json +++ b/packages/features/fixtures/json-schema/coding-agent-view-codingAgentPullRequestActionParamsSchema.json @@ -37,6 +37,93 @@ ], "additionalProperties": false }, + { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "merge" + ] + }, + "method": { + "type": "string", + "enum": [ + "merge", + "squash", + "rebase" + ] + }, + "headSha": { + "type": "string", + "maxLength": 64 + } + }, + "required": [ + "action", + "method", + "headSha" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "enable-auto-merge" + ] + }, + "method": { + "type": "string", + "enum": [ + "merge", + "squash", + "rebase" + ] + }, + "headSha": { + "type": "string", + "maxLength": 64 + } + }, + "required": [ + "action", + "headSha" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "disable-auto-merge" + ] + } + }, + "required": [ + "action" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "dequeue" + ] + } + }, + "required": [ + "action" + ], + "additionalProperties": false + }, { "type": "object", "properties": { diff --git a/packages/features/fixtures/json-schema/coding-agent-wire-codingAgentOverviewPullRequestAnswerSchema.json b/packages/features/fixtures/json-schema/coding-agent-wire-codingAgentOverviewPullRequestAnswerSchema.json index 684d18d4ac..4a13170353 100644 --- a/packages/features/fixtures/json-schema/coding-agent-wire-codingAgentOverviewPullRequestAnswerSchema.json +++ b/packages/features/fixtures/json-schema/coding-agent-wire-codingAgentOverviewPullRequestAnswerSchema.json @@ -121,6 +121,118 @@ }, "maxItems": 50 }, + "merging": { + "type": "object", + "properties": { + "methods": { + "type": "array", + "items": { + "type": "string", + "enum": [ + "merge", + "squash", + "rebase" + ] + } + }, + "autoMergeAllowed": { + "type": "boolean" + }, + "queue": { + "type": "boolean" + }, + "queued": { + "anyOf": [ + { + "type": "object", + "properties": { + "position": { + "type": "number", + "minimum": 0 + } + }, + "required": [ + "position" + ], + "additionalProperties": false + }, + { + "type": "null" + } + ] + }, + "autoMerge": { + "anyOf": [ + { + "type": "object", + "properties": { + "method": { + "anyOf": [ + { + "type": "string", + "enum": [ + "merge", + "squash", + "rebase" + ] + }, + { + "type": "null" + } + ] + } + }, + "required": [ + "method" + ], + "additionalProperties": false + }, + { + "type": "null" + } + ] + }, + "headSha": { + "type": "string", + "maxLength": 64 + }, + "review": { + "anyOf": [ + { + "type": "string", + "enum": [ + "approved", + "changes-requested", + "review-required" + ] + }, + { + "type": "null" + } + ] + }, + "checks": { + "type": "string", + "enum": [ + "pending", + "passing", + "failing", + "none" + ] + } + }, + "required": [ + "methods", + "autoMergeAllowed", + "queue", + "queued", + "autoMerge", + "headSha", + "review", + "checks" + ], + "additionalProperties": false + }, "labels": { "type": "array", "items": { diff --git a/packages/features/fixtures/json-schema/pull-request-write-wire-mergeMethodSchema.json b/packages/features/fixtures/json-schema/pull-request-write-wire-mergeMethodSchema.json new file mode 100644 index 0000000000..31166e6eff --- /dev/null +++ b/packages/features/fixtures/json-schema/pull-request-write-wire-mergeMethodSchema.json @@ -0,0 +1,8 @@ +{ + "type": "string", + "enum": [ + "merge", + "squash", + "rebase" + ] +} diff --git a/packages/features/fixtures/json-schema/pull-request-write-wire-pullRequestActionRequestSchema.json b/packages/features/fixtures/json-schema/pull-request-write-wire-pullRequestActionRequestSchema.json index a980a71697..91279f49db 100644 --- a/packages/features/fixtures/json-schema/pull-request-write-wire-pullRequestActionRequestSchema.json +++ b/packages/features/fixtures/json-schema/pull-request-write-wire-pullRequestActionRequestSchema.json @@ -30,6 +30,93 @@ ], "additionalProperties": false }, + { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "merge" + ] + }, + "method": { + "type": "string", + "enum": [ + "merge", + "squash", + "rebase" + ] + }, + "headSha": { + "type": "string", + "maxLength": 64 + } + }, + "required": [ + "action", + "method", + "headSha" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "enable-auto-merge" + ] + }, + "method": { + "type": "string", + "enum": [ + "merge", + "squash", + "rebase" + ] + }, + "headSha": { + "type": "string", + "maxLength": 64 + } + }, + "required": [ + "action", + "headSha" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "disable-auto-merge" + ] + } + }, + "required": [ + "action" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "dequeue" + ] + } + }, + "required": [ + "action" + ], + "additionalProperties": false + }, { "type": "object", "properties": { diff --git a/packages/features/src/coding-agent-wire.ts b/packages/features/src/coding-agent-wire.ts index bb56815014..90a16df1f2 100644 --- a/packages/features/src/coding-agent-wire.ts +++ b/packages/features/src/coding-agent-wire.ts @@ -3,6 +3,7 @@ import { readEither, verbatimJsonSchema, wireKeys } from "@luke/wire/effect"; import { Schema as EffectSchema, Result, SchemaTransformation } from "effect"; import { integrationConnectionNameSchema, integrationConnectionOf } from "./integration-wire.js"; import { pinPositionSchema } from "./pin-order.js"; +import { mergeMethodSchema, PULL_REQUEST_WRITE_BOUNDS } from "./pull-request-write-wire.js"; import { countedNumber, memberRefSchema, wireUuidSchema } from "./service-wire.js"; /** @@ -953,6 +954,39 @@ export const MERGE_STATE = { export type MergeState = (typeof MERGE_STATE)[keyof typeof MERGE_STATE]; +/** What the reviews a branch requires say of a pull request, as GitHub sums them up. */ +export const REVIEW_DECISION = { + APPROVED: "approved", + CHANGES_REQUESTED: "changes-requested", + REVIEW_REQUIRED: "review-required", +} as const; + +export type ReviewDecision = (typeof REVIEW_DECISION)[keyof typeof REVIEW_DECISION]; + +/** + * How an open pull request may be merged, as its merge box offers it: the + * methods the repository allows, whether it allows auto-merge, whether its + * base is merged through a merge queue, where the pull request stands in that + * queue or whether auto-merge is on for it, by which method, the head a merge + * is made against, what its required reviews say, and its checks as one word. + */ +const pullRequestMergingSchema = EffectSchema.Struct({ + methods: EffectSchema.Array(mergeMethodSchema), + autoMergeAllowed: EffectSchema.Boolean, + queue: EffectSchema.Boolean, + queued: EffectSchema.NullOr(EffectSchema.Struct({ position: countedNumber })), + autoMerge: EffectSchema.NullOr( + EffectSchema.Struct({ method: EffectSchema.NullOr(mergeMethodSchema) }), + ), + headSha: EffectSchema.String.check( + EffectSchema.isMaxLength(PULL_REQUEST_WRITE_BOUNDS.MAX_COMMIT_CHARS), + ), + review: EffectSchema.NullOr(EffectSchema.Literals(Object.values(REVIEW_DECISION))), + checks: EffectSchema.Literals(Object.values(CHECK_SUMMARY)), +}); + +export type PullRequestMerging = typeof pullRequestMergingSchema.Type; + /** Where one check on a pull request's head stands: running or queued, passed, failed, or skipped or neutral, which passes nothing and fails nothing. */ export const CHECK_STATE = { PENDING: "pending", @@ -1182,6 +1216,8 @@ export const codingAgentOverviewPullRequestAnswerSchema = EffectSchema.Struct({ requested: EffectSchema.Array(githubPersonSchema).check( EffectSchema.isMaxLength(CHANGES_BOUNDS.MAX_PEOPLE), ), + /** How it may be merged; absent where GitHub did not say, or from a service before the merge box. */ + merging: EffectSchema.optionalKey(pullRequestMergingSchema), labels: EffectSchema.optionalKey( EffectSchema.Array(pullRequestLabelSchema).check( EffectSchema.isMaxLength(OVERVIEW_BOUNDS.MAX_LABELS), diff --git a/packages/features/src/hosted-wire-schemas.test.ts b/packages/features/src/hosted-wire-schemas.test.ts index 25df3d1bf3..b88a411d33 100644 --- a/packages/features/src/hosted-wire-schemas.test.ts +++ b/packages/features/src/hosted-wire-schemas.test.ts @@ -432,6 +432,7 @@ const EFFECT_MODULE_SCHEMAS = { providerKeySaveRequestSchema: providerKeyWire.providerKeySaveRequestSchema, } satisfies RecordedEffectJsonSchemas, "pull-request-write-wire": { + mergeMethodSchema: pullRequestWriteWire.mergeMethodSchema, pullRequestActionRequestSchema: pullRequestWriteWire.pullRequestActionRequestSchema, pullRequestWriteAnswerSchema: pullRequestWriteWire.pullRequestWriteAnswerSchema, } satisfies RecordedEffectJsonSchemas, diff --git a/packages/features/src/pull-request-write-wire.ts b/packages/features/src/pull-request-write-wire.ts index 217649db50..ffcf070b69 100644 --- a/packages/features/src/pull-request-write-wire.ts +++ b/packages/features/src/pull-request-write-wire.ts @@ -10,7 +10,9 @@ import { Schema as EffectSchema } from "effect"; * answer says it was made, that the member must sign in with GitHub again * because they hold no sign-in GitHub takes, or that GitHub refused the * change itself, in GitHub's own words: a branch protection, a conflict, or - * a permission the member's account does not hold. A refusal of Luke's own, + * a permission the member's account does not hold. A merge, and turning + * auto-merge on, name the head commit the member saw, so GitHub refuses one + * made against a head pushed since. A refusal of Luke's own, * an agent out of reach or a service that could not answer, travels as the * hosted vocabulary's refusal as every other call's does. */ @@ -18,18 +20,49 @@ import { Schema as EffectSchema } from "effect"; export const PULL_REQUEST_WRITE_BOUNDS = { /** The most of GitHub's own words on a refused change the answer carries. */ MAX_REFUSAL_CHARS: 1_000, + /** A commit's id as GitHub spells one: forty hex digits for SHA-1, sixty-four for SHA-256. */ + MAX_COMMIT_CHARS: 64, /** The longest body a review may carry, GitHub's own bound on a comment's. */ MAX_BODY_CHARS: 65_536, /** The longest GitHub id for a review thread the request names; GitHub's run near thirty. */ MAX_NODE_ID_CHARS: 256, } as const; +/** How a pull request is merged, as GitHub offers each where the repository allows it. */ +export const MERGE_METHOD = { + MERGE: "merge", + SQUASH: "squash", + REBASE: "rebase", +} as const; + +export type MergeMethod = (typeof MERGE_METHOD)[keyof typeof MERGE_METHOD]; + +export const mergeMethodSchema = EffectSchema.Literals(Object.values(MERGE_METHOD)); + +/** The head commit a merge is made against, as GitHub spells it. */ +const headShaSchema = EffectSchema.String.check( + EffectSchema.isMaxLength(PULL_REQUEST_WRITE_BOUNDS.MAX_COMMIT_CHARS), + EffectSchema.isPattern(/^[0-9a-f]+$/u), +); + /** The changes the Pull request tab makes, each one press. */ export const PULL_REQUEST_ACTION = { /** A draft marked ready for review. */ READY_FOR_REVIEW: "ready-for-review", /** An open pull request turned back into a draft. */ CONVERT_TO_DRAFT: "convert-to-draft", + /** Merged now, by the method named, against the head named. */ + MERGE: "merge", + /** + * Merged by GitHub once what it waits on passes, against the head named: by + * the method named, or, on a base merged through a merge queue, added to + * the queue once ready, by the queue's own method. + */ + ENABLE_AUTO_MERGE: "enable-auto-merge", + /** Auto-merge turned off again. */ + DISABLE_AUTO_MERGE: "disable-auto-merge", + /** Taken out of the merge queue. */ + DEQUEUE: "dequeue", /** A review submitted: the member's own pending review where they hold one, else a new one. */ SUBMIT_REVIEW: "submit-review", /** A reply on a review thread of the pull request. */ @@ -69,6 +102,18 @@ const replyBodySchema = reviewBodySchema.check(EffectSchema.isPattern(/\S/u)); export const pullRequestActionRequestSchema = EffectSchema.Union([ EffectSchema.Struct({ action: EffectSchema.Literal(PULL_REQUEST_ACTION.READY_FOR_REVIEW) }), EffectSchema.Struct({ action: EffectSchema.Literal(PULL_REQUEST_ACTION.CONVERT_TO_DRAFT) }), + EffectSchema.Struct({ + action: EffectSchema.Literal(PULL_REQUEST_ACTION.MERGE), + method: mergeMethodSchema, + headSha: headShaSchema, + }), + EffectSchema.Struct({ + action: EffectSchema.Literal(PULL_REQUEST_ACTION.ENABLE_AUTO_MERGE), + method: EffectSchema.optionalKey(mergeMethodSchema), + headSha: headShaSchema, + }), + EffectSchema.Struct({ action: EffectSchema.Literal(PULL_REQUEST_ACTION.DISABLE_AUTO_MERGE) }), + EffectSchema.Struct({ action: EffectSchema.Literal(PULL_REQUEST_ACTION.DEQUEUE) }), EffectSchema.Struct({ action: EffectSchema.Literal(PULL_REQUEST_ACTION.SUBMIT_REVIEW), event: EffectSchema.Literals(Object.values(REVIEW_EVENT)), diff --git a/packages/service/src/agents/coding-agents-app.test.ts b/packages/service/src/agents/coding-agents-app.test.ts index d4dca4edb1..45349eec1c 100644 --- a/packages/service/src/agents/coding-agents-app.test.ts +++ b/packages/service/src/agents/coding-agents-app.test.ts @@ -113,6 +113,7 @@ import { codingAgentAnswerSchema, codingAgentListAnswerSchema, codingAgentMessagesAnswerSchema, + codingAgentOverviewPullRequestAnswerSchema, codingAgentPageAnswerSchema, codingAgentPullRequestAnswerSchema, PULL_REQUEST_STATE, @@ -191,6 +192,7 @@ import { tasksApp } from "../tasks/tasks-app.js"; import { connectWorkspaceGitHub, type FakeGitHub, + GITHUB_FIXTURE_INSTALLATION_TOKEN, githubReaching, type InstallationFixture, openGithubUser, @@ -1282,6 +1284,7 @@ it.layer(getTestDB())("the coding-agent routes", (it) => { const AGENT_PULL_REQUEST_ACTION = "/api/agents/pull-request-action"; const AGENT_PULL_REQUEST = "/api/agents/pull-request"; +const AGENT_OVERVIEW_PULL_REQUEST = AGENT_OVERVIEW[0]; /** The GraphQL id GitHub gives the published pull request. */ const PULL_REQUEST_NODE_ID = "PR_kwDOfixture12"; @@ -1318,15 +1321,45 @@ function graphqlAnswering( }; } -/** A change to the agent's pull request as the bearer given. */ -function actionAs(bearer: string, agentId: string, action: string): Request { +/** A change to the agent's pull request as the bearer given, with the fields beside its action given. */ +function actionAs( + bearer: string, + agentId: string, + action: string, + fields: Readonly> = {}, +): Request { return request(AGENT_PULL_REQUEST_ACTION, bearer, { method: "POST", id: agentId, - body: { action }, + body: { action, ...fields }, }); } +/** How the published pull request may be merged, as GitHub's GraphQL answers the merge box's read. */ +const MERGING_ANSWER = { + data: { + repository: { + mergeCommitAllowed: false, + squashMergeAllowed: true, + rebaseMergeAllowed: true, + autoMergeAllowed: true, + pullRequest: { + headRefOid: PUBLISHED.headSha, + isMergeQueueEnabled: false, + mergeQueueEntry: null, + autoMergeRequest: { mergeMethod: "SQUASH" }, + reviewDecision: "REVIEW_REQUIRED", + commits: { nodes: [{ commit: { statusCheckRollup: { state: "PENDING" } } }] }, + }, + }, + }, +} as const; + +/** The mutation a GraphQL call made, by the field it named; none for a read. */ +function mutationOf(call: GraphqlCall): string | undefined { + return /mutation[^{]*\{\s*(\w+)/u.exec(call.query)?.[1]; +} + /** A review submitted on the agent's pull request as the bearer given. */ function reviewAs( bearer: string, @@ -1445,6 +1478,110 @@ it.layer(getTestDB())("changing an agent's pull request", (it) => { }), ); + it.effect( + "a merge, auto-merge on and off, and leaving the merge queue are each made on GitHub as the member, a merge and auto-merge against the head they saw", + () => + Effect.gen(function* () { + const { other, agent, calls, ask } = yield* published({ otherToken: "ghu_teammate" }); + const head = { headSha: PUBLISHED.headSha }; + + const answers = [ + yield* ask(actionAs(other, agent.id, "merge", { method: "squash", ...head })), + yield* ask(actionAs(other, agent.id, "enable-auto-merge", { method: "rebase", ...head })), + yield* ask(actionAs(other, agent.id, "enable-auto-merge", head)), + yield* ask(actionAs(other, agent.id, "disable-auto-merge")), + yield* ask(actionAs(other, agent.id, "dequeue")), + ]; + + assert.deepEqual( + answers.map((answer) => answer.body), + answers.map(() => ({ outcome: "done" })), + ); + assert.ok(calls.every((call) => call.authorization === "Bearer ghu_teammate")); + const id = PULL_REQUEST_NODE_ID; + assert.deepEqual( + calls.flatMap((call) => { + const mutation = mutationOf(call); + return mutation === undefined ? [] : [[mutation, call.variables]]; + }), + [ + ["mergePullRequest", { id, method: "SQUASH", head: PUBLISHED.headSha }], + ["enablePullRequestAutoMerge", { id, head: PUBLISHED.headSha, method: "REBASE" }], + ["enablePullRequestAutoMerge", { id, head: PUBLISHED.headSha }], + ["disablePullRequestAutoMerge", { id }], + ["dequeuePullRequest", { id }], + ], + ); + }), + ); + + it.effect( + "a merge naming no head, or a head that is no commit, is refused before anything reaches GitHub", + () => + Effect.gen(function* () { + const { other, agent, calls, ask } = yield* published({}); + const invalid = refusal(HOSTED_HTTP_STATUS.BAD_REQUEST, HOSTED_API_ERROR.INVALID_REQUEST); + + assert.deepEqual( + yield* ask(actionAs(other, agent.id, "merge", { method: "squash" })), + invalid, + ); + assert.deepEqual( + yield* ask( + actionAs(other, agent.id, "merge", { method: "squash", headSha: "main; drop" }), + ), + invalid, + ); + assert.deepEqual(calls, []); + }), + ); + + it.effect( + "the pull request's header read says how it may be merged, read on the App's token, and stands without it where GitHub would not say", + () => + Effect.gen(function* () { + let answering = true; + const { owner, agent, calls, ask } = yield* published({ + answer: () => + answering + ? Response.json(MERGING_ANSWER) + : Response.json({ message: "Server Error" }, { status: 502 }), + }); + const header = () => + Effect.map( + ask(request(AGENT_OVERVIEW_PULL_REQUEST, owner, { id: agent.id })), + (answered) => + readAnswer( + codingAgentOverviewPullRequestAnswerSchema, + HOSTED_HTTP_STATUS.OK, + answered, + ).pullRequest, + ); + + const read = yield* header(); + answering = false; + const without = yield* header(); + + assert.deepEqual(read?.merging, { + methods: ["squash", "rebase"], + autoMergeAllowed: true, + queue: false, + queued: null, + autoMerge: { method: "squash" }, + headSha: PUBLISHED.headSha, + review: "review-required", + checks: "pending", + }); + assert.ok( + calls.every( + (call) => call.authorization === `Bearer ${GITHUB_FIXTURE_INSTALLATION_TOKEN}`, + ), + ); + assert.equal(without?.title, read?.title); + assert.equal(without?.merging, undefined); + }), + ); + it.effect( "a member with no GitHub sign-in is asked to sign in with GitHub, and nothing is sent to GitHub as anyone else", () => diff --git a/packages/service/src/agents/coding-agents-app.ts b/packages/service/src/agents/coding-agents-app.ts index 5b195cbca2..52e1641d5f 100644 --- a/packages/service/src/agents/coding-agents-app.ts +++ b/packages/service/src/agents/coding-agents-app.ts @@ -41,6 +41,7 @@ import { codingAgentServersRequestSchema, codingAgentStartRequestSchema, PULL_REQUEST_STATE, + type PullRequestMerging, } from "@luke/features/coding-agent-wire"; import { githubImageOf } from "@luke/features/github-image"; import { @@ -77,13 +78,18 @@ import { namedOverviewConversation, namedOverviewPullRequest, } from "../github/github-people.js"; -import { githubReadOrRefusal } from "../github/github-refusal.js"; +import { githubReadOrRefusal, logGitHubReadFailure } from "../github/github-refusal.js"; +import { pullRequestMerging } from "../github/pull-request-merging.js"; import { + type GitHubWriteFailure, + mergePullRequest, pullRequestNodeId, replyToReviewThread, + setAutoMerge, setPullRequestDraft, setReviewThreadResolved, submitPullRequestReview, + takeOutOfMergeQueue, } from "../github/pull-request-writes.js"; import { type PullRequestReadFailure, @@ -1015,14 +1021,33 @@ const overviewPullRequestEndpoint = /* @__PURE__ */ Effect.fn( "web/agentOverviewPullRequestEndpoint", )(function* (seams: CodingAgentsAppSeams, published: PublishedCache) { return yield* overviewPart(seams, published, (token, agent, pullRequest) => - Effect.flatMap( - pullRequestOverview(token, agent.repository, pullRequest.number, stackable(pullRequest)), - (read) => - Option.isNone(read) ? Effect.succeed(null) : namedOverviewPullRequest(agent, read.value), - ), + Effect.gen(function* () { + const [read, merging] = yield* Effect.all( + [ + pullRequestOverview(token, agent.repository, pullRequest.number, stackable(pullRequest)), + mergingOrNone(token, agent.repository, pullRequest.number), + ], + { concurrency: "unbounded" }, + ); + if (Option.isNone(read)) return null; + const named = yield* namedOverviewPullRequest(agent, read.value); + return Option.isNone(merging) ? named : { ...named, merging: merging.value }; + }), ); }); +/** + * How the pull request may be merged, for its merge box; none where GitHub + * would not say, logged by its kind, since the header it rides with stands + * without it. + */ +const mergingOrNone = (token: Redacted.Redacted, repository: string, number: number) => + pullRequestMerging(token, repository, number).pipe( + Effect.catch((failure) => + Effect.as(logGitHubReadFailure(failure), Option.none()), + ), + ); + /** * GET reads the checks on the agent's pull request's head, failures first, * up to the bound, and its head's deployments beside them: three requests @@ -1061,34 +1086,50 @@ const overviewConversationEndpoint = /* @__PURE__ */ Effect.fn( ); }); +/** The write a change makes on the pull request, on the member's token, by the pull request's GraphQL id. */ +function writeOf( + request: PullRequestActionRequest, + token: Redacted.Redacted, + id: string, +): Effect.Effect { + switch (request.action) { + case PULL_REQUEST_ACTION.READY_FOR_REVIEW: + return setPullRequestDraft(token, id, false); + case PULL_REQUEST_ACTION.CONVERT_TO_DRAFT: + return setPullRequestDraft(token, id, true); + case PULL_REQUEST_ACTION.MERGE: + return mergePullRequest(token, id, request.method, request.headSha); + case PULL_REQUEST_ACTION.ENABLE_AUTO_MERGE: + return setAutoMerge(token, id, { method: request.method, headSha: request.headSha }); + case PULL_REQUEST_ACTION.DISABLE_AUTO_MERGE: + return setAutoMerge(token, id, undefined); + case PULL_REQUEST_ACTION.DEQUEUE: + return takeOutOfMergeQueue(token, id); + case PULL_REQUEST_ACTION.SUBMIT_REVIEW: + return submitPullRequestReview(token, id, request); + case PULL_REQUEST_ACTION.REPLY_TO_THREAD: + return replyToReviewThread(token, id, request); + case PULL_REQUEST_ACTION.RESOLVE_THREAD: + case PULL_REQUEST_ACTION.UNRESOLVE_THREAD: + return setReviewThreadResolved( + token, + id, + request.threadId, + request.action === PULL_REQUEST_ACTION.RESOLVE_THREAD, + ); + } +} + /** The write a change makes on the pull request, on the member's token. */ function actionWrite( request: PullRequestActionRequest, -): (pullRequest: MemberPullRequest) => ReturnType { +): ( + pullRequest: MemberPullRequest, +) => Effect.Effect { return ({ token, repository, number }) => - Effect.flatMap(pullRequestNodeId(token, repository, number), (id) => { - switch (request.action) { - case PULL_REQUEST_ACTION.READY_FOR_REVIEW: - case PULL_REQUEST_ACTION.CONVERT_TO_DRAFT: - return setPullRequestDraft( - token, - id, - request.action === PULL_REQUEST_ACTION.CONVERT_TO_DRAFT, - ); - case PULL_REQUEST_ACTION.SUBMIT_REVIEW: - return submitPullRequestReview(token, id, request); - case PULL_REQUEST_ACTION.REPLY_TO_THREAD: - return replyToReviewThread(token, id, request); - case PULL_REQUEST_ACTION.RESOLVE_THREAD: - case PULL_REQUEST_ACTION.UNRESOLVE_THREAD: - return setReviewThreadResolved( - token, - id, - request.threadId, - request.action === PULL_REQUEST_ACTION.RESOLVE_THREAD, - ); - } - }); + Effect.flatMap(pullRequestNodeId(token, repository, number), (id) => + writeOf(request, token, id), + ); } /** diff --git a/packages/service/src/github/pull-request-merging.ts b/packages/service/src/github/pull-request-merging.ts new file mode 100644 index 0000000000..045b1bea3b --- /dev/null +++ b/packages/service/src/github/pull-request-merging.ts @@ -0,0 +1,143 @@ +/** + * pull-request-merging.ts -- how an open pull request may be merged, as its merge box offers it, read from GitHub's GraphQL API on the token the overview reads on. + * + * One query reads what the REST pull request does not carry: the merge + * methods the repository allows, whether it allows auto-merge, whether the + * pull request's base is merged through a merge queue and where it stands + * in it, whether auto-merge is on for it and by which method, its head + * commit, what its required reviews say, and its head's checks as one word. + * It runs on the installation token the App minted for the agent's one + * repository (`github-app.ts`'s `repositoryPullRequestToken`), as every + * other overview read does, over the App module's own kit (`githubPost`, + * `send`, `readBody`). An answer GitHub gave in no shape this reads, errors + * among it, is unreadable, which the caller answers as no merge box rather + * than failing the header it rides with. + */ + +import { + CHECK_SUMMARY, + type CheckSummary, + type PullRequestMerging, + REVIEW_DECISION, + type ReviewDecision, +} from "@luke/features/coding-agent-wire"; +import { MERGE_METHOD, type MergeMethod } from "@luke/features/pull-request-write-wire"; +import { Effect, Option, type Redacted, Schema } from "effect"; +import type * as HttpClient from "effect/http/HttpClient"; +import { type GitHubUnavailable, githubPost, readBody, send } from "./github-app.js"; + +const MERGING_QUERY = `query($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + mergeCommitAllowed + squashMergeAllowed + rebaseMergeAllowed + autoMergeAllowed + pullRequest(number: $number) { + headRefOid + isMergeQueueEnabled + mergeQueueEntry { position } + autoMergeRequest { mergeMethod } + reviewDecision + commits(last: 1) { nodes { commit { statusCheckRollup { state } } } } + } + } +}`; + +const MergingAnswerSchema = Schema.Struct({ + data: Schema.Struct({ + repository: Schema.NullOr( + Schema.Struct({ + mergeCommitAllowed: Schema.Boolean, + squashMergeAllowed: Schema.Boolean, + rebaseMergeAllowed: Schema.Boolean, + autoMergeAllowed: Schema.Boolean, + pullRequest: Schema.NullOr( + Schema.Struct({ + headRefOid: Schema.String, + isMergeQueueEnabled: Schema.Boolean, + mergeQueueEntry: Schema.NullOr(Schema.Struct({ position: Schema.Number })), + autoMergeRequest: Schema.NullOr( + Schema.Struct({ mergeMethod: Schema.NullOr(Schema.String) }), + ), + reviewDecision: Schema.NullOr(Schema.String), + commits: Schema.Struct({ + nodes: Schema.Array( + Schema.Struct({ + commit: Schema.Struct({ + statusCheckRollup: Schema.NullOr(Schema.Struct({ state: Schema.String })), + }), + }), + ), + }), + }), + ), + }), + ), + }), +}); + +/** GitHub's word for each merge method, as GraphQL spells it. */ +const GITHUB_MERGE_METHOD = new Map([ + ["MERGE", MERGE_METHOD.MERGE], + ["SQUASH", MERGE_METHOD.SQUASH], + ["REBASE", MERGE_METHOD.REBASE], +]); + +const GITHUB_REVIEW_DECISION = new Map([ + ["APPROVED", REVIEW_DECISION.APPROVED], + ["CHANGES_REQUESTED", REVIEW_DECISION.CHANGES_REQUESTED], + ["REVIEW_REQUIRED", REVIEW_DECISION.REVIEW_REQUIRED], +]); + +/** A head's status rollup as one word; one GitHub has not rolled up is none run. */ +const GITHUB_ROLLUP = new Map([ + ["SUCCESS", CHECK_SUMMARY.PASSING], + ["PENDING", CHECK_SUMMARY.PENDING], + ["EXPECTED", CHECK_SUMMARY.PENDING], + ["FAILURE", CHECK_SUMMARY.FAILING], + ["ERROR", CHECK_SUMMARY.FAILING], +]); + +/** + * How the pull request may be merged, read on the token; none where the + * repository or the pull request reads as none. Note that the methods are + * the repository's own settings alone: a ruleset narrowing them on the base + * is GitHub's to refuse, in its own words, when a merge names one. + */ +export const pullRequestMerging = /* @__PURE__ */ Effect.fn("web/pullRequestMerging")(function* ( + token: Redacted.Redacted, + fullName: string, + number: number, +): Effect.fn.Return, GitHubUnavailable, HttpClient.HttpClient> { + const [owner = "", name = ""] = fullName.split("/"); + const request = githubPost("/graphql", token, { + query: MERGING_QUERY, + variables: { owner, name, number }, + }); + const answer = yield* Effect.scoped( + Effect.flatMap(send(request), (response) => readBody(response, MergingAnswerSchema)), + ); + const repository = answer.data.repository; + const pull = repository?.pullRequest; + if (repository === null || pull === null || pull === undefined) return Option.none(); + const methods = [ + ...(repository.mergeCommitAllowed ? [MERGE_METHOD.MERGE] : []), + ...(repository.squashMergeAllowed ? [MERGE_METHOD.SQUASH] : []), + ...(repository.rebaseMergeAllowed ? [MERGE_METHOD.REBASE] : []), + ]; + const rollup = pull.commits.nodes[0]?.commit.statusCheckRollup?.state; + return Option.some({ + methods, + autoMergeAllowed: repository.autoMergeAllowed, + queue: pull.isMergeQueueEnabled, + queued: pull.mergeQueueEntry === null ? null : { position: pull.mergeQueueEntry.position }, + autoMerge: + pull.autoMergeRequest === null + ? null + : { method: GITHUB_MERGE_METHOD.get(pull.autoMergeRequest.mergeMethod ?? "") ?? null }, + headSha: pull.headRefOid, + review: GITHUB_REVIEW_DECISION.get(pull.reviewDecision ?? "") ?? null, + checks: + rollup === undefined ? CHECK_SUMMARY.NONE : (GITHUB_ROLLUP.get(rollup) ?? CHECK_SUMMARY.NONE), + }); +}); diff --git a/packages/service/src/github/pull-request-writes.ts b/packages/service/src/github/pull-request-writes.ts index 40aaa8d9c1..79bfe65819 100644 --- a/packages/service/src/github/pull-request-writes.ts +++ b/packages/service/src/github/pull-request-writes.ts @@ -17,6 +17,8 @@ */ import { + MERGE_METHOD, + type MergeMethod, PULL_REQUEST_WRITE_BOUNDS, REVIEW_EVENT, type ReviewEvent, @@ -326,3 +328,92 @@ export const setReviewThreadResolved = /* @__PURE__ */ Effect.fn("web/setReviewT ); }, ); + +/** Each merge method as GitHub's GraphQL spells it. */ +const GRAPHQL_MERGE_METHOD = { + [MERGE_METHOD.MERGE]: "MERGE", + [MERGE_METHOD.SQUASH]: "SQUASH", + [MERGE_METHOD.REBASE]: "REBASE", +} as const satisfies Record; + +const MERGE_MUTATION = `mutation($id: ID!, $method: PullRequestMergeMethod!, $head: GitObjectID!) { + mergePullRequest(input: { pullRequestId: $id, mergeMethod: $method, expectedHeadOid: $head }) { + pullRequest { merged } + } +}`; + +/** + * The pull request merged now by the method given, on the member's token, + * against the head they saw: GitHub refuses it where the head has moved + * since, the method is not allowed, or a rule still stands in the way. + */ +export const mergePullRequest = /* @__PURE__ */ Effect.fn("web/mergePullRequest")(function* ( + token: Redacted.Redacted, + pullRequestId: string, + method: MergeMethod, + headSha: string, +): Effect.fn.Return { + yield* githubWrite( + token, + MERGE_MUTATION, + { id: pullRequestId, method: GRAPHQL_MERGE_METHOD[method], head: headSha }, + Schema.Unknown, + ); +}); + +const ENABLE_AUTO_MERGE_MUTATION = `mutation($id: ID!, $method: PullRequestMergeMethod, $head: GitObjectID!) { + enablePullRequestAutoMerge(input: { pullRequestId: $id, mergeMethod: $method, expectedHeadOid: $head }) { + clientMutationId + } +}`; + +const DISABLE_AUTO_MERGE_MUTATION = `mutation($id: ID!) { + disablePullRequestAutoMerge(input: { pullRequestId: $id }) { clientMutationId } +}`; + +/** What auto-merge is turned on with: the method, none on a base a merge queue merges, and the head the member saw. */ +interface AutoMerge { + readonly method: MergeMethod | undefined; + readonly headSha: string; +} + +/** + * Auto-merge turned on for the pull request as given, or off where none is + * given, on the member's token. On a base merged through a merge queue, + * turning it on is how a pull request is added to the queue once it is + * ready, as `gh pr merge --auto` adds one, the queue's method standing. + */ +export const setAutoMerge = /* @__PURE__ */ Effect.fn("web/setAutoMerge")(function* ( + token: Redacted.Redacted, + pullRequestId: string, + autoMerge: AutoMerge | undefined, +): Effect.fn.Return { + if (autoMerge === undefined) { + yield* githubWrite(token, DISABLE_AUTO_MERGE_MUTATION, { id: pullRequestId }, Schema.Unknown); + return; + } + yield* githubWrite( + token, + ENABLE_AUTO_MERGE_MUTATION, + { + id: pullRequestId, + head: autoMerge.headSha, + ...(autoMerge.method === undefined + ? undefined + : { method: GRAPHQL_MERGE_METHOD[autoMerge.method] }), + }, + Schema.Unknown, + ); +}); + +const DEQUEUE_MUTATION = `mutation($id: ID!) { + dequeuePullRequest(input: { id: $id }) { clientMutationId } +}`; + +/** The pull request taken out of its base's merge queue, on the member's token. */ +export const takeOutOfMergeQueue = /* @__PURE__ */ Effect.fn("web/takeOutOfMergeQueue")(function* ( + token: Redacted.Redacted, + pullRequestId: string, +): Effect.fn.Return { + yield* githubWrite(token, DEQUEUE_MUTATION, { id: pullRequestId }, Schema.Unknown); +});