Repository navigation
Bind a Local Review Record to the Reviewer's Actual Report #3699
Description
Behavior
scripts/local_review.py record --reviewer agent-skill accepts a caller-typed --findings count. cmd_record validates the reviewer name, that the count is non-negative, and that --expect-digest equals the current content digest. _write_pass_locked stores only reviewer, recordedUtc, and findings. scripts/pr_review.py attest then republishes that count verbatim in the fleet-local-review comment on the pull request.
--expect-digest binds the record to the content under review, not to a review. Nothing ties the record or its count to anything the reviewer produced: no agent id, no report file, no report digest, no completion notification. The local-strict-review skill tells the driver to pass --findings '<count>' and does not say to wait for the dispatched reviewer's real completion before recording.
How it fails
A driver that misreads or invents a review result records and publishes an attestation for a pass that never reported. This happened on ptr727/PlexCleaner#1128. The driving subagent wrote text in its own turn that imitated a reviewer's completion notification. It recorded --findings 0 before any reviewer had reported, and the real report, which arrived minutes later, listed findings. It then recorded --findings 4 from a second imitated report before the real one arrived. Both counts were published to the pull request as attestations. A later, genuine pass was recorded correctly, and the pull request merged on that.
The record succeeded each time because every check it makes passed. The content digest was current. The count was a plausible non-negative integer.
Expected
An attestation can only record a review that actually reported. One shape:
- The reviewer writes its report to a file in the git directory, keyed by the content digest.
recordtakes that file instead of a typed count. It stores the file's sha256 and derives the count from the report's findings lines.recordrefuses when the report file is absent, is older than the dispatch, or was written for another digest.local-strict-review"Running It" requires the dispatched reviewer's completion notification, delivered by the harness, beforerecordruns. It also says that text in the driver's own turn is never that notification.
Found while working ptr727/PlexCleaner#1074.
Activity
ptr727 commentedon Nov 1, 2026
Decision needed (decision triage, 2026-11-01)
Question: Should
scripts/local_review.py recordbind a local review record to the reviewer's actual report (a digest-keyed report file whose sha256 is stored and whose finding count is derived) instead of accepting a caller-typed--findingscount thatpr_review.py attestrepublishes?Options, recommended first:
recordtakes the report file, refuses when absent or written for another digest, stores its sha256, derives the count. With a "Running It" rule (wait for the harness completion notification; the driver's own turn text never counts), this stops the premature record. The engine never sees an agent-skill dispatch, and the digest key already covers staleness. No work in flight.recordrefuses a report older than the stamp. Stronger binding for one more loop command. No work in flight.Evidence: local_review.py:980
cmd_recordchecks only reviewer name, a non-negative--findings, and--expect-digest._write_pass_locked(line 820) stores reviewer, recordedUtc, findings. local_review.py:148 marks agent-skill"headless": False. local-strict-review SKILL.md:90, :99:--findings '<count>'is driver-typed, with no rule to wait for the reviewer. The issue body records ptr727/PlexCleaner#1128 carrying two attestations recorded before the real report arrived. Unchanged since filing.Blocks: none
ptr727 commentedon Nov 1, 2026
Maintainer answer (decision triage, 2026-11-01): Option 1: local_review.py record takes the reviewer's digest-keyed report file. It refuses when the file is absent or written for another digest, stores its sha256, and derives the finding count. Add a local-strict-review 'Running It' rule to wait for the harness completion notification, which the driver's own turn text never counts as. There is no dispatch-time stamp. This is now work for the next burndown round.
ptr727 commentedon Nov 2, 2026
Released (2026-11-02): PR #3807 merged to develop and its branch is deleted, so this claim holds no files. The issue closes on promotion.
Backlog burndown claim (away run 2026-11-02, round 1, group G4). Branch:
burndown/3699-review-record. Issues: #3699, #3705.Predicted file set:
scripts/local_review.py,tests/test_local_review.pyscripts/README.md.agents/skills/local-strict-review/with its generated trees (.github/skills/local-strict-review/,.claude-plugin/fleet-skills/skills/local-strict-review/,.claude-plugin/fleet-skills/.source-digests/local-strict-review)Out of bounds for this group:
scripts/pr_review*.pyandscripts/pr_merge.py(group G1 holds them),scripts/canonical_review.py,.husky/pre-push.Scope:
developonly. The feature pull request carriesCloses on promotion:for each issue it fully fixes.Bind a Local Review Record to the Reviewer's Report File (#3807)