[kixelated]

Implements and deletes quest/m0/review-merge-base.md. quest review now reviews only the branch's own change, even when the branch is behind its base. It also reviews a detached HEAD.

Summary

• Root cause. In moq-dev/quest#337, Codex's own exec review --base origin/main turn was correct: it was told the merge base and ran git diff <merge-base>. The bad findings came from the follow-up findings turn, which quest review resumes. That prompt named only the head, and the developer instructions called main's tip "the base commit". So Codex ran a two-dot git diff <main tip> <head> and reported main's newer land code as reverted (session 01a198ee-…).
• Fix (src/cli/review.ts).
  • review() computes the merge base of HEAD with the fetched base tip and passes it to every turn.
    • Codex runs exec review --base <merge-base>. Given a commit, Codex's merge base with HEAD is that commit, and its prompt names it.
    • Claude Code runs /code-review <effort> <merge-base>...HEAD.
  • The findings prompt and every later-round prompt now open with the range: `git diff <merge-base>..<head>`. They also say the base's newer commits are not part of the change.
  • The instructions and REVIEW.md preambles now call the base's tip "the base branch's tip", not "the base commit".
  • The list of changed paths that picks instruction files also uses the merge base. This only matters when the base moves during a round.
• Detached HEAD. --branch <name> keys the review session by that branch instead of the checked-out one, the same way quest check --branch does. A detached HEAD requires it, and the error now says so. The name is checked with git check-ref-format --branch. So a checkout of a branch that another worktree holds, the moq-dev/quest#348 case, continues that branch's session. So does a differently named local branch.
• Docs. The quest review --help text and the README "Review locally" section are updated. /quest-review (assets/skills/review.md) now says to use a detached checkout plus --branch when another worktree holds the PR's head branch.
• Tests (test/review.test.ts).
  • A branch behind its base, for both reviewers: the first turn gets the merge base, the follow-up names the merge-base range, and the base's newer docs/ change brings no docs/AGENTS.md.
  • A detached checkout with --branch resumes the branch's session in round 2. A differently named branch with --branch continues it in round 3, and without --branch starts its own.
  • The detached error and an invalid --branch name.

Choices made without asking

• A detached HEAD names its session with --branch; there is no automatic key. The quest suggested "by the PR or by a name passed on the command line". Finding the PR from a merge commit's HEAD would need the forge and guesswork, while --branch matches quest check --branch and needs no network. Without it, a detached HEAD is a usage error that names the flag.
• The merge base is pinned once per round as a SHA, not passed as <remote>/<base>...HEAD. So a concurrent fetch in another worktree cannot move the range mid-round, and both reviewers get the same commit.
• The output is unchanged. base still prints the base's tip, and the merge base is not printed.

Checks

At 1fd183d: just check passes, just test passes (939 passed, 2 skipped), and quest check --base origin/main is ok.

Review

mechanical fix by quest land watch: merged main at fc7bab34c2a56e2620f9a6b9e7da8db5d3d377b3 with the quest merge driver (head dacb855)

mechanical fix by codex / Codex: Merged base, preserving the exported Context and PR mergeBase field; renamed the fixture field; passed bun run build, bun run check, and bun run test (979 passed, 2 skipped). (head 38415c8)

mechanical fix by codex / Codex: Merged base and re·solved ReviewOptions conflict by preserving both branch and signal; bun run build, bun run check, and bun run test passed (962 tests passed, 2 skipped). (head 4ec9d3a)

Codex (quest review, run from this branch, so it exercised the fix live), gpt-6.1-sol, medium effort, session 01a19f5e-8003-78d2-9903-c027cb06d57d, against origin/main 91058d2. Reviewed head 1fd183d.

• Round 1: no findings. The session log shows Codex's review turn was given the merge-base SHA and its findings turn ran git diff <merge-base>..<head>.

Suggested follow-ups

None.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[coderabbitai]

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: a4a5a032-37d5-425e-a69b-f268efccdc5f

• 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

• X
• Mastodon
• Reddit
• LinkedIn

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

[moq-bot]

quest land watch: the merge queue removed this pull request at 1fd183d because it conflicts with main (merge_conflict).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

(written by quest land watch)

[moq-bot]

quest land watch: armed auto-merge on this pull request at 4ec9d3a, so it joins the back of the merge queue once its required checks pass, with the land fixer's fix (codex / Codex): Merged base and re·solved ReviewOptions conflict by preserving both branch and signal; bun run build, bun run check, and bun run test passed (962 tests passed, 2 skipped)..

(written by quest land watch)

[moq-bot]

quest land watch: the merge queue removed this pull request at 4ec9d3a because it conflicts with main (merge_conflict).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

(written by quest land watch)

[moq-bot]

quest land watch: armed auto-merge on this pull request at 38415c8, so it joins the back of the merge queue once its required checks pass, with the land fixer's fix (codex / Codex): Merged base, preserving the exported Context and PR mergeBase field; renamed the fixture field; passed bun run build, bun run check, and bun run test (979 passed, 2 skipped)..

(written by quest land watch)

[moq-bot]

quest land watch: the merge queue removed this pull request at 38415c8 because it conflicts with main (merge_conflict).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

(written by quest land watch)

[moq-bot]

quest land watch: armed auto-merge on this pull request at dacb855, so it joins the back of the merge queue once its required checks pass, with main at fc7bab3 merged in by the quest merge driver.

(written by quest land watch)