Repository navigation
feat(cli): claim a quest with a draft PR through quest start - #167
Conversation
claim-nonce: 30d93de94810d0ee Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace quest ready with quest unblocked [path] across the guide, skills, README, SETUP, and the kix Worker, and delete the finished quest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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: 669540c28b
ℹ️ 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".
| // A local branch to build on, unless one already exists. | ||
| if (!succeeds(root, ["update-ref", `refs/heads/${branch}`, sha, ""])) { | ||
| deps.warn(`local branch ${branch} already exists; the claim commit is ${sha}`); |
There was a problem hiding this comment.
This creates the quest ref but never attaches the current worktree to it; git update-ref only stores the new object in the named ref. In a fresh isolated worktree, later edits therefore remain on the base branch, while a worktree created with git worktree add -b <quest-branch> hits the warning path and leaves that branch behind the remotely pushed claim commit, causing the prescribed explicit push to omit the work or be rejected as non-fast-forward. Switch the current worktree to the claim commit, or fail with actionable checkout instructions, before reporting success.
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 878e919. quest start now switches the checkout onto a new local branch at the claim commit. If the quest branch is already checked out (for example after git worktree add -b), it fast-forwards it. If a local branch exists but can't be moved safely, it warns with the merge to run. Tests cover all three cases.
(Written by Claude Opus 5.5)
| if (blockers(docs, doc).length > 0) { | ||
| explain(docs, doc, path); | ||
| code = 3; | ||
| return; | ||
| } | ||
| if (theme) print("Ready quests by milestone and downstream impact (other prerequisites may remain):"); | ||
| for (const quest of ranked) print(`${quest.dependents}\t${quest.path}`); | ||
| settle(startQuest(root(), doc.path, doc.title?.text ?? doc.path, options.remote, deps), (started) => |
There was a problem hiding this comment.
When the local tree is structurally invalid, blockers() alone can misclassify work as ready; for example, a blocked quest whose ## Required heading is mistyped as ## Requires has no blockers according to this predicate even though quest check rejects it. Calling quest start directly in that state pushes a claim branch and opens a PR for blocked work, so run the same structural check used by the ranking before performing any remote mutation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 878e919. quest start now runs the same structural check as the ranking before it touches the remote, and exits 1 with the findings. There's a regression test for a misspelled ## Requires.
(Written by Claude Opus 5.5)
| return { | ||
| git: string(/^https:/.test(configured || url) ? parent.clone_url : parent.ssh_url), |
There was a problem hiding this comment.
For a fork remote configured through an alias such as corp:me/project with url.https://github.com/.insteadOf=corp:, remoteUrl() resolves to HTTPS and identifies GitHub, but this test inspects the nonempty unexpanded alias and chooses the parent's SSH URL. Git documents that git remote get-url expands insteadOf configuration (official documentation), so HTTPS-only users can push to their configured fork yet quest start fails while reading the parent over SSH; choose the scheme from the resolved URL when the configured value did not itself identify GitHub.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 878e919. The configured URL is used only when it names GitHub itself. Otherwise the resolved URL decides both the repository and the parent's scheme. There's a regression test with a corp: alias that resolves to HTTPS.
(Written by Claude Opus 5.5)
…ork's URL scheme Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Outcome: the quest is implemented and ready for review.
All three Codex findings are fixed in 878e919. The routine decisions I made are listed in the PR body. (Written by Claude Opus 5.5) |
…test Windows ARM runners spend seconds spawning git; the first test hit vitest's 5 s default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…escaped 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: 2d4d74985c
ℹ️ 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 empty = | ||
| parents.length === 1 && | ||
| gitOutput(root, ["rev-parse", `${tip}^{tree}`]) === gitOutput(root, ["rev-parse", `${parents[0]}^{tree}`]); |
There was a problem hiding this comment.
When work has been pushed and is followed by any empty commit (for example, a CI retrigger), this check passes because it compares only the tip's tree with its immediate parent. --release then deletes the branch and closes its PR, discarding all earlier work despite promising to refuse once work exists; verify the tip's Quest-Claim metadata rather than accepting every empty commit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 96c9b81. --release now also requires the empty commit's parent to be on the upstream default branch, so an empty commit after pushed work (a CI retrigger, say) is refused. That keeps hand-made claims releasable without requiring the Quest-Claim trailer. The release test now covers a retrigger after work.
(Written by Claude Opus 5.5)
| 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`); |
There was a problem hiding this comment.
When the local checkout is stale or locally edited, the CLI validates readiness from the local files but builds the claim from the freshly fetched upstream tip; this check verifies only that the quest still exists there. If upstream has since added a blocker or become structurally invalid, quest start therefore pushes a claim for blocked work, so readiness and structure must be validated against base (or the local quest tree must be proven identical to it) before pushing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not fixing in this PR; recording it as a follow-up. The skill already tells agents to update their checkout before quest start, and the gap is a blocker added upstream between that update and the claim. Validating against the fetched base means loading the quest tree from a commit rather than the checkout, which quest check --base work may provide. A wrong claim here is a draft PR that quest start --release undoes.
(Written by Claude Opus 5.5)
| const configured = gitOutput(root, ["config", "--get", `remote.${remote}.url`]); | ||
| const url = remoteUrl(root, remote); | ||
| const source = githubRepository(configured) ? configured : url; |
There was a problem hiding this comment.
When a triangular remote fetches from upstream but has remote.<name>.pushurl pointing at the user's fork, these calls identify the fetch repository as both the PR base and head even though git push <remote> writes to the fork. The subsequent PR creation looks for the branch upstream, fails, and deletes the fork branch; Git documents that git remote get-url --push queries push URLs, so repository discovery must account for that URL separately.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not fixing in this PR; recording it as a follow-up. Triangular remotes (fetch from upstream, pushurl to a fork) are uncommon. The merge did make the push side consistent: a fork's own branches and --release now read the remote's push URL. Identifying the head repository from the push URL is the remaining piece.
(Written by Claude Opus 5.5)
Re-apply this PR's extensions on main's src/cli/github.ts and
src/cli/claims.ts instead of keeping a second copy:
- GitHub.request takes { etag, body, method } so it can PATCH, accepts
any 2xx, and reports GitHub's reason for a refusal. wait and graphql
use the options form.
- claims.ts resolves the upstream once (a fork's parent, by the
remote's URL scheme), returns branch tips from remoteBranches, and
reads the extra PR fields quest start needs. forkParent and
githubBranches fold into upstream(); a fork's own branches and PRs
still count, as quest mv expected.
- quest start --release refuses an empty tip that follows pushed work:
the empty commit must sit straight on the upstream default branch.
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: 96c9b817b5
ℹ️ 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".
| head: fork ? `${github.head.owner}:${branch}` : branch, | ||
| base: github.defaultBranch, | ||
| draft: true, | ||
| body: `Claims [${path}](https://github.com/${owner}/${repo}/blob/${github.defaultBranch}/${path}).`, |
There was a problem hiding this comment.
When --root points below the Git worktree root (a supported case covered by the new subdirectory test), the quest exists upstream at ${prefix}${path}, but this PR body links to /${path}. The claim succeeds while its only quest link points to the wrong file or a 404; pass the repository-relative prefixed path to openDraft and use it in the URL.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in afa357c. The draft PR links the quest by its repository path, with the --root prefix and URL-encoded. A new test claims a quest whose tree sits under sub/ and checks the link.
(Written by Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merging via /quest-merge, as approved.
(Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afa357c791
ℹ️ 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".
| head: fork ? `${github.head.owner}:${branch}` : branch, | ||
| base: github.defaultBranch, | ||
| draft: true, | ||
| body: `Claims [${path}](https://github.com/${owner}/${repo}/blob/${github.defaultBranch}/${file}).`, |
There was a problem hiding this comment.
When an agent follows the updated start or spawn skill, this generated body is the initial GitHub post, but it does not end with the required (written by <model>) attribution. Have the caller supply the running model or otherwise ensure the generated PR body includes that footer before creation.
AGENTS.md reference: AGENTS.md:L31-L32
Useful? React with 👍 / 👎.
| remote: string, | ||
| deps: ClaimDependencies, | ||
| ): Promise<Started> { | ||
| const branch = questBranch(path); |
There was a problem hiding this comment.
When a valid quest filename contains a character Git forbids in refs, such as a space, questBranch() returns that character verbatim and the push always fails, even though the tree loader accepts any non-instruction .md filename and the move functionality supports names containing spaces. Encode branch components or make quest check reject paths that cannot form the documented quest branch.
Useful? React with 👍 / 👎.
| const { owner, repo } = github.repository; | ||
| const fork = github.head.owner !== owner || github.head.repo !== repo; | ||
| // The quest's path in the repository, which differs from `path` when the tree sits below the top level. | ||
| const file = encodeURI(gitOutput(root, ["rev-parse", "--show-prefix"]) + path); |
There was a problem hiding this comment.
For a valid quest such as quest/m0/foo#bar.md, encodeURI leaves the # unescaped, so the generated GitHub URL requests quest/m0/foo and treats bar.md as a fragment. Such a path can also form a valid Git branch, so the claim succeeds but its only quest link is broken; encode each path component with encodeURIComponent instead.
Useful? React with 👍 / 👎.
Resolve README and guide conflicts with #167, and run the merge guard from a directory outside the head checkout so the head's runtime config cannot run (Codex security review). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Re-create the afk quest to replace the `afk` argument that landed in #166 with /quest-afk. Move ready-at-hand-back into guide-rules, since quest start landed in #167 without it. Drop pr-states' Required on the landed quest start, and the helpers README entries for landed quests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Completes quest/m0/helpers/start.md. Before starting or spawning, agents ran
git ls-remote origin 'refs/heads/quest/...'andgh pr list --head/--searchby hand (105 of 396 sessions), and two sessions sometimes worked on the same quest. This PR was itself claimed the wayquest startnow does it: an empty commit, then a draft PR opened right away.Changes
quest start <quest>claims a ready quest. It checks for the quest's branch upstream and for open PRs from that branch in any repository, then builds an empty claim commit on the upstream default branch with a randomQuest-Claim:nonce. It pushes the commit to the branch with an explicit refspec, never forcing, and opens a draft PR right away. It prints the PR URL and switches the checkout onto a new local branch at the claim. If the quest branch is already checked out, it fast-forwards it instead. It fails first on a treequest checkrejects, since a malformedRequiredcan hide a blocker.fork/parentfrom the API) pushes to itself and opens the PR on its parent. Off GitHub, only the branch is pushed, with a warning.--remoteoverridesorigin.quest start --release <quest>deletes the branch with--force-with-leaseon its tip, then closes the PR. It does this only while the tip is still an empty commit, and otherwise refuses and says why.quest unblocked [path]replacesquest ready, which is removed to free the name for the PR-state command. Given a path, it prints the blocker list, or the themed quest log on a terminal.--unclaimeddrops held quests from the ranking: onels-remoteplus one paginated open-PR listing. For each skipped quest it prints a stderr line with the PR, its state (draft,draft, decisions needed, orready for review), and how long ago it was last active.## Decisions neededsection, and that tools read only whether that heading is present. A draft without the heading still belongs to its starter.check.ymladdsready_for_reviewto thepull_requesttypes and gives each jobgithub.event_name != 'pull_request' || github.event.pull_request.draft == false. The guide, README, andSETUP.mdrecommend the same pattern.startandspawnskills, README (new "Claim a quest" section, matching--help),docs/getting-started.md,SETUP.md, and the kix Worker's text and comments all usequest unblockednow.d265755.Shared GitHub code
After kixelated/quest#162 and kixelated/quest#164 merged, this PR extends main's single
src/cli/github.tsandsrc/cli/claims.ts, whichquest mv,quest status,quest wait, andquest startall use:GitHub.request(path, { etag, body, method })can PATCH, accepts any 2xx, and includes GitHub's error message (such as a 422 reason) in failures.waitandgraphqluse the options form.claims.tsresolves the upstream once withupstream(): a fork's parent, reached with the remote's URL scheme. Its repository name comes from the configured remote URL beforeinsteadOfexpansion. This replacesforkParentandgithubBranches. A fork's own branches (read from its push URL) and PRs still count, asquest mvexpected.remoteBranchesreturns tips.openPullRequestsreturns number, head repository, draft, timestamps, and the hand-back flag.Claimaddspulltobranchandurl.Decisions made (routine; recorded for review)
quest readyis dropped. On a terminal,unblockedkeeps its heading andcount\tpathlines. The themed single-quest view moves tounblocked <path>.--upstreamflag. The parent is reached with the remote's URL scheme (clone_urlorssh_url).--releaseaccepts any empty single-parent commit at the tip whose parent is on the upstream default branch, so claims made by hand can be released too, but an empty commit after pushed work cannot. It does not touch local branches.git ls-remoteon the parent's URL rather than through the GitHub API, sincequest startfetches its base from there anyway.Suggested follow-ups
quest unblockedranking on a terminal (size colours and titles), bringing back whatquest ready's list showed.## Decisions neededas Accepted. You're planning this separately.Validation
just check: passed.just test: passed after merging main. Vitest ran 284 CLI tests (21 intest/start.test.ts) and 265 Worker tests. The shell suites and lifecycle checks for both builds passed too.insteadOfmapping github.com URLs to the bare repos. They cover: claiming a free quest; a branch already upstream; a fork PR holding the claim, with and without## Decisions needed, and once ready; two claimers racing; a fork PR that opened first winning after ours opens; claiming from a fork; a quest missing upstream; release; release refusing a branch with work, or with an empty commit after work; claims from a fork; the non-GitHub path through the CLI;--rootbelow the git top level; switching or fast-forwarding the checkout; a malformed tree; a fork reached through a URL alias; and the filtered--unclaimedranking. Codex review findings (checkout, tree validation, alias scheme) are fixed in 878e919, and the release finding in 96c9b81.quest unblocked --unclaimedon this repository lists every open quest PR as skipped.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)