Skip to content

quest land reports head_moved only when a writer moved the head - #321

Merged
moq-bot[bot] merged 5 commits into
mainfrom
quest/m0/land-head-moved
Nov 1, 2026
Merged

moq-bot[bot] merged 5 commits into
mainfrom
quest/m0/land-head-moved

Conversation

@kixelated

@kixelated kixelated commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Completes and deletes quest/m0/land-head-moved.md.

The case the record missed

The host gate's journal and private state on nixos show one event behind all four PRs the quest cites. At 2026-10-31T03:04Z the batch moq-dev/moq#7406, #7407, #7413, #7416, #7353 passed its check, and #7406 landed. For #7407, prepareHead pushed the base merge 70bb5275 as moq-bot at 03:04:34Z. The PR's timeline shows no other push. The merge was ejected 5 seconds later with this detail:

GitHub refused /repos/moq-dev/moq/pulls/7407: Pull Request has merge conflicts

GitHub had not caught up with the push. It refused the merge with a 405 "merge conflicts", judged on the old pin, which conflicts with the new base. When the gate re-read the PR, head.sha still showed the pin. mergePinned treated the PR as "not yet propagated" only after a 409, so this case fell through to head.sha !== head and was reported as head_moved. The private record and forgetHeads were fine: the record was never consulted at this step. #7413, #7416 and #7353 were the batch-mates, and they were all given #7407's reason.

