From 04197e981e8a3b6920bc731f8604ce8dd2d9d310 Mon Sep 17 00:00:00 2001 From: Daniel Ehrhardt Date: Tue, 6 Oct 2026 08:32:51 +0200 Subject: [PATCH 1/3] Approve merges the pull request; workspaces can merge delivered tickets automatically Approve (task sheet, board menu, drag to Done, phone) merges the ticket's open pull request with gh before it moves to Done; when merging fails the ticket stays in review and the reason is shown with a Mark done anyway action. A new workspace setting merges a delivered ticket's pull request right away. --- .../src/components/tasks/task-board.tsx | 14 +++- .../src/components/tasks/task-meta.tsx | 3 + .../src/components/tasks/task-sheet.tsx | 20 +++-- .../workspaces/workspace-dialog.tsx | 16 +++- apps/desktop/src/lib/api.ts | 2 + apps/desktop/src/pages/tasks/tasks-page.tsx | 28 ++++++- apps/mobile/src/app/task/[id].tsx | 21 ++++- apps/mobile/src/lib/api.ts | 2 + packages/core/src/db/migrations.ts | 8 ++ packages/core/src/server/routes/tasks.ts | 7 +- packages/core/src/server/routes/workspaces.ts | 1 + packages/core/src/services/workspaces.ts | 4 + packages/core/src/tasks/git.ts | 25 ++++++ packages/core/src/tasks/service.ts | 66 ++++++++++++++- packages/core/test/tasks.test.ts | 80 +++++++++++++++++++ packages/shared/src/api.ts | 2 + packages/shared/src/models.ts | 2 + packages/shared/src/tasks.ts | 11 ++- 18 files changed, 287 insertions(+), 25 deletions(-) diff --git a/apps/desktop/src/components/tasks/task-board.tsx b/apps/desktop/src/components/tasks/task-board.tsx index 4ed05526..848bf03c 100644 --- a/apps/desktop/src/components/tasks/task-board.tsx +++ b/apps/desktop/src/components/tasks/task-board.tsx @@ -20,9 +20,9 @@ import { import { SortableContext, arrayMove, sortableKeyboardCoordinates, useSortable, verticalListSortingStrategy } from "@dnd-kit/sortable"; import { CSS } from "@dnd-kit/utilities"; import { Link } from "react-router"; -import { Archive, Check, ChevronsLeftRight, MessageSquareReply, MessagesSquare, PanelRightOpen, Play, Plus, RotateCcw, RotateCw, Trash2 } from "lucide-react"; +import { Archive, Check, ChevronsLeftRight, GitMerge, MessageSquareReply, MessagesSquare, PanelRightOpen, Play, Plus, RotateCcw, RotateCw, Trash2 } from "lucide-react"; import type { Agent, Task, TaskStatus, Workspace } from "@godmode/shared"; -import { reopenStatus } from "@godmode/shared"; +import { mergesOnApprove, reopenStatus } from "@godmode/shared"; import { toast } from "sonner"; import { Button } from "@/components/ui/button"; import { ContextMenu, ContextMenuContent, ContextMenuItem, ContextMenuSeparator, ContextMenuShortcut, ContextMenuTrigger } from "@/components/ui/context-menu"; @@ -397,7 +397,15 @@ function SortableCard({ <> onMove(task, "done", null)}> - Approve + {mergesOnApprove(task) ? ( + <> + Approve & merge + + ) : ( + <> + Approve + + )} {task.agentId && task.conversationId && ( diff --git a/apps/desktop/src/components/tasks/task-meta.tsx b/apps/desktop/src/components/tasks/task-meta.tsx index 95355b15..3f8b46aa 100644 --- a/apps/desktop/src/components/tasks/task-meta.tsx +++ b/apps/desktop/src/components/tasks/task-meta.tsx @@ -186,6 +186,9 @@ export function waitingLabel(task: Task): string | null { return isWaiting(task) && task.followup ? `Waiting — continues ${followupWhen(task.followup.dueAt)}` : null; } +/** The approve mutation (tasks page); its variables are the ticket being approved. */ +export const APPROVE_KEY = ["tasks", "approve"] as const; + /** Moving the ticket away from In progress would end something: a run working, standing still, or a follow-up. */ export function needsConfirm(task: Task): boolean { return task.status === "in_progress" && (isWorking(task) || !!task.pause || !!task.followup); diff --git a/apps/desktop/src/components/tasks/task-sheet.tsx b/apps/desktop/src/components/tasks/task-sheet.tsx index d1401c7a..a4108f68 100644 --- a/apps/desktop/src/components/tasks/task-sheet.tsx +++ b/apps/desktop/src/components/tasks/task-sheet.tsx @@ -1,6 +1,6 @@ import { useCallback, useEffect, useRef, useState, type ReactNode } from "react"; import { Link } from "react-router"; -import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { useMutation, useMutationState, useQueryClient } from "@tanstack/react-query"; import { format, formatDistanceToNowStrict } from "date-fns"; import { toast } from "sonner"; import { @@ -15,6 +15,7 @@ import { CornerDownRight, EllipsisVertical, GitBranch, + GitMerge, GitPullRequestCreateArrow, Hourglass, ListTree, @@ -31,7 +32,7 @@ import { Trash2, } from "lucide-react"; import type { Agent, Task, TaskEvent, TaskPatch, TaskStatus, Workspace } from "@godmode/shared"; -import { MAX_TASK_TITLE_LENGTH, githubBranchUrl, isWaiting, reopenStatus, waitsForTickets } from "@godmode/shared"; +import { MAX_TASK_TITLE_LENGTH, githubBranchUrl, isWaiting, mergesOnApprove, reopenStatus, waitsForTickets } from "@godmode/shared"; import { WorkingTicks } from "@/components/aicss/Motion"; import { Markdown } from "@/components/chat/markdown"; import { ChatFilesScope } from "@/components/chat/local-files"; @@ -58,7 +59,7 @@ import { AgentSelect, DueDateField, LabelsInput, PrioritySelect, StatusSelect, a import { TaskTimeline } from "./task-timeline"; import { TaskDialog } from "./task-dialog"; import { PullRequestChip, useTaskActivity } from "./task-card"; -import { BLOCKED_META, StatusIcon, TYPE_META, TypeIcon, formatCost, formatWork, isWorking, pauseLabel, repoLabel, taskRepoLabel, workspaceRepos } from "./task-meta"; +import { APPROVE_KEY, BLOCKED_META, StatusIcon, TYPE_META, TypeIcon, formatCost, formatWork, isWorking, pauseLabel, repoLabel, taskRepoLabel, workspaceRepos } from "./task-meta"; import { followupWhen, useFollowupActions } from "@/components/chat/followup"; import { useNow } from "@/components/vault/use-now"; import { usePauseActions } from "@/components/chat/pause"; @@ -637,9 +638,11 @@ function WorkPanel({ onSuccess: (t) => qc.setQueriesData({ queryKey: qk.tasks }, (list) => list?.map((x) => (x.id === t.id ? t : x))), onError: (e) => toastApiError(e, "Couldn't publish", qc), }); + const merging = useMutationState({ filters: { mutationKey: APPROVE_KEY, status: "pending" }, select: (m) => (m.state.variables as Task).id }).includes(task.id); const approve = (t: Task) => { onMove(t, "done"); - toast.success(`#${t.number} approved`, { action: { label: "Undo", onClick: () => onMove(t, "in_review") } }); + // Merging can't be undone; the page tells how that went. + if (!mergesOnApprove(t)) toast.success(`#${t.number} approved`, { action: { label: "Undo", onClick: () => onMove(t, "in_review") } }); }; const reopen = (t: Task) => { const back = reopenStatus(t); @@ -900,8 +903,8 @@ function WorkPanel({ )}
- {task.agentId && task.conversationId && ( )}
- {pr?.number && pr.state === "open" && ( + {mergesOnApprove(task) && (

- Approve marks it done here — merging stays on {new URL(pr.url).hostname}. A merged pull request also moves it to Done. + Merges #{pr!.number} into {task.baseBranch} on {new URL(pr!.url).hostname} and moves the + ticket to Done.

)} diff --git a/apps/desktop/src/components/workspaces/workspace-dialog.tsx b/apps/desktop/src/components/workspaces/workspace-dialog.tsx index e430ad7c..25c7aad5 100644 --- a/apps/desktop/src/components/workspaces/workspace-dialog.tsx +++ b/apps/desktop/src/components/workspaces/workspace-dialog.tsx @@ -10,6 +10,7 @@ import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover"; import { Spinner } from "@/components/ui/spinner"; +import { Switch } from "@/components/ui/switch"; import { Textarea } from "@/components/ui/textarea"; import { toastApiError } from "@/components/vault/vault-utils"; import { WorkspaceProfileField } from "@/components/browser/workspace-profile-field"; @@ -42,6 +43,7 @@ interface WorkspaceForm { vmId: string | null; browserProfileId: string | null; sources: WorkspaceSourceInput[]; + autoMerge: boolean; } function randomLook() { @@ -58,6 +60,7 @@ function formFrom(workspace: Workspace | null | undefined, look: { icon: string; vmId: workspace?.vmId ?? null, browserProfileId: workspace?.browserProfileId ?? null, sources: workspace?.sources.map(toSourceInput) ?? [], + autoMerge: workspace?.autoMerge ?? false, }; } const CONTEXT_EXAMPLE = "We are ACME GmbH. Write to clients in German.\nInvoices go to finance@acme.example.\nNever touch the production database."; @@ -88,7 +91,7 @@ export function WorkspaceDialog({ const closing = useRef(live); if (open) closing.current = live; const form = open ? live : closing.current; - const { name, icon, color, description, instructions, vmId, browserProfileId, sources } = form; + const { name, icon, color, description, instructions, vmId, browserProfileId, sources, autoMerge } = form; const set = (key: K) => (value: WorkspaceForm[K]) => @@ -130,6 +133,7 @@ export function WorkspaceDialog({ description: description.trim(), instructions: instructions.trim(), sources, + autoMerge, // Only when the VM control is shown: otherwise leave the assignment as it is. ...(vmChoices.available ? { vmId } : {}), ...(browserProfileId !== (workspace?.browserProfileId ?? null) ? { browserProfileId } : {}), @@ -321,6 +325,16 @@ export function WorkspaceDialog({ +
+
+ +

+ When an agent delivers a ticket with a pull request, Godmode merges it into its base branch and moves the ticket to Done — no + approval needed. +

+
+ +
{vmChoices.available && ( post(`/api/tasks/${id}/messages`, { content, attachments }), + /** Approve a ticket in review: its open pull request is merged first, then it's done. */ + approve: (id: string) => post(`/api/tasks/${id}/approve`), /** Push the task's branch (only coding tasks push theirs by themselves). */ push: (id: string) => post(`/api/tasks/${id}/push`), /** Push the branch and open its pull request — or, when gh can't, link to the page that opens one. */ diff --git a/apps/desktop/src/pages/tasks/tasks-page.tsx b/apps/desktop/src/pages/tasks/tasks-page.tsx index afa1894e..a2128668 100644 --- a/apps/desktop/src/pages/tasks/tasks-page.tsx +++ b/apps/desktop/src/pages/tasks/tasks-page.tsx @@ -4,7 +4,7 @@ import { useMutation, useQueryClient } from "@tanstack/react-query"; import { Archive, FolderGit2, ListFilter, Plus, Search, SquareKanban } from "lucide-react"; import { toast } from "sonner"; import type { Task, TaskPriority, TaskStatus, Workspace } from "@godmode/shared"; -import { TASK_PRIORITIES, isOverdue, localDay } from "@godmode/shared"; +import { TASK_PRIORITIES, isOverdue, localDay, mergesOnApprove } from "@godmode/shared"; import { AgentAvatar, EmptyState, PageHeader } from "@/components/common"; import { DropdownMenu, @@ -36,11 +36,11 @@ import { useArchiveTasks } from "@/components/tasks/task-actions"; import { TaskBoard } from "@/components/tasks/task-board"; import { TaskDialog } from "@/components/tasks/task-dialog"; import { TaskSheet } from "@/components/tasks/task-sheet"; -import { PRIORITY_META, PriorityIcon, STATUS_META, isWorking, needsConfirm, workspaceRepos } from "@/components/tasks/task-meta"; +import { APPROVE_KEY, PRIORITY_META, PriorityIcon, STATUS_META, isWorking, needsConfirm, workspaceRepos } from "@/components/tasks/task-meta"; import { followupWhen } from "@/components/chat/followup"; import { toastApiError } from "@/components/vault/vault-utils"; import { WorkspaceDialog } from "@/components/workspaces/workspace-dialog"; -import { api, errorMessage } from "@/lib/api"; +import { ApiRequestError, api, errorMessage } from "@/lib/api"; import { useAllAgents, useArchivedTasks, useGoals, useTasks, useWorkspaces } from "@/lib/hooks"; import { GoalsStrip } from "@/components/tasks/goals-strip"; import { qk } from "@/lib/queryKeys"; @@ -218,8 +218,28 @@ export default function TasksPage() { }, }); + // Approving a ticket with an open pull request merges it: it moves to Done once that worked. + const approve = useMutation({ + mutationKey: APPROVE_KEY, + mutationFn: (task: Task) => api.tasks.approve(task.id), + onSuccess: (t, task) => { + upsertTask(qc, t); + toast.success(`#${t.number} merged into ${t.baseBranch}`, { description: `Pull request #${task.pullRequest?.number} is merged and the ticket is done.` }); + }, + onError: (e, task) => { + void qc.invalidateQueries({ queryKey: qk.tasks }); + if (!(e instanceof ApiRequestError && e.code === "merge_failed")) return toastApiError(e, "Could not approve the task", qc); + toast.error(`Couldn't merge #${task.pullRequest?.number}`, { + description: e.message, + duration: 12_000, + action: { label: "Mark done anyway", onClick: () => move.mutate({ task, status: "done", beforeId: null }) }, + }); + }, + }); + const requestMove = (task: Task, status: TaskStatus, beforeId: string | null = null) => { - if (needsConfirm(task) && status !== "in_progress") setStopping({ task, status, beforeId }); + if (status === "done" && mergesOnApprove(task)) approve.mutate(task); + else if (needsConfirm(task) && status !== "in_progress") setStopping({ task, status, beforeId }); else move.mutate({ task, status, beforeId }); }; diff --git a/apps/mobile/src/app/task/[id].tsx b/apps/mobile/src/app/task/[id].tsx index 300ba6c8..f984a5b4 100644 --- a/apps/mobile/src/app/task/[id].tsx +++ b/apps/mobile/src/app/task/[id].tsx @@ -1,4 +1,4 @@ -import { isWaiting, reopenStatus, waitsForAnswer, waitsForSubtasks, waitsForTickets, type TaskBlockedKind } from "@godmode/shared"; +import { isWaiting, mergesOnApprove, reopenStatus, waitsForAnswer, waitsForSubtasks, waitsForTickets, type TaskBlockedKind } from "@godmode/shared"; import { useMutation, useQuery } from "@tanstack/react-query"; import { router, Stack, useLocalSearchParams } from "expo-router"; import { Alert, Linking, Pressable, ScrollView, StyleSheet, View } from "react-native"; @@ -10,7 +10,7 @@ import { Markdown } from "@/components/markdown"; import { openChat } from "@/components/rows"; import { STATUS_META, TaskStatusBadge, TYPE_META } from "@/components/task-row"; import { Avatar, Badge, Button, Card, Row, SectionTitle, T, tap } from "@/components/ui"; -import { api, errorText } from "@/lib/api"; +import { ApiError, api, errorText } from "@/lib/api"; import { encodeFiles, type PendingFile } from "@/lib/attachments"; import { activityText } from "@/lib/format"; import { useAgents } from "@/lib/hooks"; @@ -35,6 +35,18 @@ export default function TaskScreen() { onSuccess: onDone, onError: (err) => Alert.alert("Couldn't change the task", errorText(err)), }); + const approve = useMutation({ + mutationFn: () => api.tasks.approve(id), + onSuccess: onDone, + onError: (err) => { + // A computer on an older Godmode: approving only marks it done. + if (err instanceof ApiError && err.status === 404) return update.mutate({ status: "done" }); + Alert.alert(`Couldn't merge #${task.data?.pullRequest?.number ?? ""}`, errorText(err), [ + { text: "Cancel", style: "cancel" }, + { text: "Mark done anyway", onPress: () => update.mutate({ status: "done" }) }, + ]); + }, + }); const t = task.data; if (!t) return {task.isError ? "Task not found" : "Task"}; @@ -50,6 +62,7 @@ export default function TaskScreen() { const move = (status: TaskStatus) => { tap(); + if (status === "done" && mergesOnApprove(t)) return approve.mutate(); if (!(working || waiting) || status === "in_progress") return update.mutate({ status }); Alert.alert( "Stop the agent?", @@ -154,7 +167,7 @@ export default function TaskScreen() { patch(`/api/tasks/${id}`, input), + /** Approve a ticket in review: its open pull request is merged first, then it's done. */ + approve: (id: string) => post(`/api/tasks/${id}/approve`), /** Feedback for the agent in the task's chat; the task goes back to work. */ message: (id: string, content: string, attachments?: UploadFile[]) => post(`/api/tasks/${id}/messages`, attachments?.length ? { content, attachments } : { content }), diff --git a/packages/core/src/db/migrations.ts b/packages/core/src/db/migrations.ts index 177a0ad4..0378acf1 100644 --- a/packages/core/src/db/migrations.ts +++ b/packages/core/src/db/migrations.ts @@ -1147,6 +1147,14 @@ CREATE TABLE IF NOT EXISTS mods ( created_at TEXT NOT NULL, updated_at TEXT NOT NULL ); +`, + }, + { + id: 72, + name: "workspace_auto_merge", + sql: /* sql */ ` +-- 1: a delivered ticket's pull request is merged into its base branch right away (no review step). +ALTER TABLE workspaces ADD COLUMN auto_merge INTEGER NOT NULL DEFAULT 0; `, }, ]; diff --git a/packages/core/src/server/routes/tasks.ts b/packages/core/src/server/routes/tasks.ts index 8502a8e7..920ac49f 100644 --- a/packages/core/src/server/routes/tasks.ts +++ b/packages/core/src/server/routes/tasks.ts @@ -1,7 +1,7 @@ import type { Context, Hono } from "hono"; import { requestDevice } from "../auth"; import { MAX_GOAL_TITLE_LENGTH, MAX_GOAL_WHY_LENGTH, MAX_TASK_ATTACHMENT_BYTES, MAX_TASK_DESCRIPTION_LENGTH, MAX_TASK_TITLE_LENGTH, TASK_PRIORITIES, TASK_STATUSES, TASK_TYPES } from "@godmode/shared"; -import { archiveTasks, createTask, deleteTask, getTask, listTaskEvents, listTasks, pushTaskBranch, sendTaskMessage, updateTask } from "../../tasks/service"; +import { approveTask, archiveTasks, createTask, deleteTask, getTask, listTaskEvents, listTasks, pushTaskBranch, sendTaskMessage, updateTask } from "../../tasks/service"; import { readTaskAttachment, saveTaskAttachment } from "../../tasks/attachments"; import { HttpError, badRequest } from "../../util"; import { body, z } from "../validate"; @@ -150,6 +150,11 @@ export function registerTaskRoutes(app: Hono): void { return c.json(await sendTaskMessage(c.req.param("id"), content, attachments, { actor: "user", via: requestDevice(c) ? "phone" : "task" })); }); + app.post("/api/tasks/:id/approve", async (c) => { + expectSlow(c); + return c.json(await approveTask(c.req.param("id"))); + }); + app.post("/api/tasks/:id/push", async (c) => { expectSlow(c); return c.json(await pushTaskBranch(c.req.param("id"), { pullRequest: false })); diff --git a/packages/core/src/server/routes/workspaces.ts b/packages/core/src/server/routes/workspaces.ts index 30d791d0..d8492e78 100644 --- a/packages/core/src/server/routes/workspaces.ts +++ b/packages/core/src/server/routes/workspaces.ts @@ -17,6 +17,7 @@ const workspaceSchema = z.object({ instructions: z.string().max(MAX_INSTRUCTIONS_LENGTH).optional(), vmId: z.string().trim().max(100).nullable().optional(), browserProfileId: z.string().trim().max(100).nullable().optional(), + autoMerge: z.boolean().optional(), sources: z.array(sourceSchema).max(MAX_SOURCES, `A workspace can have up to ${MAX_SOURCES} folders and repositories.`).optional(), }); diff --git a/packages/core/src/services/workspaces.ts b/packages/core/src/services/workspaces.ts index ec2968b0..baae00d9 100644 --- a/packages/core/src/services/workspaces.ts +++ b/packages/core/src/services/workspaces.ts @@ -26,6 +26,7 @@ interface WorkspaceRow { icon: string; instructions: string; vm_id: string | null; + auto_merge: number; created_at: string; updated_at: string; } @@ -45,6 +46,7 @@ function toModel(r: WorkspaceRow & { browser_profile_id?: string | null }, sourc vmId: r.vm_id ?? null, browserProfileId: r.browser_profile_id ?? null, sources, + autoMerge: !!r.auto_merge, createdAt: r.created_at, updatedAt: r.updated_at, }; @@ -86,6 +88,7 @@ export function createWorkspace(input: WorkspaceInput): Workspace { icon: input.icon?.trim() || "🗂️", instructions: input.instructions?.trim() ?? "", vm_id: normalizeVmId(input.vmId) ?? null, + auto_merge: input.autoMerge ? 1 : 0, created_at: ts, updated_at: ts, }; @@ -123,6 +126,7 @@ export function updateWorkspace(id: string, patch: Partial): Wor icon: patch.icon !== undefined ? patch.icon.trim() || "🗂️" : undefined, instructions: patch.instructions?.trim(), vm_id: vmId, + auto_merge: patch.autoMerge === undefined ? undefined : patch.autoMerge ? 1 : 0, updated_at: now(), }); const apply = patch.sources ? setSources(id, patch.sources) : undefined; diff --git a/packages/core/src/tasks/git.ts b/packages/core/src/tasks/git.ts index efe93dfa..9fc062ae 100644 --- a/packages/core/src/tasks/git.ts +++ b/packages/core/src/tasks/git.ts @@ -609,3 +609,28 @@ export async function pullRequestState(dir: string, prUrl: string): Promise { + if (!ghBin()) return { merged: false, problem: "Install the GitHub CLI (gh) and run `gh auth login` to merge pull requests from Godmode." }; + const cwd = existsSync(dir) ? dir : tmpdir(); + let last = ""; + for (const method of ["--merge", "--squash", "--rebase"]) { + const res = await gh(["pr", "merge", prUrl, method], cwd); + if (res.ok || (await viewPullRequest(cwd, prUrl))?.state === "merged") return { merged: true, problem: null }; + last = res.err; + if (!/not allowed|not enabled|method/i.test(res.err)) break; + } + return { merged: false, problem: mergeProblem(last) }; +} + +function mergeProblem(err: string): string { + if (/not mergeable|cannot be cleanly created|conflict/i.test(err)) return "It has merge conflicts with its base branch — request changes so the agent resolves them."; + if (/required status check|checks? (are|is) (pending|failing|expected)|review is required|approving review|protected branch|base branch policy/i.test(err)) { + return `The base branch's rules don't allow merging it yet: ${err}`; + } + return `gh couldn't merge it: ${err || "unknown error"}`; +} diff --git a/packages/core/src/tasks/service.ts b/packages/core/src/tasks/service.ts index 5b3be97c..1c14bcd7 100644 --- a/packages/core/src/tasks/service.ts +++ b/packages/core/src/tasks/service.ts @@ -85,6 +85,7 @@ import { isRepoFolder, listSources, reposDir } from "../services/workspaceSource import { commitWork, commitsAhead, + mergePullRequest, needsClone, openPullRequest, prepareWorktree, @@ -1972,14 +1973,16 @@ async function publish(task: TaskRow, summary: string | null, runId: string): Pr return; } if (task.pr_url && task.pr_number && task.pr_state === "open") { - if (deliver(id, runId)) notify("success", `Task #${task.number}: pull request updated`, title, link); + if (deliver(id, runId) && !(await autoMerge(id))) notify("success", `Task #${task.number}: pull request updated`, title, link); return; } const { pullRequest, problem } = await openTaskPullRequest(task, summary); const opened = requireRow(id); if (opened.pr_url && (opened.pr_url !== task.pr_url || opened.pr_number !== task.pr_number)) record(id, "pr_opened", "system", { data: { number: opened.pr_number, url: opened.pr_url } }); if (!deliver(id, runId)) return; - if (pullRequest?.number) notify("success", `Task #${task.number}: pull request #${pullRequest.number} is open`, title, link); + if (pullRequest?.number) { + if (!(await autoMerge(id))) notify("success", `Task #${task.number}: pull request #${pullRequest.number} is open`, title, link); + } else notify("warning", `Task #${task.number}: open the pull request`, `The branch ${task.branch} was pushed. ${problem ?? ""}`.trim(), link); } catch (err) { const message = err instanceof Error ? err.message : String(err); @@ -1988,6 +1991,65 @@ async function publish(task: TaskRow, summary: string | null, runId: string): Pr } } +const mergeable = (task: TaskRow) => !!task.pr_url && !!task.pr_number && task.pr_state === "open"; + +/** Merge the task's open pull request. Null when it was merged, else why not. */ +async function mergeTaskPullRequest(task: TaskRow, actor: TaskActor): Promise { + setActivity(task.id, "Merging the pull request…"); + try { + const { merged, problem } = await mergePullRequest(checkoutDir(task.id), task.pr_url!); + if (!merged) return problem; + sql("UPDATE tasks SET pr_state = 'merged', updated_at = ? WHERE id = ?", now(), task.id); + record(task.id, "pr_merged", actor, { data: { number: task.pr_number!, url: task.pr_url!, ...(actor === "system" ? { auto: true } : {}) } }); + return null; + } finally { + activity.delete(task.id); + emit(task.id); + } +} + +/** + * The workspace merges its tickets' work without a review: the delivered pull request is merged and the ticket is + * done. `true` when the human was told what happened (merged, or why it wasn't). + */ +async function autoMerge(id: string): Promise { + const task = requireRow(id); + if (task.status !== "in_review" || !mergeable(task) || !task.workspace_id) return false; + if (!get<{ auto_merge: number }>("SELECT auto_merge FROM workspaces WHERE id = ?", task.workspace_id)?.auto_merge) return false; + const link = `/tasks?task=${id}`; + const problem = await mergeTaskPullRequest(task, "system"); + if (problem) { + record(id, "note", "system", { body: `Not merged automatically. ${problem}` }); + emit(id); + notify("warning", `Task #${task.number}: pull request #${task.pr_number} not merged`, problem, link); + return true; + } + if (requireRow(id).status === "in_review") updateTask(id, { status: "done" }, "system"); + notify("success", `Task #${task.number}: merged into ${task.base_branch}`, redact(task.title), link); + log.info(`task #${task.number}: pull request merged automatically — done`); + return true; +} + +/** + * The human approves the delivered work: its open pull request is merged first (an error says why it couldn't be, and + * the ticket stays in review), then the ticket is done. + */ +export async function approveTask(id: string): Promise { + const task = requireRow(id); + if (task.status !== "in_review") throw conflict("Only a ticket in review can be approved"); + if (mergeable(task)) { + if (busy.has(id)) throw conflict(`Godmode is ${activity.get(id)?.replace(/…$/, "").toLowerCase() ?? "busy with the task"} — try again in a moment`); + busy.add(id); + try { + const problem = await mergeTaskPullRequest(task, "user"); + if (problem) throw new HttpError(409, redact(problem), "merge_failed"); + } finally { + release(id); + } + } + return requireRow(id).status === "in_review" ? updateTask(id, { status: "done" }, "user") : getTask(id); +} + /** * Push the task's branch from the board (general and research tasks never push theirs by themselves) and, with * `pullRequest`, open its pull request. What's left uncommitted in the worktree is committed first. diff --git a/packages/core/test/tasks.test.ts b/packages/core/test/tasks.test.ts index b00cc880..fea06881 100644 --- a/packages/core/test/tasks.test.ts +++ b/packages/core/test/tasks.test.ts @@ -12,6 +12,7 @@ import { createWorkspace, deleteWorkspace, updateWorkspace } from "../src/servic import { workingDirectoryProblem } from "../src/services/folders"; import { __setTaskRetryDelaysForTests, + approveTask, archiveTasks, checkPullRequests, checkoutDir, @@ -1037,6 +1038,11 @@ if [ "$1 $2" = "pr view" ]; then if [ -n "$FAKE_GH_STATE" ]; then echo "{\\"url\\":\\"https://github.com/acme/app/pull/7\\",\\"number\\":7,\\"state\\":\\"$FAKE_GH_STATE\\"}"; exit 0; fi echo "no pull requests found" >&2; exit 1 fi +if [ "$1 $2" = "pr merge" ]; then + if [ -n "$FAKE_GH_MERGE_CONFLICT" ]; then echo "Pull request acme/app#7 is not mergeable: the merge commit cannot be cleanly created." >&2; exit 1; fi + if [ -n "$FAKE_GH_SQUASH_ONLY" ] && [ "$4" != "--squash" ]; then echo "GraphQL: Merge commits are not allowed on this repository. (mergePullRequest)" >&2; exit 1; fi + exit 0 +fi if [ "$1 $2" = "pr create" ]; then if [ -n "$FAKE_GH_CREATE_FAIL" ]; then echo "GraphQL: Resource not accessible by integration" >&2; exit 1; fi echo "Creating pull request"; echo "https://github.com/acme/app/pull/7"; exit 0 @@ -1094,6 +1100,80 @@ exit 1 expect(t.completedAt).not.toBeNull(); }); + test("approving merges the open pull request first, then the ticket is done", async () => { + const reviewed = () => { + const task = createTask({ workspaceId, title: "Approve me", status: "backlog" }); + sql("UPDATE tasks SET status = 'in_review', pr_url = ?, pr_number = 7, pr_state = 'open' WHERE id = ?", "https://github.com/acme/app/pull/7", task.id); + return task; + }; + const log = join(env.dataDir, "gh-calls.log"); + __setGhForTests(fakeGh()); + try { + const conflicted = reviewed(); + process.env.FAKE_GH_MERGE_CONFLICT = "1"; + const err = await catchHttp(() => approveTask(conflicted.id)); + delete process.env.FAKE_GH_MERGE_CONFLICT; + expect(err.status).toBe(409); + expect(err.code).toBe("merge_failed"); + expect(err.message).toContain("merge conflicts"); + expect(getTask(conflicted.id).status).toBe("in_review"); + expect(getTask(conflicted.id).pullRequest?.state).toBe("open"); + + const approved = await approveTask(conflicted.id); + expect(approved.status).toBe("done"); + expect(approved.pullRequest?.state).toBe("merged"); + expect(approved.completedAt).not.toBeNull(); + expect(readFileSync(log, "utf8")).toContain("pr merge https://github.com/acme/app/pull/7 --merge"); + const merged = listTaskEvents(approved.id).find((e) => e.kind === "pr_merged"); + expect(merged?.actor).toBe("user"); + + // Only squash allowed: it's squashed. + const squashed = reviewed(); + process.env.FAKE_GH_SQUASH_ONLY = "1"; + expect((await approveTask(squashed.id)).pullRequest?.state).toBe("merged"); + delete process.env.FAKE_GH_SQUASH_ONLY; + expect(readFileSync(log, "utf8")).toContain("pr merge https://github.com/acme/app/pull/7 --squash"); + } finally { + delete process.env.FAKE_GH_MERGE_CONFLICT; + delete process.env.FAKE_GH_SQUASH_ONLY; + __setGhForTests(null); + } + // Nothing to merge: approving marks it done. + const plain = createTask({ workspaceId, title: "No pull request", status: "backlog" }); + sql("UPDATE tasks SET status = 'in_review' WHERE id = ?", plain.id); + expect((await approveTask(plain.id)).status).toBe("done"); + expect((await catchHttp(() => approveTask(plain.id))).status).toBe(409); + }); + + test("a workspace that merges automatically merges delivered pull requests", async () => { + const { url } = makeRemote("auto-merge"); + const ws = createWorkspace({ name: "Ship it", autoMerge: true }); + expect(ws.autoMerge).toBe(true); + const task = createTask({ workspaceId: ws.id, title: "TASK_EDIT merged without review", type: "coding", repoUrl: url, agentId: agent.id }); + await settled(task.id, ["in_review"]); + // Its pull request is open on GitHub (the push still goes to the task's origin). + sql("UPDATE tasks SET repo_url = ?, pr_url = ?, pr_number = 7, pr_state = 'open' WHERE id = ?", "https://github.com/acme/app.git", "https://github.com/acme/app/pull/7", task.id); + __setGhForTests(fakeGh()); + try { + process.env.FAKE_GH_MERGE_CONFLICT = "1"; + await sendTaskMessage(task.id, "TASK_EDIT once more"); + await until(() => listTaskEvents(task.id).some((e) => e.kind === "note" && e.body.includes("Not merged automatically")), 20_000, "merge attempt"); + await settled(task.id, ["in_review"]); + expect(getTask(task.id).pullRequest?.state).toBe("open"); + delete process.env.FAKE_GH_MERGE_CONFLICT; + + await sendTaskMessage(task.id, "TASK_EDIT and again"); + await settled(task.id, ["done"]); + } finally { + delete process.env.FAKE_GH_MERGE_CONFLICT; + __setGhForTests(null); + } + const done = getTask(task.id); + expect(done.pullRequest?.state).toBe("merged"); + expect(listTaskEvents(task.id).find((e) => e.kind === "pr_merged")?.data).toMatchObject({ number: 7, auto: true }); + expect(updateWorkspace(ws.id, { autoMerge: false }).autoMerge).toBe(false); + }, 60_000); + test("the board pushes a task's branch and opens its pull request", async () => { const { url, bare } = makeRemote("board-push"); const task = createTask({ title: "TASK_EDIT share from the board", repoUrl: url, agentId: agent.id }); diff --git a/packages/shared/src/api.ts b/packages/shared/src/api.ts index c96b8a95..710a4c70 100644 --- a/packages/shared/src/api.ts +++ b/packages/shared/src/api.ts @@ -44,6 +44,8 @@ export interface WorkspaceInput { browserProfileId?: ID | null; /** Every attached folder and repository, in order. One naming an existing source keeps it (and its clone). */ sources?: WorkspaceSourceInput[]; + /** Merge tickets' pull requests as soon as they're delivered. */ + autoMerge?: boolean; } export type WorkspaceSourceInput = { kind: "folder"; path: string } | { kind: "git"; url: string; branch?: string | null }; diff --git a/packages/shared/src/models.ts b/packages/shared/src/models.ts index 6f20d3a8..09af1671 100644 --- a/packages/shared/src/models.ts +++ b/packages/shared/src/models.ts @@ -34,6 +34,8 @@ export interface Workspace { browserProfileId: ID | null; /** Folders and git repositories every agent in the workspace works with. */ sources: WorkspaceSource[]; + /** A ticket's pull request is merged as soon as its agent delivers it, and the ticket is done — no review step. */ + autoMerge: boolean; createdAt: ISODate; updatedAt: ISODate; } diff --git a/packages/shared/src/tasks.ts b/packages/shared/src/tasks.ts index 0edb9da6..6ed47d66 100644 --- a/packages/shared/src/tasks.ts +++ b/packages/shared/src/tasks.ts @@ -92,7 +92,8 @@ export interface TaskEventData { /** body: the note. */ note: Record; pr_opened: { number: number | null; url: string }; - pr_merged: { number: number; url: string }; + /** auto: merged by Godmode because its workspace merges delivered work automatically. */ + pr_merged: { number: number; url: string; auto?: boolean }; pr_closed: { number: number; url: string }; /** body: what was asked (see AgentQuestion). */ asked: { questionId: ID; kind: "question" | "approval" }; @@ -337,6 +338,11 @@ export function reopenStatus(t: Pick): boolean { + return t.status === "in_review" && !!t.pullRequest?.number && t.pullRequest.state === "open"; +} + /** One label, cleaned: no leading #, single spaces, at most MAX_TASK_LABEL_LENGTH characters. "" = drop it. */ export function cleanTaskLabel(label: string): string { return label.replace(/^#+/, "").replace(/\s+/g, " ").trim().slice(0, MAX_TASK_LABEL_LENGTH).trim(); @@ -417,7 +423,8 @@ export function taskEventText(e: TaskEvent, o: { you: string; youObject: string; case "pr_opened": return e.data.number ? `Pull request #${e.data.number} opened` : "Branch pushed — the pull request still has to be opened"; case "pr_merged": - return `Pull request #${e.data.number} merged`; + if (e.data.auto) return `Pull request #${e.data.number} merged automatically`; + return e.actor === "user" ? `${a} merged pull request #${e.data.number}` : `Pull request #${e.data.number} merged`; case "pr_closed": return `Pull request #${e.data.number} closed without merging`; case "asked": From b5cf1e234c8c4250746c767436599306af3aa214 Mon Sep 17 00:00:00 2001 From: Daniel Ehrhardt Date: Tue, 6 Oct 2026 08:35:30 +0200 Subject: [PATCH 2/3] Cloud scope: allow approving tasks --- packages/core/src/cloud/scope.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/cloud/scope.ts b/packages/core/src/cloud/scope.ts index 9b050b17..0032e0d8 100644 --- a/packages/core/src/cloud/scope.ts +++ b/packages/core/src/cloud/scope.ts @@ -254,6 +254,7 @@ const RULES: [methods: string, path: string, rule: Rule][] = [ ["GET", "/api/tasks/attachments/:id/:name", A], ["GET|PATCH|DELETE", "/api/tasks/:id", A], ["POST", "/api/tasks/:id/messages", A], + ["POST", "/api/tasks/:id/approve", A], ["POST", "/api/tasks/:id/push", A], ["POST", "/api/tasks/:id/pull-request", A], From 1844762e967cde6cd0a1ff794b12e0fbb72e518f Mon Sep 17 00:00:00 2001 From: Daniel Ehrhardt Date: Tue, 6 Oct 2026 08:37:43 +0200 Subject: [PATCH 3/3] Merge only from Approve; handle merge queues, branch rules and races Dragging a ticket to Done or picking Done in its status menu no longer merges. A pull request a merge queue takes stays in review until GitHub merges it, branch-policy errors aren't reported as conflicts, and the watcher and an approval can't both note the same merge. --- .../src/components/tasks/task-board.tsx | 9 +++--- .../src/components/tasks/task-sheet.tsx | 8 ++--- apps/desktop/src/pages/tasks/tasks-page.tsx | 11 ++++--- apps/mobile/src/app/task/[id].tsx | 6 +++- packages/core/src/tasks/git.ts | 19 +++++++----- packages/core/src/tasks/service.ts | 30 ++++++++++++------- packages/core/test/tasks.test.ts | 14 +++++++++ 7 files changed, 65 insertions(+), 32 deletions(-) diff --git a/apps/desktop/src/components/tasks/task-board.tsx b/apps/desktop/src/components/tasks/task-board.tsx index 848bf03c..3a7798cc 100644 --- a/apps/desktop/src/components/tasks/task-board.tsx +++ b/apps/desktop/src/components/tasks/task-board.tsx @@ -57,7 +57,8 @@ export interface TaskBoardProps { /** Given when the board spans several workspaces: cards show theirs. */ workspaces?: Map; onOpen: (task: Task) => void; - onMove: (task: Task, status: TaskStatus, beforeId: string | null) => void; + /** merge: Approve — its open pull request is merged first. */ + onMove: (task: Task, status: TaskStatus, beforeId: string | null, merge?: boolean) => void; onQuickAdd: (status: TaskStatus, title: string) => Promise; onArchive: (tasks: Task[]) => void; onDelete: (task: Task) => void; @@ -220,7 +221,7 @@ function Column({ dragging: boolean; activeId: string | null; onOpen: (task: Task) => void; - onMove: (task: Task, status: TaskStatus, beforeId: string | null) => void; + onMove: (task: Task, status: TaskStatus, beforeId: string | null, merge?: boolean) => void; onArchive: (tasks: Task[]) => void; onDelete: (task: Task) => void; onCollapse: () => void; @@ -346,7 +347,7 @@ function SortableCard({ workspace?: Workspace | null; ghost: boolean; onOpen: (task: Task) => void; - onMove: (task: Task, status: TaskStatus, beforeId: string | null) => void; + onMove: (task: Task, status: TaskStatus, beforeId: string | null, merge?: boolean) => void; onArchive: (tasks: Task[]) => void; onDelete: (task: Task) => void; }) { @@ -396,7 +397,7 @@ function SortableCard({ {task.status === "in_review" && ( <> - onMove(task, "done", null)}> + onMove(task, "done", null, true)}> {mergesOnApprove(task) ? ( <> Approve & merge diff --git a/apps/desktop/src/components/tasks/task-sheet.tsx b/apps/desktop/src/components/tasks/task-sheet.tsx index a4108f68..50d892be 100644 --- a/apps/desktop/src/components/tasks/task-sheet.tsx +++ b/apps/desktop/src/components/tasks/task-sheet.tsx @@ -79,7 +79,7 @@ export function TaskSheet({ agents: Agent[]; workspaces: Map; onClose: () => void; - onMove: (task: Task, status: TaskStatus) => void; + onMove: (task: Task, status: TaskStatus, merge?: boolean) => void; onArchive: (task: Task, archived: boolean) => void; onDelete: (task: Task) => void; onReassign: (task: Task, agentId: string | null) => void; @@ -222,7 +222,7 @@ function TaskDetail({ workspaces: Map; onClose: () => void; onUploading: (uploading: boolean) => void; - onMove: (task: Task, status: TaskStatus) => void; + onMove: (task: Task, status: TaskStatus, merge?: boolean) => void; onArchive: (task: Task, archived: boolean) => void; onDelete: (task: Task) => void; onReassign: (task: Task, agentId: string | null) => void; @@ -621,7 +621,7 @@ function WorkPanel({ }: { task: Task; agent?: Agent; - onMove: (task: Task, status: TaskStatus) => void; + onMove: (task: Task, status: TaskStatus, merge?: boolean) => void; onReason: (reason: string) => void; /** Drop what it waits for (it starts then). */ onStartWithoutWaiting: () => void; @@ -640,7 +640,7 @@ function WorkPanel({ }); const merging = useMutationState({ filters: { mutationKey: APPROVE_KEY, status: "pending" }, select: (m) => (m.state.variables as Task).id }).includes(task.id); const approve = (t: Task) => { - onMove(t, "done"); + onMove(t, "done", true); // Merging can't be undone; the page tells how that went. if (!mergesOnApprove(t)) toast.success(`#${t.number} approved`, { action: { label: "Undo", onClick: () => onMove(t, "in_review") } }); }; diff --git a/apps/desktop/src/pages/tasks/tasks-page.tsx b/apps/desktop/src/pages/tasks/tasks-page.tsx index a2128668..cb88ae51 100644 --- a/apps/desktop/src/pages/tasks/tasks-page.tsx +++ b/apps/desktop/src/pages/tasks/tasks-page.tsx @@ -224,7 +224,10 @@ export default function TasksPage() { mutationFn: (task: Task) => api.tasks.approve(task.id), onSuccess: (t, task) => { upsertTask(qc, t); - toast.success(`#${t.number} merged into ${t.baseBranch}`, { description: `Pull request #${task.pullRequest?.number} is merged and the ticket is done.` }); + const pr = task.pullRequest?.number; + if (t.pullRequest?.state === "merged") { + toast.success(`#${t.number} merged into ${t.baseBranch}`, { description: t.status === "done" ? `Pull request #${pr} is merged and the ticket is done.` : `Pull request #${pr} is merged.` }); + } else toast.info(`#${pr} is queued to merge`, { description: "The ticket moves to Done once GitHub merges it." }); }, onError: (e, task) => { void qc.invalidateQueries({ queryKey: qk.tasks }); @@ -237,8 +240,8 @@ export default function TasksPage() { }, }); - const requestMove = (task: Task, status: TaskStatus, beforeId: string | null = null) => { - if (status === "done" && mergesOnApprove(task)) approve.mutate(task); + const requestMove = (task: Task, status: TaskStatus, beforeId: string | null = null, merge = false) => { + if (merge && status === "done" && mergesOnApprove(task)) approve.mutate(task); else if (needsConfirm(task) && status !== "in_progress") setStopping({ task, status, beforeId }); else move.mutate({ task, status, beforeId }); }; @@ -507,7 +510,7 @@ export default function TasksPage() { agents={agents} workspaces={workspaces} onClose={() => openTask(null)} - onMove={(task, status) => requestMove(task, status)} + onMove={(task, status, merge) => requestMove(task, status, null, merge)} onArchive={(task, value) => (value ? archive([task]) : setArchived(task, false))} onDelete={setDeleting} onReassign={requestReassign} diff --git a/apps/mobile/src/app/task/[id].tsx b/apps/mobile/src/app/task/[id].tsx index f984a5b4..49d60e54 100644 --- a/apps/mobile/src/app/task/[id].tsx +++ b/apps/mobile/src/app/task/[id].tsx @@ -37,10 +37,14 @@ export default function TaskScreen() { }); const approve = useMutation({ mutationFn: () => api.tasks.approve(id), - onSuccess: onDone, + onSuccess: (next) => { + onDone(next); + if (next.pullRequest?.state === "open") Alert.alert(`#${next.pullRequest.number} is queued to merge`, "The ticket moves to Done once GitHub merges it."); + }, onError: (err) => { // A computer on an older Godmode: approving only marks it done. if (err instanceof ApiError && err.status === 404) return update.mutate({ status: "done" }); + if (!(err instanceof ApiError && err.code === "merge_failed")) return Alert.alert("Couldn't approve it", errorText(err)); Alert.alert(`Couldn't merge #${task.data?.pullRequest?.number ?? ""}`, errorText(err), [ { text: "Cancel", style: "cancel" }, { text: "Mark done anyway", onPress: () => update.mutate({ status: "done" }) }, diff --git a/packages/core/src/tasks/git.ts b/packages/core/src/tasks/git.ts index 9fc062ae..689b5c48 100644 --- a/packages/core/src/tasks/git.ts +++ b/packages/core/src/tasks/git.ts @@ -612,25 +612,28 @@ export async function pullRequestState(dir: string, prUrl: string): Promise { - if (!ghBin()) return { merged: false, problem: "Install the GitHub CLI (gh) and run `gh auth login` to merge pull requests from Godmode." }; +export async function mergePullRequest(dir: string, prUrl: string): Promise<{ merged: boolean; queued: boolean; problem: string | null }> { + if (!ghBin()) return { merged: false, queued: false, problem: "Install the GitHub CLI (gh) and run `gh auth login` to merge pull requests from Godmode." }; const cwd = existsSync(dir) ? dir : tmpdir(); let last = ""; for (const method of ["--merge", "--squash", "--rebase"]) { const res = await gh(["pr", "merge", prUrl, method], cwd); - if (res.ok || (await viewPullRequest(cwd, prUrl))?.state === "merged") return { merged: true, problem: null }; + const state = (await viewPullRequest(cwd, prUrl))?.state; + if (state === "merged") return { merged: true, queued: false, problem: null }; + if (res.ok) return state === "open" ? { merged: false, queued: true, problem: null } : { merged: true, queued: false, problem: null }; last = res.err; - if (!/not allowed|not enabled|method/i.test(res.err)) break; + if (!/not allowed|not enabled|merge method/i.test(res.err)) break; } - return { merged: false, problem: mergeProblem(last) }; + return { merged: false, queued: false, problem: mergeProblem(last) }; } function mergeProblem(err: string): string { - if (/not mergeable|cannot be cleanly created|conflict/i.test(err)) return "It has merge conflicts with its base branch — request changes so the agent resolves them."; - if (/required status check|checks? (are|is) (pending|failing|expected)|review is required|approving review|protected branch|base branch policy/i.test(err)) { + if (/policy|required status check|checks? (are|is) (pending|failing|expected)|review is required|approving review|protected branch/i.test(err)) { return `The base branch's rules don't allow merging it yet: ${err}`; } + if (/not up to date|behind/i.test(err)) return `The branch is behind its base branch, which must be up to date to merge: ${err}`; + if (/cannot be cleanly created|conflict/i.test(err)) return "It has merge conflicts with its base branch — request changes so the agent resolves them."; return `gh couldn't merge it: ${err || "unknown error"}`; } diff --git a/packages/core/src/tasks/service.ts b/packages/core/src/tasks/service.ts index 1c14bcd7..f075eac0 100644 --- a/packages/core/src/tasks/service.ts +++ b/packages/core/src/tasks/service.ts @@ -1993,15 +1993,16 @@ async function publish(task: TaskRow, summary: string | null, runId: string): Pr const mergeable = (task: TaskRow) => !!task.pr_url && !!task.pr_number && task.pr_state === "open"; -/** Merge the task's open pull request. Null when it was merged, else why not. */ -async function mergeTaskPullRequest(task: TaskRow, actor: TaskActor): Promise { +/** Merge the task's open pull request: merged, queued (GitHub merges it later; the watcher moves it to Done), or why not. */ +async function mergeTaskPullRequest(task: TaskRow, actor: TaskActor): Promise<{ queued: boolean; problem: string | null }> { setActivity(task.id, "Merging the pull request…"); try { - const { merged, problem } = await mergePullRequest(checkoutDir(task.id), task.pr_url!); - if (!merged) return problem; - sql("UPDATE tasks SET pr_state = 'merged', updated_at = ? WHERE id = ?", now(), task.id); - record(task.id, "pr_merged", actor, { data: { number: task.pr_number!, url: task.pr_url!, ...(actor === "system" ? { auto: true } : {}) } }); - return null; + const { merged, queued, problem } = await mergePullRequest(checkoutDir(task.id), task.pr_url!); + if (!merged) return { queued, problem }; + if (sql("UPDATE tasks SET pr_state = 'merged', updated_at = ? WHERE id = ? AND pr_state = 'open'", now(), task.id).changes) { + record(task.id, "pr_merged", actor, { data: { number: task.pr_number!, url: task.pr_url!, ...(actor === "system" ? { auto: true } : {}) } }); + } + return { queued: false, problem: null }; } finally { activity.delete(task.id); emit(task.id); @@ -2017,7 +2018,11 @@ async function autoMerge(id: string): Promise { if (task.status !== "in_review" || !mergeable(task) || !task.workspace_id) return false; if (!get<{ auto_merge: number }>("SELECT auto_merge FROM workspaces WHERE id = ?", task.workspace_id)?.auto_merge) return false; const link = `/tasks?task=${id}`; - const problem = await mergeTaskPullRequest(task, "system"); + const { queued, problem } = await mergeTaskPullRequest(task, "system"); + if (queued) { + notify("success", `Task #${task.number}: pull request #${task.pr_number} is queued to merge`, "It moves to Done once GitHub merges it.", link); + return true; + } if (problem) { record(id, "note", "system", { body: `Not merged automatically. ${problem}` }); emit(id); @@ -2032,7 +2037,7 @@ async function autoMerge(id: string): Promise { /** * The human approves the delivered work: its open pull request is merged first (an error says why it couldn't be, and - * the ticket stays in review), then the ticket is done. + * the ticket stays in review), then the ticket is done. A pull request a merge queue takes stays in review until merged. */ export async function approveTask(id: string): Promise { const task = requireRow(id); @@ -2041,8 +2046,9 @@ export async function approveTask(id: string): Promise { if (busy.has(id)) throw conflict(`Godmode is ${activity.get(id)?.replace(/…$/, "").toLowerCase() ?? "busy with the task"} — try again in a moment`); busy.add(id); try { - const problem = await mergeTaskPullRequest(task, "user"); + const { queued, problem } = await mergeTaskPullRequest(task, "user"); if (problem) throw new HttpError(409, redact(problem), "merge_failed"); + if (queued) return getTask(id); } finally { release(id); } @@ -2102,9 +2108,11 @@ export async function checkPullRequests(): Promise { // Approved tickets too: their pull request may be merged after the human marked them done. const open = all("SELECT * FROM tasks WHERE status IN ('in_review', 'done') AND pr_number IS NOT NULL AND pr_state = 'open' AND pr_url IS NOT NULL"); for (const task of open) { + if (busy.has(task.id)) continue; const state = await pullRequestState(checkoutDir(task.id), task.pr_url!).catch(() => null); if (!state || state === "open") continue; - sql("UPDATE tasks SET pr_state = ?, updated_at = ? WHERE id = ?", state, now(), task.id); + // Approving merged it meanwhile: noted there. + if (!sql("UPDATE tasks SET pr_state = ?, updated_at = ? WHERE id = ? AND pr_state = 'open'", state, now(), task.id).changes) continue; record(task.id, state === "merged" ? "pr_merged" : "pr_closed", "system", { data: { number: task.pr_number!, url: task.pr_url! } }); if (state === "merged" && transition(task.id, "done", ["in_review"])) { sql("UPDATE tasks SET completed_at = ? WHERE id = ?", now(), task.id); diff --git a/packages/core/test/tasks.test.ts b/packages/core/test/tasks.test.ts index fea06881..f10948f7 100644 --- a/packages/core/test/tasks.test.ts +++ b/packages/core/test/tasks.test.ts @@ -1039,6 +1039,7 @@ if [ "$1 $2" = "pr view" ]; then echo "no pull requests found" >&2; exit 1 fi if [ "$1 $2" = "pr merge" ]; then + if [ -n "$FAKE_GH_MERGE_POLICY" ]; then echo "X Pull request acme/app#7 is not mergeable: the base branch policy prohibits the merge." >&2; exit 1; fi if [ -n "$FAKE_GH_MERGE_CONFLICT" ]; then echo "Pull request acme/app#7 is not mergeable: the merge commit cannot be cleanly created." >&2; exit 1; fi if [ -n "$FAKE_GH_SQUASH_ONLY" ] && [ "$4" != "--squash" ]; then echo "GraphQL: Merge commits are not allowed on this repository. (mergePullRequest)" >&2; exit 1; fi exit 0 @@ -1127,6 +1128,17 @@ exit 1 const merged = listTaskEvents(approved.id).find((e) => e.kind === "pr_merged"); expect(merged?.actor).toBe("user"); + // Branch rules: not called a conflict. A merge queue takes it: it stays in review until GitHub merges it. + const ruled = reviewed(); + process.env.FAKE_GH_MERGE_POLICY = "1"; + expect((await catchHttp(() => approveTask(ruled.id))).message).toContain("rules don't allow"); + delete process.env.FAKE_GH_MERGE_POLICY; + process.env.FAKE_GH_STATE = "OPEN"; + const queued = await approveTask(ruled.id); + delete process.env.FAKE_GH_STATE; + expect(queued.status).toBe("in_review"); + expect(queued.pullRequest?.state).toBe("open"); + // Only squash allowed: it's squashed. const squashed = reviewed(); process.env.FAKE_GH_SQUASH_ONLY = "1"; @@ -1135,6 +1147,8 @@ exit 1 expect(readFileSync(log, "utf8")).toContain("pr merge https://github.com/acme/app/pull/7 --squash"); } finally { delete process.env.FAKE_GH_MERGE_CONFLICT; + delete process.env.FAKE_GH_MERGE_POLICY; + delete process.env.FAKE_GH_STATE; delete process.env.FAKE_GH_SQUASH_ONLY; __setGhForTests(null); }