From a9a1579d3878c5011edbf826c6324c933dd9eb40 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 10:21:42 -0700 Subject: [PATCH 1/2] chore(quest): claim quest/m0/kix/merge.md Co-Authored-By: Claude Opus 5.5 From 2221b78d6c48a85bc2b65cbe9d59b2e0412af85d Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 10:32:31 -0700 Subject: [PATCH 2/2] feat(kix): merge clean pull requests from the decision inbox A PR item offers "Merge this PR" when GitHub reports its merge state as CLEAN or HAS_HOOKS and no job holds it. The form binds the head on screen; mergePullRequest picks the repository's allowed method, and a moved head or GitHub's refusal is shown plainly and never retried. Co-Authored-By: Claude Opus 5.5 --- cloudflare/GITHUB.md | 5 ++- cloudflare/src/github/service.ts | 58 +++++++++++++++++++++++---- cloudflare/src/github/state.ts | 4 +- cloudflare/src/inbox/page.tsx | 23 ++++++++++- cloudflare/src/index.tsx | 22 +++++++++- cloudflare/src/jobs/store.ts | 10 +++++ cloudflare/test/inbox.test.ts | 62 +++++++++++++++++++++++++++++ cloudflare/test/project-fixtures.ts | 28 +++++++++++-- quest/m0/kix/README.md | 1 - quest/m0/kix/merge-job.md | 11 ++--- quest/m0/kix/merge.md | 31 --------------- quest/m0/landed-quests.md | 2 +- 12 files changed, 203 insertions(+), 54 deletions(-) delete mode 100644 quest/m0/kix/merge.md diff --git a/cloudflare/GITHUB.md b/cloudflare/GITHUB.md index c5f815d6..1e1e6e76 100644 --- a/cloudflare/GITHUB.md +++ b/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 diff --git a/cloudflare/src/github/service.ts b/cloudflare/src/github/service.ts index 26bc8679..448d9c89 100644 --- a/cloudflare/src/github/service.ts +++ b/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(token: string, path: string, method = "GET", body?: unknown): Promise { + /** `explain` keeps GitHub's one-line `message` from a refusal, for a maintainer to read. */ + async #api(token: string, path: string, method = "GET", body?: unknown, explain = false): Promise { 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); } @@ -914,10 +936,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 @@ -926,10 +948,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 }; }); } } diff --git a/cloudflare/src/github/state.ts b/cloudflare/src/github/state.ts index 33b78c7e..4ee13c4f 100644 --- a/cloudflare/src/github/state.ts +++ b/cloudflare/src/github/state.ts @@ -26,6 +26,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 }[]; @@ -72,7 +74,7 @@ export function activityQuery(scan: Scan) { if (scan.pulls !== false) { variables.push("$pulls: String"); fields.push( - `pullRequests(first:50,after:$pulls,orderBy:{field:CREATED_AT,direction:ASC}) { nodes { ${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 } } } } } pageInfo { hasNextPage endCursor } }`, + `pullRequests(first:50,after:$pulls,orderBy:{field:CREATED_AT,direction:ASC}) { nodes { ${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 } } } } } pageInfo { hasNextPage endCursor } }`, ); } if (scan.refs !== false) { diff --git a/cloudflare/src/inbox/page.tsx b/cloudflare/src/inbox/page.tsx index 982b6256..e5d53f76 100644 --- a/cloudflare/src/inbox/page.tsx +++ b/cloudflare/src/inbox/page.tsx @@ -281,6 +281,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 (
@@ -297,9 +307,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 (
+ {merge && ( + + + + + + )} {reason ? ( <>

{reason}

@@ -311,7 +330,9 @@ function Actions(props: { item: Item; view: ProjectView | undefined }) { {job.target && } - + ) )} diff --git a/cloudflare/src/index.tsx b/cloudflare/src/index.tsx index 21eb839f..22078cfd 100644 --- a/cloudflare/src/index.tsx +++ b/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) => { diff --git a/cloudflare/src/jobs/store.ts b/cloudflare/src/jobs/store.ts index 1b852b07..22b6db08 100644 --- a/cloudflare/src/jobs/store.ts +++ b/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 diff --git a/cloudflare/test/inbox.test.ts b/cloudflare/test/inbox.test.ts index 9fd8479a..021e3a04 100644 --- a/cloudflare/test/inbox.test.ts +++ b/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('