Changes

  • mergePinned (src/cli/land-serve.ts): right after the gate's own push, if GitHub still shows the pin as the head, it retries with the existing backoff whatever the refusal status. The merge stays pinned to the prepared SHA, so a retry can never merge another head. If GitHub has not caught up after three retries, it throws as before. The daemon then retries the whole round, and on that round isPrepared accepts the recorded head. Only another writer's head is reported as head_moved.
  • serveBatch: a tree mismatch now affects only the mismatched PR, as main (moq-dev/quest#355) already does after conflicts and moved heads:
    • A prepared tree that differs from the tested one logs CRITICAL and ejects only that PR as tree_mismatch.
    • A GitHub squash whose tree differs logs CRITICAL. That PR is already merged, so nothing is ejected.
    • In both cases the rest of the batch stays queued without a comment. The next batch rebuilds and checks it on the current base, which includes the mismatched squash in the second case.
  • The ejection detail now says "the rest of the batch stays queued", not "was not merged".
  • README, CONTRIBUTING, and the quest land serve help text describe the retry and the per-PR tree mismatch. CONTRIBUTING's line that conflicts, moved heads, and tree mismatches "eject the remaining batch" was out of date on main too.
  • Tests:
    • A regression test for #7407: GitHub refuses with a 405 conflict while it lags the gate's push, and a retry then merges.
    • The post-squash tree-mismatch test now expects the batch-mates to stay queued with no comments. A second round then lands them on the mismatched squash, and the sandbox receives that squash as its base.
    • A new prepared-tree-mismatch test switches the merge driver after the check. PR 2 is ejected alone as tree_mismatch. PR 3 stays queued, and the next round leaves PR 2 out and lands the rest on PR 1's squash.

Decisions

  • Maintainer decision, 2026-11-01 (comment): after a tree_mismatch, only the mismatched PR leaves the queue, and its batch-mates stay queued. This replaces the earlier batch_stopped reason, which this PR no longer adds. It also supersedes the quest Goal's "naming the batch-mate" clause: batch-mates are no longer ejected at all, so none of them inherits another PR's reason.

Checks

On 8632fe3, with CLAUDE_CONFIG_DIR unset: just check passed. just test passed: 764 passed and 2 skipped, plus 426 Worker tests.

Suggested follow-ups

  • Stale mergeable: false on the prepared head itself, where GitHub has the new head but its conflict flag still comes from the old one. This is out of scope here and belongs to quest/m0/land-mergeability.md.
  • A post-squash tree mismatch leaves no comment on the merged PR, only the host log. This was already true on main. Whether to post one is a separate choice.

Review

  • Reviewer: Codex gpt-6.1-sol, medium effort, via quest review --base origin/main --new. This was a new session after the design change, round 1.
  • Head reviewed: 8632fe3, against origin/main 1733726
  • Findings: none

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 2 commits October 31, 2026 08:11
Right after the gate pushes its base merge, GitHub can still report the
pin as the PR's head and refuse the merge for it with a 405 "merge
conflicts", since the pin conflicts with the base that push merged.
mergePinned retried only a 409 there, so moq-dev/moq#7407 was ejected as
head_moved after the gate's own push. Retry any refusal while GitHub
still shows the pin.

When one PR stops a batch, the later PRs in it are now ejected as
batch_stopped, with a comment naming that PR, instead of inheriting its
reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review October 31, 2026 15:33
@coderabbitai

coderabbitai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

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: d66435d7-0cc3-4820-93bc-916f7d282968
  • 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

@moq-bot

moq-bot Bot commented Nov 1, 2026

Copy link
Copy Markdown

quest land removed this pull request from the queue: Conflicts with the base: README.md, src/cli/land-serve.ts, src/cli/main.ts, test/land.test.ts
.

@moq-bot

moq-bot Bot commented Nov 1, 2026

Copy link
Copy Markdown

codex / Codex land fixer: Failure log: 'Conflicts with the base: README.md, src/cli/land-serve.ts, src/cli/main.ts, test/land.test.ts'. PR 321 ejects later PRs as batch_stopped, while base explicitly keeps them queued and stop. Recommendation: Rebase with author review: retain base's per-PR parking/ejection policy, keep the stale prepared-head retry, and decide where batch_stopped remains appropriate (such as tree mismatches); update docume

  			for (let i = 1; i <= 4; i++) forge.pull(`PR ${i}`, { [`${i}.txt`]: "yes" });
@@@ -1177,30 -1520,29 +1556,45 @@@ mv '${result}.new' '${result}
  `,
  				{ mode: 0o755 },
  			);
- 			let watcher: ReturnType<typeof watch>;
- 			const launched = new Promise<void>((done) => {
- 				watcher = watch(options.state, (_event, name) => {
- 					if (name === "fixer-result" && existsSync(result)) done();
- 				});
- 			});
+ 			const launched = eventFile(options.state, (name) => name === "fixer-result");
  			try {
  				const output = await serveBatch(forge.clone(), plan, options, deps, async () => deps.token);
++<<<<<<< HEAD
 +				await launched;
 +				expect(output).toEqual([
 +					`landed 1 ${forge.repo.pulls[0].squash}`,
 +					`ejected 2 ${reason}`,
 +					"ejected 3 batch_stopped",
 +				]);
++=======
++>>>>>>> base
  				expect(forge.merges.map(({ number }) => number)).toEqual([1, 2]);
- 				expect(readFileSync(result, "utf8").trim().split("\n").slice(1)).toEqual(["2", "3"]);
  				expect(forge.repo.pulls[0].merged).toBe(true);
+ 				// PR 3 passed the check with it: it stays queued, without a comment, to be rebuilt on the new base.
+ 				expect(forge.repo.pulls[2].landing).toBe(true);
+ 				expect(forge.comment(3)).toBeUndefined();
  				expect(forge.repo.pulls[3].landing).toBe(true);
++<<<<<<< HEAD
 +				expect(forge.comment(2)).toContain(`<!-- quest land: ${reason} -->`);
 +				// The batch-mate names the PR that stopped the batch instead of inheriting its reason.
 +				expect(forge.comment(3)).toContain(`PR #2 stopped the batch as ${reason}`);
 +				expect(forge.comment(3)).toContain("<!-- quest land: batch_stopped -->");
++=======
+ 				if (reason === "merge_conflict") {
+ 					// A conflict parks the PR for the fixer, keeping its auto-merge.
+ 					expect(output).toEqual([`landed 1 ${forge.repo.pulls[0].squash}`, "parked 2"]);
+ 					await launched;
+ 					expect(readFileSync(result, "utf8").trim().split("\n").slice(1)).toEqual(["2"]);
+ 					expect(forge.repo.pulls[1].landing).toBe(true);
+ 				} else {
+ 					// A writer moved the head, which no fixer repairs.
+ 					expect(output).toEqual([`landed 1 ${forge.repo.pulls[0].squash}`, `ejected 2 ${reason}`]);
+ 					expect(forge.comment(2)).toContain(`<!-- quest land: ${reason} -->`);
+ 					launched.cancel();
+ 				}
++>>>>>>> base
  			} finally {
- 				watcher!.close();
+ 				launched.cancel();
  			}
  		},
  	);
