feat(kix): merge clean pull requests from the decision inbox

cloudflare/GITHUB.md

@@ -132,7 +132,10 @@ GitHub keeps pull request status, checks, and reviews. `mergePullRequest`
 merges only an exact head a maintainer explicitly chose, with the first method
 the repository allows: squash, then a merge commit, then rebase. GitHub's
 branch rules and refusals stand, and an agent's recommendation is never
-approval.
+approval. The decision inbox offers it on a pull request item only when
+GitHub reports the PR's merge state as `CLEAN` or `HAS_HOOKS` and no job holds
+the PR. The form binds the head on screen. A moved head asks for another look,
+GitHub's refusal is shown in its own words, and neither is retried.
 
 Fork pull requests do not make privileged CI safe. Workflows that run a pull
 request's code with secrets or write tokens, such as `pull_request_target` or

cloudflare/src/github/service.ts

@@ -48,6 +48,8 @@ export class GitHubError extends Error {
 	constructor(
 		message: string,
 		readonly status?: number,
+		/** GitHub's own words for a refusal, read only where a request asks for them. */
+		readonly reason: string | null = null,
 	) {
 		super(message);
 	}
@@ -118,6 +120,20 @@ function sha(value: string) {
 export function refName(name: string) {
 	return !/[\x00-\x20\x7f~^:?*[\\]|\.\.|@\{|\/\/|(^|\/)\.|\.lock($|\/)|[/.]$|^\/|^@$/.test(name);
 }
+/** The `message` GitHub gives for a refused request, as one bounded line of display text. */
+async function refusal(response: Response) {
+	try {
+		const { message } = (await response.json()) as { message?: unknown };
+		if (typeof message === "string")
+			return (
+				message
+					.replace(/[\x00-\x1f\x7f]+/g, " ")
+					.trim()
+					.slice(0, 300) || null
+			);
+	} catch {}
+	return null;
+}
 function pullNumber(target: string) {
 	const match = target.match(/^pulls\/(\d{1,15})$/);
 	if (!match) throw new GitHubError("Invalid pull request target");
@@ -145,7 +161,8 @@ export class GitHubService {
 		return `${body}.${sign("RSA-SHA256", Buffer.from(body), createPrivateKey(this.#env.GITHUB_APP_PRIVATE_KEY)).toString("base64url")}`;
 	}
 
-	async #api<T>(token: string, path: string, method = "GET", body?: unknown): Promise<T> {
+	/** `explain` keeps GitHub's one-line `message` from a refusal, for a maintainer to read. */
+	async #api<T>(token: string, path: string, method = "GET", body?: unknown, explain = false): Promise<T> {
 		const response = await this.#fetch(`https://api.github.com${path}`, {
 			method,
 			redirect: "error",
@@ -159,7 +176,12 @@ export class GitHubService {
 			body: body === undefined ? undefined : JSON.stringify(body),
 		});
 		// Never include provider response bodies, request headers, or secrets in errors.
-		if (!response.ok) throw new GitHubError(`GitHub request failed (${response.status})`, response.status);
+		if (!response.ok)
+			throw new GitHubError(
+				`GitHub request failed (${response.status})`,
+				response.status,
+				explain ? await refusal(response) : null,
+			);
 		return response.status === 204 ? (undefined as T) : ((await response.json()) as T);
 	}
 
@@ -931,10 +953,10 @@ export class GitHubService {
 	/**
 	 * Merge exactly `expectedHead` with the first method the repository
 	 * allows: squash, then a merge commit, then rebase. GitHub enforces branch
-	 * rules, reviews, and checks.
+	 * rules, reviews, and checks; a refusal says why and is never retried.
 	 */
 	mergePullRequest(repo: GitHubRepository, userId: string, pull: number, expectedHead: string) {
-		return this.#authorized(repo, userId, { contents: "write", pull_requests: "write" }, (token, current) => {
+		return this.#authorized(repo, userId, { contents: "write", pull_requests: "write" }, async (token, current) => {
 			const method = current.allow_squash_merge
 				? "squash"
 				: current.allow_merge_commit
@@ -943,10 +965,30 @@ export class GitHubService {
 						? "rebase"
 						: null;
 			if (!method) throw new GitHubError("The repository allows no merge method");
-			return this.#api(token, `${repositoryPath(repo)}/pulls/${number(pull)}/merge`, "PUT", {
-				sha: sha(expectedHead),
-				merge_method: method,
-			});
+			let merged: { sha: string };
+			try {
+				merged = await this.#api<{ sha: string }>(
+					token,
+					`${repositoryPath(repo)}/pulls/${number(pull)}/merge`,
+					"PUT",
+					{ sha: sha(expectedHead), merge_method: method },
+					true,
+				);
+			} catch (error) {
+				if (!(error instanceof GitHubError)) throw error;
+				if (error.status === 409)
+					throw new GitHubError("The pull request's head moved since you looked; review it again", 409);
+				if (error.reason && [403, 405, 422].includes(error.status!))
+					throw new GitHubError(`GitHub refused the merge: ${error.reason}`, error.status);
+				throw error;
+			}
+			// As a webhook would, mark observations stale so the inbox stops offering the merged pull request.
+			await this.#env.DB.prepare(
+				"UPDATE github_project_state SET invalidated=1,revision=revision+1 WHERE repository=?",
+			)
+				.bind(repo.id)
+				.run();
+			return { sha: merged.sha };
 		});
 	}
 }

