feat(source-control): judge a review lane by what it produced, not by its check row

plugins/source-control/.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json",
   "name": "source-control",
-  "version": "0.55.74",
+  "version": "0.55.75",
   "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-authored-by trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop, safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /babysit-loop (the loop-lane merge lane: a standing or drain loop that invokes babysit-prs per cycle, configured through repo-scoped babysit_loop_* keys on the layered source-control.md seam, with merge authority human-only until the target repo's tracked config adopts the lane, a gate-proven C2-mechanical baseline once adopted, and standing merge-rung raises binding from the team-tracked layer only, with one named exception, where an invocation line explicitly typing both the autopilot tier keyword and the dedicated raise argument --merge c3-this-run widens that single invocation's merge authority up to C3 behind a fresh independent frontier-tier resolver, while C4-structural and C5-untrusted-provenance stay unconditionally human-merge), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention merged across its config layers and the babysit-prs config, or apply, which interviews the repo and writes the convention config to a chosen layer), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep, never --abort). The commit-subject / PR-title convention is configurable via a source-control.md config written by a re-runnable setup skill, layered across a ~/.claude user-global file, the tracked team file, and a gitignored .claude/source-control.local.md personal overlay merged per key; Conventional Commits is the default when no convention is declared.",
   "author": {
     "name": "Melodic Software",

plugins/source-control/CHANGELOG.md

@@ -3,6 +3,30 @@
 All notable changes to the `source-control` plugin are documented here. Format follows
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning.
 
+## [0.55.75]
+
+### Added
+
+- **The PR monitor judges a review lane by what it produced, not by its check row.** New step B2
+  in the per-iteration checklist: an AI-review lane that concluded success having produced no
+  review body and no finding **for the current round** reviewed nothing, and is classified ABSENT
+  rather than PASS. These lanes report on their session rather than on their output, so a session
+  that ends without error goes green whether or not it reviewed anything, and cost is no signal
+  either since the session is billed either way. Productivity is scoped to the head under review
+  through the per-surface commit fields Gate 5 already names, so an artifact left by an earlier
+  head or an earlier rerun cannot stand in for a round that emitted nothing. An absent lane is
+  substituted with a local review over the same diff (`/review:fanout`, or the bundled
+  `/code-review` against an explicit target for a correctness lane) and named in the report.
+- **The substitution is enforced on the readiness gate, not only in the monitor loop.** The same
+  invariant is now a Gate 5 item in `reference/readiness.md`, the single source of truth both
+  `monitor.md` (Phase 3.4) and `merge.md` (Phase 4.1) rerun, so a direct `merge` invocation and
+  the final re-verification in `full` enforce it too. It states what reaching Gate 5's bound hands
+  off to: the bound ends the wait for a silent reviewer, it does not supply the review. The
+  `babysit-prs` worker and autopilot tiers merge through their own gate and do not read this file,
+  so they are unchanged by this entry.
+- **Both readiness templates gained a `Review lanes:` line**, so an absent lane and its local
+  substitute are reported rather than dropped from a verdict that otherwise reads all-clear.
+
 ## [0.55.74]
 
 ### Changed

plugins/source-control/skills/pull-request/SKILL.md

@@ -161,6 +161,7 @@ When a channel event, Monitor notification, or poll iteration fires, complete ev
 
 - [ ] **A. Terminal state:** `gh pr view <N> --json state -q .state`. MERGED/CLOSED → self-terminate
 - [ ] **B. CI checks:** `gh pr checks <N>`. Classify EVERY non-pending check (pass/fail/skipped). Read logs for ANY failure per §3.1 fetch chain
+- [ ] **B2. Review-lane productivity:** a green AI-review lane is not evidence that a review happened. Take the lane roster from step B's `gh pr checks`, never from the authors who posted: a lane that posted nothing is missing from every author-derived roster by construction, so such a roster applies this step to an empty set. For each lane, confirm it produced something readable **for the current round**, scoped per surface the way [readiness.md](reference/readiness.md) Gate 5 scopes its wait: `original_commit_id` on inline review comments, `commit_id` on the reviews endpoint, and `created_at` against the push time on issue-level comments, which carry no commit field at all and are where AI-review summaries land. An artifact from an earlier HEAD cannot satisfy this. An artifact from an earlier RERUN of the current head is indistinguishable by commit field alone, since it carries the same SHA, so bound that case by timestamp against the run's start. A lane that concluded success having produced nothing for this round reviewed nothing, whether it never started, could not load the skill it was told to invoke, or stopped early inside its own session. Classify it ABSENT, never PASS, and stand a local review in its place over the same diff: `/review:fanout` for breadth, or the bundled `/code-review` against an explicit target for a correctness lane, whichever resolves in this session. A lane whose posted body admits it fell back to a manual pass is the same verdict. Name every absent lane and its substitute on the F report's `Review lanes:` line <!-- contract-restatement: B2-lane-productivity -->
 - [ ] **C. Fetch ALL comments from ALL sources:** read every update on the PR regardless of author or format. Three API surfaces + reviews:
   - [ ] C1. Review-thread comments: `gh api --paginate "repos/<owner>/<repo>/pulls/<N>/comments?per_page=100"`
   - [ ] C2. Issue-level comments: `gh api --paginate "repos/<owner>/<repo>/issues/<N>/comments?per_page=100"` (includes AI-review summaries, user replies, bot task-completion posts)
@@ -181,7 +182,7 @@ When a channel event, Monitor notification, or poll iteration fires, complete ev
   - [ ] D7. Post a follow-up reply citing the fix commit SHA
     - [ ] **Verify follow-up reply posted, same surface routing as D5:** inline thread → `pulls/<N>/comments` filtered by `in_reply_to_id`; issue-level → `gh api --paginate "repos/<owner>/<repo>/issues/<N>/comments?per_page=100" --jq '.[] | select((.body | contains("<sha>")) and .user.login == "<posting-identity>") | .body'`. Confirm the follow-up with SHA appears on GitHub. Constrain on BOTH the SHA and the posting identity, and never on `.[-1]`. `.[-1]` is wrong because these endpoints return oldest-first, so on an unpaginated list it is the 30th-oldest comment. SHA alone is wrong because this is a control gate you act on: anyone else quoting the fix SHA, a reviewer, another bot, satisfies it, and the check reports your reply as posted when the write failed. `<posting-identity>` is the login you posted as (the bot-identity wrapper's account when the project has one, your own otherwise)
   - [ ] D7.5. Resolve review thread, **author- and classification-conditional, inline only**. Eligible only when EVERY finding in the thread carries a recorded disposition: `VALID (fix now)` with the fix pushed and cited, `VALID (defer)` grounded per D4.6 with the item id cited, or `INCORRECT` with counter-evidence posted, one dispositioned finding never makes a multi-finding thread eligible, and a single `UNCERTAIN` escalates and holds the thread open. Resolve threads opened by a BOT reviewer that you addressed. NEVER resolve HUMAN-authored threads (the human resolves their own). NEVER resolve your OWN (your posting identity. Bot or personal). **A `VALID (defer)` never clears the gate for a merge this same session performs**. `full` mode monitors and merges in one session, so route the deferral to an independent adjudicating context, or leave the thread unresolved and do not merge. Fail closed. Detect bot via the API surface in use. REST `user.type==Bot`; GraphQL `author.__typename==Bot` (resolution runs via GraphQL). Verify `isResolved == true` via GraphQL <!-- contract-restatement: D7.5-thread-eligibility --> <!-- contract-restatement: D7.5-merge-authorization -->
-- [ ] **E. Readiness gate:** ALL checks terminal + ALL comments addressed + 2-min cooldown since last activity per [readiness.md](reference/readiness.md)
+- [ ] **E. Readiness gate:** ALL checks terminal + every review lane productive or substituted per B2 + ALL comments addressed + 2-min cooldown since last activity per [readiness.md](reference/readiness.md)
 - [ ] **F, Report:** present the full readiness table OR list remaining blockers
 
 **Receiving an event is not processing it.** Each event drives at least steps A-C, and a new comment event drives D1-D7 for that comment. Readiness is declared only after E.
@@ -242,6 +243,7 @@ Public action for retrieving failed-CI evidence. Tiered fetch chain. Cheapest si
 - **Bot comments need reactions AND replies.** React (👍/👎/👀) on every substantive comment AND reply with a per-finding classification table with evidence. Don't skip any reviewer, each gets individual attention. Verify BOTH the reaction and the reply landed on GitHub via API query. Resolve the thread only when D7.5's conditions are met for EVERY finding in it, and only IF bot-authored; never human-authored, never your own. D7.5 in the checklist above is the single statement of which dispositions qualify and who may act on one, do not restate it here
 - **Monitor is async, not serial.** Process comments as they arrive while CI is still running. Don't wait for all checks to complete before reading comments, bots post at different times
 - **Check mergeable BEFORE polling CI.** `gh pr view <pr_number> --json mergeable`, if `CONFLICTING`, GitHub won't trigger workflows. Integrate the default branch first, then poll
+- **A green review lane can mean nothing was reviewed.** These lanes report on their session, not on their output: a session that ends without error is a success even when it posted no review and produced no finding, so the check row goes green and any evidence guard beside it has no posted body to judge. Cost is no signal either, since the session is billed whether or not it reviewed anything. Judge a review lane by what it produced (B2), never by its check row, and run the review locally when it produced nothing
 - **Never merge with unclassified FAILURE check runs.** Every FAILURE must be investigated, classified (real failure vs informational), and documented before merge is even suggested. See [readiness.md](reference/readiness.md) for the full 6-gate checklist
 - **"No comments" does NOT mean "ready."** Comment-only actors post at unpredictable times. A 2-minute cooldown after the last check-run completion or comment arrival prevents the race condition. See readiness.md Gate 5
 - **Security scans are always blocking.** Any check run or bot comment reporting security findings (secrets, vulnerabilities) triggers mandatory triage, even if the finding is a false positive, it must be explicitly classified and documented before merge

plugins/source-control/skills/pull-request/reference/monitor.md

@@ -407,6 +407,7 @@ When all readiness gates pass:
 **Check runs:** X passed, Y skipped, Z failed-informational
 **Security:** [scanner] evaluated, N findings classified
 **Comments:** X from N reviewers, Y fixed, Z deferred, W incorrect
+**Review lanes:** [each lane on the checks roster: productive, or ABSENT with what was run locally in its place]
 **Cooldown:** 2+ min since last activity
 **Fix iterations:** N
 **Failures classified:**

plugins/source-control/skills/pull-request/reference/readiness.md

@@ -157,6 +157,10 @@ gh api --paginate "repos/{owner}/{repo}/issues/<pr_number>/comments?per_page=100
 
   `commit_id` is the wrong field for this question: it re-anchors to the newest head while a comment's hunk still applies, so it counts surviving prior-round comments as current and short-circuits the wait. Unpaginated, the count also undercounts: the comments you are waiting on are the newest, and on a PR with prior review rounds the newest are exactly what page 1 omits. The count is slurped rather than passed to `--jq` for the reason rule 3 gives: a reduction like `length` inside `--jq` runs per page and prints one number per page, never the total.
 
+<!-- contract-restatement-begin: B2-lane-productivity -->
+- [ ] **A review lane that produced nothing for this round is SUBSTITUTED before the gate clears, not merely reported.** Take the roster for this item from `gh pr checks`, NOT from the bot authors the reviewer roster above is built from: a lane that posted nothing is missing from every author-derived roster by construction, so that roster would apply this item to an empty set, which is exactly the lane this item exists for. Its own check row cannot settle it either: these lanes report on their session rather than on their output, so a session that ends without error is green whether or not it reviewed anything, and cost is no signal since the session is billed either way. Judge the lane by the artifacts the scoping rule above attributes to this head, bounding a rerun of the same head by timestamp against the run's start, since a rerun's artifacts carry the same SHA; zero attributed artifacts means nothing was reviewed, however green the row. Run a local review over the same diff (`/review:fanout` for breadth, or the bundled `/code-review` against an explicit target for a correctness lane, whichever resolves in this session), then name every absent lane and its substitute on the verdict's `Review lanes:` line. A lane whose posted body admits it fell back to a manual pass counts as absent on the same terms. This is what the bound above hands off to: reaching the bound ends the wait, it does not supply the review.
+<!-- contract-restatement-end: B2-lane-productivity -->
+
 ### Gate 6: No pending work
 
 - [ ] No fix pushes are in flight (a push restarts the entire monitoring loop)
@@ -176,6 +180,7 @@ Only when ALL gates pass, present:
 **Comments:** X from N reviewers: Y fixed, Z deferred, W incorrect
 **Cooldown:** 2+ min since last activity
 **Reviewers:** [each discovered reviewer: responded, no-findings signal, or not yet responded at the bound with its missing artifacts named]
+**Review lanes:** [each lane on the checks roster: productive, or ABSENT with what was run locally in its place]
 **Failures classified:**
 - `review`: FAILURE, usage limit (informational, safe to proceed)
 - [any other failures with classification]

scripts/contract-clause-registry.json

@@ -173,6 +173,35 @@
           "hint": "carry the self-containment bullet with its three parts: no external requests, no remote scripts, assets inline"
         }
       ]
