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");