[kyle-sexton]

Refs: #10961

Summary

This ships part 1 of #10961 and two merge-job read cuts.

• Part 1. pr-review-main now records a designed stop instead of failing. A diff that holds a
  binary file, is over 300000 bytes, or has a line over 1800 bytes gets no verdict. The run stays
  green, and each stop leaves one countable check run. The merge gate names the stop as the lane's
  unbound reason. No person hold is added: the stop stays the same MED evidence-unbound:<lane>
  lane cause as before, and only its reason changes.
• Cuts A and B. For a pull request that changes a lane source, the merge job stops reading
  evidence that bind() never uses.

Fix

.github/workflows/pr-review-main.yml (all three legs, identical edits):

• The "Write the diff" step now has a stop() helper. On binary, too-large or long-line it
  logs ::notice title=pr-review-main stop::reason=<r> lane=<lane> pr=<n>, writes stop=<r>,
  <lane>-stop and <lane>-head to the step outputs, and exits 0.
• The part-line check and the empty-diff check still fail the step. So does a failed fetch or a
  failed write-diff.py.
• Every later step that gated on steps.diff.outcome == 'success' now also needs
  steps.diff.outputs.stop == ''.
• Each leg exports claude-review-stop, claude-security-review-stop, claude-review-head and
  claude-security-review-head. review-fallback gains an outputs: block holding only these.
• A new step in the record job, "Record the stops", does not depend on the artifact download.
  For each lane it reads the leg that set the stop. It checks the reason against
  ^(too-large|binary|long-line)$ and the head against a 40-hex SHA, and fails on anything else.
  It then posts the check run pr-review-main: <lane> on that head with conclusion neutral and
  title verdict=none reason=<r>.

Binder (plugins/source-control/skills/babysit-prs/scripts/):

• babysit_gh.py:
  • MAIN_REVIEW_STOP_TITLE reads a stop title as ("stop", reason). Any other title that does
    not read is still None, which counts one CRITICAL.
  • main_review_critical counts a stop as 0 CRITICAL.
• babysit_merge.py _main_reviews:
  • head_records keeps only count tuples.
  • A new head_stops holds each lane's stops on the head.
• babysit_review_evidence.py _main_lane: when no verdict binds and the head has a stop, the
  unbound reason is main-review-stop:<reason>. Otherwise it stays lane-source-changed.
  • A verdict that binds wins over a stop.
  • A stop never binds a lane.
  • The security lane still carries its CRITICAL count.
• babysit_risk.py is unchanged. The reason reaches the report through the existing
  evidence-unbound:<lane> detail, not bound: main-review-stop:too-large.

Cut A (babysit_merge._lane_evidence):

• needs_main_review is computed once, before collect_lane_runs, and passed into
  _main_reviews.
• When it is true, only the security lane's pull_request runs are listed. On that path bind()
  reads the code lane's runs nowhere, since _unbound and _carried_blocking read only the
  security lane.

Cut B (babysit_gh.collect_main_review_counts):

• The function now takes a list of SHAs per lane.
• The code lane is read on the head only, because head_records uses only the head and its
  carried count is checked only for None.
• The security lane still reads every head: commits plus force-pushed heads.

Fail-closed:

• A forged stop title (any same-repo workflow can post as github-actions) only renames an unbound
  reason. It adds nothing a person reads and never binds a lane.
• A missing stop record leaves today's lane-source-changed.
• An unknown title still counts one CRITICAL.

Metrics

Countable outcome event (part 1).

• Each designed stop posts one check run, pr-review-main: <lane>, on the head. It is titled
  verdict=none reason=<too-large|binary|long-line>, its conclusion is neutral, and its app is
  github-actions. The checks API has no delete, so the count is durable.
• The same step logs ::notice title=pr-review-main stop::reason=<r> lane=<lane> pr=<n>.
• pr-review-main runs that end in failure should fall by the number of stops.

Expected GITHUB_TOKEN reads saved per merge-job tick. These are estimates. They come from the
plan's figures, taken from the babysit-gh-calls lines of merge job run 40819274416, which
predates #10743. The parent records the before and after measurements.

Cut	Saved per lane-source candidate	Basis
A (_lane_evidence)	About 8 to 9 reads: 1 workflows/pr-review-hosted.yml/runs listing, plus about 3 reads per uncached claude-review run (actions/runs/{id}, attempts/{n}/jobs, check-runs/{id}/annotations). In the sample, 2 of 10 candidates took this path, about 17 reads for the tick.	Estimate. The log totals reads per endpoint, not per lane, so the per-lane figure is that total split in half. Carried lane runs make the steady-state saving smaller.
B (collect_main_review_counts)	(commits + force-pushed heads − 1) check-runs reads, for example 9 for a 10-commit pull request with no force pushes.	Estimate, computed from the code. Not in the sample, which predates #10743. The run cache does not affect it.

