Repository navigation
feat(review): converge CI review lanes across rounds, trusting only the lane's own posts #7777
Description
Refs: #7735
What
Rules that make the CI review lanes (/review:code-review, /review:security-review) settle down across rounds:
- after the first round, review only what changed since the head the lane last reviewed;
- don't post a finding the lane already posted, unless the new diff changed the code it names;
- stop after a repo-owned number of rounds. The security lane still reports a new CRITICAL or IMPORTANT finding past the cap.
#7735 first added these and then dropped them in review (threads discussion_r4303831729, r4303831724 and r4303833626 on #7735), because they had three problems:
- Security: rule 2 read every PR comment and review. A forged comment shaped like the lane's own output could suppress a real finding.
- Instruction economy (
docs/plugin-philosophy.md#instruction-economy): no observed stumble was recorded. - Volatile specific: the round cap copied an upstream article's number (
.claude/rules/skill-bodies-state-current-rules.md).
Build only when
At least one observed stumble on a merged PR:
- a review lane re-posts a finding it already posted, against unchanged code; or
- a review lane keeps producing findings beyond the round a person would stop at.
Link the PR and the comments. Until then, do not build.
Data so far (2026-10-17): across the last 80 merged PRs, no lane posted findings on more than 3 distinct heads, and none re-posted an earlier finding. #7766, #7774 and #7735 each drew new findings after a fix, but every one was a distinct, real issue, not a repeat.
Design when built
- Rule 2 trusts only the lane's own posts: comments, reviews and inline review threads whose author is the lane's bot login and that carry the lane's
claude-lane: <lane>marker. Anything else is never evidence that a finding was already reported. The bot-and-marker check used bybabysit_review_evidence.py(MARKER_AUTHOR) is a model to follow. - Rule 2 also loads inline review threads, not only
reviewsandcomments, because inline comments are where findings land. - The round cap is this repo's own value, set in a
docs/conventions/config, not copied from upstream. - A test posts a forged lookalike comment from another account and shows the finding is still reported.
🤖 Generated with Claude Code
Activity
github-actions commentedon Oct 17, 2026
Proposed work class: work-class: scoped
Basis: automated intake triage; the lanes App applied this class.
Size: Medium
Complexity: Medium
Uncertainty: Knowable
Flags: none
Blocked by: none
#7777 (comment)
This decision settles the open questions: the build gate stays unmet until a host run of
/harness-ops:morning-brief precisionshows a re-posted finding or an over-long round count. That run is host work, not a decision for a person. Until the gate is met, the issue stays parked. If the data meets the gate, the body's "Design when built" section is the spec.Automated triage, applied by the lanes App.
kyle-sexton commentedon Oct 22, 2026
This was generated by AI during attend-queue.
Parked by operator: build only after at least one observed stumble on a merged PR, per this issue's "Build only when" section.
kyle-sexton commentedon Oct 25, 2026
Released for re-triage under the owner's 2026-10-24 rule: an issue stops for a person only for an intent or vision question, a security call, a host or attended run, or spend outside prepaid credit. The hold label came off and intake triage runs again now; it re-applies a hold, with a numbered question, when one of those applies.
To hold this issue again, add
needs-human(orstatus: needs-decision) and say why in a comment.8 remaining items
github-actions commentedon Nov 1, 2026
The triage sweep dispatched intake triage for this issue (attempt 1 of 2).
kyle-sexton commentedon Nov 1, 2026
Claimed by host lane 4 (melo-lap-001 WSL), 2026-11-01T15:10Z, on hub assignment: refresh the 'Build only when' evidence. I'll survey merged PRs since 2026-10-17 for a review lane that re-posts a finding against unchanged code or keeps finding things past a reasonable round, and post the record here. Building waits until that record shows a stumble.
kyle-sexton commentedon Nov 1, 2026
Evidence survey (2026-11-01, lane 4): the "Build only when" bar is met. Stumble 1 is observed: a review lane re-posted a finding against unchanged code, and in five PRs the owner had already settled that finding before the re-post.
Scope: 2,778 PRs merged from 2026-10-17 to 2026-11-01T14:46Z. 392 had claude-review lane reviews and 152 had claude-security-review lane reviews. Evidence counted only the lane's own posts:
github-actionsreviews carrying aclaude-lane: <lane>line, plus their inline comments. The survey cost about 220 GraphQL points.Strongest example (I checked it myself): #10397, claude-security-review. The same CRITICAL ("Trust control … merge hold narrowed") at
.github/scripts/pr-merge/enable-auto-merge.mjs:967was posted three times:cb11548, 02:50Zb5e8c6a, 03:13Zdaa86e4, 03:46ZThe owner answered the first two at 03:18Z and 03:19Z ("Settled without code by an owner decision (Q1 = A …)"). The compare
b5e8c6a...daa86e4changes 194 files, andenable-auto-merge.mjsis not one of them (only its.test.mjschanged).Other repeats the survey found on settled findings (worker-reported, not re-verified by me):
userConfig.pr_open_state.defaultflag atplugin.json:46, posted on 5 heads. Only theversionline changed between heads, and each re-post came about a minute after the owner settled the previous one.hooksflag on the eval-onlygh-forge-pathmanifest, posted on 4 heads with the file unchanged, after the owner classified it INTENTIONAL.Distribution (PRs by the number of distinct heads with at least one finding; a lower bound, because each lane edits one summary comment in place):
This reverses the 2026-10-17 data point. The highest counts are 11 heads on #10252 and #10583 (claude-review) and 10 on #9835 (security).
Stumble 2 (rounds), data only: across the 39 lane-and-PR pairs with findings on 4 or more heads, the author's replies to findings from round 4 on classify as 89 fixed, 17 no-change or settled, 14 with no reply and 11 unclear. The classification is a regex heuristic. Most late findings were real. The no-change share concentrates in the security lane's repeated manifest and trust flags.
Not verified: why the security lane re-posts these flags. It may re-derive manifest and trust flags from base against head on every review. Also not verified: when incremental review ("incremental, A → B" in recent lane comments) arrived. #11820, on 2026-11-01, still repeated a finding on an unchanged file.
Next: per the body, the issue is now buildable as designed, with rule 2 trusting only the lane's own bot-and-marker posts and a forged-comment test. Lane 4 queues the build PR behind its open PR.
fix(review): count prior-findings rounds on earlier finding heads only
fix(review): count a suppressed repeat finding as if posted
kyle-sexton commentedon Nov 1, 2026
Status (lane 4): rules 2 and 3 merged in #12011.
pr-review-cli.yml(incremental-review).plugins/review/lib/prior-findings.mjsand both lane skills. A suppressed repeat still counts toward the findings count,critical-findingsandhold, so no merge gate reads a review as clean because of it.kyle-sexton commentedon Nov 1, 2026
Owner decision (2026-11-01, lane 4 session, "go w/ recommendations"): option (a). The security lane keeps never dropping a settled CRITICAL, trust-control widening or manifest flag (
security-review/SKILL.md"Settled points"). Repeats on owner-resolved threads (#10397, #9483, #11820) stay out of scope by design. This issue closes once ci-workflows#950 runsprior-findings.mjsin the lanes.kyle-sexton commentedon Nov 1, 2026
Done: rules 2 and 3 merged in #12011 and went live in every review lane with melodic-software/ci-workflows#955 (closes ci-workflows#950). Rule 1 already shipped as incremental-review. Repeats on owner-resolved threads stay out of scope by owner decision (issuecomment-6562109658).