src/cli/main.ts:555:<<<<<<< HEAD
src/cli/main.ts:557:=======
src/cli/main.ts:559:>>>>>>> base
test/land.test.ts:1533:<<<<<<< HEAD
test/land.test.ts:1535:=======
test/land.test.ts:1537:>>>>>>> base
test/land.test.ts:1562:<<<<<<< HEAD
test/land.test.ts:1569:=======
test/land.test.ts:1570:>>>>>>> base
test/land.test.ts:1577:<<<<<<< HEAD
test/land.test.ts:1582:=======
test/land.test.ts:1595:>>>>>>> base
README.md:645:<<<<<<< HEAD
README.md:650:=======
README.md:653:>>>>>>> base
src/cli/land-serve.ts:658:<<<<<<< HEAD
src/cli/land-serve.ts:675:=======
src/cli/land-serve.ts:683:>>>>>>> base
/nix/store/216mr46yy8xspaydgapjhf4a21y643ji-bun-1.4.2/bin/bun

codex
The conflict involves competing behavior: PR 321 ejects later PRs as `batch_stopped`, while base keeps those PRs queued and stops only the failing PR. Resolving that requires a behavior decision, so I’ll hand it back with a diagnosis and leave the branch unchanged.
exec
/bin/bash -lc "git merge --abort; python3 - <<'PY'
import json
result = [{\"number\":321,\"action\":\"handback\",\"diagnosis\":\"Failure log: 'Conflicts with the base: README.md, src/cli/land-serve.ts, src/cli/main.ts, test/land.test.ts'. PR 321 ejects later PRs as batch_stopped, while base explicitly keeps them queued and stops only the failing PR; resolving these conflicts requires a behavior decision rather than a mechanical merge.\",\"recommendation\":\"Rebase with author review: retain base's per-PR parking/ejection policy, keep the stale prepared-head retry, and decide where batch_stopped remains appropriate (such as tree mismatches); update documentation and tests accordingly.\"}]
with open('quest-land-result.json','w') as f:
    json.dump(result,f,indent=2)
    f.write('\\n')
PY
 git status --short; git rev-parse pr-321; git diff --stat" in /work
 succeeded in 0ms:
?? quest-land-result.json
8be9669164a8f498cc70cc8e883c108c67e1bc12

codex
PR 321 handed back: its batch ejection behavior conflicts with base’s policy of keeping later PRs queued. Resolving this requires author review.

Branch unchanged. Diagnosis and recommendation saved in [quest-land-result.json](/work/quest-land-result.json), uncommitted. No build or tests run.
tokens used
20,772
PR 321 handed back: its batch ejection behavior conflicts with base’s policy of keeping later PRs queued. Resolving this requires author review.

Branch unchanged. Diagnosis and recommendation saved in [quest-land-result.json](/work/quest-land-result.json), uncommitted. No build or tests run.
fixer result written

(written by codex / Codex)

Keep main's per-PR parking and ejection; batch_stopped now names the PR
whose tree mismatch stopped the batch for its later batch-mates.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated enabled auto-merge (squash) November 1, 2026 21:22
@kixelated
kixelated disabled auto-merge November 1, 2026 21:52
@kixelated

Copy link
Copy Markdown
Collaborator Author

I've taken this PR out of the queue for one more change. On 2026-11-01 the maintainer decided that after a tree_mismatch, only the mismatched PR is ejected and its batch-mates stay queued for the next batch. That matches main's policy after conflicts and moved heads, and the land-fixer goal of dequeuing a PR only when a decision is needed. So batch_stopped goes away.

(Written by Claude Opus 5.5)

A maintainer decided (2026-11-01) that a tree mismatch, prepared or after
GitHub's squash, ejects only the mismatched PR, as tree_mismatch. Its
batch-mates stay queued and the next batch rebuilds and checks them on the
current base, as after conflicts and moved heads. Drop the batch_stopped
reason and its /quest-complete line.

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

Copy link
Copy Markdown
Collaborator Author

Design change: per the maintainer's 2026-11-01 decision above, a tree_mismatch (prepared or after GitHub's squash) ejects only the mismatched PR, and its batch-mates stay queued to be rebuilt and checked on the current base by the next batch. batch_stopped is removed from the code, tests, README, quest land serve --help, and /quest-complete. After a mismatch in GitHub's squash, the PR is already merged, so nothing is ejected and only the CRITICAL log line remains. The review restarts with quest review --new.

(Written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge (squash) November 1, 2026 22:00
@moq-bot
moq-bot Bot merged commit ba4e938 into main Nov 1, 2026
@moq-bot
moq-bot Bot deleted the quest/m0/land-head-moved branch November 1, 2026 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant