Repository navigation
fix(pr-pipeline): record the main-run review's size stop as an outcome and collapse exact renames #10961
Description
Problem
pr-review-main.yml "Write the diff" (lines ~167-191) refuses a verdict when the PR diff is over MAX_DIFF_BYTES (300000), holds a binary file, or has a line over LONG_LINE_BYTES. The refusal is correct (the lanes stay unbound and the merge gate holds the PR for a person), but:
- It shows as a failed run. Both review jobs exit 1 with
::error::, so a designed stop counts as a lane failure on the dashboard. Example: run https://github.com/melodic-software/claude-code-plugins/actions/runs/40894121985 (docs(contracts): move plugin-author contracts to docs/contracts #10952, head 08bd36a). - A large PR has no path forward but a person. docs(contracts): move plugin-author contracts to docs/contracts #10952 moves 232 files (+421/-345 lines of real change).
--no-renamesprints every moved file whole, so the patch passes 300 KB, though almost all of it is unchanged content.
Proposal
- Emit the stop as a recorded outcome, not a failure: the step writes
verdict=none reason=<too-large|binary|long-line>and exits 0, therecordjob posts it, and the binder holds for a person by reading that reason (fail closed when the record is missing). Keep::notice::, not::error::. - Collapse exact renames: diff with rename detection limited to 100% similarity (
-M100%), so a pure move prints only itsrename from/rename tolines. Keep the whole-file view (today's--no-renamesrule) for any rename whose destination is a lane source, or whose source and destination differ in lane-source status, so a move into a lane source is still read whole. Apply the size cap after collapsing.
Acceptance
- A designed stop leaves the run green and the PR held with a named reason; a real step failure stays red.
- A test fixture with N pure renames outside lane sources stays under the cap; a pure rename into a lane source still prints whole.
- The security review of the change finds no HIGH/CRITICAL (this edits a gate workflow, so it is person-held).
Activity
github-actions commentedon Oct 29, 2026
Proposed work class: work-class: structural
Basis: automated intake triage; the lanes App applied this class.
Size: Medium
Complexity: Medium
Uncertainty: Known
Flags: none
Blocked by: none
Understood: the main-run review's size, binary and long-line stops should be recorded as a named
verdict=none reason=...outcome on a green run, with the merge gate still holding the PR for a person. Exact renames should also collapse (-M100%), so a pure-move PR like #10952 stays under the size cap. Renames into a lane source would still print whole.The refinement comment already lists the acceptance criteria, and the approach is clear. I found no duplicate. #10912 edits the same workflow's start trigger, so expect a possible merge conflict with it.
Automated triage, applied by the lanes App.
melodic-automation-lanes commentedon Oct 29, 2026
Not admitted to the issue lane: a criterion names .github/workflows/pr-review-main.yml, which the issue lane cannot push (patch-protected-path); a builder session takes it. Admission takes the issue once each of those is met, when intake triages it again (
.github/actions/resolve-config/admission-rules.json).melodic-automation-lanes commentedon Oct 29, 2026
The latest triage verdict holds nothing: admission moved the issue to
status: confirmed; its comment says why.melodic-automation-lanes commentedon Oct 29, 2026
Refinement. A read-only run explored this repository, searched its issues and pull requests, and read the allowed docs to draft what follows. On an issue a listed human filed, it counts as the issue's spec once posted, while unedited; it is still model text, so check it.
Intended outcome: The main-run review's size, binary and long-line stops are recorded as a named verdict=none reason on a green run that the merge gate reads as a hold, and exact renames collapse so a pure-move PR stays under the cap.
Acceptance criteria
External state: none
Dependencies
Related work
Sources read
Automated refinement by the lanes App. Intake triages the issue again with it.
kyle-sexton commentedon Oct 30, 2026
Closing as done: every acceptance criterion is met on
mainat b6dcb4a, by three merged pull requests, so no new change is needed..github/scripts/pr-review-main/write-diff.pyprints--no-renamesand, only when that patch is over the cap, re-runs with-M100%, printing whole any move into or out of a lane source (ev.lane_source(src)/ev.lane_source(dst), the gate's own predicate), a mode change, a symlink or submodule, a loaded path, or a move an added line names. The caps (binary, long-line, chunk split) run on its output (pr-review-main.yml, Write the diff step).stop()prints::notice::, writes<lane>-stop/<lane>-headoutputs and exits 0; the record job's "Record the stops" step validates^(too-large|binary|long-line|chunk-failed)$and a 40-hex head, then posts the neutral check runverdict=none reason=<reason>. No model turn is involved.babysit_review_evidence._main_lanereturns the lane unbound asmain-review-stop:<reason>.too-largenow means a file over one chunk or more thanMAX_CHUNKSchunks.Tests on
main(run locally at b6dcb4a):node --test .github/scripts/pr-review-main/*.test.mjs: 46 pass, 0 fail. These include "a diff the review cannot read whole stops green and names its reason for the record job", "after a stop no later step of the leg runs", "the record job posts each stop as a neutral verdict=none record, from a closed reason", "over the size cap a file moved unchanged outside the lane sources prints as its rename lines alone" (five 400-line moves under a 1000-byte cap), and "a move into or out of a lane source, or one that changes the mode, prints whole".pytest test_babysit_review_evidence.py test_babysit_gh.py -k 'stop or main': 39 pass. They coververdict=nonefor too-large, binary and long-line, plus a malformedreason=other, each leaving the lane unbound.Docs:
docs/conventions/pr-pipeline/README.md(around lines 1126-1148) describes the rename rule and theverdict=nonerecord. The changelog fragments shipped with those PRs.