Verification

New tests, each run on the old code first (git restore --source=HEAD on the source files only)
and then on the new code:

• .github/scripts/pr-review-main/workflow.test.mjs (node --test: 29 pass after; 4 fail before):
  • a diff the review cannot read whole stops green and names its reason for the record job:
    runs the real "Write the diff" script under bash -e -o pipefail with git and python3
    stubs, in every leg. It covers a too-large stop, a long-line stop, a failed write-diff.py
    (non-zero, no stop=), and a whole diff (parts written, no stop).
  • after a stop no later step of the leg runs, and each lane's stop leaves the leg.
  • the record job posts each stop as a neutral verdict=none record, from a closed reason: runs
    the step with a gh stub. A malformed reason or head fails and posts nothing.
  • a later leg runs only for the lanes the previous leg handed on: the fallback leg's
    outputs: assertion is narrowed to the four stop outputs.
• tests/test_babysit_gh.py (stop title parse; stop counts 0 and an unknown reason counts 1; one
  read per listed (sha, lane)): 3 fail before, pass after. The case of a stop on one head beside
  critical=1 on another passes before and after. It guards that the stop skip drops no other
  head's count.
• tests/test_babysit_merge.py:
  • The _main_reviews input test fails before and passes after. It checks per-lane SHAs,
    head_stops, and that head_records excludes stops.
  • LaneRunsReadTests checks that a lane-source PR lists only the security lane's runs; it fails
    before and passes after. Its paired case, where any other PR lists every lane, passes both.
• tests/test_babysit_review_evidence.py:
  • The stop names main-review-stop:too-large, keeps the carried CRITICAL, and scores as MED
    evidence-unbound with no new person code. This fails before and passes after.
  • A verdict beside a stop binds. This passes before and after, as a guard.
  • A pin for cut A: bind() with security-only runs equals bind() with both lanes' runs. This
    passes before and after.

Runs on the final tree:

• pytest on the five touched or neighboring babysit test files: 812 passed.
• pytest tests/test_babysit_risk.py: 135 passed.
• node --test on workflow.test.mjs, reconcile.test.mjs and enable-auto-merge.test.mjs:
  all pass.
• actionlint and zizmor --offline on pr-review-main.yml: clean.
• scripts/run-ruff.sh format and check: clean.
• scripts/check-changelog-fragments.sh --check: valid.

CI runs the full suites.

Related

• Part 1 of fix(pr-pipeline): record the main-run review's size stop as an outcome and collapse exact renames #10961. Part 2 is not in this PR, so this PR refs the issue and does not close it.
• The plan's person-hold option (a main-review-stop:<lane> HIGH person code, its prefix and its
  lane-causes-person-codes pin) is dropped by owner direction. A stop is recorded and reported,
  and nothing waits on a person.
• feat(pr-pipeline): turn path-based merge holds off behind one switch (owner Q15) #11058 (merge.diff-check.path-holds) is the base this branch was cut from. feat(pr-pipeline): lane-arm the merge gate's own code on a verified pr-merge attestation #10966 was closed
  unmerged, so nothing here depends on it.

🤖 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-29T23:28:29.115256Z	8b6e77f	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-security-review reviewed 8b6e77fb8be9c4c78ea18d2c348e31550024f45b: 0 finding(s), 0 as inline comments.

No security issues found in PR #11084. I read the whole diff and PR context, and the consuming code in babysit_review_evidence.py (_main_lane, _main_bound, _unbound, _carried_blocking) and the handoff steps in pr-review-main.yml. I dispatched no verifiers.

