feat(pr-pipeline): request a whole review when a lane PR is held only by findings an incremental review cannot clear

.changes/pr-pipeline/feat-8565-request-full-review-ce021004.md

@@ -0,0 +1,7 @@
+---
+bump: minor
+---
+
+### Added
+
+- pr-merge requests a whole review of a lane pull request held only by findings an incremental review cannot clear: it records the request in the reasons comment, then converts the pull request to a draft and marks it ready, once per head, at most twice per pull request and once per run, only while fewer than 35 jobs run or wait and both tokens have 30% of each rate-limit bucket left ([README.md, Unattended merge](https://github.com/melodic-software/claude-code-plugins/blob/main/docs/conventions/pr-pipeline/README.md)).

.changes/source-control/feat-8565-request-full-review-88c15588.md

@@ -0,0 +1,7 @@
+---
+bump: minor
+---
+
+### Added
+
+- pr-merge requests a whole review of a lane pull request held only by findings an incremental review cannot clear: it records the request in the reasons comment, then converts the pull request to a draft and marks it ready, once per head, at most twice per pull request and once per run, only while fewer than 35 jobs run or wait and both tokens have 30% of each rate-limit bucket left ([README.md, Unattended merge](https://github.com/melodic-software/claude-code-plugins/blob/main/docs/conventions/pr-pipeline/README.md)).

.github/workflows/pr-merge.yml

@@ -539,7 +539,7 @@ jobs:
           for author in "${authors[@]}"; do
             for attempt in 1 2 3; do
               if gh pr list --repo "$REPO" --base main --state open --limit 500 \
-                --author "$author" --json number,headRefOid,isDraft,updatedAt,labels,body >>"$listing"; then
+                --author "$author" --json id,number,headRefOid,isDraft,updatedAt,labels,body >>"$listing"; then
                 break
               fi
               if [ "$attempt" = 3 ]; then
@@ -556,6 +556,8 @@ jobs:
               --human-gated="$human_gated" >"$RUNNER_TEMP/prefilter.json"
           jq -c .candidates "$RUNNER_TEMP/prefilter.json" >"$RUNNER_TEMP/candidates.json"
           jq -c .held "$RUNNER_TEMP/prefilter.json" >"$RUNNER_TEMP/held.json"
+          jq -cs '[add[] | select(.isDraft) | {number, id, head: .headRefOid, updatedAt,
+              labels: [.labels[].name]}] | unique_by(.number)' "$listing" >"$RUNNER_TEMP/drafts.json"
           echo "count=$(jq length "$RUNNER_TEMP/candidates.json")" >>"$GITHUB_OUTPUT"
 
       # The gate loop below can stop at its time budget, so it starts after
@@ -668,6 +670,23 @@ jobs:
             echo "::warning::the post-merge remediation exited $code (re-run or dispatch refused: the lane token has no Actions permission)"
           fi
 
+      # A whole-review request (babysit_whole_review.py) converts a lane PR
+      # to a draft and marks it ready again; a run that ended between the two
+      # left a draft the listing above skips. Each lane draft updated in the
+      # last day whose newest lane comment records a request at its current
+      # head is marked ready here: one comments read per such draft.
+      - name: Re-ready a draft a whole-review request left
+        if: steps.mode.outputs.report-only == 'false' && steps.candidates.outcome == 'success'
+        continue-on-error: true
+        env:
+          GH_TOKEN: ${{ steps.lane-token.outputs.token }}
+          REPO: ${{ github.repository }}
+        run: |
+          set -euo pipefail
+          identity=$(yq '.merge."agent-identity" // ""' .base/docs/conventions/pr-pipeline.yaml)
+          python3 .base/plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py recover \
+            --repo "$REPO" --identity "$identity" --drafts "$RUNNER_TEMP/drafts.json"
+
       # A candidate the last run refused on inputs that have not changed
       # since (head, updatedAt, labels, base, check rollup, reviews and
       # threads, comments, closing issues, verdict artifacts, the gate's
@@ -721,10 +740,14 @@ jobs:
         if: steps.candidates.outcome == 'success'
         env:
           GH_TOKEN: ${{ steps.lane-token.outputs.token }}
-          # Only the verdict reader's Actions calls use this, as their own
-          # GH_TOKEN (babysit_verify_evidence.py); the job runs main's code
-          # alone, never a PR's.
+          # Only the verdict reader's Actions calls and the whole-review
+          # request's limit reads use this, as their own GH_TOKEN
+          # (babysit_verify_evidence.py, babysit_whole_review.py); the job
+          # runs main's code alone, never a PR's.
           BABYSIT_VERIFY_GH_TOKEN: ${{ github.token }}
+          # At most one whole-review request per run: the first candidate to
+          # reach its limits creates this file (babysit_whole_review.py).
+          BABYSIT_WHOLE_REVIEW_SPENT: ${{ runner.temp }}/whole-review-spent
           # The first candidate saves its artifact listing here and the rest
           # of the run reads it while it is under two minutes old; a
           # candidate the saved listing would allow lists again before it is
@@ -788,6 +811,9 @@ jobs:
             echo "::group::PR #$number at $head: exit $code"
             cat "$out"
             echo "::endgroup::"
+            if jq -e '.unattended.writes.wholeReview.toggled == true' "$out" >/dev/null 2>&1; then
+              echo "::notice::pr-merge whole-review-toggle pr=$number head=$head"
+            fi
             # Keep only the verdict fields; a gate that printed no JSON object
             # is recorded as an error with its exit code.
             jq -e 'type == "object"' "$out" >/dev/null 2>&1 || echo '{"error": "gate printed no JSON"}' >"$out"

docs/conventions/pr-pipeline/README.md

@@ -730,6 +730,20 @@ it never closes an unchanged thread: a lane that resolved an unfixed finding and
 commit leaves it open. A resolved thread also closes when a human resolved it, or when it is
 outdated with none of its flagged lines left at the head; an unresolved thread stays open.
 
+Whole-review request (`babysit_whole_review.py`): when a lane pull request passes every other gate
+and its only risk reasons are open findings a clean whole review of the head would close (no review
+of that head reported a finding or was whole, and no lane thread is unresolved), the gate writes a
+`Whole review requested at head` line into its reasons comment, then converts the pull request to a
+draft and marks it ready with the lane token, so `ready_for_review` runs both review lanes whole.
+That event also re-runs `pr-require-checks.yml`, `pr-test-windows.yml` and pr-merge itself. The
+line, stamped with the time it was written, is kept on every rewrite, so a head is toggled once and a pull
+request at most twice; past that it waits for a person. A run toggles at most one
+pull request, and only while fewer than 35 jobs are running or queued and the lane token and the job token
+each have 30% of every rate-limit bucket's hourly limit left; an escalated comment is not toggled. Each toggle logs
+`::notice::pr-merge whole-review-toggle pr=<n> head=<sha>`. A draft a run left between the two
+writes is marked ready by the next run, only when the lane token made its newest conversion to draft within ten minutes after a record's time;
+a draft a person made stays one.
+
 While `merge.report-only` is true and `merge.dependabot.mode` is not `enforce`, the workflow starts
 no full CI run either; its summary says whether it would have.
 

plugins/source-control/skills/babysit-prs/reference/guard-contract.md

@@ -32,6 +32,7 @@ This table scopes `mutation` to DOMAIN state -- GitHub, the queue state file, wo
 | `skills/babysit-prs/scripts/pr_queue_snapshot.py` | -- | conditionally mutating | writes the queue snapshot to local state | --write-state (absent: reports the snapshot without persisting it) | No GitHub write on any path. The only domain-state mutation is the local snapshot file. | `snapshot.write-state-gates-the-only-write` |
 | `skills/babysit-prs/scripts/babysit_findings.py` | -- | read-only | nothing | n/a | Pure classification over supplied input; no write of any kind. | -- |
 | `skills/babysit-prs/scripts/babysit_merge_prefilter.py` | -- | read-only | nothing | n/a | Pure classification of the candidate list on stdin into kept and held, printed on stdout; no write of any kind and no GitHub call. | -- |
+| `skills/babysit-prs/scripts/babysit_whole_review.py` | -- | mutating | a pull request's draft state | pr-merge.yml runs `recover` with the lane token, live mode only | `recover` reads each listed draft's comments and newest conversion to draft, and marks ready a draft whose newest reasons comment (`<!-- babysit-merge-risk -->`) by the lane identity records a whole-review request, when the lane identity also made that conversion within ten minutes after a record. | `whole-review.marks-ready` |
 | `skills/babysit-prs/scripts/babysit_merge_skip.py` | -- | read-only | nothing on GitHub | n/a | Reads GitHub (one GraphQL query per 20 candidates, the artifact listing) and writes only the local files its arguments name: the kept candidates, the skipped rows, the keys and the refusal store, plus the artifact listing `BABYSIT_VERIFY_LISTING` names. No GitHub write on any path. | -- |
 | `skills/babysit-prs/scripts/pr_review_threads.py` | `scripts/source-control-review-threads` | read-only | nothing | n/a | The pull-request skill's readiness thread gate. It reads unresolved review threads and reports THREADS_OK / THREADS_BLOCKED / THREADS_UNPROVEN; it writes no file and performs no GitHub write. | -- |
 | `scripts/babysit-readiness-gate.sh` | -- | read-only | nothing | n/a | The lane's one entry point outside the skill's own scripts directory, invoked by name from SKILL.md. It counts findings and classification rows over fetched comments and reports READINESS_OK / READINESS_BLOCKED; it writes no file and performs no GitHub write. | -- |
@@ -120,6 +121,7 @@ Asserted against the entry point's source, because the mechanism -- not just the
 | ID | Entry point | Claim |
 | --- | --- | --- |
 | `refresh.uses-server-side-update-branch` | `skills/babysit-prs/scripts/refresh_pr_branch.py` | refresh_pr_branch.py never pushes. It asks GitHub to update the branch server-side via PUT repos/{repo}/pulls/{number}/update-branch, pinned with expected_head_sha. A consumer reasoning about push guards for this script is reasoning about the wrong mechanism -- it needs no local write access to the branch at all. |
+| `whole-review.marks-ready` | `skills/babysit-prs/scripts/babysit_whole_review.py` | babysit_whole_review.py recover performs a GitHub write: it marks a draft pull request ready for review through GraphQL. |
 | `review.posts-a-comment` | `skills/babysit-prs/scripts/request_review.py` | request_review.py --apply performs a GitHub write: it POSTs an issue comment carrying the review-trigger phrase. It is a mutating helper despite its read-sounding name. |
 | `prune.removes-worktrees-via-git` | `skills/babysit-prs/scripts/prune_babysit_worktrees.py` | prune_babysit_worktrees.py --apply runs `git worktree remove`, a destructive local filesystem operation. Its dry-run default is what makes the flagless form safe, not anything about the name. |
 | `snapshot.write-state-gates-the-only-write` | `skills/babysit-prs/scripts/pr_queue_snapshot.py` | pr_queue_snapshot.py's only queue-state write is the local snapshot, and --write-state is the only thing that reaches it. Nothing on any path writes to GitHub, so a flagless invocation reads the queue and persists nothing of it. Housekeeping is out of scope per the entry-point table's mutation definition: this script loads state unconditionally, so load_state may quarantine an already-corrupt state file whatever the flags say. |

plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py

@@ -151,6 +151,7 @@
 import babysit_risk
 import babysit_run_cache as run_cache
 import babysit_verify_evidence as verify_evidence
+import babysit_whole_review as whole_review
 import babysit_repo_config as repo_policy
 from babysit_state import resolve_state_dir, state_lock, write_state
 from babysit_checks import check_identity_key, classify_checks
@@ -2739,7 +2740,7 @@ def queued_entry_hold(
 
 # --- The unattended path ------------------------------------------------------------------------
 
-RISK_MARKER = "<!-- babysit-merge-risk -->"
+RISK_MARKER = whole_review.RISK_MARKER
 # Appended to the reasons comment when its refusal persisted: the rewrite
 # stamps the comment beside the label it explains, which the head-moved
 # clear (clear-escalations.mjs) needs to pair them.
@@ -3065,6 +3066,9 @@ def score_unattended(
             for lane in lanes
         ],
     }
+    # Every other gate passed and only findings a clean whole review of the
+    # head would close hold it (babysit_whole_review.py).
+    details["wholeReviewWanted"] = whole_review.wanted(policy.reason, risk, lanes)
     inputs.update(risk=risk, comments=comments)
     reason = policy.reason or (
         None if risk.score is babysit_risk.Risk.LOW else f"risk:{risk.score.name}"
@@ -3628,6 +3632,9 @@ def judge(n: int) -> str | None:
                 else merge_policy.RISK_ESCALATION_MINUTES
             )
             prior = _newest_marker_comment(comments, identity, RISK_MARKER)
+            body, report["wholeReview"] = _whole_review_record(
+                repo, policy, body, head, prior
+            )
             escalated = f"{body}\n\n{RISK_ESCALATION_LINE}"
             if prior is not None and (
                 str(prior.get("body") or "") == escalated
@@ -3643,16 +3650,25 @@ def judge(n: int) -> str | None:
                 report["riskComment"] = upsert_marker_comment(
                     repo, number, identity, body, comments
                 )
+            # Recorded first, so a failed write never toggles twice at a head.
+            if report["wholeReview"] == {"toggle": True}:
+                node_id = str(json_object(inputs.get("pr")).get("node_id") or "")
+                report["wholeReview"] = whole_review.toggle(node_id)
         except (RuntimeError, ValueError, json.JSONDecodeError) as exc:
             report["errors"].append(f"reasons comment: {exc}")
+        if report.get("wholeReview") is None:
+            report.pop("wholeReview", None)
     # A LOW score ends the refusal at this head: the identity's refusal
     # comment naming it says so once, from comments already read. An
     # escalated one stays beside the label it explains.
     elif not stopped and risk is not None and inputs.get("comments") is not None:
         head = str(json_object(inputs["pr"].get("head")).get("sha"))
         prior = _newest_marker_comment(inputs["comments"], identity, RISK_MARKER)
         text = str((prior or {}).get("body") or "")
-        cleared = risk_cleared_body(head)
+        # Every whole-review record stays, so the cap counts across heads.
+        cleared = whole_review.with_records(
+            risk_cleared_body(head), whole_review.record_lines(text)
+        )
         if (
             prior is not None
             and f" at head `{head}`" in text
@@ -3702,6 +3718,44 @@ def judge(n: int) -> str | None:
     return report
 
 
+def _whole_review_record(
+    repo: str,
+    policy: merge_policy.UnattendedPolicy,
+    body: str,
+    head: str,
+    prior: dict[str, Any] | None,
+) -> tuple[str, dict[str, Any] | None]:
+    """The reasons comment body with every whole-review record line the
+    prior comment holds, and the request: `{"toggle": True}` when this run
+    records a new one. A lane pull request scored `wholeReviewWanted` whose
+    comment is not escalated, with no record at this head and fewer than
+    `MAX_TOGGLES` in all, is recorded when this sweep's limits allow it."""
+    text = str((prior or {}).get("body") or "")
+    lines = whole_review.record_lines(text)
+    kept = whole_review.with_records(body, lines)
+    if whole_review.recorded(text, head):
+        return kept, None
+    inputs = policy.inputs or {}
+    pr = json_object(inputs.get("pr"))
+    user = json_object(pr.get("user"))
+    author = merge_policy.actor_key(
+        {"__typename": user.get("type"), "login": user.get("login")}
+    )
+    if (
+        (policy.details or {}).get("wholeReviewWanted") is not True
+        or author != str(inputs.get("identity") or "").casefold()
+        or text.endswith(RISK_ESCALATION_LINE)
+    ):
+        return kept, None
+    if len(lines) >= whole_review.MAX_TOGGLES:
+        return kept, {"toggled": False, "skipped": f"toggle-cap:{len(lines)}"}
+    refusal = whole_review.limits(repo)
+    if refusal is not None:
+        return kept, {"toggled": False, "skipped": refusal}
+    line = whole_review.record_line(head, _now())
+    return whole_review.with_records(body, [*lines, line]), {"toggle": True}
+
+
 def _identity_queue_entries(
     repo: str, branch: str, identity: str
 ) -> list[dict[str, Any]] | None: