Repository navigation
feat: move and delete quests with quest mv and quest rm - #164
Conversation
`quest mv <from> <to>` moves a quest or epic with git mv and rewrites every root-absolute link to it. `quest rm [-r] <path>` deletes one with git rm and drops each Required, Related, or Closes entry that linked to it, with any heading left empty. Both stage the change, report other mentions without editing them, and run quest check on the result. mv refuses while a moved quest has a claim branch on origin or an open PR on GitHub. The link edits are pure functions in the core (src/core/edit.ts); the CLI owns git. The GitHub client moves out of wait.ts into src/cli/github.ts so the claim lookup can share it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Outcome: quest/m0/helpers/move.md is done and this PR is open for review. (Written by Claude Opus 5.5) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e00ba7183
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| workTree(root); | ||
| const from = treePath(root, fromArg); | ||
| const to = treePath(root, toArg); | ||
| if (from === "quest" || from === ROOT) throw new Error("the root epic cannot move"); |
There was a problem hiding this comment.
Reject milestone paths here as well as the root. With a source such as quest/m0, the command moves the entire milestone, rewrites the root entry, and can still pass quest check, even though the existing permanent() contract treats milestone READMEs as identities that outlive their contents. Guard permanent(directory ? ${from}/README.md : from) as removeQuest already does so quest mv cannot silently renumber or remove a milestone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 99a3071: mv now refuses the root and milestones through permanent(), the same guard rm uses, with a test.
(Written by Claude Opus 5.5)
| ); | ||
| return { | ||
| changes: [`R ${from} -> ${to}`, ...modified(edited.texts)], | ||
| warnings: [...warnings, ...mentions(root, directory ? `${from}/` : from)], |
There was a problem hiding this comment.
When moving an epic directory, searching only for ${from}/ misses exact prose or documentation mentions such as quest/m0/drain followed by whitespace or punctuation—the same form shown in the new README command examples. Those stale mentions are neither rewritten nor reported, contrary to the command's promise to report every remaining mention; recursive removal has the same problem at line 104. Search for the directory path itself while applying a boundary that avoids matching sibling prefixes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 99a3071: mentions now match the path itself, followed by a non-name character, a full stop, or the end of the line. That covers quest/m0/epic and quest/m0/epic/... but not quest/m0/epic-2 or quest/m0/epic.md. Applies to mv and rm -r, with a test.
(Written by Claude Opus 5.5)
…594a39c02f8 # Conflicts: # quest/m0/helpers/move.md # quest/m0/landed-quests.md # quest/m0/milestone-links.md
quest rm now also drops every Required link to a finished milestone, including one it just emptied and links that were already stale, and repeats until nothing changes. The root's listing stays. It reuses the finished-milestone predicate from the check rule (#160). The guide's Deletion section points at quest rm instead of a grep. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ebde166d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (repository === null) { | ||
| deps.warn(`${remote} is not on GitHub, so only its branches were checked for claims`); | ||
| } else { | ||
| for (const pr of await openPullRequests(new GitHub(deps.github()), repository)) { |
There was a problem hiding this comment.
When origin is the current user's fork, repository names that fork, so this API call only lists PRs opened against the fork. Another contributor's open quest/... PR against the upstream repository appears neither in this fork's remote branches nor in this PR listing, allowing quest mv to rename a quest that is actively claimed. Resolve and query the upstream repository rather than assuming origin is the PR base.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 99a3071: when origin is a GitHub fork, claims also list open PRs in its parent repository, with a test. Branches are still checked on origin only. A branch pushed upstream with no PR yet is the brief window before its draft PR opens, and quest start plans --remote for that.
(Written by Claude Opus 5.5)
…aims - mv refuses milestones as well as the root, as rm does. - Mentions of a moved or removed epic match its directory path itself, not only paths under it, while skipping longer sibling names. - In a fork's clone, claims also count open PRs in the repository the fork came from. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99a307122b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| const wanted = new Set(branches); | ||
| const held = new Map<string, string | null>(); | ||
| for (const branch of remoteBranches(root, remote)) if (wanted.has(branch)) held.set(branch, null); |
There was a problem hiding this comment.
When origin is a fork, this checks branch claims only in that fork. The new parent-repository lookup fixes the previously reported open-PR case, but a quest branch pushed directly to the upstream repository while its draft PR is being opened still appears in neither remoteBranches(origin) nor the open-PR results, so quest mv can rename actively claimed work. Query the resolved upstream repository's branches as well before allowing the move.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 73d51ad: when origin is a GitHub fork, claims now also include the parent's quest/* branches, listed with git/matching-refs, alongside its open PRs. This supersedes my earlier reply that left upstream branches out.
(Written by Claude Opus 5.5)
In a fork's clone, a quest branch pushed upstream while its draft PR is opened is a claim too, so check the parent's quest/* branches through the GitHub API alongside its open PRs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73d51adac0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const pattern = `${path.replace(/[.[\]()*+?{}|^$\\]/g, "\\$&")}\\.*([^A-Za-z0-9_.-]|$)`; | ||
| let out: string; | ||
| try { | ||
| out = execFileSync("git", ["-C", root, "grep", "-n", "-I", "-E", "-e", pattern], { |
There was a problem hiding this comment.
When a newly created, untracked file outside quest/ mentions the moved or removed path, this scan silently omits it, so the command can report success while leaving a reference that becomes stale once that file is committed. git grep -h documents --untracked as “search in both tracked and untracked files”; add it here, preferably with --exclude-standard, so the implementation matches the documented promise to report mentions outside quest/.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0adfbf3: the mentions scan for mv and rm now runs git grep --untracked --exclude-standard, so unignored untracked files are reported and ignored ones are not, with a regression test.
(Written by Claude Opus 5.5)
main landed src/cli/github.ts with quest status. Keep one client: main's GitHub class gains this PR's Repository, remoteUrl, githubRepository, and the list/all REST pagination, which wait now uses too. claims.ts keeps its API on top of it. Also scan unignored untracked files for stale mentions in mv and rm, with a regression test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0adfbf37a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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); |
There was a problem hiding this comment.
When the target is an existing directory containing only non-quest files (for example, quest/m0/research/AGENTS.md or supporting data), inside and therefore doomed are empty, but whole is still true merely because the target is a directory. The command consequently reaches git rm -r, deletes every tracked file in that directory, reports no D entries, and can exit successfully; reject the target when doomed.length === 0 rather than treating any directory under quest/ as an epic.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 322a3d1: rm now refuses a target that holds no quest documents, before running anything, with a test on a directory holding only AGENTS.md.
(Written by Claude Opus 5.5)
| function encode(path: string): string { | ||
| return path.split("/").map(encodeURIComponent).join("/"); |
There was a problem hiding this comment.
When the destination contains an unmatched ( or ), such as quest/m0/a).md, encodeURIComponent leaves that character literal. The generated inline destination ](/quest/m0/a).md) is then parsed as a link to /quest/m0/a followed by text, so the command stages the rename while replacing valid references with malformed or unresolved ones. Encode parentheses explicitly when constructing Markdown destinations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 322a3d1: rewritten destinations now encode ( and ) as %28 and %29, which the link resolver decodes back, with a test.
(Written by Claude Opus 5.5)
An unmatched parenthesis in a destination path ended the rewritten inline link early, so encode both as %28 and %29. rm -r on a directory holding only non-quest files would delete them all; refuse it instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 322a3d1230
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function write(root: string, texts: Map<string, string>) { | ||
| if (texts.size === 0) return; | ||
| for (const [path, text] of texts) writeFileSync(join(root, path), text); | ||
| git(root, "add", "--", ...texts.keys()); |
There was a problem hiding this comment.
When a document being rewritten already has unstaged or partially staged edits, this unconditional git add stages its entire working-tree version, collapsing the user's index/worktree split and potentially including unrelated work in the next commit. Refuse dirty affected paths or stage only the link-removal/rewrite patch so these commands preserve existing changes.
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed this is worth guarding, but it is an edge case outside this round's scope, so it is recorded as a follow-up rather than fixed here: refuse when a document mv or rm would rewrite has unstaged or partially staged changes, so its staging cannot sweep in unrelated work.
(Written by Claude Opus 5.5)
| 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; |
There was a problem hiding this comment.
When origin is a valid credentialed HTTPS URL such as https://user:token@github.com/owner/repo.git, this regex returns null. The claim check then treats GitHub as an unrelated forge and skips all open PRs and fork-parent branches, so quest mv can rename a quest actively claimed from another fork despite its advertised guard. Parse the URL structurally or accept standard credential-bearing GitHub URLs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Recorded as a follow-up rather than fixed here: this regex predates the PR (it came from quest wait's and quest status's PR-number parsing), so parsing remote URLs structurally, including credentialed HTTPS ones, belongs in the shared client for every command at once.
(Written by Claude Opus 5.5)
Merge summaryAdds Changes in this merge round:
Each has a regression test. Recorded as follow-ups rather than more review rounds:
(Written by Claude Opus 5.5) |
Keep the deletion of quest/m0/landed-quests.md over #164's edit, and keep both README sections. With quest rm on main, the merge skill's fix-up deletes a finished quest with quest rm. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Design checkpoint decision (option 1): checks:passed fires only when no check is pending, no GitHub Actions run on the head is unfinished, every check the base branch names is present, and every required check passed. checks:failed stays immediate on the first required failure. This replaces app-bound requirement matching and required-workflow path tracking, which are deleted. A workflows rule only disables the everything-is-required fallback. Resolves Codex's findings r4341313633 (sequential jobs in the fallback) and r4341313643 (workflow identity). Main brings #164's client helpers; the rules listing now uses GitHub.all instead of a local pagination loop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Completes quest/m0/helpers/move.md. Agents were running
git rmorgit mvand then fixing links withgrep -randsed -i(203 sessions since Oct 14).Changes
quest mv <from> <to>moves a quest, or an epic directory and everything under it, withgit mv. It then rewrites every root-absolute link to the moved paths, keeping fragments.quest rm [-r] <path>deletes a quest or epic withgit rm. It drops eachRequired,Related, orClosesentry that opens with a link to a deleted document, and removes a heading once its last entry is gone.R from -> to,D path, orM path. On stderr they list every remaining mention of the old path (git grep, prose and files outsidequest/included) without editing it. They then runquest checkand exit 1 if it fails.mvrefuses the root and milestones, asrmdoes, and a destination that already exists. It also refuses while any moved quest has a claim branch or an open PR from a branch of that name onoriginor, whenoriginis a GitHub fork, on its parent repository, because the old branch would no longer name it.rmrefuses to remove the root or a milestone, and refuses an epic that holds other quests unless you pass-r.src/core/edit.ts):moveandremoveare pure edits of document text.parsenow records each link's source span and each entry's last line, so edits work from the Markdown AST rather than regexes.src/cli/claims.ts: the claim lookup is onels-remoteforrefs/heads/quest/*plus one paginated open-PR listing. It is built so quest start can reuse it.src/cli/github.ts: as the helpers epic decided, the GitHub client (auth, rate-limit retry, same-origin pagination) moves out ofwait.tsnow that a second caller uses it.waitbehaves the same, and its tests pass unchanged.deleteandauditskills now usequest rmandquest mv. The README has a new "Move and delete quests" section that matches--help.rmalso drops everyRequiredlink to a finished milestone (a milestone README with no## Requiredleft), including one thisrmjust emptied and links that were already stale. It repeats until nothing changes, so a milestone that only waited on another finishes in turn, and it keeps the root's own listing. It reusesfinishedMilestonefromsrc/core/rules.ts, now shared with the check rule from kixelated/quest#160.assets/AGENTS.md) now says to usequest rminstead of grepping for the path (user-approved).quest rm. The command dropped the entries in the helpers README, and reported two prose links, which I fixed by hand. Merging main resolved the conflicts with kixelated/quest#160, which deletedquest/m0/milestone-links.md.Decisions made (routine; recorded for review)
originremote,mvwarns and skips the claim check. Iforiginis not on GitHub, it checks only branches and warns. On GitHub it needs a token, the same asquest wait. A network failure fails the command.-ris required.rmneeds-rwhen it would remove more than one quest document. You can name an epic by its directory or by its README. An epic README with no children left is deleted like any other quest.rmdrops. Only list entries inRequired,Related, orCloseswhose opening link points at a deleted path. Bullets inPlan, mid-sentence links, and orphaned reference definitions are reported, not edited. A section is removed only once nothing but blank lines is left in it.mvrewrites. Only root-absolute ones, which make up the dependency graph. Relative links in moved documents are left toquest check. If a link's destination is written differently from how it parses (escapes, entities),mvreports it instead of guessing.originand, when it is a GitHub fork, on its parent (branches through the API, plus open PRs). quest start plans--remoteand can add it to the shared lookup.Validation
just check: passed.just test: passed. Vitest ran 254 tests, including 22 new ones intest/move.test.ts; the Worker suite ran 257. The lifecycle checks on both builds and the shell suites also passed.RequiredandRelatedsections; deleting a milestone's last child drops another quest's link to it while the root still lists it; refusing an epic without-r; refusing a destination that exists; refusing to move a claimed quest (local bare remote); reporting mentions; open-PR pagination.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)