Skip to content

feat(land): quest land watch fixes each PR the merge queue drops and re-enqueues it - #389

Merged
moq-bot[bot] merged 7 commits into
mainfrom
quest/m0/queue-watcher/fix
Nov 2, 2026
Merged

moq-bot[bot] merged 7 commits into
mainfrom
quest/m0/queue-watcher/fix

Conversation

@kixelated

@kixelated kixelated commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Implements Fix and re-enqueue and deletes the quest.

Problem

quest land watch (moq-dev/quest#388) recorded each PR the merge queue dropped, but nothing fixed it or put it back in the queue. It also treated every App enqueue as part of an existing pick. moq's Dependabot workflow enqueues with moq-bot's token, so under that rule every round of a Dependabot PR looked like a new pick, and the stop rules would never fire.

Behavior

quest land watch --fixer <wrapper> (the same wrapper and argument as serve --fixer) now does the following for each removal recorded with a fix verdict, oldest first, while the PR still sits where the queue dropped it:

  • Conflict: it first merges the current base with the quest merge driver, in the daemon's private repository (mergeBase, as prepareHead does). A clean merge needs no fixer: it becomes a merged round.
  • Failure, or a conflict the driver can't resolve: it starts the Codex land fixer in the background, with the same detached supervisor, wrapper, and timeouts as the host gate. A failed build is fixed on the base its merge_group was built on. A conflict is fixed on the current base, and the fixer gets the merge's CONFLICT lines as its log.
    • --fixer-jobs (default 2) caps how many run at once; the others wait.
    • --fixer-effort (default medium) reaches the wrapper as QUEST_LAND_FIXER_EFFORT, and quest land fix --effort (default: that variable, else medium) passes it to Codex as model_reasoning_effort.
  • Rounds and stop rules: each collected round goes into the pick's history with saveRound, and stopRule decides whether it hands back, as in moq-dev/quest#384. Outcomes are fixed, merged, flaky (a failed build requeued unchanged), or unchanged (a conflict requeued unchanged). The failure signature is the fixer's failure, or else the failing job and step (Queue / Queue: <step> on moq), or conflictFailure(base) for a conflict.
  • Publishing: a fixed or merged head is validated (importFix/validateFix), lease-pushed, and recorded as a mechanical fix in the PR body's Review (publishHead). A refused push is handed back with its resolution for quest land apply. These two helpers now come from collectJob itself, so the host gate shares them unchanged.
  • Re-enqueue: the head goes back at the back of the queue with expectedHeadOid bound to it. A head whose required checks have passed is enqueued. A head whose checks still run, such as a freshly pushed fix, gets auto-merge armed instead; this is queueHead, the same choice quest merge makes, now shared from hold.ts. The watcher then comments with what it did, after any note the fixer wrote.
  • Handback: a stop rule, the fixer itself, a refusal, a crash, or a timeout posts a handback with the <!-- quest land: … --> marker quest status reads. The PR then leaves the watcher's care.
  • Pick ended mid-round: if a writer pushed to the PR, enqueued it, removed it, or retargeted it while the fixer ran, the round is not applied, and the PR gets a comment saying so. A removal that ends a pick (a push, a person's removal, an unknown reason) also ends its fixer history.
  • Errors after a re-enqueue: once a round has asked GitHub to take its head back, a collection error is retried on the next pass instead of becoming a handback; requeue is idempotent.

Picks: a pick is now the latest enqueue by anyone, the App included. The one exception is an App enqueue, since the pick began, of a head the watcher itself put back in the queue for that pick (requeuedHeads). That covers the watcher's own re-enqueue, GitHub's auto-merge of it, and Dependabot's workflow re-picking the fixer's push (the workflow fires on every synchronize). A handback ends the pick, so the App's next enqueue starts a new one.

Token permissions: with a fixer, the watcher's API token asks for merge_queues: write (granted to moq-bot 2026-11-02; enqueuePullRequest needs it), contents: write, pull_requests: write, and actions: read. Pushes ask for contents/pull_requests: write, plus workflows: write when the push carries a workflow change, as serve does. Without a fixer the token is unchanged.

Host command

Same user, state directory, and fixer launcher as moq's quest land serve, with no --sandbox:

quest land watch --app-id 1039466 --fixer <moq's fixer launcher client>

Choices made without asking

  1. Arm auto-merge instead of enqueueing while checks run. A pushed fix's own required checks are pending, and enqueuePullRequest is refused in that state, which is why quest merge arms instead. Bad picks now notes that these armed re-enqueues are among the picks GitHub may never queue.
  2. The quest-driver merge is recorded in Review as a mechanical fix by quest land watch, like a fixer's fix.
  3. A separate collection (land-watch-fix.ts) instead of extending collectJob. collectJob is built around the gate's parking, batch blame, base checks, and auto-merge pins. The watcher shares the per-PR publication steps (importFix, publishHead, launch, settle, comment), and everything gate-only can go with Retire.
  4. Running fixer builds below the queue's builds needs nothing here: merge_group builds get the landing class from moq-dev/quest#324, fixer builds use the default class, and the host already weights fixers at CPUWeight=20.
  5. The effort travels as an environment variable, because the nixos launcher passes only the log and the PR numbers. Until the launcher forwards it, quest land fix uses its medium default, which is the wanted value. host.md says so.

Validation

  • just check and just test pass on 388a0e53.
  • New test/land-watch.test.ts cases run against a git-backed fake upstream with real fixer wrapper processes. They cover:
    • fix, push, Review record, and re-enqueue, then a repeated failure handed back;
    • a quest-file conflict merged by the driver and armed, a code conflict fixed on the current base, and the --fixer-jobs cap holding a failed build back;
    • a Dependabot (App) pick re-enqueued flaky twice and handed back on the third, after which the App's next enqueue is a new pick;
    • a writer's push mid-round, a maintainer's re-enqueue dropped again mid-round, a retarget before and during a round, a fixer crash, and a fixer handback;
    • a person's removal ending the pick (even after a restart left a re-enqueue unrecorded), and a failed read and a refused outcome comment after a successful re-enqueue, both retried rather than handed back;
    • the requested token permissions.
  • Live, with every write intercepted: one pass against moq-dev/moq read real removals from 2026-10-29/30 and classified them. 7029 and 7135 had since been re-armed by the host gate, so they were correctly left alone (dropped … picked). Separately, the merge path on those PRs: 7135's conflict merges cleanly with the quest driver (moq-bot merge commit, parents head and base); 7029 has a real code conflict in py/moq-rs/moq/types.py and would go to the fixer.

Suggested follow-ups

  • kixelated/nixos: have the fixer launcher forward QUEST_LAND_FIXER_EFFORT as quest land fix --effort, so it can be set to something other than medium. This is part of host.md.

Review

Codex gpt-6.1-sol, medium effort, via quest review --base origin/main. Latest round: session 01a19aed, round 2, head 388a0e530b7cda4b452f58b4431671a2e9e0576a, 0 findings.

First session (01a19ab5), rounds 1–4, all findings fixed:

  • Round 1 (ef913d32), fixed in 31625bb6:
    • a superseded round could apply after a newer drop;
    • a crash or timeout handback recorded no round, so the pick stayed open.
  • Round 2 (31625bb6), fixed in 120db0d8:
    • a recorded re-enqueue ignored a later manual removal;
    • a pick-ending removal left its history live.
  • Round 3 (120db0d8), fixed in 5f9c3de4: a comment failure after a successful re-enqueue became a handback.
  • Round 4 (5f9c3de4), fixed in 518b9b44 after the round-4 checkpoint (maintainer: continue): a read failure right after a successful enqueue became a handback. Once a round has recorded requeuing, any collection error is now retried on the next pass instead.

Second session (01a19aed, --new after the checkpoint):

  • Round 1 (518b9b44), fixed in 388a0e53: a PR retargeted while it waited or while its fixer ran could get the old base's fix and re-enqueue. The base is now checked when the removal is taken, when the round is collected, and when it is re-enqueued.
  • Round 2 (388a0e53): no findings.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 5 commits November 1, 2026 20:22
…re-enqueues it

With --fixer, the watcher takes each removal recorded for the fixer, oldest
first. A conflict is first merged with the current base by the quest merge
driver; otherwise the land fixer runs in the background on the failure (at
most --fixer-jobs at once, with --fixer-effort as its reasoning effort). Each
round is recorded in its pick's history and the stop rules decide whether it
hands back; a fixed or merged head is validated, lease-pushed, and recorded
in Review, and every head that goes back is re-enqueued at the back of the
queue, or armed with auto-merge while its checks run.

App enqueues count as picks, as Dependabot's workflow makes them, except
those of heads the watcher itself put back in the queue for the pick.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…every handback ends its pick

A round whose PR was enqueued again and dropped again while the fixer ran is
not applied, and a handback from a crash, timeout, or invalid result records
a handback round, so the App's next enqueue of a head the pick put back
starts a new pick. The watcher's API token asks for merge_queues: write with
a fixer, which enqueuePullRequest needs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… a re-enqueued round is never redone

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…fail

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title The fixer runs on each dropped PR and the watcher re-enqueues it feat(land): quest land watch fixes each PR the merge queue drops and re-enqueues it Nov 2, 2026
@kixelated

Copy link
Copy Markdown
Collaborator Author

Design change: round-4 review checkpoint (head 5f9c3de4), decided 2026-11-02 by the maintainer through the coordinator ("go for it", proceeding on recommendations).

Prompt: review round 4 reported that a read failure right after a successful queueHead turns a round into a handback while the PR stays queued. Rounds 3 and 4 found the same class of edge: collectAll, following serve's collection, converts any collection error into a handback, even after GitHub may already have taken the head back.

  • ✅ Continue: once a round has recorded requeuing, a collection error is retried on the next pass instead of becoming a handback (requeue is idempotent), with a test; then review round 5 with quest review --new.
  • Merge as is
  • Rethink the approach

(Written by Claude Opus 5.5)

kixelated and others added 2 commits November 1, 2026 22:23
… on an error, never handed back

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 2, 2026 06:38
@coderabbitai

coderabbitai Bot commented Nov 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 16 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 47efb366-e6b0-48fe-aa5b-4833a553ec47
Commits

Reviewing files that changed from the base of the PR and between 6629545 and 388a0e5.

Files selected for processing: 16
  • README.md
  • quest/m0/land-fixer/README.md
  • quest/m0/land-fixer/pr-signal.md
  • quest/m0/queue-watcher/README.md
  • quest/m0/queue-watcher/bad-picks.md
  • quest/m0/queue-watcher/fix.md
  • quest/m0/queue-watcher/flaky.md
  • quest/m0/queue-watcher/host.md
  • src/cli/hold.ts
  • src/cli/land-fix.ts
  • src/cli/land-fixer.ts
  • src/cli/land-watch-fix.ts
  • src/cli/land-watch.ts
  • src/cli/main.ts
  • test/land-watch.test.ts
  • test/land.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@kixelated
kixelated enabled auto-merge (squash) November 2, 2026 06:49
@moq-bot
moq-bot Bot merged commit 085251c into main Nov 2, 2026
16 checks passed
@moq-bot
moq-bot Bot deleted the quest/m0/queue-watcher/fix branch November 2, 2026 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant