A box pass on an already merged pull reviews the merge's own diff, never “(no diff available)” #8766

A box pass on a pull that already landed now reviews the merge commit's own diff, so a reviewer is never sent to tick boxes against "(no diff available)".

• Where the diff comes from: a new module, src/crew/review_range.ts, picks the two commits to diff.

- Landed pull: the merge commit on the base against its first parent. refs/pull/N/merged keeps that commit, so it is always there. A squash has one parent and is read the same way. A merge commit's first parent is the base, so only the pull's own change shows. - Open pull: unchanged in effect — the head against its merge base with the live base branch. - Pull that closed as carried (#2036): the commit it records is not its own. This is detected by checking whether the merge commit's first parent equals the pull's recorded base_oid, so rows from before the carried column also work. Those pulls are read from their head.

• The brief: reviewInputs in src/crew/runner.ts diffs that range. A git failure now leaves "(no diff available)" instead of an empty diff. It also returns landedAs, and src/crew/brief.ts uses it to label the diff "as it landed: commit X … against its first parent".
• Nothing to diff: passLanded in src/crew/box_pass.ts is now async and checks for a range before queuing the review. If there is none, no reviewer is sent and the issue stays held. One note from the crew reviewer says why (No box pass for pull #N: …), and later ticks don't repeat it. autonomy.ts awaits the call.
• Docs: docs/features/box-pass.md says what a landed pull is read from.

What to look at: deleting the head branch alone doesn't lose anything, because refs/pull/N/head keeps the head reachable. The objects only go when that ref is deleted too. The new tests in test/box-pass.test.ts do that, then expire the reflog and run git gc --prune=now, and check the head commit is really gone:

• Squash-merged pull, head gone: the pass sends the reviewer, and the brief holds the real diff (+++ b/gone.txt, +x), not "(no diff available)".
• Merge-strategy pull, base moved underneath: the diff holds the pull's file and not the base's own new file. Its head is the merge commit's second parent and can't be pruned, so this test doesn't prune.
• No merge commit and no head: nobody is sent, the issue stays held, and there is exactly one note, by the reviewer, even after two passes.

Each of these fails on the old code. The existing passLanded tests now await it.

Checks run: bun scripts/gate.ts and bun x tsc --noEmit are clean. scripts/fast.ts named 1357 files (runner.ts reaches almost everything), so I picked 11 covering suites by hand: box-pass, landed-held, autonomy, autonomy_sweep, crew-review, crew-review-rounds, the crew-brief suites (docs, gate, foreign-fence), review-design and review-from-check. All 67 tests pass. The full suite was left to the forge's check.

🤖 Generated with Claude Code

Closes #8739

A box pass on a pull that already landed now reviews the merge commit's own diff, so a reviewer is never sent to tick boxes against "(no diff available)".

• Where the diff comes from: a new module, src/crew/review_range.ts, picks the two commits to diff.

- Landed pull: the merge commit on the base against its first parent. refs/pull/N/merged keeps that commit, so it is always there. A squash has one parent and is read the same way. A merge commit's first parent is the base, so only the pull's own change shows. - Open pull: unchanged in effect — the head against its merge base with the live base branch. - Pull that closed as carried (#2036): the commit it records is not its own. This is detected by checking whether the merge commit's first parent equals the pull's recorded base_oid, so rows from before the carried column also work. Those pulls are read from their head.

• The brief: reviewInputs in src/crew/runner.ts diffs that range. A git failure now leaves "(no diff available)" instead of an empty diff. It also returns landedAs, and src/crew/brief.ts uses it to label the diff "as it landed: commit X … against its first parent".
• Nothing to diff: passLanded in src/crew/box_pass.ts is now async and checks for a range before queuing the review. If there is none, no reviewer is sent and the issue stays held. One note from the crew reviewer says why (No box pass for pull #N: …), and later ticks don't repeat it. autonomy.ts awaits the call.
• Docs: docs/features/box-pass.md says what a landed pull is read from.

What to look at: deleting the head branch alone doesn't lose anything, because refs/pull/N/head keeps the head reachable. The objects only go when that ref is deleted too. The new tests in test/box-pass.test.ts do that, then expire the reflog and run git gc --prune=now, and check the head commit is really gone:

• Squash-merged pull, head gone: the pass sends the reviewer, and the brief holds the real diff (+++ b/gone.txt, +x), not "(no diff available)".
• Merge-strategy pull, base moved underneath: the diff holds the pull's file and not the base's own new file. Its head is the merge commit's second parent and can't be pruned, so this test doesn't prune.
• No merge commit and no head: nobody is sent, the issue stays held, and there is exactly one note, by the reviewer, even after two passes.

Each of these fails on the old code. The existing passLanded tests now await it.

Checks run: bun scripts/gate.ts and bun x tsc --noEmit are clean. scripts/fast.ts named 1357 files (runner.ts reaches almost everything), so I picked 11 covering suites by hand: box-pass, landed-held, autonomy, autonomy_sweep, crew-review, crew-review-rounds, the crew-brief suites (docs, gate, foreign-fence), review-design and review-from-check. All 67 tests pass. The full suite was left to the forge's check.

🤖 Generated with Claude Code

A box pass on a pull that already landed now reviews the merge commit's own diff, so the reviewer is never sent to tick boxes against "(no diff available)". This branch now has main merged in, with both of its conflicts resolved.

Where the diff comes from: a new module, src/crew/review_range.ts, picks the two commits to diff.

• A landed pull is read as its merge commit on the base against that commit's first parent. refs/pull/N/merged keeps the merge commit, so it is there even when the head's refs and objects are gone. A squash has one parent and is read the same way.
• An open pull is read as before: its head against the merge base with the live base branch.
• A pull that closed as carried (#2036) records a commit that is not its own. This is spotted by checking whether the merge commit's first parent equals the pull's recorded base_oid, which also works for rows from before the carried column. Those pulls are read from their head.

reviewInputs in src/crew/runner.ts diffs that range and returns landedAs. src/crew/brief.ts then labels the diff "as it landed: commit X … against its first parent".

When nothing can be read: passLanded in src/crew/box_pass.ts is now async and checks for a range before queuing the review. If there is none, no reviewer is sent and the issue stays held. The crew reviewer leaves one note saying why (No box pass for pull #N: …), and later ticks don't repeat it. docs/features/box-pass.md describes this.

The merge with main: passLanded keeps both changes. The reviewer's slot limit from #8734 (room from reviewerSlots and reviewsInFlight) still bounds how many reviews it sends, and the no-diff check is still there. In test/box-pass.test.ts, main's slot-limit test now awaits passLanded, alongside this branch's tests.

What to look at: deleting the head branch alone loses nothing, because refs/pull/N/head keeps the head alive. The new tests delete that ref as well, expire the reflog and run git gc --prune=now, then check the head commit is really gone. They cover:

• a squash-merged pull whose head is gone: the brief holds the real diff;
• a merge-commit pull whose base moved underneath it: the diff shows the pull's file and not the base's;
• a pull with no merge commit and no head: nobody is sent, the issue stays held, and there is exactly one note.

Each of these fails on the old code.

Checks run after the merge: bun scripts/gate.ts and bun x tsc --noEmit are clean. scripts/fast.ts named 1357 test files, too many to run here, so I picked 9 covering suites by hand: box-pass, landed-held, autonomy, autonomy_sweep, crew-review, review-load, crew-brief-docs, review-design and review-from-check. All 65 tests pass. The full suite was left to the forge's check.

🤖 Generated with Claude Code