From 10afb2c1c450e1e3581406e53c14a9a43f2b7c13 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 22:44:51 -0700 Subject: [PATCH 1/2] chore(quest): claim quest/m0/helpers/claims.md Quest-Claim: 265ab2d5c3627d77 From 1f197bedae5536945baae325f0061da3fbf1c02e Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 23:26:20 -0700 Subject: [PATCH 2/2] feat(claims): read claims and readiness from the upstream tree A quest branch whose tip is a merged PR's head no longer counts as a claim, and quest start replaces it with a lease on that tip; a closed, unmerged PR's branch still holds its quest. quest unblocked --unclaimed and quest start rank and validate the fetched upstream default branch's tree instead of the checkout. The head repository comes from the remote's push URL, parsed through remoteRepository in github.ts. Completes quest/m0/helpers/claims.md. Co-Authored-By: Claude Opus 5.5 --- README.md | 27 ++-- quest/m0/helpers/README.md | 1 - quest/m0/helpers/claims.md | 32 ----- src/cli/claims.ts | 167 +++++++++++++++++++---- src/cli/github.ts | 41 +++++- src/cli/main.ts | 114 +++++++++------- src/cli/start.ts | 55 ++++---- src/cli/tree.ts | 72 +++++++++- test/start.test.ts | 262 ++++++++++++++++++++++++++++--------- 9 files changed, 560 insertions(+), 211 deletions(-) delete mode 100644 quest/m0/helpers/claims.md diff --git a/README.md b/README.md index e4355494..40f0499d 100644 --- a/README.md +++ b/README.md @@ -139,13 +139,18 @@ when no quests are ready; `--limit ` requires a positive integer. Invalid trees fail with diagnostics and no ranking. `quest-spawn` uses the ranking unless you explicitly select work in another order. -`--unclaimed` drops quests someone already holds: the quest's branch on the +`--unclaimed` ranks the fetched upstream default branch's tree rather than +your checkout, so a stale checkout cannot offer a finished or deleted quest. +It then drops quests someone already holds: the quest's branch on the upstream repository, or an open PR from that branch in any repository, forks -included. It lists each one it skips on stderr with its PR and how long ago -that PR was last active, so you can decide whether a stale claim should be -taken over. In a fork's clone, the upstream is the fork's parent, and the -fork's own branches and PRs count too; `--remote` picks a remote other than -`origin`. Off GitHub, only branches are checked. +included. A branch whose tip is the head of a merged PR holds nothing, since +its work landed; a closed, unmerged PR's branch still holds its quest. It +lists each one it skips on stderr with its PR and how long ago that PR was +last active, so you can decide whether a stale claim should be taken over. +In a fork's clone, the upstream is the fork's parent, and the fork's own +branches and PRs count too; a remote that fetches from upstream and pushes to +a fork works the same way. `--remote` picks a remote other than `origin`. +Off GitHub, only branches are checked. Neither form inspects kix.quest runner locks or outside conditions. ## Claim a quest @@ -158,14 +163,16 @@ quest start --release quest/m0/export.md ``` It pushes an empty claim commit, built on the upstream default branch, to the -quest's branch (its path without `.md`) without forcing, then opens a draft PR +quest's branch (its path without `.md`), then opens a draft PR from it right away and prints the PR's URL. It switches your checkout to the claim on a new local branch, or fast-forwards that branch if it is already -checked out. A fork pushes to itself and opens the PR on its parent. It fails -on a tree `quest check` rejects, which can hide a blocker, and refuses a +checked out. A fork pushes to itself and opens the PR on its parent. It reads +readiness from the upstream default branch it builds on, not your checkout. It +fails on a tree `quest check` rejects, which can hide a blocker, and refuses a blocked quest and one someone else holds; a rejected push means another claimer got there first, and when two PRs race, the earliest open one -keeps the claim and the later one is closed. Off GitHub, it pushes the branch +keeps the claim and the later one is closed. A branch left at a merged PR's +head is replaced, with a lease on that tip. Off GitHub, it pushes the branch without a PR. `--release` deletes the branch, with a lease on its tip, and closes its PR, but diff --git a/quest/m0/helpers/README.md b/quest/m0/helpers/README.md index 71061094..5de2d651 100644 --- a/quest/m0/helpers/README.md +++ b/quest/m0/helpers/README.md @@ -63,7 +63,6 @@ commands from Sep-Oct. Session counts in each child come from that window. - [PR state commands](/quest/m0/helpers/pr-states.md) - `quest iterate`, `ready`, `merge`, and `release` label the one owner of a PR - [Complete loop](/quest/m0/helpers/complete-loop.md) - `/quest-complete` re-scans PRs, keeps picks and holds, and counts rounds per PR - [Headless agents](/quest/m0/helpers/headless-agents.md) - `/quest-spawn api` launches, resumes, and stops billed agents through one command auto mode allows -- [Upstream claims](/quest/m0/helpers/claims.md) - claims and readiness read from the upstream tree, ignoring merged branches - [Wait gaps](/quest/m0/helpers/wait-gaps.md) - `quest wait` reports every event after its starting point exactly once - [Guard in CI](/quest/m0/helpers/ci-guard.md) - pull request CI runs the landed-quest guard diff --git a/quest/m0/helpers/claims.md b/quest/m0/helpers/claims.md deleted file mode 100644 index 1920e0d3..00000000 --- a/quest/m0/helpers/claims.md +++ /dev/null @@ -1,32 +0,0 @@ -# [S] Read claims from the upstream tree - -## Goal - -`quest unblocked --unclaimed` and `quest start` agree with GitHub about which -quests are ready and claimed, even when the local checkout is behind or the -repository keeps merged branches. - -## Plan - -Decided 2026-10-21 from the follow-ups of -[kixelated/quest#164](https://github.com/kixelated/quest/pull/164) and -[kixelated/quest#167](https://github.com/kixelated/quest/pull/167), and a -spawn on 2026-10-21 where two quests that had partly landed were skipped as -"held by branch ..., with no open PR": - -- **Merged heads (user).** A branch whose tip is the head of a merged PR no - longer counts as a claim: its work landed. A closed, unmerged PR's branch - still counts, since it may hold abandoned work someone wants back. - Repositories without GitHub's "Automatically delete head branches" keep - every merged branch; kixelated/quest turned that setting on and deleted - 115 such branches the same day. -- **Upstream tree.** Check readiness against the fetched upstream default - branch's tree, not the local checkout, so a stale checkout cannot report a - finished prerequisite as open or a deleted quest as ready. The landed-quest - guard from [kixelated/quest#169](https://github.com/kixelated/quest/pull/169) - already loads a tree from a ref; reuse it. -- **Head repository.** Derive the head repository from the remote's push URL - (which may differ from its fetch URL), parsing SSH and HTTPS remote URLs - through one helper in `src/cli/github.ts`. -- Test a merged-head branch, a closed-PR branch, a checkout behind upstream, - and a remote whose push URL differs from its fetch URL. diff --git a/src/cli/claims.ts b/src/cli/claims.ts index b5ff04d2..49f596ff 100644 --- a/src/cli/claims.ts +++ b/src/cli/claims.ts @@ -1,17 +1,20 @@ -// Who holds a quest: its claim branch upstream, or an open PR from a branch of -// that name in any repository. Commands that must not disturb held work look -// here first, and `quest start` records a claim the same way. +// Who holds a quest: its claim branch upstream, unless that branch is a merged +// PR's head, or an open PR from a branch of that name in any repository. +// Commands that must not disturb held work look here first, and `quest start` +// records a claim the same way, judging readiness by the upstream tree. import { execFileSync } from "node:child_process"; import { type GitHubDependencies, + type ObjectValue, type Repository, GitHub, - githubRepository, object, + remoteRepository, remoteUrl, string, } from "./github"; +import { type Snapshot, snapshot } from "./tree"; /** The heading that hands a draft back to the maintainer; only its presence matters. */ const HAND_BACK = /^#{2}[ \t]+Decisions needed[ \t]*#*[ \t]*$/im; @@ -70,29 +73,31 @@ export interface Upstream { /** * The upstream of `remote`. A GitHub repository that is a fork resolves to its * parent, reached with the same URL scheme as the remote; anything else is - * its own upstream with no GitHub side. + * its own upstream with no GitHub side. The head repository is where the + * remote pushes, which may be a fork even when it fetches from upstream. */ export async function upstream(root: string, remote: string, deps: ClaimDependencies): Promise { - // The configured URL first, so a URL rewrite (`insteadOf`) still names the GitHub repository. - const configured = gitOutput(root, ["config", "--get", `remote.${remote}.url`]); - const source = githubRepository(configured) ? configured : remoteUrl(root, remote); - const named = githubRepository(source); + const source = remoteRepository(root, remote); const push = gitOutput(root, ["remote", "get-url", "--push", remote]); - if (named === null) return { git: remote, push, github: null }; + if (source.repository === null) return { git: remote, push, github: null }; const client = new GitHub(deps.github()); - const info = object(await (await client.request(repositoryPath(named))).json()); - const self = { owner: string(object(info.owner).login), repo: string(info.name) }; - if (info.fork === true && info.parent !== undefined && info.parent !== null) { - const parent = object(info.parent); + const info = await repositoryInfo(client, source.repository); + const self = info.repository; + // Push somewhere other than the fetched repository, such as a fork, and that is the head; a push + // URL off GitHub can open no PR, so it falls back to the fetched repository. + const pushed = remoteRepository(root, remote, true).repository; + const head = pushed === null || same(pushed, self) ? self : (await repositoryInfo(client, pushed)).repository; + if (info.parent !== null) { + const parent = info.parent; return { // Reach the parent the way the remote is reached, so the same credentials work. - git: string(/^https:/.test(source) ? parent.clone_url : parent.ssh_url), + git: string(/^https:/.test(source.url) ? parent.clone_url : parent.ssh_url), push, github: { client, repository: { owner: string(object(parent.owner).login), repo: string(parent.name) }, - head: self, + head, defaultBranch: string(parent.default_branch), }, }; @@ -100,10 +105,28 @@ export async function upstream(root: string, remote: string, deps: ClaimDependen return { git: remote, push, - github: { client, repository: self, head: self, defaultBranch: string(info.default_branch) }, + github: { client, repository: self, head, defaultBranch: info.defaultBranch }, + }; +} + +/** A repository as GitHub names it, its default branch, and its parent when it is a fork. */ +async function repositoryInfo( + client: GitHub, + named: Repository, +): Promise<{ repository: Repository; defaultBranch: string; parent: ObjectValue | null }> { + const info = object(await (await client.request(repositoryPath(named))).json()); + return { + repository: { owner: string(object(info.owner).login), repo: string(info.name) }, + defaultBranch: string(info.default_branch), + parent: info.fork === true && info.parent !== undefined && info.parent !== null ? object(info.parent) : null, }; } +/** Whether two names are the same repository; GitHub ignores case. */ +function same(a: Repository, b: Repository): boolean { + return a.owner.toLowerCase() === b.owner.toLowerCase() && a.repo.toLowerCase() === b.repo.toLowerCase(); +} + /** The REST path of a repository. */ export function repositoryPath(repository: Repository): string { return `/repos/${encodeURIComponent(repository.owner)}/${encodeURIComponent(repository.repo)}`; @@ -155,38 +178,126 @@ export async function claims( branches: string[], deps: ClaimDependencies, ): Promise { + const up = await reach(root, remote, deps); + return up === null ? [] : (await held(root, up, branches)).claims; +} + +/** + * The upstream of `remote` for reading claims, warning when off GitHub that + * only branches count. Without the remote it warns and returns `null`. + */ +export async function reach(root: string, remote: string, deps: ClaimDependencies): Promise { try { remoteUrl(root, remote); } catch { deps.warn(`no ${remote} remote, so claims were not checked`); - return []; + return null; } const up = await upstream(root, remote, deps); if (up.github === null) deps.warn(`${remote} is not on GitHub, so only its branches were checked for claims`); - return held(root, up, branches); + return up; +} + +/** Which branches someone holds, and which landed branches a new claim replaces. */ +export interface Holdings { + /** Sorted by branch. */ + claims: Claim[]; + /** + * Branches where the remote pushes whose tip is the head of a merged PR, + * by that tip. Their work landed, so they hold nothing and a claim may replace them. + */ + landed: Map; } /** - * Which of `branches` someone holds, sorted by branch. A fork's own branches - * and PRs count too, so a claim pushed there is seen before its PR opens. + * Which of `branches` someone holds. A fork's own branches and PRs count too, + * so a claim pushed there is seen before its PR opens. On GitHub, a branch + * whose tip is a merged PR's head holds nothing: its work landed. A closed, + * unmerged PR's branch still holds its quest, since that work may be wanted back. */ -export async function held(root: string, up: Upstream, branches: string[]): Promise { - const pushed = new Set(remoteBranches(root, up.git).keys()); +export async function held(root: string, up: Upstream, branches: string[]): Promise { + // Where claim branches live: upstream, then the fork the remote pushes to. + const sources: { repository: Repository | null; tips: Map }[] = [ + { repository: up.github?.repository ?? null, tips: remoteBranches(root, up.git) }, + ]; const pulls: OpenPull[] = []; if (up.github) { pulls.push(...(await openPullRequests(up.github.client, up.github.repository))); const { head, repository } = up.github; if (head.owner !== repository.owner || head.repo !== repository.repo) { - for (const branch of remoteBranches(root, up.push).keys()) pushed.add(branch); + sources.push({ repository: head, tips: remoteBranches(root, up.push) }); pulls.push(...(await openPullRequests(up.github.client, head))); } } - const result: Claim[] = []; - for (const branch of [...new Set(branches)].sort()) { + const pushTo = sources[sources.length - 1]; + const landed = new Map(); + const holds = async (branch: string): Promise => { const pull = pullsFrom(pulls, branch)[0] ?? null; - if (pull !== null || pushed.has(branch)) result.push({ branch, url: pull?.url ?? null, pull }); + if (pull !== null) return { branch, url: pull.url, pull }; + let holding = false; + for (const source of sources) { + const tip = source.tips.get(branch); + if (tip === undefined) continue; + if ( + up.github && + source.repository && + (await merged(up.github.client, up.github.repository, source.repository, branch, tip)) + ) { + if (source === pushTo) landed.set(branch, tip); + } else { + holding = true; + } + } + return holding ? { branch, url: null, pull: null } : null; + }; + const found = await Promise.all([...new Set(branches)].sort().map(holds)); + return { claims: found.filter((claim): claim is Claim => claim !== null), landed }; +} + +/** Whether `tip` is the head of a PR from `head`'s `branch` that merged into `repository`. */ +async function merged( + github: GitHub, + repository: Repository, + head: Repository, + branch: string, + tip: string, +): 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); +} + +/** Where a claim lands: the remote, its upstream, and the upstream default branch's tip and quest tree. */ +export interface Target { + remote: string; + up: Upstream; + /** The upstream default branch's tip, fetched. */ + base: string; + tree: Snapshot; +} + +/** The target for claims pushed to `remote`, read from the fetched upstream default branch. */ +export async function target(root: string, remote: string, deps: ClaimDependencies): Promise { + const up = await upstream(root, remote, deps); + const base = upstreamHead(root, up.git); + return { remote, up, base, tree: snapshot(root, base) }; +} + +/** The tip of `remote`'s default branch (a name or URL), fetched so it can be read and built on. */ +export function upstreamHead(root: string, remote: string): string { + const sha = gitOutput(root, ["ls-remote", remote, "HEAD"]).split("\t")[0]; + if (!sha) throw new Error(`${remote} has no default branch`); + ensure(root, remote, sha); + return sha; +} + +/** Make `sha` from `remote` available locally. */ +export function ensure(root: string, remote: string, sha: string): void { + try { + gitOutput(root, ["cat-file", "-e", `${sha}^{commit}`]); + } catch { + gitOutput(root, ["fetch", "--quiet", "--no-tags", "--no-write-fetch-head", remote, sha]); } - return result; } /** Run git in `root` and return its trimmed stdout; an unset config key is empty rather than an error. */ diff --git a/src/cli/github.ts b/src/cli/github.ts index 78c1ecb7..4cab269f 100644 --- a/src/cli/github.ts +++ b/src/cli/github.ts @@ -38,7 +38,7 @@ export type ObjectValue = Record; /** Accept a GitHub PR URL, or a number in the origin repository. */ export function pullRequest(value: string, root: string): PullRequest { if (/^[1-9]\d*$/.test(value)) { - const repository = githubRepository(remoteUrl(root, "origin")); + const { repository } = remoteRepository(root, "origin"); if (!repository) throw new Error("origin must name a github.com repository; pass a GitHub PR URL instead"); return { ...repository, number: Number(value) }; } @@ -55,10 +55,45 @@ export function remoteUrl(root: string, remote: string): string { }).trim(); } -/** The github.com repository a remote URL names, or `null` for any other host. */ +/** + * The URL `remote` fetches from, or pushes to with `push`, and the github.com + * repository it names. The configured URL comes first, so a rewrite + * (`insteadOf`) to another host still names GitHub; otherwise the rewritten one. + */ +export function remoteRepository( + root: string, + remote: string, + push = false, +): { url: string; repository: Repository | null } { + const git = (args: string[]) => { + try { + return execFileSync("git", ["-C", root, ...args], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }).trim(); + } catch { + // An unset config key exits 1. + return ""; + } + }; + const configured = + (push ? git(["config", "--get", `remote.${remote}.pushurl`]) : "") || + git(["config", "--get", `remote.${remote}.url`]); + const named = githubRepository(configured); + if (named !== null) return { url: configured, repository: named }; + const url = push ? git(["remote", "get-url", "--push", remote]) : remoteUrl(root, remote); + return { url, repository: githubRepository(url) }; +} + +/** + * The github.com repository a remote URL names, or `null` for any other host: + * HTTPS with or without credentials, `ssh://`, or the scp-like `user@github.com:` form. + */ export function githubRepository(url: string): Repository | null { const match = - /^(?:https:\/\/github\.com\/|git@github\.com:|ssh:\/\/git@github\.com\/)([^/]+)\/([^/]+?)(?:\.git)?$/.exec(url); + /^(?:https:\/\/(?:[^@/]+@)?github\.com\/|ssh:\/\/(?:[^@/]+@)?github\.com(?::\d+)?\/|[^@/:]+@github\.com:)([^/]+)\/([^/]+?)(?:\.git)?\/?$/.exec( + url, + ); return match ? { owner: match[1], repo: match[2] } : null; } diff --git a/src/cli/main.ts b/src/cli/main.ts index 44c2a2d9..f07e6089 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -18,16 +18,16 @@ import { unblocked, } from "../core"; import { AgentInputError, HARNESSES, type Launch, processDependencies, runAgent, stopAgent, stopAll } from "./agent"; -import { claims, questBranch } from "./claims"; +import { held, questBranch, reach, target, upstreamHead } from "./claims"; import { gate } from "./gate"; import { gitHubDependencies, GitHubInputError, pullRequest } from "./github"; import { landed } from "./landed"; import { type Report, moveQuest, removeQuest } from "./move"; import { init, uninstall } from "./setup"; -import { Refusal, describe, releaseQuest, startQuest } from "./start"; +import { Refusal, describe, notUpstream, releaseQuest, startQuest } from "./start"; import { GUIDE, SKILLS, skill } from "./skills"; import { type Terminal, Theme } from "./theme"; -import { collect, exists, load, under } from "./tree"; +import { type Snapshot, checkout, collect, exists, load, snapshot, under } from "./tree"; import { pullRequestStatus, statusLines } from "./status"; import { waitForActivity, WaitTimeout } from "./wait"; import { Worktrees } from "./worktree"; @@ -117,16 +117,21 @@ export function main(argv: string[], out: Output): number | Promise { }); /** The quest document `path` names, as the tree writes it, as the shell completes it, or as a filesystem path. */ - const find = (docs: Doc[], path: string): Doc => { + const locate = (docs: Doc[], path: string): Doc | null => { const inside = withinRoot(root(), path); - const doc = lookup(docs, path) ?? (inside === null ? null : lookup(docs, inside)); + return lookup(docs, path) ?? (inside === null ? null : lookup(docs, inside)); + }; + + /** Like `locate`, failing when `path` names no quest. */ + const find = (docs: Doc[], path: string): Doc => { + const doc = locate(docs, path); if (doc === null) throw new Error(`${path} is not a quest document under ${under(root(), "quest")}`); return doc; }; /** Whether the tree passes `quest check`; otherwise report its findings and fail. */ - const valid = (docs: Doc[]): boolean => { - const findings = check(docs, (path) => exists(root(), path)); + const valid = ({ docs, exists }: Snapshot): boolean => { + const findings = check(docs, exists); for (const finding of findings) warn(formatFinding(finding)); if (findings.length > 0) code = 1; return findings.length === 0; @@ -154,12 +159,9 @@ export function main(argv: string[], out: Output): number | Promise { }; /** A claim command's outcome: 0 when done, 3 when refused, and 1 for any other failure. */ - const settle = (work: Promise, done: (result: T) => void) => { + const settle = (work: Promise, done: (result: T) => number | void) => { pending = work.then( - (result) => { - done(result); - return 0; - }, + (result) => done(result) ?? 0, (error: unknown) => { warn(error instanceof Error ? error.message : String(error)); return error instanceof Refusal ? 3 : 1; @@ -176,10 +178,13 @@ export function main(argv: string[], out: Output): number | Promise { "Counts exclude structural epic membership. Other prerequisites may remain for dependents. " + "Invalid trees fail before printing a ranking. Prints a count and path per line; --json prints an " + "ordered array of {path, dependents} objects.\n\n" + - "--unclaimed drops quests someone holds: a quest branch upstream, or an open PR from that branch in " + - "any repository. A GitHub fork's upstream is its parent, and the fork's own branches and PRs count " + - "too. Each skipped quest is listed on stderr with its PR and how long ago that PR was last active, so a stale claim can be offered for takeover. Off " + - "GitHub, only branches are checked. Outside conditions are never checked.\n\n" + + "--unclaimed ranks the fetched upstream default branch's tree instead of the checkout, then drops " + + "quests someone holds: a quest branch upstream, or an open PR from that branch in any repository. A " + + "branch whose tip is a merged PR's head holds nothing; a closed, unmerged PR's branch still does. A " + + "GitHub fork's upstream is its parent, and the branches and PRs of the fork the remote pushes to " + + "count too. Each skipped quest is listed on stderr with its PR and how long ago that PR was last " + + "active, so a stale claim can be offered for takeover. Off GitHub, only branches are checked. " + + "Outside conditions are never checked.\n\n" + "Given a path, piped output lists its blockers, one per line with nesting indented, with an " + "explanation on stderr; no output means it is ready. On a terminal it prints a quest log instead: a " + "yellow ! for a ready quest, a grey ! for a blocked one, and each size label in its colour. Set " + @@ -196,16 +201,14 @@ export function main(argv: string[], out: Output): number | Promise { path: string | undefined, options: { json?: boolean; limit?: number; unclaimed?: boolean; remote: string }, ) => { - const docs = load(root()); if (path !== undefined) { if (options.json || options.limit !== undefined || options.unclaimed) { throw new UsageError("--json, --limit, and --unclaimed apply to the ranking; omit the path"); } + const docs = load(root()); explain(docs, find(docs, path), path); return; } - if (!valid(docs)) return; - const ranked = unblocked(docs); const show = (quests: RankedQuest[]) => { const shown = quests.slice(0, options.limit); if (options.json) { @@ -217,21 +220,32 @@ export function main(argv: string[], out: Output): number | Promise { for (const quest of shown) print(`${quest.dependents}\t${quest.path}`); }; if (!options.unclaimed) { - show(ranked); + const tree = checkout(root()); + if (valid(tree)) show(unblocked(tree.docs)); return; } - const branches = ranked.map((quest) => questBranch(quest.path)); + // Readiness comes from the upstream default branch, so a stale checkout cannot offer + // a deleted quest or one whose prerequisite has since finished. + const deps = { github: () => gitHubDependencies(warn), warn }; settle( - claims(root(), options.remote, branches, { github: () => gitHubDependencies(warn), warn }), - (held) => { - const byBranch = new Map(held.map((claim) => [claim.branch, claim])); + (async () => { + const up = await reach(root(), options.remote, deps); + const tree = up === null ? checkout(root()) : snapshot(root(), upstreamHead(root(), up.git)); + if (!valid(tree)) return null; + const ranked = unblocked(tree.docs); + const branches = ranked.map((quest) => questBranch(quest.path)); + return { ranked, holdings: up === null ? [] : (await held(root(), up, branches)).claims }; + })(), + (result) => { + if (result === null) return 1; + const byBranch = new Map(result.holdings.map((claim) => [claim.branch, claim])); const now = Date.now(); - for (const quest of ranked) { + for (const quest of result.ranked) { const claim = byBranch.get(questBranch(quest.path)); if (claim) warn(`skipped ${quest.path}: held by ${describe(claim.branch, claim.pull, now)}`); } - show(ranked.filter((quest) => !byBranch.has(questBranch(quest.path)))); + show(result.ranked.filter((quest) => !byBranch.has(questBranch(quest.path)))); }, ); }, @@ -242,16 +256,19 @@ export function main(argv: string[], out: Output): number | Promise { .summary("Claim a ready quest with a draft PR, or release that claim") .description( "Claim a ready quest: push an empty claim commit, built on the upstream default branch, to the quest's " + - "branch (its path without .md) without forcing, then open a draft PR from it right away. Switches the " + + "branch (its path without .md), then open a draft PR from it right away. Switches the " + "checkout to the claim on a new local branch, or fast-forwards that branch when it is checked out, " + "and prints the PR URL, or the branch off GitHub.\n\n" + - "Fails on a tree quest check rejects, which can hide a blocker. " + - "Refuses a blocked quest, one missing from the upstream default branch, and one already held: a " + - "branch upstream or an open PR from that branch in any repository. A rejected push means someone " + - "else pushed the branch first. When two PRs race, as from forks, the earliest open PR keeps the " + - "claim; a later one is closed and its branch deleted.\n\n" + + "Readiness comes from the fetched upstream default branch's tree, not the checkout. Fails on a tree " + + "quest check rejects, which can hide a blocker. Refuses a blocked quest, one missing from the " + + "upstream default branch, and one already held: a branch upstream or an open PR from that branch in " + + "any repository. A branch whose tip is a merged PR's head holds nothing, and the claim replaces it " + + "with a lease on that tip. A rejected push means someone else pushed the branch first. When two PRs " + + "race, as from forks, the earliest open PR keeps the claim; a later one is closed and its branch " + + "deleted.\n\n" + "The remote's upstream holds claims: a GitHub fork pushes the branch to itself and opens the PR on its " + - "parent. Off GitHub, only the branch is pushed, with a warning.\n\n" + + "parent, and so does a remote that fetches from upstream but pushes to a fork. Off GitHub, only the " + + "branch is pushed, with a warning.\n\n" + "--release deletes the quest's branch on the remote, with a lease on its tip, and closes its PR, only " + "while that tip is still an empty commit.\n\n" + "Uses GH_TOKEN, GITHUB_TOKEN, or gh auth token on GitHub. Exits 0 when done, 3 when refused " + @@ -261,25 +278,32 @@ export function main(argv: string[], out: Output): number | Promise { .option("--release", "Release the claim instead: delete the empty claim branch and close its PR") .option("--remote ", "Remote to push the claim to", "origin") .action((path: string, options: { release?: boolean; remote: string }) => { - const docs = load(root()); - const doc = find(docs, path); const deps = { github: () => gitHubDependencies(warn), warn }; if (options.release) { - settle(releaseQuest(root(), doc.path, options.remote, deps), (released) => { + settle(releaseQuest(root(), find(load(root()), path).path, options.remote, deps), (released) => { print(`deleted ${released.branch}`); for (const url of released.closed) print(`closed ${url}`); }); return; } - // A malformed tree can hide a blocker, as a misspelled `## Required` does. - if (!valid(docs)) return; - if (blockers(docs, doc).length > 0) { - explain(docs, doc, path); - code = 3; - return; - } - settle(startQuest(root(), doc.path, doc.title?.text ?? doc.path, options.remote, deps), (started) => - print(started.url ?? started.branch), + settle( + (async () => { + // Readiness comes from the upstream default branch the claim builds on, not the checkout. + const at = await target(root(), options.remote, deps); + const doc = locate(at.tree.docs, path); + if (doc === null) throw notUpstream(find(load(root()), path).path); + // A malformed tree can hide a blocker, as a misspelled `## Required` does. + if (!valid(at.tree)) return 1; + if (blockers(at.tree.docs, doc).length > 0) { + explain(at.tree.docs, doc, path); + return 3; + } + return startQuest(root(), at, doc.path, doc.title?.text ?? doc.path, deps); + })(), + (started) => { + if (typeof started === "number") return started; + print(started.url ?? started.branch); + }, ); }); diff --git a/src/cli/start.ts b/src/cli/start.ts index 9721d449..ee7f9242 100644 --- a/src/cli/start.ts +++ b/src/cli/start.ts @@ -7,7 +7,9 @@ import { randomBytes } from "node:crypto"; import { type ClaimDependencies, type OpenPull, + type Target, type Upstream, + ensure, gitOutput, held, openPullRequests, @@ -16,6 +18,7 @@ import { questBranch, repositoryPath, upstream, + upstreamHead, } from "./claims"; import { object, string } from "./github"; @@ -30,25 +33,28 @@ export interface Started { url: string | null; } -/** Claim `path`, a ready quest, on `remote`. Throws `Refusal` when someone else holds it. */ +/** The refusal for a quest the upstream default branch lacks: finished upstream, or not merged yet. */ +export function notUpstream(path: string): Refusal { + return new Refusal(`${path} is not on the upstream default branch; update your checkout`); +} + +/** + * Claim `path`, a quest ready in the target's upstream tree. Throws `Refusal` + * when someone else holds it or the upstream tree lacks it. A branch left by + * a merged PR holds nothing, so the claim replaces it with a lease on its tip. + */ export async function startQuest( root: string, + { remote, up, base, tree }: Target, path: string, title: string, - remote: string, deps: ClaimDependencies, ): Promise { const branch = questBranch(path); - const up = await upstream(root, remote, deps); - const [claim] = await held(root, up, [branch]); + if (!tree.docs.some((doc) => doc.path === path)) throw notUpstream(path); + const { claims, landed } = await held(root, up, [branch]); + const [claim] = claims; if (claim) throw new Refusal(`${path} is held by ${describe(claim.branch, claim.pull, Date.now())}`); - - // Build the claim on the upstream default branch, which must still have the quest. - const base = remoteHead(root, up.git); - const prefix = gitOutput(root, ["rev-parse", "--show-prefix"]); - if (!succeeds(root, ["cat-file", "-e", `${base}:${prefix}${path}`])) { - throw new Refusal(`${path} is not on the upstream default branch; update your checkout`); - } // A nonce keeps two claimers on the same base, identity, and second from building the same commit. const nonce = randomBytes(8).toString("hex"); const sha = gitOutput(root, [ @@ -61,7 +67,13 @@ export async function startQuest( "-m", `Quest-Claim: ${nonce}`, ]); - push(root, remote, [`${sha}:refs/heads/${branch}`], `someone pushed ${branch} first`); + // The lease expects the landed tip, or no branch at all. + push( + root, + remote, + [`--force-with-lease=refs/heads/${branch}:${landed.get(branch) ?? ""}`, `${sha}:refs/heads/${branch}`], + `someone pushed ${branch} first`, + ); let url: string | null = null; if (up.github === null) { @@ -175,7 +187,7 @@ export async function releaseQuest( const tip = remoteBranches(root, up.push).get(branch); if (!tip) throw new Error(`${remote} has no branch ${branch} to release`); ensure(root, up.push, tip); - const base = remoteHead(root, up.git); + const base = upstreamHead(root, up.git); const parents = gitOutput(root, ["rev-list", "--parents", "-n", "1", tip]).split(" ").slice(1); // Only the claim itself: one empty commit straight on the default branch, not one following pushed work. const claim = @@ -229,21 +241,6 @@ async function close(up: Upstream, number: number): Promise { }); } -/** The upstream default branch's tip, fetched so a commit can build on it. */ -function remoteHead(root: string, remote: string): string { - const sha = gitOutput(root, ["ls-remote", remote, "HEAD"]).split("\t")[0]; - if (!sha) throw new Error(`${remote} has no default branch to build a claim on`); - ensure(root, remote, sha); - return sha; -} - -/** Make `sha` from `remote` available locally. */ -function ensure(root: string, remote: string, sha: string): void { - if (!succeeds(root, ["cat-file", "-e", `${sha}^{commit}`])) { - gitOutput(root, ["fetch", "--quiet", "--no-tags", "--no-write-fetch-head", remote, sha]); - } -} - /** Delete `branch` on `remote` only while its tip is still `sha`. */ function deleteBranch(root: string, remote: string, branch: string, sha: string): void { push( @@ -255,7 +252,7 @@ function deleteBranch(root: string, remote: string, branch: string, sha: string) } /** - * Push without forcing. A rejection, which means the remote ref is not what + * Push, forcing only where a lease in `args` allows. A rejection, which means the remote ref is not what * this push expects, is a `Refusal` with `rejected`; anything else fails. */ function push(root: string, remote: string, args: string[], rejected: string): void { diff --git a/src/cli/tree.ts b/src/cli/tree.ts index 8b6edef7..6a5c6689 100644 --- a/src/cli/tree.ts +++ b/src/cli/tree.ts @@ -1,5 +1,7 @@ -// Loading the quest tree from a checkout on disk for the core to read. +// Loading the quest tree for the core to read: from a checkout on disk, or +// from a commit, such as the upstream default branch's tip. +import { execFileSync } from "node:child_process"; import { existsSync, readFileSync, readdirSync } from "node:fs"; import { join } from "node:path"; @@ -53,6 +55,66 @@ export function exists(root: string, path: string): boolean { return existsSync(join(root, path)); } +/** A quest tree as `quest check` reads it: its documents, and whether a repository-relative path exists. */ +export interface Snapshot { + docs: Doc[]; + exists(path: string): boolean; +} + +/** The quest tree in the checkout at `root`. */ +export function checkout(root: string): Snapshot { + return { docs: load(root), exists: (path) => exists(root, path) }; +} + +/** + * The quest tree at `commit`, read from git rather than the checkout, with + * paths relative to `root` as on disk. A symbolic link is a path, not a quest. + */ +export function snapshot(root: string, commit: string): Snapshot { + const paths = new Set(); + const quests: { path: string; oid: string }[] = []; + // ` \t`; run in `root`, git lists only its subtree, relative to it. + for (const entry of git(root, ["ls-tree", "-r", "-z", commit]).toString("utf8").split("\0")) { + if (entry === "") continue; + const tab = entry.indexOf("\t"); + const [mode, type, oid] = entry.slice(0, tab).split(" "); + const path = entry.slice(tab + 1); + paths.add(path); + for (let dir = path; dir.includes("/");) { + dir = dir.slice(0, dir.lastIndexOf("/")); + paths.add(dir); + } + if (type === "blob" && mode !== "120000" && path.startsWith("quest/") && isQuest(path)) { + quests.push({ path, oid }); + } + } + if (quests.length === 0) throw new Error(`no quest documents found under ${under(root, "quest")} at ${commit}`); + + // One `cat-file --batch` reads every document: ` blob \n\n` each, in input order. + const batch = git(root, ["cat-file", "--batch"], `${quests.map((quest) => quest.oid).join("\n")}\n`); + let offset = 0; + const docs = quests.map(({ path }) => { + const header = batch.indexOf(10, offset); + const size = Number(batch.toString("utf8", offset, header).split(" ")[2]); + const text = decode(batch.subarray(header + 1, header + 1 + size), path); + offset = header + size + 2; + return { path, text }; + }); + return { + docs: docs.sort((a, b) => comparePaths(a.path, b.path)).map((source) => parse(source.path, source.text)), + exists: (path) => paths.has(path), + }; +} + +function git(root: string, args: string[], input = ""): Buffer { + try { + return execFileSync("git", ["-C", root, ...args], { input, maxBuffer: 1 << 30, stdio: "pipe" }); + } catch (error) { + const stderr = String((error as { stderr?: unknown }).stderr ?? "").trim(); + throw new Error(`git ${args[0]} failed${stderr ? `: ${stderr}` : ""}`); + } +} + // Fatal, so a file that is not UTF-8 is an error rather than silently mangled // text; ignoreBOM keeps a byte-order mark in the text, as the parser expects. const UTF8 = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }); @@ -64,3 +126,11 @@ function read(root: string, path: string): string { throw new Error(`reading ${path}: ${(error as Error).message}`); } } + +function decode(bytes: Uint8Array, path: string): string { + try { + return UTF8.decode(bytes); + } catch (error) { + throw new Error(`reading ${path}: ${(error as Error).message}`); + } +} diff --git a/test/start.test.ts b/test/start.test.ts index d09bafbb..67ac510b 100644 --- a/test/start.test.ts +++ b/test/start.test.ts @@ -9,11 +9,11 @@ import { join } from "node:path"; import { afterAll, describe, expect, test, vi } from "vitest"; -import { claims } from "../src/cli/claims"; -import { type GitHubDependencies, type ObjectValue } from "../src/cli/github"; +import { claims, target } from "../src/cli/claims"; +import { type GitHubDependencies, type ObjectValue, githubRepository } from "../src/cli/github"; import { type Output, main } from "../src/cli/main"; import { Refusal, ago, releaseQuest, startQuest } from "../src/cli/start"; -import { TWO, Tree, tempDir } from "./fixture"; +import { EPIC_README, TWO, Tree, tempDir } from "./fixture"; const ONE = "quest/m0/epic/one.md"; const BRANCH = "quest/m0/epic/one"; @@ -84,9 +84,10 @@ function clone(url: string, routes: Record = {}): string { interface Pull { number: number; base: string; - head: { ref: string; repo: { full_name: string } | null }; + head: { ref: string; sha?: string; repo: { full_name: string } | null }; draft: boolean; state: string; + merged_at?: string | null; body: string; created_at: string; updated_at: string; @@ -110,6 +111,21 @@ class FakeGitHub { this.repos.set(name, { owner: { login: owner }, name: repo, full_name: name, ...extra }); } + /** A fork of example/project. */ + fork(name: string): void { + this.repo(name, { + fork: true, + default_branch: "main", + parent: { + owner: { login: "example" }, + name: "project", + clone_url: UPSTREAM, + ssh_url: "git@github.com:example/project.git", + default_branch: "main", + }, + }); + } + /** Open a PR as another contributor, `minutes` before now. */ open(base: string, ref: string, headRepo: string, options: Partial = {}): Pull { const number = this.pulls.length + 1; @@ -129,10 +145,31 @@ class FakeGitHub { return pull; } + /** Close a PR from `ref` of `headRepo` whose head was `sha`, merged or not. */ + closed(headRepo: string, ref: string, sha: string, merged: boolean): Pull { + return this.open("example/project", ref, headRepo, { + head: { ref, sha, repo: { full_name: headRepo } }, + state: "closed", + merged_at: merged ? new Date(this.clock).toISOString() : null, + }); + } + open_(base: string): Pull[] { return this.pulls.filter((pull) => pull.base === base && pull.state === "open"); } + /** The listing's `state` (default open) and `head` (`owner:ref`) filters. */ + private list(base: string, query: URLSearchParams): Pull[] { + const state = query.get("state") ?? "open"; + const head = query.get("head"); + return this.pulls.filter( + (pull) => + pull.base === base && + (state === "all" || pull.state === state) && + (head === null || `${pull.head.repo?.full_name.split("/")[0]}:${pull.head.ref}` === head), + ); + } + deps(): GitHubDependencies { return { fetch: async (input, init) => this.handle(new URL(String(input)), init ?? {}), @@ -153,7 +190,7 @@ class FakeGitHub { return repo ? json(200, repo) : json(404, { message: "Not Found" }); } match = /^\/repos\/([^/]+\/[^/]+)\/pulls$/.exec(url.pathname); - if (match && method === "GET") return json(200, this.open_(match[1])); + if (match && method === "GET") return json(200, this.list(match[1], url.searchParams)); if (match && method === "POST") { const [owner, ref] = String(body.head).includes(":") ? String(body.head).split(":") : [null, body.head]; const headRepo = owner ? `${owner}/${match[1].split("/")[1]}` : match[1]; @@ -178,6 +215,23 @@ function deps(github: FakeGitHub, warnings: string[] = []) { return { github: () => github.deps(), warn: (message: string) => warnings.push(message) }; } +/** Commit `files` in `work`, where `null` deletes one, and push them to origin's default branch. */ +function land(work: string, files: Record): void { + for (const [path, text] of Object.entries(files)) { + if (text === null) rmSync(join(work, path)); + else writeFileSync(join(work, path), text); + } + git(work, "add", "-A"); + git(work, "commit", "-q", "-m", "land"); + git(work, "push", "-q", "origin", "HEAD:refs/heads/main"); +} + +/** Claim `path` from `work` as `quest start` does, reading the upstream tree first. */ +async function start(work: string, github: FakeGitHub, warnings: string[] = [], path = ONE) { + const dependencies = deps(github, warnings); + return startQuest(work, await target(work, "origin", dependencies), path, "[S] One", dependencies); +} + /** The branch tip on a bare repository, or "" when it is missing. */ function tip(bare: string, branch = BRANCH): string { return git(bare, "for-each-ref", "--format=%(objectname)", `refs/heads/${branch}`); @@ -197,7 +251,7 @@ describe("start on GitHub", () => { const bare = upstream(); const github = new FakeGitHub(); const work = clone(UPSTREAM, { [UPSTREAM]: bare }); - const started = await startQuest(work, ONE, "[S] One", "origin", deps(github)); + const started = await start(work, github); expect(started).toEqual({ branch: BRANCH, sha: tip(bare), url: "https://github.com/example/project/pull/1" }); const base = git(bare, "rev-parse", "main"); @@ -228,7 +282,7 @@ describe("start on GitHub", () => { git(work, "push", "-q", "origin", "HEAD:refs/heads/main"); const github = new FakeGitHub(); - await startQuest(join(work, "sub"), ONE, "[S] One", "origin", deps(github)); + await start(join(work, "sub"), github); expect(github.pulls[0].body).toBe(`Claims [${ONE}](https://github.com/example/project/blob/main/sub/${ONE}).`); }); @@ -237,7 +291,7 @@ describe("start on GitHub", () => { const work = clone(UPSTREAM, { [UPSTREAM]: bare }); git(work, "switch", "-q", "-c", BRANCH); const warnings: string[] = []; - const started = await startQuest(work, ONE, "[S] One", "origin", deps(new FakeGitHub(), warnings)); + const started = await start(work, new FakeGitHub(), warnings); expect(warnings).toEqual([]); expect(git(work, "rev-parse", "HEAD")).toBe(started.sha); }); @@ -247,7 +301,7 @@ describe("start on GitHub", () => { const work = clone(UPSTREAM, { [UPSTREAM]: bare }); git(work, "branch", BRANCH); const warnings: string[] = []; - const started = await startQuest(work, ONE, "[S] One", "origin", deps(new FakeGitHub(), warnings)); + const started = await start(work, new FakeGitHub(), warnings); expect(warnings).toEqual([ `local branch ${BRANCH} already exists; merge the claim commit ${started.sha} into it before pushing`, ]); @@ -259,17 +313,7 @@ describe("start on GitHub", () => { const forkBare = tempDir(); git(forkBare, "init", "-q", "--bare", "-b", "main"); const github = new FakeGitHub(); - github.repo("me/project", { - fork: true, - default_branch: "main", - parent: { - owner: { login: "example" }, - name: "project", - clone_url: UPSTREAM, - ssh_url: "git@github.com:example/project.git", - default_branch: "main", - }, - }); + github.fork("me/project"); // `corp:me/project` expands to the fork's HTTPS URL, so the parent is read over HTTPS too. // The SSH form leads nowhere, so choosing it fails fast; pushes go to the fork's bare repository. const work = clone("corp:me/project.git", { @@ -278,7 +322,7 @@ describe("start on GitHub", () => { }); git(work, "config", "url.https://github.com/.insteadOf", "corp:"); git(work, "config", "remote.origin.pushurl", forkBare); - const started = await startQuest(work, ONE, "[S] One", "origin", deps(github)); + const started = await start(work, github); expect(tip(forkBare)).toBe(started.sha); }); @@ -286,23 +330,90 @@ describe("start on GitHub", () => { const bare = upstream(); git(bare, "branch", BRANCH, "main"); const work = clone(UPSTREAM, { [UPSTREAM]: bare }); - await expect(startQuest(work, ONE, "[S] One", "origin", deps(new FakeGitHub()))).rejects.toThrow( + await expect(start(work, new FakeGitHub())).rejects.toThrow( new Refusal(`${ONE} is held by branch ${BRANCH}, with no open PR`), ); }); + test("replaces a branch whose tip a merged PR landed", async () => { + const bare = upstream(); + git(bare, "branch", BRANCH, "main"); + const landed = tip(bare); + const github = new FakeGitHub(); + github.closed("example/project", BRANCH, landed, true); + const work = clone(UPSTREAM, { [UPSTREAM]: bare }); + + expect(await claims(work, "origin", [BRANCH], deps(github))).toEqual([]); + const started = await start(work, github); + expect(started.sha).not.toBe(landed); + expect(tip(bare)).toBe(started.sha); + expect(github.open_("example/project")).toMatchObject([{ head: { ref: BRANCH } }]); + }); + + test("a branch whose PR closed unmerged, or that moved on after merging, still holds its quest", async () => { + const bare = upstream(); + git(bare, "branch", BRANCH, "main"); + const github = new FakeGitHub(); + const closed = github.closed("example/project", BRANCH, tip(bare), false); + const work = clone(UPSTREAM, { [UPSTREAM]: bare }); + const held = new Refusal(`${ONE} is held by branch ${BRANCH}, with no open PR`); + await expect(start(work, github)).rejects.toThrow(held); + + // A merged PR whose head is no longer the tip: the work pushed since has not landed. + Object.assign(closed, { merged_at: closed.updated_at, head: { ...closed.head, sha: "0".repeat(40) } }); + await expect(start(work, github)).rejects.toThrow(held); + expect(tip(bare)).toBe(git(bare, "rev-parse", "main")); + }); + + test("a fork replaces its own branch that a merged PR landed", async () => { + const bare = upstream(); + const forkBare = upstream(); + git(forkBare, "branch", BRANCH, "main"); + const github = new FakeGitHub(); + github.fork("me/project"); + github.closed("me/project", BRANCH, tip(forkBare), true); + const work = clone(FORK, { [FORK]: forkBare, [UPSTREAM]: bare }); + + const started = await start(work, github); + expect(tip(forkBare)).toBe(started.sha); + expect(tip(bare)).toBe(""); + }); + + test("pushes where the push URL points, as a fork, when the remote fetches from upstream", async () => { + const bare = upstream(); + const forkBare = tempDir(); + git(forkBare, "init", "-q", "--bare", "-b", "main"); + const github = new FakeGitHub(); + github.fork("me/project"); + const forkSsh = "git@github.com:me/project.git"; + const work = clone(UPSTREAM, { [UPSTREAM]: bare, [forkSsh]: forkBare }); + git(work, "config", "remote.origin.pushurl", forkSsh); + + const started = await start(work, github); + expect(tip(forkBare)).toBe(started.sha); + expect(tip(bare)).toBe(""); + expect(github.pulls).toMatchObject([ + { base: "example/project", head: { ref: BRANCH, repo: { full_name: "me/project" } } }, + ]); + // The fork's branch is seen as a claim even before its PR is listed. + github.pulls.length = 0; + expect(await claims(work, "origin", [BRANCH], deps(github))).toEqual([ + { branch: BRANCH, url: null, pull: null }, + ]); + }); + test("refuses a quest a fork's PR claims, and says whether it was handed back", async () => { const bare = upstream(); const work = clone(UPSTREAM, { [UPSTREAM]: bare }); const github = new FakeGitHub(); const pull = github.open("example/project", BRANCH, "someone/project"); - const start = () => startQuest(work, ONE, "[S] One", "origin", deps(github)); + const attempt = () => start(work, github); - await expect(start()).rejects.toThrow(`${ONE} is held by ${pull.html_url} (draft, last active just now)`); + await expect(attempt()).rejects.toThrow(`${ONE} is held by ${pull.html_url} (draft, last active just now)`); pull.body = "Work so far.\n\n## Decisions needed\n\n1. Which name?\n"; - await expect(start()).rejects.toThrow(`held by ${pull.html_url} (draft, decisions needed,`); + await expect(attempt()).rejects.toThrow(`held by ${pull.html_url} (draft, decisions needed,`); pull.draft = false; - await expect(start()).rejects.toThrow(`held by ${pull.html_url} (ready for review,`); + await expect(attempt()).rejects.toThrow(`held by ${pull.html_url} (ready for review,`); expect(tip(bare)).toBe(""); }); @@ -310,10 +421,7 @@ describe("start on GitHub", () => { const bare = upstream(); const github = new FakeGitHub(); const [a, b] = [clone(UPSTREAM, { [UPSTREAM]: bare }), clone(UPSTREAM, { [UPSTREAM]: bare })]; - const results = await Promise.allSettled([ - startQuest(a, ONE, "[S] One", "origin", deps(github)), - startQuest(b, ONE, "[S] One", "origin", deps(github)), - ]); + const results = await Promise.allSettled([start(a, github), start(b, github)]); const won = results.filter((result) => result.status === "fulfilled"); const lost = results.filter((result) => result.status === "rejected"); @@ -336,7 +444,7 @@ describe("start on GitHub", () => { }); }; - await expect(startQuest(work, ONE, "[S] One", "origin", deps(github))).rejects.toThrow( + await expect(start(work, github)).rejects.toThrow( `${fork?.html_url ?? "https://github.com/example/project/pull/2"} claimed ${ONE} first; closed https://github.com/example/project/pull/1`, ); expect(github.open_("example/project").map((pull) => pull.number)).toEqual([2]); @@ -348,20 +456,10 @@ describe("start on GitHub", () => { const forkBare = tempDir(); git(forkBare, "init", "-q", "--bare", "-b", "main"); const github = new FakeGitHub(); - github.repo("me/project", { - fork: true, - default_branch: "main", - parent: { - owner: { login: "example" }, - name: "project", - clone_url: UPSTREAM, - ssh_url: "git@github.com:example/project.git", - default_branch: "main", - }, - }); + github.fork("me/project"); const work = clone(FORK, { [FORK]: forkBare, [UPSTREAM]: bare }); - const started = await startQuest(work, ONE, "[S] One", "origin", deps(github)); + const started = await start(work, github); expect(tip(forkBare)).toBe(started.sha); expect(tip(bare)).toBe(""); expect(github.pulls).toMatchObject([ @@ -370,7 +468,7 @@ describe("start on GitHub", () => { // Another clone of the upstream sees the fork's claim. const other = clone(UPSTREAM, { [UPSTREAM]: bare }); - await expect(startQuest(other, ONE, "[S] One", "origin", deps(github))).rejects.toThrow(Refusal); + await expect(start(other, github)).rejects.toThrow(Refusal); const released = await releaseQuest(work, ONE, "origin", deps(github)); expect(released).toEqual({ branch: BRANCH, closed: [started.url] }); @@ -382,7 +480,7 @@ describe("start on GitHub", () => { const work = clone(UPSTREAM, { [UPSTREAM]: bare }); const extra = "quest/m0/epic/three.md"; writeFileSync(join(work, extra), "# [S] Three\n\n## Goal\n\nOnly local.\n"); - await expect(startQuest(work, extra, "[S] Three", "origin", deps(new FakeGitHub()))).rejects.toThrow( + await expect(start(work, new FakeGitHub(), [], extra)).rejects.toThrow( new Refusal(`${extra} is not on the upstream default branch; update your checkout`), ); expect(tip(bare, "quest/m0/epic/three")).toBe(""); @@ -392,7 +490,7 @@ describe("start on GitHub", () => { const bare = upstream(); const github = new FakeGitHub(); const work = clone(UPSTREAM, { [UPSTREAM]: bare }); - const started = await startQuest(work, ONE, "[S] One", "origin", deps(github)); + const started = await start(work, github); expect(await releaseQuest(work, ONE, "origin", deps(github))).toEqual({ branch: BRANCH, @@ -406,7 +504,7 @@ describe("start on GitHub", () => { const bare = upstream(); const github = new FakeGitHub(); const work = clone(UPSTREAM, { [UPSTREAM]: bare }); - await startQuest(work, ONE, "[S] One", "origin", deps(github)); + await start(work, github); appendFileSync(join(work, ONE), "\nWork in progress.\n"); git(work, "commit", "-q", "-m", "work", "--", ONE); git(work, "push", "-q", "origin", `refs/heads/${BRANCH}:refs/heads/${BRANCH}`); @@ -467,10 +565,10 @@ describe("start on another forge", () => { expect(tip(bare)).not.toBe(""); }); - test("fails on a malformed tree before touching the remote", async () => { + test("fails on a malformed upstream tree before pushing a claim", async () => { const { bare, work } = plain(); // A misspelled `## Required` hides the blocker from the readiness check. - writeFileSync(join(work, "quest/m0/epic/two.md"), TWO.replace("## Required", "## Requires")); + land(work, { "quest/m0/epic/two.md": TWO.replace("## Required", "## Requires") }); const out = await cli("--root", work, "start", "quest/m0/epic/two.md"); expect(out.code).toBe(1); expect(out.stderr).toContain("unknown '## Requires'"); @@ -500,8 +598,10 @@ describe("start on another forge", () => { test("unblocked --unclaimed drops held quests and lists them on stderr", async () => { const { bare, work } = plain(); // Two ready quests, so the filter leaves one. - appendFileSync(join(work, "quest/m0/epic/README.md"), "- [Three](/quest/m0/epic/three.md)\n"); - writeFileSync(join(work, "quest/m0/epic/three.md"), "# [S] Three\n\n## Goal\n\nAlso ready.\n"); + land(work, { + "quest/m0/epic/README.md": `${EPIC_README}- [Three](/quest/m0/epic/three.md)\n`, + "quest/m0/epic/three.md": "# [S] Three\n\n## Goal\n\nAlso ready.\n", + }); git(bare, "branch", BRANCH, "main"); expect(await cli("--root", work, "unblocked", "--unclaimed")).toEqual({ @@ -515,6 +615,30 @@ describe("start on another forge", () => { "quest: no nope remote, so claims were not checked\n", ); }); + + test("readiness comes from the upstream tree, not a checkout behind it", async () => { + const { bare, work } = plain(); + const behind = clone(bare); + // Upstream finished One: it deleted the quest and its links, so Two is ready there. + land(work, { + "quest/m0/epic/README.md": EPIC_README.replace("- [One](/quest/m0/epic/one.md)\n", ""), + "quest/m0/epic/two.md": TWO.replace(/\n## Required\n[^]*$/, ""), + [ONE]: null, + }); + + expect(await cli("--root", behind, "unblocked")).toMatchObject({ stdout: `1\t${ONE}\n` }); + expect(await cli("--root", behind, "unblocked", "--unclaimed")).toEqual({ + code: 0, + stdout: "0\tquest/m0/epic/two.md\n", + stderr: "quest: origin is not on GitHub, so only its branches were checked for claims\n", + }); + const finished = await cli("--root", behind, "start", ONE); + expect(finished.code).toBe(3); + expect(finished.stderr).toBe(`quest: ${ONE} is not on the upstream default branch; update your checkout\n`); + const ready = await cli("--root", behind, "start", "quest/m0/epic/two.md"); + expect(ready.code, ready.stderr).toBe(0); + expect(tip(bare, "quest/m0/epic/two")).not.toBe(""); + }); }); test("claims reports the earliest open PR on each branch", async () => { @@ -533,17 +657,7 @@ test("claims from a fork count the parent's branches and PRs, and the fork's own const forkBare = tempDir(); git(forkBare, "init", "-q", "--bare", "-b", "main"); const github = new FakeGitHub(); - github.repo("me/project", { - fork: true, - default_branch: "main", - parent: { - owner: { login: "example" }, - name: "project", - clone_url: UPSTREAM, - ssh_url: "git@github.com:example/project.git", - default_branch: "main", - }, - }); + github.fork("me/project"); const work = clone(FORK, { [FORK]: forkBare, [UPSTREAM]: bare }); git(bare, "branch", "quest/m0/a", "main"); git(work, "push", "-q", "origin", "HEAD:refs/heads/quest/m0/b"); @@ -564,6 +678,30 @@ test("claims from a fork count the parent's branches and PRs, and the fork's own ]); }); +test("githubRepository reads HTTPS, ssh://, and scp-like remote URLs", () => { + const repository = { owner: "example", repo: "project" }; + for (const url of [ + "https://github.com/example/project.git", + "https://github.com/example/project", + "https://x-access-token:secret@github.com/example/project.git", + "https://github.com/example/project/", + "ssh://git@github.com/example/project.git", + "ssh://git@github.com:22/example/project", + "git@github.com:example/project.git", + "org-123@github.com:example/project.git", + ]) { + expect(githubRepository(url), url).toEqual(repository); + } + for (const url of [ + "https://gitlab.com/example/project.git", + "https://github.com.evil.example/example/project", + "git@github.com:example", + "/srv/git/project.git", + ]) { + expect(githubRepository(url), url).toBeNull(); + } +}); + test("ago rounds to minutes, hours, then days", () => { const now = Date.parse("2026-10-21T12:00:00Z"); expect(ago(now, now - 10_000)).toBe("just now");