Repository navigation
Conversation
A merge queue dequeue (merge_conflict, failed_checks, a quest land ejection) used to remove quest:merge, so the maintainer's merge pick was lost and the next /quest-complete asked about the PR again. The label now stays on as the pick, which no session holds: - `quest merge` without a token takes it over with a fresh token; iterate and ready refuse it, naming the pick. - `quest status` prints it as a `pick` record instead of `hold`. - `quest land` ejections and the kix.quest bot leave the label on; the bot still removes it when a merge job ends without landing. - /quest-complete dispatches /quest-merge for every `pick` without asking. Removing the label withdraws the pick. 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: e4843464da
ℹ️ 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 (token === undefined && state === "merge" && spentMergeHold(snapshot)) { | ||
| await labels.remove(MERGE_LABEL); | ||
| snapshot = { ...snapshot, labels: snapshot.labels.filter((name) => name !== MERGE_LABEL) }; |
There was a problem hiding this comment.
After a landing ends unmerged, reacquiring merge ownership with the original token skips this refresh and reaches the mine === label early return. The acquisition succeeds, but spentMergeHold remains true, so status still reports an unowned pick and another session can immediately take it over while the original session repairs the PR. This newly permits overlapping work and invalidates the first session's supposedly reacquired token. Refresh the spent hold on token-bearing reacquisition too, or refuse that acquisition and require a fresh take.
Useful? React with 👍 / 👎.
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: e484346.
No new actionable defects found. Separating a persistent merge pick from an active session hold is a sensible direction. The CLI's fresh-token takeover, status classification, queue-ejection preservation, and bot's token-checked release of ended jobs are consistent with that distinction. The existing non-atomic label acquisition limitations are not resolved by this change.
Nonblocking follow-up: test refused takeover of a picked draft and consider performing refusal checks before removing its label, so an unsuccessful attempt preserves the pick.
GitHub-only static review of the diff, hold classification/acquisition, bot release/store lifecycle, skills, and test source. No CLI/Worker tests or live GitHub lifecycle experiments were run; reported check counts were not independently verified.
(Written by OpenAI)
|
Closing as superseded by the host-land plan (#291). An ejection will now remove (written by Claude Opus 5.5) |
Problem
When a PR left the merge queue unmerged (
merge_conflict,failed_checks, or aquest landejection), the CLI and the kix.quest bot removedquest:merge. That erased the maintainer's merge pick, so the next/quest-completeasked about the same PR again. In one moq session that happened to dozens of PRs.Change
The label now stays on as the pick, which no session holds.
quest mergewithout a token takes over a pick: it removes the label and adds it back, getting a fresh token, so the PR is exclusively held again.quest iterate/readyrefuse it with… is picked for merge …. The session that landed can still use its old token.quest statusprints a pick aspick quest:merge <token> <time> <url> <age>instead ofhold.quest landejections keep the label.releasePullRequestleaves the label on when the hold is spent, meaning a landing ended unmerged. A merge job that ends without landing (for example, stopped to ask) still removes the label.holdPullRequestalready replaced a spent label./quest-completedispatches/quest-mergefor everypickwithout asking./quest-mergesays the label persists.--helptext and README document this.Choices made without asking
quest mergetakes over a pick. Letting iterate/ready take it would silently drop the pick.Checks
just checkpasses.just testpasses: 620 CLI tests and 426 Worker tests. Twotest/agent.test.tscases fail whenCLAUDE_CONFIG_DIR/QUEST_WORKTREE_HOMEare set in the caller's environment; that happens on main too, and they pass with those variables unset.Suggested follow-ups
test/agent.test.tsshould clearCLAUDE_CONFIG_DIRandQUEST_WORKTREE_HOMEfrom the environment it inherits.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code