[kyle-sexton]

No related issue: lane defect fix (the shared read-patch collector wrote exact moves as delete plus add and exceeded the patch cap), found while the issue lane worked #11124; it delivers none of #11124's done-when items, which stay with the issue lane.

Summary

The read-patch collector ran git diff --no-renames, so a pure move became a full deletion plus a full addition and went over the 262144-byte patch cap in pr-run-activity-read.yml. Issue #11124 moves the 422 KB morning-brief skill, and that move was refused three times. The collector now writes exact renames as rename headers. This collector is shared by every patch-mode row, not only the issue lane.

Fix

• .github/scripts/read-patch/collect-patch.sh passes -M100% in place of --no-renames, the same form pr-review-main/write-diff.py uses (fix(pr-pipeline): record the main-run review's size stop as an outcome and collapse exact renames #10961, feat(pr-pipeline): review an oversize main-run diff in chunks #11160).
  • An exact move (100% similarity) becomes a rename header with no content.
  • A rename with edits is still a deletion and an addition.
• apply-patch.mjs needs no change:
  • It applies the patch with git apply --cached, which accepts rename headers.
  • It then checks the tree the patch builds with diff-tree --no-renames. A rename's old path therefore meets every refusal as a deletion, and its new path as an addition. Both also meet the allow predicates, max-files, and the blob size and credential checks.
  • rename-into-refused still runs with -M on that tree.
• The leak scan in pr-run-activity-read.yml applies the patch and scans the bytes of the tree it builds, so the patch's form does not change what it reads.
• docs/conventions/pr-pipeline/pr-run-activity.md now says how the patch writes renames.

Verification

Red first. Before the collector change, 6 of the new or changed collect-patch.test.sh checks failed:

• the patch had no exact-rename headers;
• it had 4 deleted/new file lines and 4 content hunks;
• it was not small;
• config: the exact move is one rename failed.

After the change:

• bash .github/scripts/read-patch/collect-patch.test.sh: all pass. Cases:

  • a pure move of a 200 KB file plus an executable script gives two similarity index 100% renames, no content hunk and under 1 KiB, and the patch applies to a fresh checkout and rebuilds the tree;
  • a move with an edit is still a deletion plus an addition, with no rename header, and applies;
  • repository config (diff.renames copies) still changes nothing: no copy headers.

• bash .github/scripts/read-patch/collect-patch-lane.test.sh: pass.

• node --test in .github/scripts/apply-patch: 181 pass, 0 fail. These new issue-row tests use the collector's -M100% form:

  • an exact move pushes, with the old path gone and the new one in place;
  • an exact move out of .github/workflows/ or CODEOWNERS is refused by the old path's rule;
  • an exact move into .github/workflows/ or the central lane grant file is refused by rename-into-refused.

  They passed before the collector change too, because the apply side already checked both paths. They guard against regressions; they were not red first.

• python3 -I scripts/sync-hold-floor.py --check: pass.

• CHECK_DATA_ROOT=$PWD bash scripts/check-affected.sh --tests: 12 passed, 4 skipped over 5 changed files.

• No workflow was edited, so actionlint has nothing new to check.

Metrics

I measured locally by running git mv plugins/harness-ops/skills/morning-brief plugins/work-items/skills/morning-brief (9 files) on 95622795f7 and running both collectors:

Collector	Patch bytes	Cap 262144
Before (--no-renames)	864697	over
After (-M100%)	2765	under

This covers the pure move only. Any moved file that #11124 also edits (for example its own skill name) drops below 100% similarity and still costs its full content twice. The 25 reference edits add their own hunks. The move is 18 tree entries against the issue row's max-files: 300.

Related

• refactor(work-items): move /harness-ops:morning-brief to /work-items:morning-brief #11124: the move the cap refused; the issue lane delivers it in its own PR once this merges.
• fix(pr-pipeline): record the main-run review's size stop as an outcome and collapse exact renames #10961 and feat(pr-pipeline): review an oversize main-run diff in chunks #11160: exact-rename handling on the review side (pr-review-main).

🤖 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	✅ Completed 2026-10-30T13:01:23.357813Z	b474c71	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.

[github-actions]

claude-review needs no new review of e1d2642a6136bcec68f2569568cec0f1a4a16a12: no file of this pull request changed since the review of b474c71.

No findings. The change swaps --no-renames for -M100% in collect-patch.sh, so an exact move becomes a content-free rename header.

What I checked by reading the code (I ran no tests):

• Apply side: apply-patch.mjs applies the patch with git apply --cached and then judges the tree it builds. Every diff-tree call I found there passes --no-renames (lines 703, 886, 887, 897, 912). A rename's old path therefore meets the deletion refusals and its new path meets the addition refusals and predicates, as the PR says. The new tests assert this.
• Leak scan in pr-run-activity-read.yml: it applies the patch to the head objects and reads the resulting tree through changedBytes. That function in lib/publish-gate.mjs:207 also uses --no-renames, so a moved file's bytes at its new path are still scanned. The patch's form does not change what the scan reads.
• Other consumers: the other --no-renames users (scan-reuse.mjs, merge-base/tree-entries.sh, work-resolve-issue.yml) run their own diffs, not the collector's patch.
• Partial renames: a rename below 100% similarity still prints as a deletion and an addition, so the 100% threshold keeps partial moves checked as before.
• No closing issue: the PR says Refs: #11124 and does not close it, so there are no acceptance criteria to check.
• Manifests: no plugin manifest is touched, so no manifest lines apply.
• Changelog fragment: bump: patch fits a bug fix to the collector.

Residual note, not a finding: in merge-base (text-patch) mode, the comment at pr-run-activity-read.yml around line 2647 says every added byte was read by the text scan. A pure rename adds no new bytes to the tree, only moves existing ones, so the claim still holds in effect. The wording is slightly looser than before.

Spend: none. This change has no cost or paid-plan effect.

The test results in the PR description (181 apply-patch tests, the collect-patch.test.sh cases) are the author's claims. I did not run them.

[github-actions]

claude-security-review needs no new review of e1d2642a6136bcec68f2569568cec0f1a4a16a12: no file of this pull request changed since the review of b474c71.

No security issues found in this pull request.

Change: collect-patch.sh now passes -M100% instead of --no-renames, so an exact move is written as a rename header rather than a full delete plus add. The rest of the diff is tests, docs and a changelog entry.

Why it opens no path: the patch comes from the read activity, which handles model-influenced output, so I checked every consumer of the patch.

• Apply side (apply-patch.mjs):
  • git apply --cached builds the tree from the patch (lines 566-567).
  • Every path check then reads that tree with diff-tree --no-renames (lines 897 and 912). A rename's old path therefore meets the refusals as a deletion and its new path as an addition. Both also meet the allow predicates, max-files and the content rule.
  • The blob size and credential checks read the new blob (lines 935-947).
  • rename-into-refused still runs with -M on the built tree (lines 913 and 918), so a move into a protected path is still caught.
• Leak scan in pr-run-activity-read.yml: it applies the patch and scans the built tree through changedBytes. That function in lib/publish-gate.mjs uses diff-tree --no-renames (line 207), so it reads both the path and the blob of a renamed file. The patch's form changes nothing.
• Other patch consumers: I found none that parse the patch text for paths. The split-diff.py and write-diff.py matches are the review lane's own diff, which already handles rename headers.
• Collector hardening: the collector keeps its other pinned options (--no-ext-diff, --no-textconv, --no-relative, a clean config). -M100% matches only exact renames, so repository config such as diff.renames=copies cannot add copy headers. The new test covers that.

Trusted-path matches: apply-patch/**, pr-run-activity-read.yml and docs/conventions/** were read, not changed. The changed files are the collector, its tests and docs, so no trust control is widened or weakened.

Spend: none. The change shrinks the patch size and adds no turns, runs or billing.

critical-findings: 0

[github-actions]

The pr-verify verifier failed head b474c716733e9da715f8bd337f71ddabc5710ae6. What it wrote, as data:

What it wrote (24 lines)

Summary: The PR says `Refs: #11124`, but none of its changes fall within that issue's scope. Issue
  #11124 asks to move the morning-brief skill from harness-ops to work-items as
  `/work-items:morning-brief`, together with its scripts, tests, evals, probe and settings, and to
  repoint every reference. The diff instead changes the shared read-patch collector (`-M100%` in
  place of `--no-renames`), its tests, the apply-patch tests and a convention doc. That pipeline fix
  may be what makes the move possible, but it delivers none of the issue's done-when items. The PR
  body also names no part of #11124 as delivered and no remaining work, so the `refs` linkage fails.
  The code change itself looks sound: the apply side still checks the tree it builds with renames
  off, and the new tests cover exact moves into and out of refused paths.
- pr body: The `Refs: #11124` line links an issue whose scope is moving the morning-brief skill
  (skill files, tests, evals, probes, schema keys, references). This PR changes only CI pipeline
  tooling, which is outside that scope. Link this PR to its own issue about the read-patch cap and
  exact renames, by closing or referencing that issue, so it is judged against a matching spec. Then
  deliver #11124 in a separate PR.
- pr body: Under a `Refs:` linkage the body must name which part of #11124 is delivered and what
  remains. It does neither: it only describes the collector fix and notes that the 25 reference
  edits will add hunks later. If the PR stays linked to #11124, it needs an explicit delivered part
  that lies within the issue's scope.
- .github/scripts/read-patch/collect-patch.sh:89: Switching `--no-renames` to `-M100%` changes the
  form of every patch-mode row's patch, not only the issue lane's. The apply side's `diff-tree
  --no-renames` checks and the new tests in `.github/scripts/apply-patch/apply-patch.test.mjs`
  support the claim that refusals still see both the old and the new path. No defect found here, but
  a human should confirm the change under the issue that properly owns it.
- No text in the PR or the issue appeared to be trying to steer this review.

This pull request leaves the merge queue, and ci-status holds it out while it carries pr-verify-failed. A pr-verify pass at a new commit removes the label.