pr-batch: do not chase reviewer re-review prompts when the merge gate is already satisfied

[justin808]

Fixes #702

deferred_to_update_changelog

This keeps review-cohort restarts tied to the repository review_gate seam and current-head evidence instead of treating every push as an automatic restart. Advisory reviewer UI prompts stay metadata only, while branch protection, required reviewers/checks, unresolved threads, substantive deltas, and UNKNOWN evidence still fail closed.

Checks:

• ruby skills/pr-batch/bin/review-wave-contract-test.rb
• ruby skills/pr-batch/bin/goal-completion-contract-test.rb
• ruby skills/pr-batch/bin/user-facing-coordination-contract-test.rb
• ruby skills/plan-pr-batch/scripts/check_goal_prompt_size.rb
• git diff --check
• bin/lint (stopped after rubocop and markdownlint because yamllint 1.37.1 is not installed in this environment)

Files touched:

• review-wave contract and closeout wording
• GMCC compact contract line and prompt-size guard
• monitoring/workflow wording for advisory review restarts
• prompt-template size trims in pr-batch and Plan To Goal Handoff

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[justin808]

Closeout receipt:

• Draft PR published: pr-batch: do not chase reviewer re-review prompts when the merge gate is already satisfied #708
• Checks passed locally: ruby skills/pr-batch/bin/review-wave-contract-test.rb, ruby skills/pr-batch/bin/goal-completion-contract-test.rb, ruby skills/pr-batch/bin/user-facing-coordination-contract-test.rb, ruby skills/plan-pr-batch/scripts/check_goal_prompt_size.rb, git diff --check
• bin/lint stopped after rubocop and markdownlint because yamllint 1.37.1 is not installed here
• Security preflight: SECURITY_PREFLIGHT_OK; no exact risk token
• Current-head hosted checks at snapshot: claude-review queued, Lint queued, validate queued, CodeRabbit success
• Review threads at snapshot: none
• Claim release attempt: agent-coord release reported no claim exists for shakacode/agent-workflows#708

[claude]

Stale contradictory text left in the same section. This paragraph correctly implements the new conditional restart rule ("restart only the affected cohort(s)... when the repository review_gate seam... requires fresh review"), but lines 873-875 of this very same ### Review-Wave And Validation Cohorts section still say the old, unconditional rule: "A push invalidates both review-wave and validation-CI evidence for the previous head; restart both cohorts on the new head." An agent reading this section top-to-bottom hits directly conflicting instructions about whether a push always restarts both cohorts or only when review_gate requires it. Line 873-875 should be updated to match this paragraph (or removed as now-redundant).

Failure scenario: an agent following the first (unpatched) sentence at line 873-875 always restarts the review cohort on every push, even when the repository's review_gate seam marks it stale-only for validation — burning reviewer quota and re-triggering review agents unnecessarily, which is the exact behavior this PR set out to prevent.

[claude]

Stale unconditional "restart both cohorts" text, contradicting the rule just added above. This bullet ("If a required validation fix is ready, push it and restart both cohorts.") still uses the old unconditional wording. It directly follows step 3 (lines ~143-155), which this PR just updated to say a push "restarts the review cohort only when the repository review_gate seam or current-head evidence requires fresh review." The parallel bullet in workflows/pr-batch-integration-closeout.md's Coordinator Closeout Lane was correctly updated to "restart the affected cohort(s)" — this one in pr-monitoring was missed.

Failure scenario: an agent monitoring a PR reaches step 4, pushes a ready validation fix, and (per this literal instruction) restarts the review cohort unconditionally even when review_gate says the existing review evidence is still valid — causing an unnecessary re-review cycle that this PR is otherwise trying to eliminate.

[claude]

Two issues with this new "Review-Wave Compatibility" section:

1. It duplicates, and is contradicted by, the existing "## Review And Readiness" section further down this same file (around line 409-420). That section still has the old unconditional text: "A push invalidates both review-wave and validation-CI evidence for the previous head; restart both cohorts on the new head." That's the opposite of what this new paragraph says ("A push restarts the review cohort only when the repository review_gate seam ... requires fresh review"). A reader of this single doc gets two different answers to the same question depending on which section they read.

2. This paragraph omits the fail-closed safety sentence present in every other copy of this rule (workflows/pr-processing.md, workflows/pr-batch-integration-closeout.md, skills/pr-monitoring/SKILL.md): "Branch protection, a required reviewer/check, unresolved thread, substantive delta, or UNKNOWN evidence still forces fresh review." Without it, this doc's version of the rule reads as if the review_gate-advisory shortcut always applies, with no fail-closed guard.

Failure scenario: an operator relying only on this doc's "Review-Wave Compatibility" paragraph believes a trivial delta can always skip re-review once review_gate marks AI reviewers advisory, without the fail-closed carve-outs for branch protection / required reviewer / unresolved thread / UNKNOWN evidence that the other canonical copies require.

[claude]

The new review_gate? token in auto-clear=>watch(review_gate?same:0wake,delta:gates) introduces a ?:-style conditional that isn't used anywhere else in this compact DSL (every other operator in the line is :, |, ;, or =>), and the explanatory prose two paragraphs below ("The auto-clear=>watch(review_gate?same:0wake,delta:gates) phrase ... is the preferred watcher.") never actually defines what review_gate? tests or what happens on the "false" side. The doc explicitly claims GMCC-v5's "inline semantics remain normative when the workflow reference is missing or cannot autoload" — but this token isn't self-explanatory without the surrounding prose that documents review_gate as a seam concept.

Failure scenario: an agent that only has the compact GMCC-v5 line available (per the "inline semantics remain normative" fallback) has no way to correctly interpret review_gate?same:0wake,delta:gates and may guess its meaning incorrectly, applying the wrong watcher/restart policy.

[claude]

This new test checks @workflow, @integration_closeout, and @pr_monitoring for REVIEW_GATE_SEAM_RESTART/REVIEW_PROMPT_METADATA/REVIEW_FAIL_CLOSED, and @workflow/@integration_closeout for REVIEW_TRIVIAL_DELTA, but it never checks @docs (docs/pr-batch-skills.md, already loaded in setup) for any of these new rules — even though other tests in this same file (e.g. test_pr_entry_points_preserve_the_same_review_wave_contract) do check @docs for the older shared rules. That gap is why docs/pr-batch-skills.md can (and does, see the review comment on that file) drift out of sync with the review_gate restart policy without failing CI.

Suggested fix: add @docs to the loop(s) here (and consider a refute_includes guard against the retired unconditional "restart both cohorts" phrasing) so a future edit that forgets to update docs/pr-batch-skills.md fails the test suite instead of silently shipping a stale doc.