Repository navigation
Fix the pull request preview listing trunk's files when the site's trunk is behind - #702
Merged
ryanwelcher merged 2 commits intoOct 20, 2026
Conversation
A site whose trunk is behind the commit a pull request branched from listed every file trunk changed in between as the pull request's. The preview now fetches origin's trunk first and measures from that commit, falling back to the site's trunk when it cannot be fetched. Fixes #701.
When origin's trunk cannot be fetched, the preview falls back to the site's own trunk, and the list can carry trunk's changes again. The handler now reports that fallback and the preview dialog adds a sentence saying so. Also corrects the fetchOriginTrunk comment, which said the list was measured against the trunk the pull request was written on.
|
Warning Review limit reachedYou'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 57 minutes. View limit details
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. Comment |
Collaborator
Author
|
@coderabbitai review |
|
ryanwelcher
deleted the
ryanwelcher/a-pull-requests-file-list-includes-trunk-commits
branch
October 20, 2026 14:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
When a site's local
trunkis older than the commit a pull request branched from, the pull request preview lists every file trunk changed in between as well as the pull request's own. @zaerl hit this on #677: WordPress/wordpress-develop#14541 changes 2 files, and the app listed dozens.What changes
The root cause:
describePullRequestHeadmeasured the list from the merge base with the site's localtrunk, which only moves when the contributor updates trunk.git:preview-prhandler now fetches origin'strunkbefore the pull request head and measures the list from that commit. It reads the commit fromFETCH_HEAD, so it works on a site with no remote-tracking ref. The site's owntrunkand the checkout are not moved.Deliberately not in this PR: the checkout itself and the recorded base of
pr/Nare unchanged. Only the preview list was wrong.How to test this
Platforms: any. Use the Buildkite build for the current head,
0f91e22.Starting state: a wordpress-develop site on
trunkwith no edits, whose trunk is older than WordPress/wordpress-develop#14541. To make one, quit the app and run this from the site's folder (needs Git on your machine):Write down the commit it prints, or run
git rev-parse trunkand keep the output.14541in Pull request URL or number and click Apply PR.git rev-parse trunk.To see the bug, run the same steps on a trunk build: step 2 says "changes" followed by a much larger number of files.
What must not have happened:
trunkmoved (step 4). The preview downloads origin's trunk but must not update the site's own.npm installstarted, when you only opened the preview and cancelled.Tests: the two
git:preview-prtests intests/unit/ipc-wiring.test.cjsfail on the old handler. The trunk-download failure case is covered by those tests and by the newdescribePrPreviewtest intests/unit/pr-checkout-renderer.test.cjs.Risks and limitations
describePullRequestHead.Related
Fixes #701. Reported on #677.
Design decisions and alternatives considered
pr/N's base from the fetched head and does not use the list, so the bug was confined to the preview.FETCH_HEADreader ingit-read.cjs. Not worth it for now.Review outcome (required — see AGENTS.md)
0 [fix here] · 1 [follow-up]. The follow-up is fixed in
0f91e22.measuredFromSite, and the dialog shows a sentence when it is true.fetchOriginTrunkcomment said the list was measured against "the trunk it was written on". It now says origin's current trunk.FETCH_HEAD; the partial clone fetches no file contents; the wiring tests fail without the fix; bundled Git only, the quit sweep covers the new fetch, no store writes.Records:
.github/instructions/code-review.instructions.md. Reviewed head991f291/ basee43e9c8. Lint clean, unit suite 2113 pass / 0 fail, PR journeys (pr-checkout,apply-card,patch-apply,gutenberg-site) 17 passed. Outcome: 1 follow-up.991f291 → 0f91e22checked. This commit is the follow-up fix and the comment correction. Lint clean, unit suite 2114 pass / 0 fail, the same 17 journeys pass. Not re-reviewed by a separate context.Implementation notes
fetchOriginTrunkinsrc/pr-checkout.jsisfetchBranch(dir, origin, trunk). It goes throughstreamGiton the bundled binary and is registered withtrackGitChild, so quitting ends it.measuredFromSitetravels in the preview result, whichuse-apply-patch.jsxalready spreads intoapplyPreview.describePrPreviewturns it intotrunkNote, andpreviewWordspasses that to the dialog.Screenshots or recording
None. The only visible change is the fallback sentence, which can't be staged by hand (see Risks). The file count in step 2 is the before and after.
🤖 Generated with Claude Code