Trusted paths touched: .github/workflows/pr-review-main.yml, .github/scripts/pr-review-main/** and the babysit_* merge-gate scripts. I checked each for widening:

• A stop never binds a lane. _main_bound still needs a counted workflow_run verdict artifact from the default branch plus a matching head_records entry. head_records excludes stops (babysit_merge.py filters main_review_stop(c) is None). _head_stop runs only when found is None and only changes the reason string. babysit_risk.py uses unbound_reason only as MED evidence-unbound detail, so no severity or person code changes.
• A forged stop title adds nothing. It counts 0 CRITICAL where an unreadable title counted 1, but a forged findings=0 critical=0 title already had that effect. A stop title cannot create the verdict artifact that binding needs.
• The security lane's CRITICAL carry is unchanged. Cut B still reads the security lane on every head, including force-pushed ones. The code lane's carried count is used only as a None check in _main_lane, and a failed read still raises. Cut A is safe because _unbound passes carried_blocking only for the security lane, whose runs are still listed.
• A stop reaches no model step or later leg. The model-running steps and the "Fail without a verdict" steps carry steps.diff.outputs.stop == ''. The credential step is skipped on a stop, so present is empty rather than 'false', and "Hand the lane on" is skipped. Neither the subscription nor the fallback leg gets the lane.
• No shell injection. stop() takes a literal reason, $LANE comes from the matrix, and the numbers come from wc and the gate. Outputs are single-line key=value.
• The record step validates its inputs. It checks the reason against a closed set and the head against 40 hex characters before gh api. It uses the job's existing checks: write token.
• No merge hold is weakened. A stop lane is the same MED evidence-unbound as the earlier lane-source-changed. The separate long-line-review: rule in babysit_risk.py is untouched.

No plugin.json or marketplace.json changed, so there are no manifest lines. Cost: no billed resources change. The stop path saves model runs, and cuts A and B save GITHUB_TOKEN reads.

critical-findings: 0

[github-actions]

claude-review reviewed 8b6e77fb8be9c4c78ea18d2c348e31550024f45b: 0 finding(s), 0 as inline comments.

No blocking or advisory findings. I read the whole diff (all 12 files) and the PR context. I did not run any tests. The test results in the PR description are author-claimed and unverified by me.

What I checked

• pr-review-main.yml stop path (all three legs).
  • stop() writes stop, <lane>-stop and <lane>-head, then exit 0. It is called only from inside the if blocks, so the part-line check, the empty-diff check, a failed fetch and a failed write-diff.py still fail the step.
  • I listed every step-level if: in each leg. Every step that gated on steps.diff.outcome == 'success' now also requires steps.diff.outputs.stop == ''.
  • The remaining steps depend on steps.credential.outputs.present == 'true' or on steps.review/steps.verdict, so they skip after a stop. The credential step is skipped, so present is empty and the present == 'false' handoff does not fire.
  • The stop emits no handoff, so no later leg runs for that lane. The Log the API budget step runs with always() and is harmless.
• Record the stops step.
  • The reason and head reach the shell only through env. Both are checked against a closed regex (^(too-large|binary|long-line)$, 40-hex SHA) before the gh api call. I found no injection path.
  • [ -z "$reason" ] && continue is safe under bash -e.
  • The job needs all three legs, and a skipped leg's outputs are empty, so the || chain picks the leg that stopped.
• Binder.
  • MAIN_REVIEW_STOP_TITLE is a closed set. An unknown reason still reads as None and counts one CRITICAL.
  • main_review_critical counts a stop as 0.
  • _main_lane only renames the unbound reason, and only when found is None. A stop never binds a lane, and a verdict that binds wins over a stop.
  • The security lane still carries max(left, recorded), so a CRITICAL on another head still holds.
• Cut A.
  • _unbound reads runs only for SEVERITY_LANE, and _carried_blocking reads only _lane_runs(SEVERITY_LANE, …). So bind() never reads the code lane's runs when needs_main_review is true.
  • The except branch in bind() only calls _unbound, so it reads no code-lane runs either.
  • runs is not used elsewhere in _lane_evidence.
  • needs_main_review is the same call bind() makes, with the same inputs.
• Cut B.
  • The only consumer of carried_critical is _recorded_critical, so the code lane's carried value is checked only for None. A non-None value is guaranteed once the head is read, so narrowing the code lane to the head loses nothing.
  • The security lane keeps [head, *commits, *force-pushed].
  • head_records and head_stops read only counts[head_sha].

Not counted as findings

• docs/conventions/pr-pipeline/README.md:1123 still says the lanes stay unbound as lane-source-changed without mentioning main-review-stop:<reason>. The new paragraph a few lines above states it, so I left it as a nit.
• A workflow re-run on a stopped head posts a second verdict=none check run, so the stop count can be higher than the number of stopped PRs. The checks API has no delete, and the PR already calls the record "countable". This is expected.

Spend
This change adds no paid plan, billing setting or billed resource, so no cost hold. It only cuts GITHUB_TOKEN reads and turns failed runs into green ones.

Manifests / closing issue

• No plugin.json or marketplace.json change.
• The PR refs fix(pr-pipeline): record the main-run review's size stop as an outcome and collapse exact renames #10961 and does not close it, so there are no acceptance criteria to check.
• The PR context file holds no embedded instructions.