feat(board): ignore quest branches left at a merged PR's head

cloudflare/GITHUB.md

@@ -431,6 +431,10 @@ branch claims exclude that quest from candidate data; branch claims display as
 Claimed on the board. So does a draft PR its starter still holds, one without a
 `## Decisions needed` section, which the inbox lists as in progress rather than
 as a decision; a ready PR or a handed-back draft is Ready to turn in.
+As in the CLI, a `quest/` branch whose tip is the head of a merged PR from it
+claims nothing, since its work landed; one whose PR closed unmerged, or with
+commits pushed after the merge, still does. A push to an existing branch does
+not invalidate candidates, so such a push shows on the next occupancy pass.
 Runner/run associations belong to job coordination.
 
 Every cached read and refresh proves current maintainer authority, live repository

cloudflare/dev/github.ts

@@ -33,6 +33,10 @@ export type Pull = Issue & {
 	contributor?: string;
 	/** A `## Decisions needed` section in it hands a draft back to the maintainer. */
 	body?: string;
+	/** Closed by merging it; set with `closed`. */
+	merged?: boolean;
+	/** The commit it ended at, when its branch has moved on since it closed; defaults to the branch's tip. */
+	head?: string;
 	/** GitHub's mergeStateStatus; only CLEAN merges. Defaults to BLOCKED, or DRAFT for a draft. */
 	merge?: "CLEAN" | "BLOCKED" | "DIRTY" | "UNSTABLE" | "UNKNOWN";
 	/** When it was opened, in milliseconds; defaults to the world's fixed update time. */
@@ -54,7 +58,7 @@ export type Repo = {
 	/**
 	 * Upstream branches and their commits. A pull request's branch that is not
 	 * listed is at a commit derived from its number; a `quest/*` branch with no
-	 * pull request is a claim.
+	 * open pull request is a claim, unless it is still at a merged one's head.
 	 */
 	branches: Record<string, string>;
 	/** Branches on the repository's service fork, `<forks>/<name>`, and their commits. */
@@ -89,8 +93,9 @@ export const blob = (content: string | Uint8Array) => {
 /** The default branch's commit. */
 export const commitOf = (repo: Repo) => sha1(`commit ${repo.owner}/${repo.name}`);
 const treeOf = (repo: Repo) => sha1(`tree ${repo.owner}/${repo.name}`);
-/** A pull request's head commit: its branch's tip, where the world lists one. */
+/** A pull request's head commit: the one it ended at, or its branch's tip where the world lists one. */
 export const headOf = (repo: Repo, pull: Pull) =>
+	pull.head ??
 	(pull.fork ? repo.fork : pull.contributor ? {} : repo.branches)[pull.branch] ??
 	sha1(`head ${repo.owner}/${repo.name}#${pull.number}`);
 // GitHub's database IDs differ from numbers; derive stable ones per repository.
@@ -105,7 +110,7 @@ function entity(repo: Repo, item: Issue) {
 		databaseId: databaseId(repo, item.number),
 		number: item.number,
 		title: item.title,
-		state: item.closed ? "CLOSED" : "OPEN",
+		state: (item as Pull).merged ? "MERGED" : item.closed ? "CLOSED" : "OPEN",
 		updatedAt,
 		closedAt: item.closed ? updatedAt : null,
 		author: { login: item.author },
@@ -223,10 +228,28 @@ function graphql(repo: Repo, query: string, variables: Record<string, unknown>)
 	if (/(?<!:)pullRequests\(/.test(query)) result.pullRequests = page(repo.pulls.map((pull) => pullNode(repo, pull)));
 	if (query.includes("open:pullRequests(")) result.open = page(open.map((pull) => pullNode(repo, pull)));
 	if (query.includes("refs(")) {
-		const branches = [...Object.keys(repo.branches), ...upstreamPulls(repo).map((p) => p.branch)];
+		// Quest branches at their tips, with the pull requests from them that merged, newest first.
+		const tips = new Map<string, string>();
+		for (const pull of upstreamPulls(repo)) tips.set(pull.branch, headOf(repo, pull));
+		for (const [branch, oid] of Object.entries(repo.branches)) tips.set(branch, oid);
+		const merged = (branch: string) =>
+			repo.pulls
+				.filter((pull) => pull.merged && !pull.fork && !pull.contributor && pull.branch === branch)
+				.reverse();
 		result.refs = page(
-			[...new Set(branches.filter((b) => b.startsWith("quest/")).map((b) => b.slice("quest/".length)))].map(
-				(name) => ({ name, prefix: "refs/heads/quest/" }),
+			[...tips].flatMap(([branch, oid]) =>
+				branch.startsWith("quest/")
+					? [
+							{
+								name: branch.slice("quest/".length),
+								prefix: "refs/heads/quest/",
+								target: { oid },
+								associatedPullRequests: page(
+									merged(branch).map((pull) => ({ headRefOid: headOf(repo, pull) })),
+								),
+							},
+						]
+					: [],
 			),
 		);
 	}
@@ -526,6 +549,7 @@ export function fakeGitHub(world: World) {
 			if (body.sha !== headOf(repo, pull)) return json({ message: "Head branch was modified" }, 409);
 			if (pull.merge !== "CLEAN") return json({ message: "Pull Request is not mergeable" }, 405);
 			pull.closed = true;
+			pull.merged = true;
 			return json({ sha: commitOf(repo), merged: true, message: "Pull Request successfully merged" });
 		}
 		if (tail.startsWith("/contents/")) {

cloudflare/src/github/state.ts

@@ -1,4 +1,4 @@
-import { branchQuest, handedBack, parse, ready } from "quest/core";
+import { branchQuest, handedBack, landed, parse, ready } from "quest/core";
 import type { Project, Snapshot } from "../board/project";
 import { GitHubError, type GitHubRepository, ProjectChanged } from "./service";
 import { positiveId } from "./webhook";
@@ -38,11 +38,18 @@ export type Pull = Entity & {
 	};
 	commits: { nodes: { commit: { oid: string; statusCheckRollup: { state: string } | null } }[] };
 };
+/** A quest branch, its tip, and the merged PRs from it, newest first. */
+type QuestRef = {
+	name: string;
+	prefix: string;
+	target: { oid: string } | null;
+	associatedPullRequests: { nodes: { headRefOid: string }[] };
+};
 export type ActivityPage = {
 	issues?: Connection<Entity>;
 	pullRequests?: Connection<Pull>;
 	open?: Connection<Pull>;
-	refs?: Connection<{ name: string; prefix: string }>;
+	refs?: Connection<QuestRef>;
 	/** Stale entities read by number, in the order requested; null when GitHub no longer has one. */
 	[stale: `s${number}`]: Entity | Pull | null | undefined;
 };
@@ -107,7 +114,7 @@ export function activityQuery({ history, occupancy }: Scan, stale: Stale[] = [])
 	if (occupancy.refs !== false) {
 		variables.push("$refs: String");
 		fields.push(
-			'refs(refPrefix:"refs/heads/quest/",first:100,after:$refs) { nodes { name prefix } pageInfo { hasNextPage endCursor } }',
+			'refs(refPrefix:"refs/heads/quest/",first:100,after:$refs) { nodes { name prefix target { oid } associatedPullRequests(states:MERGED,first:10,orderBy:{field:CREATED_AT,direction:DESC}) { nodes { headRefOid } } } pageInfo { hasNextPage endCursor } }',
 		);
 	}
 	// Numbers come from validated webhook pointers and are interpolated as integers.
@@ -218,8 +225,18 @@ export async function reconcile(
 	};
 	if (occupancy.refs !== false)
 		for (const ref of page.refs!.nodes) {
-			if (typeof ref.name !== "string" || ref.prefix !== "refs/heads/quest/")
+			const tip = ref.target?.oid;
+			const merged = ref.associatedPullRequests?.nodes;
+			if (
+				typeof ref.name !== "string" ||
+				ref.prefix !== "refs/heads/quest/" ||
+				typeof tip !== "string" ||
+				!Array.isArray(merged)
+			)
 				throw new GitHubError("Invalid branch observation");
+			// A branch left at a merged PR's head holds nothing: its work landed.
+			const pulls = merged.map((pull) => ({ head: pull.headRefOid, merged: true }));
+			if (landed(tip, pulls) !== undefined) continue;
 			// Ref.name is relative to the requested prefix.
 			updated.occupancy.claims.push(`quest/${ref.name}`);
 		}

cloudflare/test/projects.test.ts

@@ -692,6 +692,15 @@ describe("project setup routes", () => {
 function connection<T>(nodes: T[], more = false, cursor: string | null = null) {
 	return { nodes, pageInfo: { hasNextPage: more, endCursor: cursor } };
 }
+/** A quest branch that no merged PR left behind. */
+function ref(name: string) {
+	return {
+		name,
+		prefix: "refs/heads/quest/",
+		target: { oid: "b".repeat(40) },
+		associatedPullRequests: { nodes: [] },
+	};
+}
 function issue(id = 100, number = 1) {
 	return {
 		databaseId: String(id),
@@ -724,7 +733,7 @@ function activity(): ActivityPage {
 		issues: connection([issue()]),
 		pullRequests: connection(pulls),
 		open: connection(pulls),
-		refs: connection([{ name: "second", prefix: "refs/heads/quest/" }]),
+		refs: connection([ref("second")]),
 	};
 }
 // A long history whose open PRs and quest branches fit in one page.
@@ -1286,7 +1295,7 @@ describe("webhook reconciliation", () => {
 			refs: connection([]),
 		};
 		await github({ activity: quiet }).service.projectState(repo, "enrollment-test", true);
-		mocked({ activity: { ...quiet, refs: connection([{ name: "first", prefix: "refs/heads/quest/" }]) } });
+		mocked({ activity: { ...quiet, refs: connection([ref("first")]) } });
 		await delivery("push", { ...event, ref: "refs/heads/quest/first", created: true }, undefined, {}, testEnv);
 		const state = await github().service.projectState(repo, "enrollment-test");
 		expect(state!.claims).toEqual(["quest/first.md"]);

cloudflare/test/seed.test.ts

@@ -15,15 +15,19 @@ const testEnv = { ...env, GITHUB_APP_PRIVATE_KEY: privateKey };
 const db = env.DB;
 afterEach(() => vi.restoreAllMocks());
 
-/** Seed a fresh world against the fake GitHub, returning it, a request helper, and the session cookie. */
-async function seeded() {
+/**
+ * Seed a fresh world, after `edit` changes it, against the fake GitHub,
+ * returning it, a request helper, and the session cookie.
+ */
+async function seeded(edit: (world: World) => void = () => {}) {
 	await db.batch([
 		db.prepare("DELETE FROM jobs"),
 		db.prepare("DELETE FROM runners"),
 		db.prepare("DELETE FROM github_projects"),
 		db.prepare("DELETE FROM github_installations"),
 	]);
 	const data = { ...world(), forks: env.GITHUB_FORK_OWNER };
+	edit(data);
 	vi.spyOn(globalThis, "fetch").mockImplementation(fakeGitHub(data));
 	// Webhooks read and notify in the background, so each request finishes its `waitUntil` work.
 	const request = async (path: string, init?: RequestInit) => {
@@ -144,6 +148,38 @@ it("seeds a representative inbox through the Worker's own routes", async () => {
 	expect(data.repos[0]!.pulls.find((p) => p.number === 12)!.closed).toBe(true);
 });
 
+it("ignores a quest branch left at the head of its merged pull request", async () => {
+	const { request, cookie } = await seeded((data) => {
+		const rocket = data.repos[0]!;
+		const closed = { author: "contributor", checks: "SUCCESS", review: "APPROVED", closed: true } as const;
+		rocket.pulls.push(
+			// Its work landed, so the branch holds nothing.
+			{ ...closed, number: 21, title: "Passkeys", branch: "quest/m0/sign-in", merged: true },
+			// Closed unmerged: that work may be wanted back.
+			{ ...closed, number: 22, title: "Offline", branch: "quest/m1/offline" },
+			// Pushed to after its merge: new work.
+			{ ...closed, number: 23, title: "Export", branch: "quest/m0/export", merged: true, head: "e".repeat(40) },
+		);
+		// Each branch outlived its pull request.
+		Object.assign(rocket.branches, {
+			"quest/m0/sign-in": "a".repeat(40),
+			"quest/m1/offline": "b".repeat(40),
+			"quest/m0/export": "c".repeat(40),
+		});
+	});
+	const board = await (
+		await request("/projects/github/example/rocket/board", { headers: { Cookie: cookie } })
+	).text();
+	const row = (title: string) => board.match(new RegExp(`${title}[^<]*</a>[\\s\\S]*?</li>`))?.[0] ?? "";
+	expect(row("Add passkey sign-in")).toContain("Available");
+	expect(row("Keep working offline")).toContain("Claimed");
+	expect(row("Export a project as a ZIP archive")).toContain("Claimed");
+	// The inbox agrees: it offers the landed quest, and not the one new work claims.
+	const inbox = await (await request("/inbox", { headers: { Cookie: cookie } })).text();
+	expect(inbox).toContain("Add passkey sign-in");
+	expect(inbox).not.toContain("Export a project as a ZIP archive");
+});
+
 it("gets failed and blocked jobs moving again from the inbox", async () => {
 	const { rocket, page, post } = await seeded();
 

quest/m0/kix/README.md

@@ -124,4 +124,3 @@ fixes what it finds:
 - [Merge jobs](/quest/m0/kix/merge-job.md) - a merge pick fixes conflicts, CI, and reviews on a runner, then lands the PR
 - [Web execution](/quest/m0/kix/execution.md) - spawn and iterate through the UI, ending in contribution PRs
 - [Onboarding guide](/quest/m0/kix/guide.md) - a complete maintainer path, from runner setup through a reviewed PR
-- [Merged claims](/quest/m0/kix/merged-claims.md) - the board ignores branches whose pull request merged, as the CLI does

src/cli/claims.ts

@@ -4,7 +4,7 @@
 // records a claim the same way, judging readiness by the upstream tree.
 import { execFileSync } from "node:child_process";
 
-import { handedBack } from "../core";
+import { handedBack, landed } from "../core";
 import {
 	type GitHubDependencies,
 	type ObjectValue,
@@ -257,8 +257,15 @@ export async function mergedPull(
 ): Promise<string | null> {
 	const query = new URLSearchParams({ state: "closed", head: `${head.owner}:${branch}`, per_page: "100" });
 	const rows = await github.all(`${repositoryPath(repository)}/pulls?${query}`);
-	const pull = rows.find((row) => typeof row.merged_at === "string" && object(row.head).sha === tip);
-	return pull === undefined ? null : string(pull.html_url);
+	const pull = landed(
+		tip,
+		rows.map((row) => ({
+			head: string(object(row.head).sha),
+			merged: typeof row.merged_at === "string",
+			url: string(row.html_url),
+		})),
+	);
+	return pull?.url ?? null;
 }
 
 /** Where a claim lands: the remote, its upstream, and the upstream default branch's tip and quest tree. */

src/core/claim.ts

@@ -1,6 +1,7 @@
-// How a quest's PR says whose move it is. A draft is its starter's claim until
-// its body hands it back with a `## Decisions needed` section (the guide's
-// Execution section); the CLI and the Worker read that the same way.
+// How a quest's PR and branch say whose move it is. A draft is its starter's
+// claim until its body hands it back with a `## Decisions needed` section, and
+// a branch whose work merged holds nothing (the guide's Execution section);
+// the CLI and the Worker read both the same way.
 
 // Web edits end lines in CRLF.
 const HAND_BACK = /^#{2}[ \t]+Decisions needed[ \t]*#*[ \t]*\r?$/im;
@@ -9,3 +10,20 @@ const HAND_BACK = /^#{2}[ \t]+Decisions needed[ \t]*#*[ \t]*\r?$/im;
 export function handedBack(body: string): boolean {
 	return HAND_BACK.test(body);
 }
+
+/** A closed PR from a quest's branch: the commit it ended at, and whether it merged. */
+export interface ClosedPull {
+	head: string;
+	merged: boolean;
+}
+
+/**
+ * The PR that landed a quest branch at `tip`: one from it that merged with
+ * `tip` as its head, so the branch holds no claim. One whose PR closed unmerged
+ * still holds its quest, since that work may be wanted back, and so does one
+ * with commits pushed after its merge. The CLI and the Worker share this rule.
+ */
+export function landed<P extends ClosedPull>(tip: string, pulls: Iterable<P>): P | undefined {
+	for (const pull of pulls) if (pull.merged && pull.head === tip) return pull;
+	return undefined;
+}

src/core/index.ts

@@ -25,7 +25,7 @@ export {
 	resolve,
 	rooted,
 } from "./doc";
-export { handedBack } from "./claim";
+export { type ClosedPull, handedBack, landed } from "./claim";
 export { type Missed, type Moved, type Source, move, relocate, remove } from "./edit";
 export { comparePaths, normalize } from "./path";
 export { type Blocker, blockers, label, lookup, ready, renderBlocker } from "./ready";