[kyle-sexton]

Refs: melodic-software/claude-code-plugins#11575

Summary

The review lane read 2 of the 1186 files on claude-code-plugins#11169 (head 778ceff0d0), and its comment said so. The check pr-review / claude-review-status still concluded success, and the comment opened with "0 finding(s)". This change makes a review that read only part of its scope fail closed. It does not add chunking, so acceptance item 1 of #11575 (review an oversize diff in chunks) stays open on that issue.

Fix

• Schema and prompt (all three legs): the model also returns files_read, the number of changed files in scope whose whole change it read.

• Extract step (all three legs, same script): the job counts the files itself. An incremental review uses the scope step's new files output, and a whole review uses the full-diff step's new files= output. The job also measures the diff's bytes where the diff is written: in the full-diff pipe, or from the text the scope step writes. It requires the diff path to be a regular file, so a symlink the head commits there is incomplete. It records coverage: { read, changed, over_budget? }, with read capped at changed. The review is incomplete when files_read is missing or invalid, the count is unreadable, or the diff file cannot be read. A diff over 300000 bytes is partial whatever the model reports. Basis: pr-verify's per-chunk read budget in claude-code-plugins .github/scripts/pr-verify/chunk-diff.sh.

• publish.cjs: a review without valid coverage is incomplete. A partial review (read < changed, or over budget):

  • still posts its findings;
  • writes the marker with the coverage line first, for example **claude-review** partial review of <head>: read 2 of 1186 changed file(s). 0 finding(s) in what was read ... This head is not reviewed.;
  • records sha=none in the hidden reviewed-head line, so the next push reviews the whole pull request and no merge gate reads the head as reviewed;
  • writes no claude-lane-review-complete notice;
  • fails with failure-class=partial.

  A complete review's line now reads reviewed <head>: read N of N changed file(s); F finding(s), .... In summary mode the shadow report records the outcome and coverage, and the step never fails.

• Who is affected: every caller of pr-review-cli.yml@main, both lanes, since the security lane shares the schema and extract step. In claude-code-plugins those callers are pr-review-hosted.yml (claude-review-status) and pr-review-security-hosted.yml. Until chunking lands, a pull request whose diff is over 300000 bytes, or whose review reads fewer files than it changed, keeps a non-success review check. Its ci-status therefore stays red. #11575 asks for this non-success result.

• Marker consumers: the claude-code-plugins parsers (babysit_review_evidence.py, clear-escalations.mjs, address-feedback.mjs, feedback-dispatch.mjs) match sha=[0-9a-f]{40}. They read sha=none as no reviewed head.

• README: the partial outcome and the coverage rule.

Verification

• node --test, all passing (321 tests) on:
  • publish-review
  • pr-review-cli
  • pr-review-cli-api-calls
  • apply-review-hold
  • claude-lane-review-complete
  • claude-lane-scope
  • claude-lane-status-check
  • claude-review-declared-outputs
  • claude-review-outcome-wiring
  • claude-lane-job-gates
  • claude-lane-fallback
  • claude-lane-compose-args
  • network-timeout-policy
• New tests:
  • A partial review (2 of 1186, and over budget) fails as partial with no completion notice, and its first visible line states coverage. The hidden marker records sha=none.
  • A partial review still posts its inline findings.
  • Missing or out-of-range coverage is incomplete.
  • Summary mode reports partial without failing.
  • Extract records coverage from the job's counts, caps an overclaim, uses the incremental count, and marks a 300001-byte diff over budget (300000 bytes passes).
  • Extract calls a missing files_read, count or diff file incomplete.
  • All three legs wire the counts identically.
  • A push after a partial review's marker reviews the whole pull request.