+    },
+    {
+      "id": "B2-lane-productivity",
+      "title": "A review lane is judged by what it produced for this round, and an absent one is substituted",
+      "canonical": "plugins/source-control/skills/pull-request/reference/readiness.md",
+      "detect": "review.?lane productivity|produced nothing for this round|\\bB2\\b",
+      "restates": "(ABSENT|absent)[^.]{0,60}(never PASS|not PASS|rather than PASS)|produced nothing for this round",
+      "qualifiers": [
+        {
+          "name": "roster-from-checks",
+          "pattern": "gh pr checks|checks roster",
+          "hint": "take the lane roster from the checks list, never from the authors who posted: a lane that posted nothing is missing from every author-derived roster by construction"
+        },
+        {
+          "name": "scoped-to-this-round",
+          "pattern": "current round|original_commit_id|this head",
+          "hint": "scope productivity to the head under review, so an artifact from an earlier head cannot satisfy the check"
+        },
+        {
+          "name": "substituted-not-just-reported",
+          "pattern": "/review:fanout|`/code-review`|local review",
+          "hint": "run a local review over the same diff; reporting the lane as absent without substituting leaves the change unreviewed"
+        },
+        {
+          "name": "substitution-reported",
+          "pattern": "Review lanes:|stood in for|what was run locally",
+          "hint": "name the absent lane and its substitute in the verdict, or the gap is dropped from a report that otherwise reads all-clear"
+        }
+      ]
     }
   ]
 }