diff --git a/README.md b/README.md index 337100c8..0044adec 100644 --- a/README.md +++ b/README.md @@ -146,6 +146,35 @@ Invalid trees fail with diagnostics and no ranking. This command uses the same local readiness as `ready`; verify claims and outside conditions before starting. `quest-spawn` uses the ranking unless you explicitly select work in another order. +## Move and delete quests + +`quest mv` and `quest rm` move or delete a quest or an epic with `git mv` and +`git rm`, then fix the links so the tree still passes `quest check`: + +```sh +quest mv quest/m0/export.md quest/m1/export.md +quest mv quest/m0/drain quest/m1/drain +quest rm quest/m0/download.md +quest rm -r quest/m1/drain +``` + +`mv` rewrites every root-absolute link to the moved path, and to everything +under a moved epic. `rm` drops each `Required`, `Related`, or `Closes` entry +that links to what it deleted, and the heading if that was the last entry. +It also drops every `Required` link to a finished milestone, including one it +just finished, but keeps the root's listing of the milestone. +Both stage the change, print one line per file (`R from -> to`, `D path`, or +`M path`), and report on stderr any other mention of the path, such as prose +or files outside `quest/`, without editing it. They run `quest check` on the +result and exit 1 if it fails, for example when a quest moved out of its epic. + +An epic is its directory or README; removing one that holds other quests needs +`-r`. The root and milestones are never moved or removed. `mv` refuses a +destination that exists, and refuses to move a quest while it has a claim +branch or open PR on `origin`, or on the repository a GitHub fork came from, +since the branch would no longer name it. +Neither command touches branches or PRs. + ## Wait for pull request activity `quest wait` blocks until a GitHub PR has a review, comment, review-thread diff --git a/assets/AGENTS.md b/assets/AGENTS.md index c071251a..477f2595 100644 --- a/assets/AGENTS.md +++ b/assets/AGENTS.md @@ -147,6 +147,6 @@ Any findings that seem out-of-scope, or bugs encountered during the process, sho A quest that is no longer needed or cannot be completed is abandoned. Delete it and explain why in the PR. -Grep its absolute path and remove every reference. +Use `quest rm`, which deletes the quest and every link to it. Remove the `quest` label from issues no other quest tracks. diff --git a/assets/skills/audit.md b/assets/skills/audit.md index 33cca769..6608bda2 100644 --- a/assets/skills/audit.md +++ b/assets/skills/audit.md @@ -17,4 +17,4 @@ Split the reading across parallel sub-agents, each given a slice of the scope an Apply factual corrections and changes the user has already decided. Use `/quest-plan` only for unresolved scope, priority, API, or other consequential choices; recommendations are not decisions. Once those choices are settled, apply them without another approval round. -Update every reference and dependency affected by a move or deletion, run `quest check`, and follow the repository's PR workflow within the user's authorized scope. Report the changes with their evidence and any decisions still open. An explicit read-only audit request leaves the tree unchanged. +Move and delete quests with `quest mv` and `quest rm`, which update the links to them; edit any mentions they report, run `quest check`, and follow the repository's PR workflow within the user's authorized scope. Report the changes with their evidence and any decisions still open. An explicit read-only audit request leaves the tree unchanged. diff --git a/assets/skills/delete.md b/assets/skills/delete.md index 38ee3f3f..eee1a025 100644 --- a/assets/skills/delete.md +++ b/assets/skills/delete.md @@ -4,6 +4,6 @@ description: Abandon a quest. Before you begin, run `quest guide` and read its output completely. -Delete the specified quest file and make a PR. +Delete the specified quest with `quest rm`, edit any mentions it reports, and make a PR. If the quest has an open PR, repurpose it instead of making a new PR. Merge the PR; it should only consist of quest deletions. diff --git a/quest/m0/helpers/README.md b/quest/m0/helpers/README.md index 2e991ec1..a3d823c8 100644 --- a/quest/m0/helpers/README.md +++ b/quest/m0/helpers/README.md @@ -48,7 +48,6 @@ commands from Sep-Oct. Session counts in each child come from that window. ## Required -- [Move and delete quests](/quest/m0/helpers/move.md) - `quest mv` and `quest rm` keep every root-absolute link valid - [CI checks in wait](/quest/m0/helpers/wait-checks.md) - `quest wait` wakes on required check results for the head - [quest start](/quest/m0/helpers/start.md) - claim a quest with a draft PR, the `## Decisions needed` hand-back, `quest unblocked --unclaimed`, and CI on ready PRs only - [PR state commands](/quest/m0/helpers/pr-states.md) - `quest iterate`, `ready`, `merge`, and `release` label the one owner of a PR diff --git a/quest/m0/helpers/move.md b/quest/m0/helpers/move.md deleted file mode 100644 index 0cb04eca..00000000 --- a/quest/m0/helpers/move.md +++ /dev/null @@ -1,43 +0,0 @@ -# [M] Move and delete quests with quest mv and quest rm - -## Goal - -`quest mv ` and `quest rm ` move or delete a quest or epic -and keep every root-absolute link in the tree valid. Agents stop following -`git rm` and `git mv` with `grep -r` and `sed -i` over Required links (203 -sessions since Oct 14). `quest check` already finds the broken links; these -commands avoid making them. - -## Plan - -- Decided 2026-10-19 (user): run `git mv` and `git rm`, so the change is - staged. Fail outside a git work tree rather than falling back to plain - file operations. -- `mv` rewrites every link to the moved path under `quest/`; moving an epic - directory rewrites links under it as a prefix. Moving a quest into or out - of an epic changes its structural membership; `quest check` reports what - the epic's Required still needs rather than `mv` guessing. -- `rm` drops list items that link to the deleted path (or under a deleted - epic) and deletes a section left empty, as the guide requires for an empty - Required. Removing an epic with children requires `-r`, as with `git rm`. -- Decided 2026-10-19 (user): `rm` also drops every `Required` link to a - finished milestone, meaning a milestone README with no `## Required` left, - which `quest check` already flags. That covers a milestone this `rm` just - emptied and any links that were already stale. As in that check, the - root's own entry for the milestone stays: it is membership, not a blocker, - and dropping it would leave the milestone unlisted. It repeats until - nothing changes, so `rm` never leaves a tree that fails either check. -- Both report prose mentions of the path and mentions outside `quest/` - without editing them, and run `quest check` on the result. -- Neither touches branches or PRs, since renaming an open PR's head branch - closes the PR. `mv` refuses while the quest, or any quest under a moved - epic, has a claim branch or open PR, because the old branch name would no - longer be recognized as its claim. Use the same lookup as `quest claim`, - so whichever lands second reuses it. -- Put the link rewriting in the core as a pure edit of documents; the CLI - owns git. Update the `delete` and `audit` skills to use `quest rm` and - `quest mv`. -- Test moving a quest, moving an epic, deleting a quest listed under several - Required and Related sections, deleting a milestone's last child while - another quest requires that milestone (the root still lists it), refusing a non-empty epic without `-r`, and - a destination that already exists, and refusing to move a claimed quest. diff --git a/quest/m0/landed-quests.md b/quest/m0/landed-quests.md index c24a9c90..e470d3c1 100644 --- a/quest/m0/landed-quests.md +++ b/quest/m0/landed-quests.md @@ -63,15 +63,10 @@ Decided 2026-10-20 (user, planned from moq-dev/moq): of their own. Rejected: refusing and reporting for `/quest-iterate`. - Repositories may also run the guard in CI on pull requests; document the invocation in `README.md`, but don't ship a workflow. -- [Move and delete quests](/quest/m0/helpers/move.md) gives `quest rm`, which - the fix-up step uses once it lands; until then the agent deletes and - unlinks by hand. +- The fix-up step deletes the quest with `quest rm`, which drops the links to + it. Tests: a quest-branch diff that deletes the quest, one that edits it, and one that leaves it untouched (the finding); an epic branch checked against its README; a non-quest branch; a branch naming a quest that does not exist at the base; and `--branch` overriding the current branch. - -## Related - -- [Move and delete quests](/quest/m0/helpers/move.md) - `quest rm` deletes a quest and every link to it diff --git a/src/cli/claims.ts b/src/cli/claims.ts new file mode 100644 index 00000000..17d07e10 --- /dev/null +++ b/src/cli/claims.ts @@ -0,0 +1,113 @@ +// Who holds a quest: its claim branch on the remote, or an open PR from a +// branch of that name in any repository. Commands that must not disturb held +// work look here first. +import { execFileSync } from "node:child_process"; + +import { + type GitHubDependencies, + type Repository, + GitHub, + githubRepository, + object, + remoteUrl, + string, +} from "./github"; + +/** A quest's branch: its path without `.md`. */ +export function questBranch(path: string): string { + return path.replace(/\.md$/, ""); +} + +/** A held quest branch, and the open PR from it if there is one. */ +export interface Claim { + branch: string; + url: string | null; +} + +export interface ClaimDependencies { + /** Called only when the remote is on GitHub, so other forges need no token. */ + github(): GitHubDependencies; + warn(message: string): void; +} + +/** The `quest/*` branches on `remote`. Plain git, so it works with any forge. */ +export function remoteBranches(root: string, remote: string): string[] { + const out = execFileSync("git", ["-C", root, "ls-remote", remote, "refs/heads/quest/*"], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }); + return out + .split("\n") + .filter((line) => line !== "") + .map((line) => line.slice(line.indexOf("\t") + 1).replace(/^refs\/heads\//, "")); +} + +/** Every open PR in `repository`, from any head repository, by head branch. */ +export async function openPullRequests( + github: GitHub, + repository: Repository, +): Promise<{ branch: string; url: string }[]> { + const rows = await github.all(`${path(repository)}/pulls?state=open&per_page=100`); + return rows.map((row) => ({ branch: string(object(row.head).ref), url: string(row.html_url) })); +} + +/** + * The repository `repository` was forked from, or `null` if it is not a fork. + * A fork's clone names the fork `origin`, while its quests are claimed upstream. + */ +export async function forkParent(github: GitHub, repository: Repository): Promise { + const parent = object(await (await github.request(path(repository))).json()).parent; + if (parent === undefined || parent === null) return null; + const [owner, repo] = string(object(parent).full_name).split("/"); + return { owner, repo }; +} + +/** The `quest/*` branches in a GitHub `repository`, for one that is not a local remote. */ +export async function githubBranches(github: GitHub, repository: Repository): Promise { + const rows = await github.all(`${path(repository)}/git/matching-refs/heads/quest/`); + return rows.map((row) => string(row.ref).replace(/^refs\/heads\//, "")); +} + +function path(repository: Repository): string { + return `/repos/${encodeURIComponent(repository.owner)}/${encodeURIComponent(repository.repo)}`; +} + +/** + * Which of `branches` someone holds: pushed to `remote`, or the head of an + * open PR there. When `remote` is a GitHub fork, its parent's branches and + * open PRs count too. Without the remote it warns and checks nothing; off + * GitHub it checks branches only. + */ +export async function claims( + root: string, + remote: string, + branches: string[], + deps: ClaimDependencies, +): Promise { + let url: string; + try { + url = remoteUrl(root, remote); + } catch { + deps.warn(`no ${remote} remote, so claims were not checked`); + return []; + } + const wanted = new Set(branches); + const held = new Map(); + for (const branch of remoteBranches(root, remote)) if (wanted.has(branch)) held.set(branch, null); + const repository = githubRepository(url); + if (repository === null) { + deps.warn(`${remote} is not on GitHub, so only its branches were checked for claims`); + } else { + const github = new GitHub(deps.github()); + const parent = await forkParent(github, repository); + if (parent !== null) { + for (const branch of await githubBranches(github, parent)) if (wanted.has(branch)) held.set(branch, null); + } + for (const each of parent === null ? [repository] : [repository, parent]) { + for (const pr of await openPullRequests(github, each)) { + if (wanted.has(pr.branch)) held.set(pr.branch, pr.url); + } + } + } + return [...held].map(([branch, url]) => ({ branch, url })).sort((a, b) => (a.branch < b.branch ? -1 : 1)); +} diff --git a/src/cli/github.ts b/src/cli/github.ts index 7ee670eb..705f45b4 100644 --- a/src/cli/github.ts +++ b/src/cli/github.ts @@ -1,13 +1,17 @@ -// The GitHub client shared by the PR commands, outside the repository-independent core. +// The GitHub client shared by the commands that talk to GitHub, outside the repository-independent core. import { execFileSync } from "node:child_process"; import { setTimeout as sleep } from "node:timers/promises"; const API = "https://api.github.com"; const MAX_SLEEP = 2_147_483_647; -export interface PullRequest { +/** A repository on github.com. */ +export interface Repository { owner: string; repo: string; +} + +export interface PullRequest extends Repository { number: number; } @@ -34,22 +38,30 @@ 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 origin = execFileSync("git", ["-C", root, "remote", "get-url", "origin"], { - encoding: "utf8", - stdio: ["ignore", "pipe", "pipe"], - }).trim(); - const match = - /^(?:https:\/\/github\.com\/|git@github\.com:|ssh:\/\/git@github\.com\/)([^/]+)\/([^/]+?)(?:\.git)?$/.exec( - origin, - ); - if (!match) throw new Error("origin must name a github.com repository; pass a GitHub PR URL instead"); - return { owner: match[1], repo: match[2], number: Number(value) }; + const repository = githubRepository(remoteUrl(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) }; } const match = /^https:\/\/github\.com\/([\w.-]+)\/([\w.-]+)\/pull\/([1-9]\d*)\/?(?:[?#].*)?$/.exec(value); if (!match) throw new GitHubInputError("expected a GitHub pull request URL or a PR number in origin"); return { owner: match[1], repo: match[2], number: Number(match[3]) }; } +/** The URL a git remote fetches from. */ +export function remoteUrl(root: string, remote: string): string { + return execFileSync("git", ["-C", root, "remote", "get-url", remote], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }).trim(); +} + +/** The github.com repository a remote URL names, or `null` for any other host. */ +export function githubRepository(url: string): Repository | null { + const match = + /^(?:https:\/\/github\.com\/|git@github\.com:|ssh:\/\/git@github\.com\/)([^/]+)\/([^/]+?)(?:\.git)?$/.exec(url); + return match ? { owner: match[1], repo: match[2] } : null; +} + export function gitHubDependencies(warn: (message: string) => void): GitHubDependencies { let token = process.env.GH_TOKEN || process.env.GITHUB_TOKEN; if (!token) { @@ -136,6 +148,24 @@ export class GitHub { } } + /** One page of a REST listing, and its response for the next link. */ + async list(path: string): Promise<{ rows: ObjectValue[]; response: Response }> { + const response = await this.request(path); + if (response.status !== 200) throw new Error("unexpected GitHub 304 without a cached response"); + return { rows: array(await response.json()), response }; + } + + /** Every page of a REST listing. */ + async all(path: string): Promise { + const rows: ObjectValue[] = []; + for (let next: string | undefined = path; next;) { + const page = await this.list(next); + rows.push(...page.rows); + next = link(page.response, "next"); + } + return rows; + } + /** Run a GraphQL query and return its data, failing on any reported error. */ async graphql(query: string, variables: ObjectValue): Promise { const response = await this.request("/graphql", undefined, { query, variables }); diff --git a/src/cli/main.ts b/src/cli/main.ts index 769fcba7..e0928c62 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -7,11 +7,12 @@ import { Command, CommanderError, InvalidArgumentError, Option } from "commander import { description, version } from "../../package.json"; import { blockers, check, formatFinding, label, lookup, ready, renderBlocker, unblocked } from "../core"; +import { gitHubDependencies, GitHubInputError, pullRequest } from "./github"; +import { type Report, moveQuest, removeQuest } from "./move"; import { init, uninstall } from "./setup"; import { GUIDE, SKILLS, skill } from "./skills"; import { type Terminal, Theme } from "./theme"; import { collect, exists, load, under } from "./tree"; -import { gitHubDependencies, GitHubInputError, pullRequest } from "./github"; import { pullRequestStatus, statusLines } from "./status"; import { waitForActivity, WaitTimeout } from "./wait"; @@ -201,6 +202,60 @@ export function main(argv: string[], out: Output): number | Promise { ); }); + /** Changes on stdout; mentions and check findings on stderr, which fail the command. */ + const report = (result: Report): number => { + for (const line of result.changes) print(line); + for (const line of result.warnings) warn(line); + for (const line of result.findings) warn(line); + return result.findings.length > 0 ? 1 : 0; + }; + + program + .command("mv") + .summary("Move a quest or epic and rewrite the links to it") + .description( + "Move a quest, or an epic directory and everything under it, with git mv, and rewrite every " + + "root-absolute link to it under quest/. The change is staged.\n\n" + + "Refuses the root epic and milestones, which are permanent. Refuses outside a git work tree, when " + + "the destination exists, and while a moved quest has a claim branch or open PR on origin, or on " + + "the repository a GitHub fork came from, since the old branch would no longer name it. Moving a " + + "quest into or out of an epic changes which epic lists it; the check afterwards reports what to fix.\n\n" + + "Prints R for the move and M for each edited document. Mentions of the old path that are not links, " + + "or are outside quest/, are reported on stderr and left alone. Runs quest check on the result and " + + "exits 1 if it fails.", + ) + .argument("", "Quest file or epic directory, as /quest/..., quest/..., or a filesystem path") + .argument("", "Its new path, which must not exist") + .action((from: string, to: string) => { + pending = moveQuest(root(), from, to, { github: () => gitHubDependencies(warn), warn }).then( + report, + (error: unknown) => { + warn(error instanceof Error ? error.message : String(error)); + return 1; + }, + ); + }); + + program + .command("rm") + .summary("Delete a quest or epic and drop the links to it") + .description( + "Delete a quest, or an epic and the quests under it, with git rm, and drop every Required, Related, " + + "or Closes entry that links to it, with any heading left empty. It also drops every Required link to a " + + "finished milestone, one whose README has no Required left, except the root's own listing. The change " + + "is staged.\n\n" + + "Refuses outside a git work tree. An epic is its directory or README; removing one that holds other " + + "quests requires -r. The root epic and milestones are permanent.\n\n" + + "Prints D for each deleted quest and M for each edited document. Mentions that are not list entries, " + + "or are outside quest/, are reported on stderr and left alone. Runs quest check on the result and " + + "exits 1 if it fails.", + ) + .argument("", "Quest file, or epic directory or README") + .option("-r, --recursive", "Remove an epic with the quests under it") + .action((path: string, options: { recursive?: boolean }) => { + code = report(removeQuest(root(), path, options.recursive ?? false)); + }); + program .command("guide") .description("Print the quest guide: the format, workflow, and rules agents follow") diff --git a/src/cli/move.ts b/src/cli/move.ts new file mode 100644 index 00000000..150bbf83 --- /dev/null +++ b/src/cli/move.ts @@ -0,0 +1,189 @@ +// `quest mv` and `quest rm`: git moves or deletes the files and stages the +// change, and the core edits the links in the documents that remain. +import { execFileSync } from "node:child_process"; +import { mkdirSync, statSync, writeFileSync } from "node:fs"; +import { join, posix, relative, resolve, sep } from "node:path"; + +import { check, formatFinding, isQuest, move, normalize, permanent, relocate, remove } from "../core"; +import { type ClaimDependencies, claims, questBranch } from "./claims"; +import { exists, load, sources, under } from "./tree"; + +/** What a command changed, and what it left for the caller to look at. */ +export interface Report { + /** `R from -> to`, `D path`, or `M path`, one per change. */ + changes: string[]; + /** Mentions left for a person to edit, as `path:line: message`. */ + warnings: string[]; + /** `quest check` on the result. */ + findings: string[]; +} + +/** + * Move a quest, or an epic directory and everything under it, and rewrite + * every root-absolute link to it. Refuses while a moved quest is claimed, + * since its branch would no longer name it. + */ +export async function moveQuest( + root: string, + fromArg: string, + toArg: string, + deps: ClaimDependencies, +): Promise { + workTree(root); + const from = treePath(root, fromArg); + const to = treePath(root, toArg); + if (!exists(root, from)) throw new Error(`${fromArg} does not exist`); + const directory = statSync(join(root, from)).isDirectory(); + if (from === "quest" || permanent(directory ? `${from}/README.md` : from)) { + throw new Error(`${fromArg} is permanent; the root epic and milestones never move`); + } + if (!directory && !isQuest(from)) throw new Error(`${fromArg} is not a quest document`); + if (!directory && !isQuest(to)) throw new Error(`${toArg} must name a Markdown quest document`); + if (exists(root, to)) throw new Error(`${toArg} already exists`); + if (relocate(to, from, from) !== null) throw new Error(`cannot move ${fromArg} inside itself`); + + const docs = sources(root); + const moving = docs.filter((doc) => relocate(doc.path, from, to) !== null); + if (moving.length === 0) throw new Error(`${fromArg} holds no quest documents`); + const held = await claims( + root, + "origin", + moving.map((doc) => questBranch(doc.path)), + deps, + ); + if (held.length > 0) { + const list = held.map((claim) => (claim.url ? `${claim.branch} (${claim.url})` : claim.branch)).join(", "); + throw new Error( + `refusing to move claimed quests: ${list}; their branches would no longer name them, so move them once the work merges or the claim is released`, + ); + } + + const edited = move(docs, from, to); + mkdirSync(join(root, posix.dirname(to)), { recursive: true }); + git(root, "mv", "--", from, to); + write(root, edited.texts); + const warnings = edited.missed.map( + (link) => + `${relocate(link.path, from, to) ?? link.path}:${link.line}: links ${link.target} in a form mv cannot rewrite; update it by hand`, + ); + return { + changes: [`R ${from} -> ${to}`, ...modified(edited.texts)], + warnings: [...warnings, ...mentions(root, from)], + findings: findings(root), + }; +} + +/** + * Delete a quest, or an epic with the quests under it, and drop every list + * entry that links to what was deleted. The root and milestones are permanent. + */ +export function removeQuest(root: string, arg: string, recursive: boolean): Report { + workTree(root); + const target = treePath(root, arg); + if (!exists(root, target)) throw new Error(`${arg} does not exist`); + const directory = statSync(join(root, target)).isDirectory(); + if (target === "quest" || permanent(directory ? `${target}/README.md` : target)) { + throw new Error(`${arg} is permanent; the root epic and milestones are never removed`); + } + if (!directory && !isQuest(target)) throw new Error(`${arg} is not a quest document`); + + // An epic is its directory, or its README while it still holds other quests. + const epic = directory ? target : posix.basename(target) === "README.md" ? posix.dirname(target) : null; + const docs = sources(root); + const inside = epic === null ? [] : docs.filter((doc) => relocate(doc.path, epic, epic) !== null); + const whole = epic !== null && (directory || inside.length > 1); + const doomed = whole ? inside : docs.filter((doc) => doc.path === target); + if (doomed.length === 0) throw new Error(`${arg} holds no quest documents`); + if (whole && doomed.length > 1 && !recursive) { + throw new Error(`${arg} is an epic holding ${doomed.length} quest documents; pass -r to remove them all`); + } + + const gone = new Set(doomed.map((doc) => doc.path)); + const edited = remove(docs, (path) => gone.has(path)); + git(root, "rm", "-q", ...(whole ? ["-r"] : []), "--", whole ? epic! : target); + write(root, edited); + return { + changes: [...doomed.map((doc) => `D ${doc.path}`), ...modified(edited)], + warnings: mentions(root, whole ? epic! : target), + findings: findings(root), + }; +} + +/** + * A quest path as the tree writes it (`/quest/m0/a.md`), as the shell + * completes it (`quest/m0/a.md`), or as a filesystem path to it. + */ +function treePath(root: string, arg: string): string { + const tree = normalize(arg.replace(/^\/(?=quest(\/|$))/, "")); + const path = + tree === "quest" || tree.startsWith("quest/") + ? tree + : relative(resolve(root), resolve(arg)).split(sep).join("/"); + if (path !== "quest" && !path.startsWith("quest/")) throw new Error(`${arg} is not under ${under(root, "quest")}`); + return path; +} + +/** Fail outside a git work tree: these commands stage their change. */ +function workTree(root: string) { + try { + git(root, "rev-parse", "--is-inside-work-tree"); + } catch { + throw new Error(`${root} is not in a git work tree; quest mv and rm stage their change with git`); + } +} + +function git(root: string, ...args: string[]): string { + try { + return execFileSync("git", ["-C", root, ...args], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); + } catch (error) { + const stderr = (error as { stderr?: string }).stderr?.trim(); + throw new Error(stderr || (error as Error).message); + } +} + +/** Write and stage each edited document. */ +function write(root: string, texts: Map) { + if (texts.size === 0) return; + for (const [path, text] of texts) writeFileSync(join(root, path), text); + git(root, "add", "--", ...texts.keys()); +} + +function modified(texts: Map): string[] { + return [...texts.keys()].sort().map((path) => `M ${path}`); +} + +/** + * Every line in a tracked or unignored untracked file that still names + * `path`, or a path under it, which no + * edit could safely change. A longer sibling name, such as `drain.md` or + * `drain-2` for `drain`, is not a mention; a trailing full stop is. + */ +function mentions(root: string, path: string): string[] { + const pattern = `${path.replace(/[.[\]()*+?{}|^$\\]/g, "\\$&")}\\.*([^A-Za-z0-9_.-]|$)`; + let out: string; + try { + out = execFileSync( + "git", + ["-C", root, "grep", "--untracked", "--exclude-standard", "-n", "-I", "-E", "-e", pattern], + { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }, + ); + } catch (error) { + // git grep exits 1 when nothing matches. + if ((error as { status?: number }).status === 1) return []; + throw error; + } + return out + .split("\n") + .filter((line) => line !== "") + .map((line) => { + const [file, number] = line.split(":", 2); + return `${file}:${number}: mentions ${path}; update it by hand`; + }); +} + +function findings(root: string): string[] { + return check(load(root), (path) => exists(root, path)).map(formatFinding); +} diff --git a/src/cli/tree.ts b/src/cli/tree.ts index d4154853..8b6edef7 100644 --- a/src/cli/tree.ts +++ b/src/cli/tree.ts @@ -3,7 +3,7 @@ import { existsSync, readFileSync, readdirSync } from "node:fs"; import { join } from "node:path"; -import { type Doc, comparePaths, isQuest, parse } from "../core"; +import { type Doc, type Source, comparePaths, isQuest, parse } from "../core"; /** * `/` as a user would name it in an error: `./quest`, not the @@ -36,11 +36,16 @@ export function collect(root: string): string[] { return out.sort(comparePaths); } -/** Every quest document, parsed, in tree order. */ -export function load(root: string): Doc[] { +/** Every quest document's source text, in tree order. */ +export function sources(root: string): Source[] { const paths = collect(root); if (paths.length === 0) throw new Error(`no quest documents found under ${under(root, "quest")}`); - return paths.map((path) => parse(path, read(root, path))); + return paths.map((path) => ({ path, text: read(root, path) })); +} + +/** Every quest document, parsed, in tree order. */ +export function load(root: string): Doc[] { + return sources(root).map((source) => parse(source.path, source.text)); } /** Whether a repository-relative path exists under `root`. */ diff --git a/src/cli/wait.ts b/src/cli/wait.ts index 357d8f53..aa817332 100644 --- a/src/cli/wait.ts +++ b/src/cli/wait.ts @@ -108,11 +108,6 @@ async function watch( const prUrl = `https://github.com/${pr.owner}/${pr.repo}/pull/${pr.number}`; const deadline = options.timeout === undefined ? Infinity : started + options.timeout * 1000; const github = new GitHub(deps, deadline); - const list = async (path: string): Promise<{ rows: ObjectValue[]; response: Response }> => { - const response = await github.request(path); - if (response.status !== 200) throw new Error("unexpected GitHub 304 without a cached response"); - return { rows: array(await response.json()), response }; - }; let cursor: Cursor; let historical: Activity[] = []; @@ -132,7 +127,7 @@ async function watch( let commitsPath: string | undefined = `${base}/pulls/${pr.number}/commits?per_page=100`; let listedHead: string | undefined; while (commitsPath) { - const { rows, response } = await list(commitsPath); + const { rows, response } = await github.list(commitsPath); for (const commit of rows) { listedHead = string(commit.sha); if (!fullSha(listedHead)) throw new Error("invalid GitHub commit SHA"); @@ -156,7 +151,7 @@ async function watch( let path: string | undefined = `/repos/${fullName}/activity?ref=${encodeURIComponent(string(head.ref))}&per_page=100`; while (path) { - const { rows, response } = await list(path); + const { rows, response } = await github.list(path); activities.push(...rows); if ( shaPoint diff --git a/src/core/doc.ts b/src/core/doc.ts index 5cadcff9..8fcf921a 100644 --- a/src/core/doc.ts +++ b/src/core/doc.ts @@ -39,12 +39,20 @@ export interface Link { target: string; /** Where the link sits in its section. */ position: Position; + /** + * Source offsets bounding where the destination is written: after the + * link's text to the end of the link, or after the label of the + * definition a reference-style link uses. An edit searches it for `target`. + */ + span: [number, number]; } /** One top-level list entry: the shape a `Required` child or blocker has. */ export interface Entry { /** 1-based source line, for findings. */ line: number; + /** 1-based last source line of the item, so an edit can remove it. */ + end: number; /** The enclosing `## Heading`, or `null` above the first one. */ section: string | null; /** @@ -211,7 +219,7 @@ export function parse(path: string, text: string): Doc { // Lines as grep sees them: a bare `\r` does not end one, so a CR-only file is // one line and none of its headings are literal, which is what grep finds. const lines = text.split("\n").map((line) => (line.endsWith("\r") ? line.slice(0, -1) : line)); - const definitions = collectDefinitions(tree); + const definitions = collectDefinitions(tree, text); let title: Heading | null = null; const headings: Heading[] = []; @@ -234,6 +242,8 @@ export function parse(path: string, text: string): Doc { let heading: { depth: number; line: number; text: string } | null = null; const lineOf = (node: Nodes) => node.position?.start.line ?? 1; + const offsetOf = (node: Nodes) => node.position?.start.offset ?? 0; + const endOf = (node: Nodes) => node.position?.end.offset ?? text.length; const addText = (value: string) => { if (heading) { @@ -252,7 +262,7 @@ export function parse(path: string, text: string): Doc { if (itemDepth > 0) fresh = false; }; - const link = (node: Nodes, target: string) => { + const link = (node: Nodes, target: string, span: [number, number]) => { // Reference-style links resolve to their definition, so they carry an // edge like any other link. Only a top-level, unquoted item can hold an // entry. @@ -260,7 +270,7 @@ export function parse(path: string, text: string): Doc { itemDepth === 0 ? "prose" : fresh && itemDepth === 1 && quoteDepth === 0 ? "entry" : "inside"; fresh = false; if (position === "entry" && entry) entry.target = target; - links.push({ line: lineOf(node), section, target, position }); + links.push({ line: lineOf(node), section, target, position, span }); }; const enter = (node: Nodes) => { @@ -278,20 +288,29 @@ export function parse(path: string, text: string): Doc { break; case "listItem": if (itemDepth === 0 && quoteDepth === 0) { - entry = { line: lineOf(node), section, text: "", target: null }; + entry = { + line: lineOf(node), + end: node.position?.end.line ?? lineOf(node), + section, + text: "", + target: null, + }; } itemDepth++; fresh = true; break; - case "link": - link(node, node.url); + case "link": { + // The destination follows the text, which may mention it too. + const last = node.children.at(-1); + link(node, node.url, [last ? endOf(last) : offsetOf(node), endOf(node)]); break; + } case "linkReference": { - const url = definitions.get(node.identifier); - if (url === undefined) { + const definition = definitions.get(node.identifier); + if (definition === undefined) { other(); } else { - link(node, url); + link(node, definition.url, definition.span); } break; } @@ -405,12 +424,17 @@ export function parse(path: string, text: string): Doc { return { path, title, headings, links, entries: found }; } -/** The first definition of each reference label, which is the one CommonMark uses. */ -function collectDefinitions(tree: Root): Map { - const definitions = new Map(); +/** + * The first definition of each reference label, which is the one CommonMark + * uses, with the span its destination is written in: after the label's `]:`. + */ +function collectDefinitions(tree: Root, text: string): Map { + const definitions = new Map(); const visit = (node: Nodes) => { if (node.type === "definition" && !definitions.has(node.identifier)) { - definitions.set(node.identifier, node.url); + const start = node.position?.start.offset ?? 0; + const end = node.position?.end.offset ?? text.length; + definitions.set(node.identifier, { url: node.url, span: [text.indexOf("]:", start) + 2, end] }); } if ("children" in node) for (const child of node.children) visit(child); }; diff --git a/src/core/edit.ts b/src/core/edit.ts new file mode 100644 index 00000000..8e9c0e3a --- /dev/null +++ b/src/core/edit.ts @@ -0,0 +1,161 @@ +// Moving and deleting quests, as pure edits of the documents that link to them. +// +// The caller owns the files: it moves or deletes them, then writes back what +// these return. Only root-absolute links are edited, since those are the +// dependency graph. Anything else that mentions a path is prose, which the +// caller reports rather than rewriting. + +import { type Doc, type Entry, parse, rooted, withoutFragment } from "./doc"; +import { LIST_SECTIONS, finishedMilestone } from "./rules"; + +/** One quest document as loaded: its repository-relative path and source text. */ +export interface Source { + path: string; + text: string; +} + +/** A link that names a moved path but whose destination is not written as parsed, such as one with escapes. */ +export interface Missed { + path: string; + line: number; + target: string; +} + +/** What a move changes. */ +export interface Moved { + /** The new text of every document whose links changed, keyed by its path after the move. */ + texts: Map; + /** Links to a moved path that could not be rewritten in place. */ + missed: Missed[]; +} + +/** + * Where `path` lands when `from` moves to `to`, or `null` if it stays. `from` + * is a quest, or an epic directory that moves everything beneath it. + */ +export function relocate(path: string, from: string, to: string): string | null { + if (path === from) return to; + return path.startsWith(`${from}/`) ? to + path.slice(from.length) : null; +} + +/** + * Rewrite every root-absolute link to `from`, or to a document under it, to + * point at its new path under `to`. Fragments are kept; each path segment is + * percent-encoded the way a Markdown destination needs. + */ +export function move(sources: Source[], from: string, to: string): Moved { + const texts = new Map(); + const missed: Missed[] = []; + for (const source of sources) { + const doc = parse(source.path, source.text); + const edits = new Map(); + for (const link of doc.links) { + const target = rooted(link.target); + const moved = target === null ? null : relocate(target, from, to); + if (moved === null) continue; + const at = source.text.indexOf(link.target, link.span[0]); + if (at < 0 || at + link.target.length > link.span[1]) { + missed.push({ path: source.path, line: link.line, target: link.target }); + continue; + } + const fragment = link.target.slice(withoutFragment(link.target).length); + // Reference-style links share their definition, which is rewritten once. + edits.set(at, { length: link.target.length, text: `/${encode(moved)}${fragment}` }); + } + if (edits.size > 0) texts.set(relocate(source.path, from, to) ?? source.path, splice(source.text, edits)); + } + return { texts, missed }; +} + +/** + * Drop every list entry, in a list section, that opens with a link to a + * deleted document, and a section heading left with no entries. Then drop + * every `Required` link to a finished milestone, whether this removal emptied + * it or the link was already stale, repeating until nothing changes so the + * result passes `quest check`. Returns the new text of every surviving + * document that changed. + */ +export function remove(sources: Source[], deleted: (path: string) => boolean): Map { + const texts = new Map(sources.filter((s) => !deleted(s.path)).map((s) => [s.path, s.text])); + const changed = new Set(); + const linksDeleted = (entry: Entry) => { + if (entry.section === null || !LIST_SECTIONS.includes(entry.section) || entry.target === null) return false; + const target = rooted(entry.target); + return target !== null && deleted(target); + }; + for (let progress = true; progress;) { + progress = false; + const docs = [...texts].map(([path, text]) => parse(path, text)); + const finished = finishedMilestone(docs); + for (const doc of docs) { + const text = drop(texts.get(doc.path)!, doc, (entry) => linksDeleted(entry) || finished(doc, entry)); + if (text === null) continue; + texts.set(doc.path, text); + changed.add(doc.path); + progress = true; + } + } + return new Map([...changed].map((path) => [path, texts.get(path)!])); +} + +/** `doc` without the list entries `dropping` accepts, or `null` if it has none. */ +function drop(text: string, doc: Doc, dropping: (entry: Entry) => boolean): string | null { + const lines = text.split("\n"); + // A trailing newline leaves an empty last element, which is not a line. + const count = text.endsWith("\n") ? lines.length - 1 : lines.length; + const dropped = new Set(); + for (const entry of doc.entries) { + if (!dropping(entry)) continue; + for (let line = entry.line; line <= entry.end; line++) dropped.add(line - 1); + } + if (dropped.size === 0) return null; + + // A section that lost entries and has nothing left but blank lines goes + // with its heading: an empty `Required` would block its quest forever. + doc.headings.forEach((heading, index) => { + if (!LIST_SECTIONS.includes(heading.text)) return; + const end = index + 1 < doc.headings.length ? doc.headings[index + 1].line - 1 : count; + const body = Array.from({ length: end - heading.line }, (_, i) => heading.line + i); + if (!body.some((i) => dropped.has(i))) return; + if (!body.every((i) => dropped.has(i) || lines[i].trim() === "")) return; + dropped.add(heading.line - 1); + for (const i of body) dropped.add(i); + }); + + // Removing a block between two blank lines would leave both; keep one. + const out: string[] = []; + let gap = false; + lines.forEach((line, i) => { + if (dropped.has(i)) { + gap = true; + return; + } + const blank = line.trim() === ""; + if (!(gap && blank && out.length > 0 && out[out.length - 1].trim() === "")) out.push(line); + gap = false; + }); + return out.join("\n"); +} + +/** + * A repository-relative path as a root-absolute Markdown destination's path. + * Parentheses are encoded too, since an unmatched one ends an inline link. + */ +function encode(path: string): string { + return path + .split("/") + .map((name) => encodeURIComponent(name).replace(/\(/g, "%28").replace(/\)/g, "%29")) + .join("/"); +} + +/** `text` with each replacement, keyed by offset, applied. */ +function splice(text: string, edits: Map): string { + let out = ""; + let from = 0; + for (const at of [...edits.keys()].sort((a, b) => a - b)) { + const edit = edits.get(at)!; + out += text.slice(from, at) + edit.text; + from = at + edit.length; + } + return out + text.slice(from); +} diff --git a/src/core/index.ts b/src/core/index.ts index d92c07bb..005bbbe1 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -23,6 +23,7 @@ export { resolve, rooted, } from "./doc"; +export { type Missed, type Moved, type Source, move, relocate, remove } from "./edit"; export { comparePaths, normalize } from "./path"; export { type Blocker, blockers, label, lookup, ready, renderBlocker } from "./ready"; export { type Finding, ROOT, check, formatFinding } from "./rules"; diff --git a/src/core/rules.ts b/src/core/rules.ts index 6469567b..8bfff295 100644 --- a/src/core/rules.ts +++ b/src/core/rules.ts @@ -11,7 +11,19 @@ // entry for a completed quest survived a rebase that produced no conflict at // all. -import { type Doc, entries, has, isEpic, children, owner, permanent, resolve, rooted, withoutFragment } from "./doc"; +import { + type Doc, + type Entry, + entries, + has, + isEpic, + children, + owner, + permanent, + resolve, + rooted, + withoutFragment, +} from "./doc"; import { comparePaths, join } from "./path"; /** @@ -27,7 +39,7 @@ const SIZES = ["XS", "S", "M", "L", "XL"]; * last entry was removed is a bug: an empty `Required` blocks its quest * forever. */ -const LIST_SECTIONS = ["Required", "Closes", "Related"]; +export const LIST_SECTIONS = ["Required", "Closes", "Related"]; /** The permanent root epic; the one document nothing has to list. */ export const ROOT = "quest/README.md"; @@ -231,14 +243,10 @@ function index(found: Finding[], known: Set, docs: Doc[]) { * membership, not a blocker, so it is exempt. */ function milestones(found: Finding[], docs: Doc[]) { - const byPath = new Map(docs.map((d) => [d.path, d])); + const finished = finishedMilestone(docs); for (const doc of docs) { - if (doc.path === ROOT) continue; for (const entry of entries(doc, "Required")) { - const target = entry.target === null ? null : rooted(entry.target); - if (target === null || target === ROOT || !permanent(target)) continue; - const milestone = byPath.get(target); - if (milestone === undefined || has(milestone, "Required")) continue; + if (!finished(doc, entry)) continue; found.push({ path: doc.path, line: entry.line, @@ -248,6 +256,21 @@ function milestones(found: Finding[], docs: Doc[]) { } } +/** + * Whether `entry`, in `doc`, is a `Required` link to a milestone of `docs` + * that is finished. Shared with `quest rm`, which drops these links. + */ +export function finishedMilestone(docs: Doc[]): (doc: Doc, entry: Entry) => boolean { + const byPath = new Map(docs.map((d) => [d.path, d])); + return (doc, entry) => { + if (doc.path === ROOT || entry.section !== "Required" || entry.target === null) return false; + const target = rooted(entry.target); + if (target === null || target === ROOT || !permanent(target)) return false; + const milestone = byPath.get(target); + return milestone !== undefined && !has(milestone, "Required"); + }; +} + /** * `Required` must be acyclic. A cycle is a set of quests none of which can ever * start, and walking the links to rule one out is exactly the manual step an diff --git a/test/move.test.ts b/test/move.test.ts new file mode 100644 index 00000000..750f796e --- /dev/null +++ b/test/move.test.ts @@ -0,0 +1,411 @@ +// `quest mv` and `quest rm`: the core edits, and the CLI's git and claim handling. + +import { execFileSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { join } from "node:path"; + +import { describe, expect, test } from "vitest"; + +import { move, remove } from "../src/core"; +import { forkParent, githubBranches, openPullRequests } from "../src/cli/claims"; +import { GitHub, type GitHubDependencies } from "../src/cli/github"; +import { main } from "../src/cli/main"; +import { ROOT_README, Tree, tempDir } from "./fixture"; + +function git(dir: string, ...args: string[]): string { + return execFileSync("git", ["-C", dir, ...args], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); +} + +/** `tree` as a git repository with everything committed. */ +function committed(tree: Tree): Tree { + git(tree.path, "init", "-q", "-b", "main"); + git(tree.path, "add", "-A"); + git( + tree.path, + "-c", + "user.name=Test", + "-c", + "user.email=test@example.com", + "-c", + "commit.gpgsign=false", + "commit", + "-q", + "-m", + "init", + ); + return tree; +} + +async function quest(tree: Tree, ...args: string[]) { + let stdout = ""; + let stderr = ""; + const code = await main(["--root", tree.path, ...args], { + stdout: (text) => { + stdout += text; + }, + stderr: (text) => { + stderr += text; + }, + }); + return { code, stdout, stderr }; +} + +/** Staged changes, as `git status --short` prints them. */ +function staged(tree: Tree): string[] { + return git(tree.path, "status", "--short").split("\n").filter(Boolean).sort(); +} + +describe("move", () => { + test("rewrites inline and reference links, keeping fragments and encoding names and parentheses", () => { + const text = + "# [S] A\n\n## Goal\n\nSee [b](/quest/b.md#plan) and [ref][b].\n\n" + + "## Related\n\n- [/quest/b.md](/quest/b.md)\n\n[b]: /quest/b.md\n"; + const moved = move([{ path: "quest/a.md", text }], "quest/b.md", "quest/m1/new (name.md"); + expect(moved.missed).toEqual([]); + expect(moved.texts.get("quest/a.md")).toBe( + "# [S] A\n\n## Goal\n\nSee [b](/quest/m1/new%20%28name.md#plan) and [ref][b].\n\n" + + "## Related\n\n- [/quest/b.md](/quest/m1/new%20%28name.md)\n\n[b]: /quest/m1/new%20%28name.md\n", + ); + }); + + test("moves an epic's documents as a prefix and keys them by their new path", () => { + const moved = move( + [ + { + path: "quest/m0/README.md", + text: "# M0\n\n## Goal\n\nG.\n\n## Required\n\n- [E](/quest/m0/e/README.md)\n", + }, + { + path: "quest/m0/e/README.md", + text: "# E\n\n## Goal\n\nG.\n\n## Required\n\n- [A](/quest/m0/e/a.md)\n", + }, + { path: "quest/m0/e/a.md", text: "# [S] A\n\n## Goal\n\nG.\n" }, + { path: "quest/m0/elsewhere.md", text: "# [S] X\n\n## Related\n\n- [E2](/quest/m0/e2.md)\n" }, + ], + "quest/m0/e", + "quest/m1/e", + ); + expect([...moved.texts.keys()]).toEqual(["quest/m0/README.md", "quest/m1/e/README.md"]); + expect(moved.texts.get("quest/m0/README.md")).toContain("- [E](/quest/m1/e/README.md)\n"); + expect(moved.texts.get("quest/m1/e/README.md")).toContain("- [A](/quest/m1/e/a.md)\n"); + }); + + test("reports a link whose destination is not written as parsed", () => { + const text = "# [S] A\n\n## Goal\n\n[b](/quest/b.md)\n"; + const moved = move([{ path: "quest/a.md", text }], "quest/b.md", "quest/c.md"); + expect(moved.texts.size).toBe(0); + expect(moved.missed).toEqual([{ path: "quest/a.md", line: 5, target: "/quest/b.md" }]); + }); +}); + +describe("remove", () => { + const gone = (path: string) => path === "quest/b.md"; + + test("drops entries, and a section left empty with its heading", () => { + const text = + "# [S] A\n\n## Goal\n\nG.\n\n## Required\n\n- [B](/quest/b.md) - first\n\n" + + "## Related\n\n- [C](/quest/c.md)\n- [B](/quest/b.md)\n wrapped\n- [D](/quest/d.md)\n"; + expect(remove([{ path: "quest/a.md", text }], gone).get("quest/a.md")).toBe( + "# [S] A\n\n## Goal\n\nG.\n\n## Related\n\n- [C](/quest/c.md)\n- [D](/quest/d.md)\n", + ); + }); + + test("keeps one blank line in a loose list and at the end of the file", () => { + const loose = + "# [S] A\n\n## Goal\n\nG.\n\n## Related\n\n- [C](/quest/c.md)\n\n- [B](/quest/b.md)\n\n- [D](/quest/d.md)\n"; + expect(remove([{ path: "quest/a.md", text: loose }], gone).get("quest/a.md")).toBe( + "# [S] A\n\n## Goal\n\nG.\n\n## Related\n\n- [C](/quest/c.md)\n\n- [D](/quest/d.md)\n", + ); + const last = "# [S] A\n\n## Goal\n\nG.\n\n## Required\n\n- [B](/quest/b.md)\n"; + expect(remove([{ path: "quest/a.md", text: last }], gone).get("quest/a.md")).toBe("# [S] A\n\n## Goal\n\nG.\n"); + }); + + test("leaves prose, sections with other content, and other sections alone", () => { + const text = + "# [S] A\n\n## Goal\n\nAfter [B](/quest/b.md).\n\n- [B](/quest/b.md) in a goal list\n\n" + + "## Related\n\nContext.\n\n- [B](/quest/b.md)\n"; + expect(remove([{ path: "quest/a.md", text }], gone).get("quest/a.md")).toBe( + "# [S] A\n\n## Goal\n\nAfter [B](/quest/b.md).\n\n- [B](/quest/b.md) in a goal list\n\n## Related\n\nContext.\n", + ); + }); + test("drops links to finished milestones until nothing changes, keeping the root's listing", () => { + const root = + "# Quests\n\n## Goal\n\nG.\n\n## Required\n\n- [M0](/quest/m0/README.md)\n- [M1](/quest/m1/README.md)\n"; + const sources = [ + { path: "quest/README.md", text: root }, + // m0 finishes once b goes, which finishes m1, which only waited on m0. + { path: "quest/m0/README.md", text: "# M0\n\n## Goal\n\nG.\n\n## Required\n\n- [B](/quest/b.md)\n" }, + { + path: "quest/m1/README.md", + text: "# M1\n\n## Goal\n\nG.\n\n## Required\n\n- [M0](/quest/m0/README.md)\n", + }, + { path: "quest/b.md", text: "# [S] B\n\n## Goal\n\nG.\n" }, + { + path: "quest/m1/a.md", + text: + "# [S] A\n\n## Goal\n\nG.\n\n## Required\n\n- [M1](/quest/m1/README.md)\n- [M2](/quest/m2/README.md)\n\n" + + "## Related\n\n- [M0](/quest/m0/README.md)\n", + }, + // m2 was already finished, so the link to it was already stale. + { path: "quest/m2/README.md", text: "# M2\n\n## Goal\n\nG.\n" }, + ]; + const texts = remove(sources, gone); + expect([...texts.keys()].sort()).toEqual(["quest/m0/README.md", "quest/m1/README.md", "quest/m1/a.md"]); + expect(texts.get("quest/m0/README.md")).toBe("# M0\n\n## Goal\n\nG.\n"); + expect(texts.get("quest/m1/README.md")).toBe("# M1\n\n## Goal\n\nG.\n"); + expect(texts.get("quest/m1/a.md")).toBe( + "# [S] A\n\n## Goal\n\nG.\n\n## Related\n\n- [M0](/quest/m0/README.md)\n", + ); + }); +}); + +describe("quest mv", () => { + test("moves a quest, rewrites its links, and stages the change", async () => { + const tree = committed(Tree.baseline()); + const out = await quest(tree, "mv", "/quest/m0/epic/one.md", "quest/m0/epic/first.md"); + expect(out).toEqual({ + code: 0, + stdout: "R quest/m0/epic/one.md -> quest/m0/epic/first.md\nM quest/m0/epic/README.md\nM quest/m0/epic/two.md\n", + stderr: "quest: no origin remote, so claims were not checked\n", + }); + expect(tree.read("quest/m0/epic/two.md")).toContain("- [One](/quest/m0/epic/first.md) - must finish first"); + expect(staged(tree)).toEqual([ + "M quest/m0/epic/README.md", + "M quest/m0/epic/two.md", + "R quest/m0/epic/one.md -> quest/m0/epic/first.md", + ]); + tree.accepts(); + }); + + test("moves an epic into a new directory", async () => { + const tree = committed(Tree.baseline()); + const out = await quest(tree, "mv", "quest/m0/epic", "quest/m1/later/epic"); + expect(out.stdout).toBe( + "R quest/m0/epic -> quest/m1/later/epic\nM quest/m0/README.md\nM quest/m1/later/epic/README.md\nM quest/m1/later/epic/two.md\n", + ); + expect(tree.read("quest/m1/later/epic/README.md")).toContain("- [One](/quest/m1/later/epic/one.md)\n"); + // The epic left m0 for a directory with no README, which check reports rather than mv guessing. + expect(out.code).toBe(1); + expect(out.stderr).toContain("quest/m1/later/epic/README.md: not listed in quest/m1/later/README.md"); + }); + + test("reports mentions it does not edit", async () => { + const tree = Tree.baseline() + .append("quest/m0/epic/two.md", "\n## Plan\n\nFollows `quest/m0/epic/one.md`.\n") + .write("notes.md", "See quest/m0/epic/one.md.\n"); + committed(tree); + const out = await quest(tree, "mv", "quest/m0/epic/one.md", "quest/m0/epic/first.md"); + expect(out.code).toBe(0); + expect(out.stderr).toContain("quest: notes.md:1: mentions quest/m0/epic/one.md; update it by hand\n"); + expect(out.stderr).toContain( + "quest: quest/m0/epic/two.md:13: mentions quest/m0/epic/one.md; update it by hand\n", + ); + }); + + test("reports mentions in untracked files but not ignored ones", async () => { + const tree = committed(Tree.baseline().write(".gitignore", "ignored.md\n")); + tree.write("draft.md", "See quest/m0/epic/one.md.\n").write("ignored.md", "See quest/m0/epic/one.md.\n"); + const out = await quest(tree, "mv", "quest/m0/epic/one.md", "quest/m0/epic/first.md"); + expect(out.code).toBe(0); + expect(out.stderr).toBe( + "quest: no origin remote, so claims were not checked\n" + + "quest: draft.md:1: mentions quest/m0/epic/one.md; update it by hand\n", + ); + }); + + test("reports mentions of a moved epic's directory but not of its siblings", async () => { + const tree = Tree.baseline().write( + "notes.md", + "See quest/m0/epic.\nRun `quest rm -r quest/m0/epic` first.\nNot quest/m0/epic-2 or quest/m0/epic.md.\n", + ); + committed(tree); + const out = await quest(tree, "mv", "quest/m0/epic", "quest/m0/feature"); + expect(out.code).toBe(0); + expect(out.stderr).toBe( + "quest: no origin remote, so claims were not checked\n" + + "quest: notes.md:1: mentions quest/m0/epic; update it by hand\n" + + "quest: notes.md:2: mentions quest/m0/epic; update it by hand\n", + ); + }); + + test("refuses the root epic and milestones", async () => { + const tree = Tree.baseline().write("quest/m1/README.md", "# M1\n\n## Goal\n\nNext.\n"); + committed(tree); + for (const path of ["quest", "quest/README.md", "quest/m0", "quest/m0/README.md"]) { + const out = await quest(tree, "mv", path, "quest/m1/old"); + expect(out.code).toBe(1); + expect(out.stderr).toBe(`quest: ${path} is permanent; the root epic and milestones never move\n`); + } + expect(staged(tree)).toEqual([]); + }); + + test("refuses a destination that exists", async () => { + const tree = committed(Tree.baseline()); + const out = await quest(tree, "mv", "quest/m0/epic/one.md", "quest/m0/epic/two.md"); + expect(out).toEqual({ code: 1, stdout: "", stderr: "quest: quest/m0/epic/two.md already exists\n" }); + expect(staged(tree)).toEqual([]); + }); + + test("refuses a claimed quest", async () => { + const tree = committed(Tree.baseline()); + const remote = tempDir(); + git(remote, "init", "-q", "--bare"); + git(tree.path, "remote", "add", "origin", remote); + git(tree.path, "push", "-q", "origin", "HEAD:refs/heads/quest/m0/epic/two"); + const out = await quest(tree, "mv", "quest/m0/epic", "quest/m0/feature"); + expect(out.code).toBe(1); + expect(out.stdout).toBe(""); + expect(out.stderr).toContain("quest: origin is not on GitHub, so only its branches were checked for claims\n"); + expect(out.stderr).toContain("quest: refusing to move claimed quests: quest/m0/epic/two;"); + expect(existsSync(join(tree.path, "quest/m0/epic/two.md"))).toBe(true); + expect(staged(tree)).toEqual([]); + }); + + test("fails outside a git work tree", async () => { + const out = await quest(Tree.baseline(), "mv", "quest/m0/epic/one.md", "quest/m0/epic/first.md"); + expect(out.code).toBe(1); + expect(out.stderr).toContain("is not in a git work tree"); + }); +}); + +describe("quest rm", () => { + test("deletes a quest listed under several Required and Related sections", async () => { + const tree = Tree.baseline() + .append("quest/m0/epic/README.md", "- [Three](/quest/m0/epic/three.md)\n") + .write( + "quest/m0/epic/three.md", + "# [S] Three\n\n## Goal\n\nMore.\n\n## Required\n\n- [Two](/quest/m0/epic/two.md)\n- [One](/quest/m0/epic/one.md)\n\n" + + "## Related\n\n- [One](/quest/m0/epic/one.md) - similar\n", + ); + committed(tree); + const out = await quest(tree, "rm", "quest/m0/epic/one.md"); + expect(out).toEqual({ + code: 0, + stdout: "D quest/m0/epic/one.md\nM quest/m0/epic/README.md\nM quest/m0/epic/three.md\nM quest/m0/epic/two.md\n", + stderr: "", + }); + expect(tree.read("quest/m0/epic/two.md")).toBe("# [S] Two\n\n## Goal\n\nAnother quest.\n"); + expect(tree.read("quest/m0/epic/three.md")).toBe( + "# [S] Three\n\n## Goal\n\nMore.\n\n## Required\n\n- [Two](/quest/m0/epic/two.md)\n", + ); + expect(staged(tree)).toContain("D quest/m0/epic/one.md"); + tree.accepts(); + }); + + test("drops links to the milestone it finishes while the root still lists it", async () => { + const tree = Tree.baseline() + .write("quest/README.md", `${ROOT_README}- [M1](/quest/m1/README.md)\n`) + .write("quest/m1/README.md", "# M1\n\n## Goal\n\nNext.\n\n## Required\n\n- [Later](/quest/m1/later.md)\n") + .write( + "quest/m1/later.md", + "# [S] Later\n\n## Goal\n\nAfter m0.\n\n## Required\n\n- [M0](/quest/m0/README.md)\n", + ); + committed(tree); + const out = await quest(tree, "rm", "-r", "quest/m0/epic"); + expect(out.code).toBe(0); + expect(out.stdout).toBe( + "D quest/m0/epic/README.md\nD quest/m0/epic/one.md\nD quest/m0/epic/two.md\nM quest/m0/README.md\nM quest/m1/later.md\n", + ); + expect(tree.read("quest/m0/README.md")).toBe("# M0\n\n## Goal\n\nA milestone.\n"); + expect(tree.read("quest/m1/later.md")).toBe("# [S] Later\n\n## Goal\n\nAfter m0.\n"); + expect(tree.read("quest/README.md")).toContain("- [M0](/quest/m0/README.md)\n"); + tree.accepts(); + }); + + test("refuses an epic that holds quests without -r, by directory or README", async () => { + const tree = committed(Tree.baseline()); + for (const path of ["quest/m0/epic", "quest/m0/epic/README.md"]) { + const out = await quest(tree, "rm", path); + expect(out.code).toBe(1); + expect(out.stderr).toBe( + `quest: ${path} is an epic holding 3 quest documents; pass -r to remove them all\n`, + ); + } + expect(staged(tree)).toEqual([]); + }); + + test("refuses the root epic and milestones", async () => { + const tree = committed(Tree.baseline()); + for (const path of ["quest/m0", "quest/m0/README.md", "quest/README.md"]) { + expect((await quest(tree, "rm", "-r", path)).stderr).toContain("is permanent"); + } + }); + + test("refuses a directory that holds no quest documents", async () => { + const tree = committed(Tree.baseline().write("quest/m0/research/AGENTS.md", "Notes.\n")); + const out = await quest(tree, "rm", "-r", "quest/m0/research"); + expect(out).toEqual({ code: 1, stdout: "", stderr: "quest: quest/m0/research holds no quest documents\n" }); + expect(existsSync(join(tree.path, "quest/m0/research/AGENTS.md"))).toBe(true); + expect(staged(tree)).toEqual([]); + }); + + test("reports a prose link it does not edit, and the check that fails on it", async () => { + const tree = Tree.baseline().append( + "quest/m0/epic/two.md", + "\n## Plan\n\nReuse [one](/quest/m0/epic/one.md).\n", + ); + committed(tree); + const out = await quest(tree, "rm", "quest/m0/epic/one.md"); + expect(out.code).toBe(1); + expect(out.stderr).toContain( + "quest: quest/m0/epic/two.md:9: mentions quest/m0/epic/one.md; update it by hand\n", + ); + expect(out.stderr).toContain("quest: quest/m0/epic/two.md:9: link does not resolve: /quest/m0/epic/one.md\n"); + }); +}); + +test("open pull requests come from every page, by head branch", async () => { + const requested: string[] = []; + const deps: GitHubDependencies = { + token: "test-token", + now: () => 0, + sleep: async () => undefined, + warn: () => undefined, + fetch: async (input) => { + const url = new URL(String(input)); + requested.push(url.pathname + url.search); + const page = url.searchParams.get("page"); + const head = (ref: string, n: number) => ({ + head: { ref }, + html_url: `https://github.com/example/project/pull/${n}`, + }); + return page === null + ? new Response(JSON.stringify([head("quest/m0/a", 1)]), { + headers: { + link: '; rel="next"', + }, + }) + : new Response(JSON.stringify([head("feature", 2)])); + }, + }; + expect(await openPullRequests(new GitHub(deps), { owner: "example", repo: "project" })).toEqual([ + { branch: "quest/m0/a", url: "https://github.com/example/project/pull/1" }, + { branch: "feature", url: "https://github.com/example/project/pull/2" }, + ]); + expect(requested).toEqual([ + "/repos/example/project/pulls?state=open&per_page=100", + "/repos/example/project/pulls?state=open&page=2", + ]); +}); + +test("a fork's parent repository and its quest branches come from GitHub", async () => { + const responses: Record = { + "/repos/me/project": { fork: true, parent: { full_name: "example/project" } }, + "/repos/example/project": { fork: false }, + "/repos/example/project/git/matching-refs/heads/quest/": [ + { ref: "refs/heads/quest/m0/a" }, + { ref: "refs/heads/quest/m0/epic/b" }, + ], + }; + const github = new GitHub({ + token: "test-token", + now: () => 0, + sleep: async () => undefined, + warn: () => undefined, + fetch: async (url) => new Response(JSON.stringify(responses[new URL(String(url)).pathname])), + }); + const parent = { owner: "example", repo: "project" }; + expect(await forkParent(github, { owner: "me", repo: "project" })).toEqual(parent); + expect(await forkParent(github, parent)).toBeNull(); + expect(await githubBranches(github, parent)).toEqual(["quest/m0/a", "quest/m0/epic/b"]); +});