• Existing fixtures gained coverage / files_read. Only the two marker-line assertions changed text.
• Mutation check: writing the head sha instead of none on a partial review fails the publish suite.
• zizmor --offline: no findings. Biome check on .github/scripts: clean.
• An independent security review (a fresh review:security-reviewer agent) of the first commit found no HIGH or CRITICAL issue. It raised one IMPORTANT issue: a head could commit .claude-lane/pr.diff as a symlink to /dev/null, so a stat of the path read 0 bytes and the budget check passed. The second commit fixes it, and a symlink test covers the fix. The review confirmed that sha=none matches no 40-hex consumer regex, that the new env values carry no injection path, and that every gap fails closed. One risk remains, and it is not new: files_read is self-reported, so a prompt injection can still claim full coverage of a diff under the budget, as it could already claim 0 findings.

Related

• fix(pr-pipeline): claude-review reports success on a partial review of an oversize diff claude-code-plugins#11575 (this lane; chunking still open there)
• feat(pr-pipeline): review an oversize main-run diff in chunks instead of holding it for a person claude-code-plugins#11000 (pr-review-main oversize)
• fix(pr-pipeline): verify a large diff in chunks with a final spec judge claude-code-plugins#11548 (pr-verify chunking)
• feat: land the rest of the UI course migration: interface design guidance, build-generator and eval cases claude-code-plugins#11169 (the 1186-file pull request)

🤖 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-31T23:45:14.930781Z	3e94beb	PR opened

ℹ️ 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 eeba85595377800578f37982ec3ee77304357c36: 0 finding(s), 0 as inline comments.

Security review (incremental, since 3e94beb)

Reviewed .claude-lane/incremental.diff for .github/workflows/pr-review-cli.yml and .github/scripts/pr-review-cli.test.cjs. I found no security issues.

• Workflow change: The three full-diff steps (the three legs of pr-review-cli.yml) now start with set -o pipefail. Before, a failing git diff … | tee | wc -c pipe could still end with written=true in $GITHUB_OUTPUT and a truncated diff file. Now the step exits non-zero before it writes any outputs. The step has continue-on-error: true, so the lane treats the full diff as unavailable and the review is flagged incomplete. This makes the lane fail closed more often. The diff adds no new input, widens no token scope, and adds no trusted actor or untrusted-text path. I read the surrounding step and saw no new expression interpolation inside run:. The token still goes through the environment, never argv.
• Test change: The new test runs the step's run: body under bash -e with a stub git that exits 128 on diff. It uses fixed environment values in a temporary directory, so no untrusted input reaches it. It asserts a non-zero exit and no written=true. The added pipefail assertion is a regex match on the workflow text.
• Trust controls: None of the listed trust controls is touched: not the trusted-trigger check, token scope, kill switch, broker, merge holds, or other .github/actions/*.
• Manifests: No plugin manifests changed.
• Instruction surface: No standing instructions were deleted or softened.

critical-findings: 0

[github-actions]

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

No findings in the incremental change since 3e94bebd.

What changed

• set -o pipefail was added to the full-diff step in all three legs of .github/workflows/pr-review-cli.yml. It carries a comment explaining why.
• .github/scripts/pr-review-cli.test.cjs gained a wiring assertion for the line, and a new test that runs the step's script with a fake git whose diff exits 128.

What I checked

• Without pipefail, the step's git diff … | tee | wc -c and git diff --name-only | wc -l pipelines both returned 0 on a git diff failure. The step would then write written=true with bogus counts. With pipefail and the default bash -e, the bytes= assignment fails the step before any output is written.
• The new test should therefore fail if the line is removed: the unguarded step would write written=true and exit 0, and the test asserts on both.
• The fake git makes fetch succeed and diff fail. The test environment supplies a 40-hex MERGE_BASE (skipping the gh compare call), GITHUB_SERVER_URL and GITHUB_OUTPUT. That is enough to reach the git diff line.
• The step is continue-on-error: true, and a failed run emits no files output. The extract step's existing handling then marks the review incomplete, as the new comment says.
• Under pipefail, the other pipelines in the step (tr | awk, and the tr | grep | paste inside the warning's echo) do not change the step's outcome.

I ran nothing myself. This is from reading the diff and the surrounding workflow and test code.

Closing issue: the PR uses Refs:, so no issue is closed and there are no acceptance criteria to check.