From edb6110df4a1baf194558fc8783f4f403f4fefd2 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sat, 17 Oct 2026 01:34:21 +0000 Subject: [PATCH 01/10] feat(service,features,desktop): a merge box on the Pull request tab, merging as the member who presses The header's merge line now ends in the one thing that may be done from where an open pull request stands: Merge by a method the repository allows (asked first, the method chosen kept per repository), Enable auto-merge while checks run where the repository allows it, Merge when ready on a base merged through a merge queue, or Disable auto-merge and Remove from queue once one was asked for. Each is made on GitHub as the member through the W1 seam, a merge and auto-merge against the head the header read. The header's read carries how the pull request may be merged, read in one GraphQL query on the App's token beside it, and a blocked merge says what it waits on. Co-authored-by: luke-agent[bot] <339870089+luke-agent[bot]@users.noreply.github.com> --- PRIVACY.md | 8 +- apps/desktop/src/renderer/styles/tasks.css | 57 ++++ .../tasks/agent-pull-request-tab.test.ts | 140 +++++++++ .../renderer/tasks/agent-pull-request-tab.tsx | 37 ++- apps/desktop/src/renderer/tasks/merge-box.tsx | 278 ++++++++++++++++++ .../renderer/tasks/pull-request-writes.tsx | 30 +- ...ntOverviewPullRequestAnswerViewSchema.json | 112 +++++++ ...ingAgentPullRequestActionParamsSchema.json | 87 ++++++ ...gAgentOverviewPullRequestAnswerSchema.json | 112 +++++++ ...-request-write-wire-mergeMethodSchema.json | 8 + ...e-wire-pullRequestActionRequestSchema.json | 87 ++++++ packages/features/src/coding-agent-wire.ts | 36 +++ .../features/src/hosted-wire-schemas.test.ts | 1 + .../features/src/pull-request-write-wire.ts | 47 ++- .../src/agents/coding-agents-app.test.ts | 143 ++++++++- .../service/src/agents/coding-agents-app.ts | 68 ++++- .../src/github/pull-request-merging.ts | 143 +++++++++ .../service/src/github/pull-request-writes.ts | 95 +++++- 18 files changed, 1462 insertions(+), 27 deletions(-) create mode 100644 apps/desktop/src/renderer/tasks/merge-box.tsx create mode 100644 packages/features/fixtures/json-schema/pull-request-write-wire-mergeMethodSchema.json create mode 100644 packages/service/src/github/pull-request-merging.ts diff --git a/PRIVACY.md b/PRIVACY.md index e87a4482dd..84fe4b9408 100644 --- a/PRIVACY.md +++ b/PRIVACY.md @@ -406,7 +406,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 @@ -420,7 +423,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 0a1d4ef0a7..ed14c764c9 100644 --- a/apps/desktop/src/renderer/styles/tasks.css +++ b/apps/desktop/src/renderer/styles/tasks.css @@ -1952,6 +1952,63 @@ button.file-ref:hover .file-ref-path { 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(--space-8); + margin-inline-start: auto; +} + +.merge-box-note { + color: var(--text-secondary); +} + +.merge-box-split { + display: inline-flex; + align-items: stretch; +} + +.merge-box-press { + height: 26px; + padding: 0 var(--space-8); + gap: var(--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(--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(--state-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 24449e2bbf..cb68f6b483 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 @@ -20,12 +20,16 @@ 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 { + MERGE_METHOD, PULL_REQUEST_ACTION, PULL_REQUEST_WRITE_OUTCOME, type PullRequestActionRequest, @@ -1448,3 +1452,139 @@ test("a change GitHub refused says so in GitHub's own words until dismissed, and act(() => dismiss.click()); 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), []); +}); 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 9fc7b0fbc1..142969f429 100644 --- a/apps/desktop/src/renderer/tasks/agent-pull-request-tab.tsx +++ b/apps/desktop/src/renderer/tasks/agent-pull-request-tab.tsx @@ -66,6 +66,7 @@ import { useConversationFilters, } from "./conversation-filters"; import { PierreDiff, useDiffLook } from "./file-diff"; +import { blockedWords, MergeActions } from "./merge-box"; import { OpenedBy, PersonRef, ReviewersLine, ReviewStanding } from "./pull-request-people"; import { PullRequestStateMenu, @@ -625,17 +626,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 ?
); } @@ -857,7 +864,18 @@ interface OverviewParts { } /** Under the title: the branches, 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 ( @@ -894,7 +912,14 @@ function PullRequestHead({ parts, now }: { parts: OverviewParts; now: number }): {reviewers.length === 0 ? null : } - {open ? : null} + {open ? ( + + } + /> + ) : null} ); } @@ -1543,7 +1568,7 @@ export function AgentPullRequestTab({ <>
- +
; + +/** 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 = useActionMenu(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 ?? []); + const confirm = useConfirmDialog({ subject: true, surfaceOpen: true }, async () => { + if (merging !== undefined && method !== undefined) { + await writes.write({ action: PULL_REQUEST_ACTION.MERGE, method, headSha: merging.headSha }); + } + 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], 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} + {method === 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 bf9e690f33..cf9301767b 100644 --- a/apps/desktop/src/renderer/tasks/pull-request-writes.tsx +++ b/apps/desktop/src/renderer/tasks/pull-request-writes.tsx @@ -45,10 +45,25 @@ import { PullRequestMark } from "./pull-request-mark"; * drawn by whatever bar the tab wears, so they move with it. */ -/** What each change says on the menu, and while it is out. */ +/** The changes the pill's own menu makes. */ +type StateAction = + | typeof PULL_REQUEST_ACTION.READY_FOR_REVIEW + | typeof PULL_REQUEST_ACTION.CONVERT_TO_DRAFT; + +/** What each of the pill's changes says on its menu. */ const ACTION_WORDS = { [PULL_REQUEST_ACTION.READY_FOR_REVIEW]: "Ready for review", [PULL_REQUEST_ACTION.CONVERT_TO_DRAFT]: "Convert to draft", +} as const satisfies Record; + +/** What the pill says while a change is out, whichever control sent it. */ +const PENDING_WORDS = { + [PULL_REQUEST_ACTION.READY_FOR_REVIEW]: "Marking ready", + [PULL_REQUEST_ACTION.CONVERT_TO_DRAFT]: "Converting to draft", + [PULL_REQUEST_ACTION.MERGE]: "Merging", + [PULL_REQUEST_ACTION.ENABLE_AUTO_MERGE]: "Enabling auto-merge", + [PULL_REQUEST_ACTION.DISABLE_AUTO_MERGE]: "Disabling auto-merge", + [PULL_REQUEST_ACTION.DEQUEUE]: "Leaving the queue", } as const satisfies Record; /** The word for where the pull request stands, which the pill says. */ @@ -95,7 +110,8 @@ export interface PullRequestWrites { /** The change out now; none while nothing is. */ readonly pending: PullRequestAction | undefined; readonly note: WriteNote | undefined; - readonly write: (request: PullRequestActionRequest) => void; + /** Sends the change; settles once GitHub answered and what it said is drawn. */ + readonly write: (request: PullRequestActionRequest) => Promise; readonly dismiss: () => void; } @@ -125,7 +141,7 @@ export function usePullRequestWrites(agentId: string, onWritten: () => void): Pu (request: PullRequestActionRequest) => { 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, @@ -145,7 +161,7 @@ export function usePullRequestWrites(agentId: string, onWritten: () => void): Pu } /** The changes the pill's menu offers from where the pull request stands. */ -function actionsFrom(state: PullRequestState): readonly PullRequestAction[] { +function actionsFrom(state: PullRequestState): readonly StateAction[] { switch (state) { case PULL_REQUEST_STATE.DRAFT: return [PULL_REQUEST_ACTION.READY_FOR_REVIEW]; @@ -160,7 +176,7 @@ function actionsFrom(state: PullRequestState): readonly PullRequestAction[] { const ACTION_GLYPHS = { [PULL_REQUEST_ACTION.READY_FOR_REVIEW]: