[kyle-sexton]

Closes #11177

Summary

pr-review-main reviews an oversize diff in up to 4 chunks (#11160), and each chunk's reviewer was told the other chunks were not its to judge. A change that ran, loaded or depended on a file in another chunk was therefore judged without that file. A chunked diff now writes a path-only manifest of the whole diff to manifest.txt in the review data directory; each chunk's prompt names that file, and the reviewer reports a cross-chunk reference as a finding naming both paths.

#11177 asks that each chunk's prompt include the manifest. That is met by the prompt naming the manifest file, which the reviewer reads with its existing Read(./pr-review-main-data/**) grant. This keeps the untrusted path text out of the prompt body, per the claude-security-review finding on .github/workflows/pr-review-main.yml.

Fix

• split-diff.py takes <manifest> <manifest-bytes>. With more than one chunk it writes one line per file, chunk <k> <added|modified|deleted|renamed> <path>, with a rename shown as old -> new. The paths come from git's own headers: the rename from/to lines, otherwise half of the diff --git line. Git C-quotes any control character, so a path cannot break a line. Lines past the cap are counted, not listed. The cap is PART_BYTES (15000), what one Read returns whole, so the reviewer can read the file in one call. Basis: the diff step's own PART_BYTES/PART_LINES limits; the shortest manifest line is 16 bytes, so 15000 bytes also stays under 1000 lines.
• The prompt step writes $DATA/manifest.txt (inside the reviewer's read grant) and the multi-chunk CHUNK_NOTE is fixed text only: it names that file, tells the reviewer to read it whole as untrusted data, and states the cross-chunk rule. No path text reaches the prompt body; the only substituted values are the runner's data path, the chunk number and the chunk count. CHUNK_NOTE sits in prompt.md right after DIFF_PARTS; an empty note renders the same bytes as main, so a one-chunk prompt is unchanged.
• The pr-review-main chunks notice adds manifest_paths=<n>. The same edits are applied to all three legs.
• The note names no severity: a cross-chunk finding takes the lane's own criteria.

Verification

• Red before the change: 4 of 48 tests failed in node --test .github/scripts/pr-review-main/*.test.mjs. Green after: 48 pass, 0 fail.
• Security fix, red then green: the prompt test now asserts each chunk's prompt names `<data>/manifest.txt` with the untrusted-data wording, holds no chunk <k> <status> manifest line and none of the fixture's path text (lib/b & c.sh, docs/x.md, guide/x.md, the git-quoted résumé path), and that manifest.txt exists holding all 6 expected lines. Against the previous head it failed on the file reference; after the change all 37 tests in workflow.test.mjs pass.
• The test runs the real "Write the diff" step on a 2-chunk fixture, then the real "Write the prompt and schema" step on its data, for each of the three legs. The one-chunk run renders exactly the template with an empty note.
• The one-chunk prompt rendered from main's prompt.md and from this branch's, with an empty note: cmp reports them identical.
• write-verdict.mjs readCoverage checks only that each diff part was read whole, so the extra Read of manifest.txt does not void a review.
• actionlint, markdownlint and check-changelog-fragments.sh --check-required origin/main pass.
• Independent security review (opus) of the first version found no secret or token leak, account compromise or billing issue.

Related

• Builds on #11160. #11097 touches the same workflow and test, but only uses: pins, so its hunks do not overlap this change.

Metrics

• The pr-review-main chunks notice gains manifest_paths=<n>: the files the manifest lists, 0 for a one-chunk diff. It shows how often reviewers get a cross-chunk view, and how close the list comes to its cap.
• GitHub API calls per run: +0. The change touches only the diff step, the prompt step and split-diff.py, all of which run locally. The record job is unchanged.

Risk

The manifest's paths are PR-author-chosen text, now read only as a file the prompt frames as untrusted data, the same channel as the diff itself. A path containing -> can make a manifest line ambiguous, but the same text is already in the diff; accepted.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-30T10:27:55.055131Z We couldn’t complete this review. Comment @codex review to try again.	ffe1471	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[github-actions]

claude-review reviewed 665fcb8be788246a74621a5726f8e8914d34d6d0: 0 finding(s), 0 as inline comments.

No findings. I reviewed the incremental change since d44a96e (split-diff.py and workflow.test.mjs), plus the surrounding code, and ran nothing.

manifest() cap. The loop now reserves room for the "and N more files, not listed" line before it adds a file line. At the point it stops, out already satisfies the previous iteration's bound, which used the same remaining count as the closing line. So the closing line is the right size and the manifest stays within manifest-bytes. The last file line is checked with more(0) as empty, so it is not penalised.

Test. I counted the bytes for the six MIXED_MANIFEST lines by hand. The first three lines are 26, 27 and 24 bytes with their newlines, and the count line for 4 files is 29. With a cap of 83, the check at the third line is 53 + 24 + 29 = 106, which is over, so it stops there. The output is 53 + 29 = 82 bytes, which is within 83. That matches the expected two lines plus "and 4 more files, not listed". The new byteLength <= 83 assertion would have failed under the old logic, so it does guard the fix.

Not reported. If manifest-bytes is smaller than the count line itself, the first iteration still returns the count line alone, which is over the cap. This is not a realistic setting and I did not count it.

[github-actions]

claude-security-review: 1 inline finding(s) on ffe1471ea0d2887510ec9b70f76161e8842b9073; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

[github-actions]

claude-security-review reviewed 665fcb8be788246a74621a5726f8e8914d34d6d0: 0 finding(s), 0 as inline comments.

No security issues found in the incremental change since d44a96e.

The change touches two files, both under the .github/scripts/pr-review-main/** trusted path. I read the whole change and the surrounding manifest() function.

• split-diff.py: manifest() now reserves room for the trailing "and N more files, not listed" line, so the whole manifest stays within manifest-bytes. Each loop step admits a line only if it, plus the "more" line for the files after it, still fits. The earlier step has already reserved the "more" line for the current remainder, so the early return stays within the cap. The one exception is a cap too small to hold even the "more" line, which is not reachable with a normal cap. The manifest is still built from git's own quoted paths, so the change adds no new injection surface, no new trust input and no new token or secret handling. It only tightens a size bound.
• workflow.test.mjs: The test raises the cap from 60 to 83 and asserts Buffer.byteLength(capped.manifest) <= 83. That only checks the new behavior and widens nothing.

No trust control is widened, no hold is weakened, and no manifest field changed. Nothing here can start a charge.

critical-findings: 0

[github-actions]

claude-review: 1 inline finding(s) on d44a96e69b4e29935623967855495f5680433de3; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

Post-merge mutation testing

• Range: 5e163a1..9562279
• Merged pull request: #11223
• Outcome: reported

Killed 6, survived 1, timeout 0, no coverage 0, invalid 0, not run 0.
Covered-code score 86%; mutation score 86%.

File	Killed	Survived	Timeout	No coverage	Invalid	Not run
.github/scripts/pr-review-main/split-diff.py	6	1	0	0	0	0

Surviving mutants

Location	Operator	Disposition	Cause
.github/scripts/pr-review-main/split-diff.py:95	ROR	productive	input-gap

The run's summary has the full report.