Repository navigation
Facts port: the snapshot's diff and files read by one compare pinned to the head commit (#284) - #303
Conversation
… the head
pull.diff and pull.changed_paths read GET /repos/{repo}/compare/{base_head}...{head}
at the head of the snapshot's first read and the base branch's head the pass read,
never off the pull request: a head pushed away and back during the snapshot gives
the parts of one commit. P-29 shows the compare's diff bytes equal the pull
request's own, so the content id is unchanged. A compare lists at most 300 files,
so a change of more is read short and deferred (the working ruling on #284).
Tests show a diff or a file list read at another commit is never asked for, a
base head that is not a commit is refused, and more than 300 files reads short;
each has a mutant. The design's section 3.3 rows and a row on decision 0020 say so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… its count alone The fresh review found the contract check for a_path_list_can_leave_out_a_file read made-up rows, and that a change of more than 300 files could be refused, not deferred, by any answer the compare gave. Such a change is now read short by changed_files alone, with nothing asked, which the fixture's 3,001 files ground; the 300 is marked as GitHub documents it, unverified, in the port, the design and decision 0020's row. The Router's docstring says what it now does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…no files is refused Muse on #284's first head: neither had a test a mutant would fail. Each now has both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…into adapters/pin-diff-to-head
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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 21 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 |
…he head and #296's closing read Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A snapshot's diff and file list now come from one compare, pinned to the head of the snapshot's first read. A head pushed away and back while the snapshot is read no longer pairs one commit's content with another head. This is the working ruling on #284, closing the hole the security pass on #278 found.
What changes
adapters/pull.py.diffreadsGET /repos/{base repo}/compare/{base_head}...{head}with the diff Accept. That is the readtimeline._contentalready makes for a review's commit, so the two content ids are made the same way. P-29 shows the bytes equal the pull request's own diff.changed_pathsreads the same compare as JSON and takesfiles.404refuses the snapshot.406 too_largereads as truncated, as before. That is grounded for the pull request's diff, not for compare's, and the design says so.changed_filesalone, with nothing asked, so no answer GitHub gives can refuse the snapshot. The Kernel defers it for the Maintainer.adapters/facts.py. The snapshot passes the base branch's head it already read, and the known-limit sentence is gone.content_idandchanged_pathssay so, and so does a new Boundaries row on decision 0020.Boundaries, each with a mutant:
files, orfilesthat are not a list, is refused.Tests. The port tests replay P-29's recorded diff and files answers as compare reads at the head (
as_compare). The diff bytes are P-29's own; the file rows have the same keys a compare gives.Not done. The fresh review raised a hardening question: could GitHub resolve a 40-hex branch name ahead of the commit? That is unverified, and
timeline._contenthas the same exposure today. It is left for a trial; this PR does not change it.Reviews, all on 1638f8e, with the fix rounds after.
a_path_list_can_leave_out_a_fileread made-up rows. Minors: the 300 was not marked unverified; a change of more than 300 files could be refused rather than deferred; the Router's docstring was stale. All are answered in 7b2293a.files. Both have a test and a mutant in ca40688.make verifypasses on a fresh export of the head.Closes #284
🤖 Generated with Claude Code