From d456e0aaf100efd020ec5b440d0a2996ae07563c Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 23 Oct 2026 23:03:53 -0700 Subject: [PATCH 1/2] chore(quest): claim quest/m0/kix/merged-claims.md Quest-Claim: 5be1a07c9e2d4f13 From f355b4089aec86039524da1788d21c1e193513f6 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 23 Oct 2026 23:21:16 -0700 Subject: [PATCH 2/2] feat(board): ignore quest branches left at a merged PR's head The Worker counted every observed quest/* branch as a claim, while the CLI already skipped a branch whose tip is the head of a merged PR. The rule now lives in quest/core as `landed`, which both use: the occupancy pass reads each quest branch's tip and its merged PRs' heads, and drops landed branches. A branch whose PR closed unmerged, or with commits pushed after its merge, still claims its quest. The fake GitHub models merged PRs and branches kept after their PR closed, and the seeded test covers all three cases. Co-Authored-By: Claude Opus 5.5 --- cloudflare/GITHUB.md | 4 ++++ cloudflare/dev/github.ts | 25 +++++++++++++++++------- cloudflare/src/github/state.ts | 25 ++++++++++++++++++++---- cloudflare/test/projects.test.ts | 13 +++++++++++-- cloudflare/test/seed.test.ts | 33 ++++++++++++++++++++++++++++++-- quest/m0/kix/README.md | 1 - quest/m0/kix/merged-claims.md | 27 -------------------------- src/cli/claims.ts | 7 +++++-- src/core/claim.ts | 24 ++++++++++++++++++++--- src/core/index.ts | 2 +- 10 files changed, 112 insertions(+), 49 deletions(-) delete mode 100644 quest/m0/kix/merged-claims.md diff --git a/cloudflare/GITHUB.md b/cloudflare/GITHUB.md index c3c93af1..efda8d12 100644 --- a/cloudflare/GITHUB.md +++ b/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 diff --git a/cloudflare/dev/github.ts b/cloudflare/dev/github.ts index a14af837..abec1535 100644 --- a/cloudflare/dev/github.ts +++ b/cloudflare/dev/github.ts @@ -16,6 +16,10 @@ export type Pull = Issue & { fork?: boolean; /** A `## Decisions needed` section in it hands a draft back to the maintainer. */ body?: string; + /** Closed by merging it; set with `closed`. */ + merged?: boolean; + /** Its branch outlived it closing: still at its head, or with commits pushed after. */ + kept?: "head" | "pushed"; /** GitHub's mergeStateStatus; only CLEAN merges. Defaults to BLOCKED, or DRAFT for a draft. */ merge?: "CLEAN" | "BLOCKED" | "DIRTY" | "UNSTABLE" | "UNKNOWN"; }; @@ -60,7 +64,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 }, @@ -108,14 +112,21 @@ function graphql(repo: Repo, query: string) { if (/(? pullNode(repo, pull))); if (query.includes("open:pullRequests(")) result.open = page(open.map((pull) => pullNode(repo, pull))); if (query.includes("refs(")) { - const branches = [ - ...repo.claims, - ...open.flatMap((p) => (p.branch.startsWith("quest/") && !p.fork ? [p.branch] : [])), - ]; + // Quest branches by tip: claims, open pull requests' heads, and branches kept after theirs closed. + const tips = new Map(repo.claims.map((branch) => [branch, sha1(`claim ${repo.owner}/${repo.name}:${branch}`)])); + for (const pull of repo.pulls) + if (pull.branch.startsWith("quest/") && !pull.fork && (!pull.closed || pull.kept)) + tips.set(pull.branch, pull.kept === "pushed" ? sha1(`pushed ${pull.branch}`) : headOf(repo, pull)); result.refs = page( - [...new Set(branches.map((b) => b.replace(/^quest\//, "")))].map((name) => ({ - name, + [...tips].map(([branch, oid]) => ({ + name: branch.replace(/^quest\//, ""), prefix: "refs/heads/quest/", + target: { oid }, + associatedPullRequests: page( + repo.pulls + .filter((pull) => pull.merged && !pull.fork && pull.branch === branch) + .map((pull) => ({ headRefOid: headOf(repo, pull) })), + ), })), ); } diff --git a/cloudflare/src/github/state.ts b/cloudflare/src/github/state.ts index 581d576d..1d679e69 100644 --- a/cloudflare/src/github/state.ts +++ b/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; pullRequests?: Connection; open?: Connection; - refs?: Connection<{ name: string; prefix: string }>; + refs?: Connection; /** 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)) continue; // Ref.name is relative to the requested prefix. updated.occupancy.claims.push(`quest/${ref.name}`); } diff --git a/cloudflare/test/projects.test.ts b/cloudflare/test/projects.test.ts index aa22ea63..8e86e8f3 100644 --- a/cloudflare/test/projects.test.ts +++ b/cloudflare/test/projects.test.ts @@ -692,6 +692,15 @@ describe("project setup routes", () => { function connection(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"]); diff --git a/cloudflare/test/seed.test.ts b/cloudflare/test/seed.test.ts index 25d98cc2..a126869b 100644 --- a/cloudflare/test/seed.test.ts +++ b/cloudflare/test/seed.test.ts @@ -15,8 +15,11 @@ 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"), @@ -24,6 +27,7 @@ async function seeded() { 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) => { @@ -149,6 +153,31 @@ 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 closed = { author: "contributor", checks: "SUCCESS", review: "APPROVED", closed: true } as const; + data.repos[0]!.pulls.push( + // Its work landed, so the branch holds nothing. + { ...closed, number: 21, title: "Passkeys", branch: "quest/m0/sign-in", merged: true, kept: "head" }, + // Closed unmerged: that work may be wanted back. + { ...closed, number: 22, title: "Offline", branch: "quest/m1/offline", kept: "head" }, + // Pushed to after its merge: new work. + { ...closed, number: 23, title: "Export", branch: "quest/m0/export", merged: true, kept: "pushed" }, + ); + }); + const board = await ( + await request("/projects/github/example/rocket/board", { headers: { Cookie: cookie } }) + ).text(); + const row = (title: string) => board.match(new RegExp(`${title}[^<]*[\\s\\S]*?`))?.[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. + const inbox = await (await request("/inbox", { headers: { Cookie: cookie } })).text(); + expect(inbox).toContain("Add passkey sign-in"); + expect(inbox).not.toContain("Keep working offline"); +}); + it("gets failed and blocked jobs moving again from the inbox", async () => { const { rocket, page, post } = await seeded(); diff --git a/quest/m0/kix/README.md b/quest/m0/kix/README.md index 8da0e5e8..1e84088c 100644 --- a/quest/m0/kix/README.md +++ b/quest/m0/kix/README.md @@ -122,4 +122,3 @@ fixes what it finds: - [Web execution](/quest/m0/kix/execution.md) - spawn and iterate through the UI, ending in contribution PRs - [Seeded GitHub fake](/quest/m0/kix/github-fake.md) - the local server runs publication end to end, and tests share its fake - [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 diff --git a/quest/m0/kix/merged-claims.md b/quest/m0/kix/merged-claims.md deleted file mode 100644 index 2ec1668c..00000000 --- a/quest/m0/kix/merged-claims.md +++ /dev/null @@ -1,27 +0,0 @@ -# [XS] Ignore merged branches on the board - -## Goal - -The kix board and inbox agree with `quest unblocked --unclaimed` about which -quests are claimed: a `quest/*` branch whose tip is the head of a merged pull -request no longer counts as a claim, while a branch whose pull request closed -unmerged still does. - -## Plan - -Decided 2026-10-22 (user) as a follow-up of -[kixelated/quest#192](https://github.com/kixelated/quest/pull/192), which -gave the CLI this rule: - -- `cloudflare/src/github/state.ts` maps every observed `quest/*` ref to a - claim (`associatedQuest`). Apply the same merged-head rule when reading - those refs, and keep the CLI and Worker on one definition of it rather - than a second copy. -- Ranked last in the epic: moq-dev/moq, moq-dev/moq.pro, and kixelated/quest - all delete head branches on merge, so only projects without that setting - see stale claims today. -- Test a merged-head branch, a closed-unmerged branch, and a merged branch - with commits pushed after the merge, with the seeded fake GitHub - (`cloudflare/dev/github.ts`). -- Dropped (user): a CLI sweep that reports or deletes merged quest branches, - since every current repository deletes them on merge. diff --git a/src/cli/claims.ts b/src/cli/claims.ts index 4d457e9f..a814350f 100644 --- a/src/cli/claims.ts +++ b/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,7 +257,10 @@ async function merged( ): Promise { const query = new URLSearchParams({ state: "closed", head: `${head.owner}:${branch}`, per_page: "100" }); const rows = await github.all(`${repositoryPath(repository)}/pulls?${query}`); - return rows.some((row) => typeof row.merged_at === "string" && object(row.head).sha === tip); + return landed( + tip, + rows.map((row) => ({ head: string(object(row.head).sha), merged: typeof row.merged_at === "string" })), + ); } /** Where a claim lands: the remote, its upstream, and the upstream default branch's tip and quest tree. */ diff --git a/src/core/claim.ts b/src/core/claim.ts index 9b441da4..b5288167 100644 --- a/src/core/claim.ts +++ b/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; +} + +/** + * Whether a quest branch at `tip` landed: `tip` is the head of a PR from it + * that merged, 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(tip: string, pulls: Iterable): boolean { + for (const pull of pulls) if (pull.merged && pull.head === tip) return true; + return false; +} diff --git a/src/core/index.ts b/src/core/index.ts index 5eae59a9..6c7da526 100644 --- a/src/core/index.ts +++ b/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";