cloudflare/src/github/state.ts

@@ -25,6 +25,8 @@ export type Pull = Entity & {
 	headRefOid: string;
 	isDraft: boolean;
 	reviewDecision: string | null;
+	/** GitHub's merge readiness for the App, such as CLEAN, BLOCKED, or DIRTY; absent in older observations. */
+	mergeStateStatus?: string;
 	reviews: { totalCount: number };
 	latestReviews: {
 		nodes: { state: string; author: { login: string } | null; commit: { oid: string } | null }[];
@@ -57,7 +59,7 @@ export type ProjectState = {
 };
 
 const entityFields = "databaseId:fullDatabaseId number title state updatedAt author { login } comments { totalCount }";
-const pullFields = `${entityFields} headRefName headRefOid isDraft reviewDecision reviews { totalCount } latestReviews(first:20) { nodes { state author {login} commit {oid} } pageInfo {hasNextPage} } commits(last:1) { nodes { commit { oid statusCheckRollup { state } } } }`;
+const pullFields = `${entityFields} headRefName headRefOid isDraft reviewDecision mergeStateStatus reviews { totalCount } latestReviews(first:20) { nodes { state author {login} commit {oid} } pageInfo {hasNextPage} } commits(last:1) { nodes { commit { oid statusCheckRollup { state } } } }`;
 // Cursor variables are declared only for unfinished connections (GraphQL rejects
 // unused variables). Fixed page sizes bound subrequests, independent of tree size.
 export function activityQuery({ history, occupancy }: Scan) {

cloudflare/src/inbox/page.tsx

@@ -282,6 +282,16 @@ function work(item: Item, view: ProjectView) {
 	return { workflow: "start", target: item.target, revision: commit, label: "Start this quest" };
 }
 
+/**
+ * The observed pull request when GitHub reports it can merge as is: no
+ * conflicts, and its checks and branch rules satisfied. Anything else is
+ * reviewed on GitHub or merged by a later merge job.
+ */
+function clean(item: Item) {
+	const pull = item.kind === "pull" ? (item.activity?.data as Pull | null | undefined) : null;
+	return pull?.state === "OPEN" && ["CLEAN", "HAS_HOOKS"].includes(pull.mergeStateStatus ?? "") ? pull : null;
+}
+
 function Refresh(props: { project: Item["project"] }) {
 	return (
 		<form method="post" action={`${projectURL(props.project.owner, props.project.name)}/refresh`}>
@@ -298,9 +308,18 @@ function Actions(props: { item: Item; view: ProjectView | undefined }) {
 	const reason = starts ? blocked(item, view) : null;
 	// Only an unblocked item has the observations its work is bound to.
 	const job = starts && !reason ? work(item, view!) : null;
+	// Only an idle item offers merge: no queued, running, or waiting job can still push to its head.
+	const merge = starts && !reason ? clean(item) : null;
 	const active = item.job && ["queued", "running", "waiting"].includes(item.job.state) && !item.job.cancel;
 	return (
 		<div class="decision-actions">
+			{merge && (
+				<form method="post" action={`${base}/pulls/${merge.number}/merge`}>
+					<input type="hidden" name="head" value={merge.headRefOid} />
+					<input type="hidden" name="version" value={item.version} />
+					<button class="button primary">Merge this PR</button>
+				</form>
+			)}
 			{reason ? (
 				<>
 					<p class="muted">{reason}</p>
@@ -312,7 +331,9 @@ function Actions(props: { item: Item; view: ProjectView | undefined }) {
 						<input type="hidden" name="workflow" value={job.workflow} />
 						{job.target && <input type="hidden" name="target" value={job.target} />}
 						<input type="hidden" name="revision" value={job.revision} />
-						<button class="button primary">{item.need === "retry" ? "Try again" : job.label}</button>
+						<button class={merge ? "button quiet" : "button primary"}>
+							{item.need === "retry" ? "Try again" : job.label}
+						</button>
 					</form>
 				)
 			)}

cloudflare/src/index.tsx

@@ -16,7 +16,7 @@ import * as inbox from "./inbox/model";
 import { InboxPage } from "./inbox/page";
 import { purge, retain } from "./jobs/output";
 import { BootstrapRequired, fromForm, jobs, resume, stop } from "./jobs/routes";
-import { JobError } from "./jobs/store";
+import { JobError, active } from "./jobs/store";
 import { Layout, type Page, type User } from "./layout";
 
 export const app = new Hono<{ Bindings: Env }>();
@@ -232,6 +232,26 @@ app.post("/projects/github/:owner/:repository/skip", async (c) => {
 	await inbox.skip(c.env.DB, session.user.id, repo.id, item, version);
 	return c.redirect("/inbox", 303);
 });
+// A merge is the maintainer's own click on the head they saw; GitHub's rules decide whether it lands.
+app.post("/projects/github/:owner/:repository/pulls/:number/merge", async (c) => {
+	const session = await createAuth(c.env).api.getSession({ headers: c.req.raw.headers });
+	if (!session) throw new GitHubError("Sign in required");
+	const github = new GitHubService(c.env);
+	const repo = await github.enrolledProject(c.req.param("owner"), c.req.param("repository"), session.user.id);
+	if (!repo) throw new GitHubError("Project is not enrolled");
+	const number = c.req.param("number");
+	const { head, version } = await c.req.parseBody();
+	if (!/^[1-9]\d{0,14}$/.test(number) || typeof head !== "string")
+		return c.json({ error: "Pull request and head required" }, 400);
+	const target = `pulls/${number}`;
+	// A job may still push to the pull request, so it never merges under one.
+	if (await active(c.env.DB, repo.id, target)) throw new JobError("Stop this pull request's job before merging it");
+	await github.mergePullRequest(repo, session.user.id, Number(number), head);
+	// The merged item is finished for this maintainer until GitHub's next observation drops it.
+	if (typeof version === "string" && version.length <= 100)
+		await inbox.skip(c.env.DB, session.user.id, repo.id, target, version);
+	return c.redirect("/inbox", 303);
+});
 
 // The signed-in home: one decision at a time across the maintainer's projects.
 app.get("/inbox", async (c) => {

cloudflare/src/jobs/store.ts

@@ -628,6 +628,16 @@ export async function listJobs(db: D1Database, repository: number) {
 	return (await Promise.all(rows.results.map((row) => readJob(db, row.id)))).filter((job) => job !== null);
 }
 
+/** Whether a job on `target` is queued, running, or waiting, and so may still push to it. */
+export async function active(db: D1Database, repository: number, target: string) {
+	return !!(await db
+		.prepare(
+			"SELECT 1 FROM jobs WHERE repository=? AND target=? AND state IN ('queued','running','waiting') LIMIT 1",
+		)
+		.bind(repository, target)
+		.first());
+}
+
 /**
  * The inbox's jobs, newest first: each target's latest job, however old, when
  * it awaits a decision (queued, running, waiting, or failed) or its target is

cloudflare/test/inbox.test.ts

@@ -148,6 +148,68 @@ describe("decision inbox", () => {
 		expect(await inbox()).toMatch(/Skipped and finished \(1\)[\s\S]*Fix/);
 	});
 
+	it("merges a clean pull request at the head on screen, or says why GitHub refused", async () => {
+		const merge = "/projects/github/upstream/project/pulls/2/merge";
+		const active = thread(await inbox());
+		expect(active).toContain(`action="${merge}"`);
+		expect(active).toContain(`name="head" value="${head}"`);
+		expect(active).toContain('<button class="button quiet">Iterate on this PR');
+		const moved = await submit(merge, [["head", "b".repeat(40)]]);
+		expect(moved.status).toBe(403);
+		expect(await moved.text()).toContain("head moved since you looked; review it again");
+		github.refuse = "At least 1 approving review is required by reviewers with write access.";
+		const refused = await submit(merge, [["head", head]]);
+		expect(refused.status).toBe(403);
+		expect(await refused.text()).toContain(
+			"GitHub refused the merge: At least 1 approving review is required by reviewers with write access.",
+		);
+		// Each click is one request bound to its head; a refusal is never retried.
+		expect(github.merged).toHaveLength(2);
+		github.refuse = null;
+		github.methods = ["squash"];
+		const merged = await submit(merge, [
+			["head", head],
+			["version", "0"],
+		]);
+		expect(merged.status).toBe(303);
+		expect(merged.headers.get("Location")).toBe("/inbox");
+		expect(github.merged.at(-1)).toEqual({ sha: head, merge_method: "squash" });
+		// The merged item is finished, and no new work starts before GitHub is read again.
+		const html = await inbox();
+		expect(html).toMatch(/Skipped and finished \(1\)[\s\S]*Fix/);
+		expect(thread(html)).toContain("GitHub reported changes");
+	});
+
+	it("offers merge only on a clean pull request no job holds, to current maintainers", async () => {
+		const merge = "/projects/github/upstream/project/pulls/2/merge";
+		await db
+			.prepare(
+				"UPDATE github_project_activity SET data=json_set(data,'$.mergeStateStatus','BLOCKED') WHERE id='21'",
+			)
+			.run();
+		let active = thread(await inbox());
+		expect(active).not.toContain(merge);
+		expect(active).toContain("Review on GitHub");
+		expect(active).toContain('<button class="button primary">Iterate on this PR');
+		await db
+			.prepare(
+				"UPDATE github_project_activity SET data=json_set(data,'$.mergeStateStatus','CLEAN') WHERE id='21'",
+			)
+			.run();
+		await running("iterate", "pulls/2");
+		active = thread(await inbox({ project: "10", item: "pulls/2" }));
+		expect(active).not.toContain(merge);
+		const held = await submit(merge, [["head", head]]);
+		expect(held.status).toBe(409);
+		expect(await held.text()).toContain("job before merging it");
+		await db.prepare("DELETE FROM jobs").run();
+		github.role = "write";
+		const outsider = await submit(merge, [["head", head]]);
+		expect(outsider.status).toBe(403);
+		expect(await outsider.text()).toContain("Current GitHub maintainer authority required");
+		expect(github.merged).toHaveLength(0);
+	});
+
 	it("offers a bootstrap job when the default branch has no flake", async () => {
 		github.flake = false;
 		const refused = await submit(jobs, [

cloudflare/test/project-fixtures.ts

@@ -35,6 +35,7 @@ export const pull = {
 	comments: { totalCount: 0 },
 	isDraft: false,
 	reviewDecision: null,
+	mergeStateStatus: "CLEAN",
 	commits: { nodes: [{ commit: { oid: head, statusCheckRollup: { state: "SUCCESS" } } }] },
 	headRefName: "contributor/fix",
 	headRefOid: head,
@@ -52,10 +53,22 @@ export const issue = {
 /**
  * Sign in a GitHub user, enroll project 10 for them with an open issue #1 and
  * pull request #2, and mock GitHub. `github` changes what the mock reports, and
- * `mock` reinstalls it after another sign-in replaces it.
+ * `mock` reinstalls it after another sign-in replaces it. Merges of #2 record
+ * their request in `merged`, and land only at `head`; `refuse` makes GitHub
+ * refuse them with that message.
  */
 export async function maintainerOf() {
-	const github = { flake: true, pullFlake: true, role: "maintain", issue: "open", pull: "open", head };
+	const github = {
+		flake: true,
+		pullFlake: true,
+		role: "maintain",
+		issue: "open",
+		pull: "open",
+		head,
+		methods: ["squash", "merge", "rebase"],
+		refuse: null as string | null,
+		merged: [] as unknown[],
+	};
 	const who = identity("github");
 	const cookie = (await oauth(who)).session!;
 	const user = (await db
@@ -80,15 +93,15 @@ export async function maintainerOf() {
 			)
 			.bind(JSON.stringify(pull), JSON.stringify(issue)),
 	]);
-	const mock = async (input: RequestInfo | URL) => {
+	const mock = async (input: RequestInfo | URL, init?: RequestInit) => {
 		const url = new URL(String(input));
 		const path = url.pathname;
 		if (path.endsWith("/installation"))
 			return Response.json({
 				id: 5,
 				app_id: 123,
 				suspended_at: null,
-				permissions: { metadata: "read", contents: "read", issues: "read", pull_requests: "read" },
+				permissions: { metadata: "read", contents: "write", issues: "read", pull_requests: "write" },
 			});
 		if (path.endsWith("/access_tokens")) return Response.json({ token: "installation-token" });
 		if (path === "/installation/token") return new Response(null, { status: 204 });
@@ -102,7 +115,17 @@ export async function maintainerOf() {
 				default_branch: "trunk",
 				archived: false,
 				disabled: false,
+				allow_squash_merge: github.methods.includes("squash"),
+				allow_merge_commit: github.methods.includes("merge"),
+				allow_rebase_merge: github.methods.includes("rebase"),
 			});
+		if (path === "/repos/upstream/project/pulls/2/merge" && init?.method === "PUT") {
+			const body = JSON.parse(String(init.body));
+			github.merged.push(body);
+			if (body.sha !== head) return Response.json({ message: "Head branch was modified." }, { status: 409 });
+			if (github.refuse) return Response.json({ message: github.refuse }, { status: 405 });
+			return Response.json({ sha: "e".repeat(40), merged: true, message: "Pull Request successfully merged" });
+		}
 		if (path.startsWith("/user/")) {
 			const id = path.split("/").at(-1)!;
 			return Response.json({ id: Number(id), login: `person${id}` });

quest/m0/kix/README.md

@@ -102,7 +102,6 @@ operator-paid-hosting decisions:
 - [Job sandbox](/quest/m0/kix/sandbox.md) - OS-sandboxed commands, with untrusted PR devShells built in a VM on Linux runners
 - [opencode adapter](/quest/m0/kix/opencode.md) - OpenCode Go, ChatGPT plans, and API-key providers through opencode
 - [Webhook reconcile](/quest/m0/kix/reconcile.md) - new issues and PRs arrive in the inbox ready to act on, without a manual refresh
-- [Inbox merge](/quest/m0/kix/merge.md) - PR items merge the reviewed head through GitHub's rules
 - [Merge jobs](/quest/m0/kix/merge-job.md) - a merge pick fixes conflicts, CI, and reviews on a runner, then lands the PR
 - [Co-maintainer inbox](/quest/m0/kix/members.md) - every current maintainer sees enrolled projects in their inbox
 - [Answer flow](/quest/m0/kix/answer-flow.md) - move to the next decision after an answer, with neutral refusals

quest/m0/kix/merge-job.md

@@ -28,10 +28,12 @@ or needs them.
   service applies labels and the job's instructions say so.
 - Decided (user): the pick covers the job's own pushes for conflicts, CI, and
   review findings. A head moved by anyone else, a design change, or the merge
-  skill's design checkpoint pauses the job into a decision. The
-  [inbox merge](/quest/m0/kix/merge.md) button keeps its exact-head binding
-  for a PR that is already clean.
-- Widen the inbox merge button: a maintainer sees merge on an open PR that
+  skill's design checkpoint pauses the job into a decision. The inbox merge
+  button keeps its exact-head binding for a PR that is already clean.
+- Widen the inbox merge button, which today appears only when GitHub's
+  `mergeStateStatus` is `CLEAN` or `HAS_HOOKS` and no job holds the PR
+  (`clean` in `cloudflare/src/inbox/page.tsx`, the merge route in
+  `cloudflare/src/index.tsx`): a maintainer sees merge on an open PR that
   is not clean (conflicted, failing, or unreviewed), and picking it starts
   this job. A clean PR keeps the direct exact-head merge.
 - Land by enabling auto-merge bound to the head the job verified, unless the
@@ -47,4 +49,3 @@ or needs them.
 
 - [Runner](/quest/m0/kix/runner.md) - jobs that run the merge skill
 - [PR state commands](/quest/m0/helpers/pr-states.md) - the label contract kix shares with local sessions
-- [Inbox merge](/quest/m0/kix/merge.md) - the merge button this widens to unclean PRs