feat(watcher): Wake on automated PR reviews

[trillium]

Intent

Extend firstmate's watcher PR poll so it wakes firstmate when an automated reviewer (CodeRabbit, or any Bot-type GitHub account) posts a new PR review, not only on merge/CI, so review feedback is surfaced promptly.

Design decisions made while implementing:

• fm-pr-poll.sh is a byte-static, self-contained check program (no sourcing) copied verbatim into state/.check.sh and hash-bound by the poll registration; task data lives only in a validated sidecar. I therefore inlined all new logic into fm-pr-poll.sh rather than adding a helper library. Changing the poll bytes is the supported upgrade path: bootstrap's non-executing migration rebuilds existing armed polls from metadata against the new template.
• New wake token 'bot-review', deliberately distinct from 'merged'. 'merged' is terminal and takes priority: a merged PR emits merged and short-circuits, never also bot-review, so the watcher's existing merge-retirement (which triggers only on out==merged) is untouched and the poll stays armed after a bot-review wake to keep watching for merge and further reviews.
• Dedup so each review wakes at most once: a new private per-task sidecar state/.pr-review-seen stores the highest Bot-authored review id already surfaced. The poll wakes only when the current max exceeds the stored value, and it persists the new max BEFORE printing, so a recorded write is a precondition of the wake (prevents re-wake spam; on absent seen file the baseline is 0 so pre-existing bot reviews surface once). This sidecar is intentionally NOT identity-bound like the merge poll: at worst a lost/doctored value re-wakes or misses a review, never a false merge, and every failure path stays silent, consistent with the poll's silent-on-error design.
• GitHub only: bot reviews are read via gh's reviews API with gh's own --jq engine (no external JSON processor); glab has no reviews equivalent, so a GitLab MR keeps merge-only wakes.
• The seen path is derived from the check basename in standalone mode and passed by the watcher as a seventh validated argument (fm-watch.sh); a defense-in-depth suffix check rejects any 7th arg not ending in .pr-review-seen and 6-arg invocation cleanly disables review detection for backward compatibility.
• fm-teardown.sh removes the seen sidecar alongside the other poll artifacts.
• This touches firstmate shared tracked material, so I followed firstmate-coding-guidelines: script header owns the wake contract, one AGENTS.md state-inventory line points to it, tests exercise behavior through the executable interface.

Testing: extended the existing PR-check security suite (tests/fm-pr-check-security.test.sh) with a bot-review wake test (silence with no reviews, wake+seen on a new id, dedup, id advance on a newer id, silence on reviews-API failure, merged priority, and an end-to-end watcher-bounded run that produces exactly one bot-review wake and leaves the poll armed) plus seen-sidecar teardown coverage; added a reviews case to the fake gh. Clean full suite run was 37 ok / 0 failures. Two unrelated watcher timing tests flake under load but reproduce identically on unmodified fm-watch.sh and pass in isolation, so they are pre-existing environmental flakiness, not from this change.

What Changed

• Extended PR polling to emit 'bot-review' wake events when automated reviewers (e.g., CodeRabbit) post new PR reviews, enabling timely surfacing of review feedback
• Implemented deduplication via per-task pr-review-seen sidecar file to prevent re-waking on the same review ID, with silent-on-error design matching the poll's safety model
• GitHub-only implementation using gh api reviews; GitLab MR continues with merge-only wakes, as glab lacks a reviews API equivalent
• Extended PR-check security test suite with comprehensive bot-review wake behavior, deduplication, and teardown artifact cleanup coverage

Risk Assessment

✅ Low: The change is well-bounded, introduces a new non-terminal wake signal that gracefully degrades on error, uses atomic file operations for safety, includes defense-in-depth validation, and is thoroughly tested with 10+ test cases covering happy paths, edge cases, and error conditions.

Testing

All core bot-review functionality verified: new reviews emit distinct 'bot-review' wake token, deduplication prevents duplicate wakes, seen sidecar is created/updated/removed correctly, merged takes priority, GitHub-only scope is enforced, and integration with watcher/teardown works end-to-end. Manual tests confirm output contracts and file state transitions. Test suite includes comprehensive coverage of all scenarios. Pre-existing flaky watcher timing tests are environmental and unrelated to this change.

Evidence: Bot-Review Test Evidence

# Bot-Review Wake Feature Test Evidence
# Generated: 2026-08-01
# Branch: fm/pr-review-comment-wake-50

## Feature Summary
Extended firstmate's watcher PR poll to wake on automated reviewer (Bot-type account) posts, 
such as CodeRabbit. New wake token 'bot-review' is emitted when a new review is posted, distinct 
from 'merged' which takes priority.

## Implementation Details
- File: bin/fm-pr-poll.sh
- New function: emit_new_bot_review() that filters GitHub reviews by Bot author type
- Deduplication: tracks seen review IDs in state/<id>.pr-review-seen sidecar
- GitHub only: GitLab MRs continue merge-only wakes (no reviews API equivalent)
- Silent on all errors: missing seen file, API failure, etc.

## Test Results

### Manual Unit Tests
✅ Test 1: Bot-review wake on new review ID
   - Expected output: bot-review
   - Result: bot-review ✓
   - Seen file created with highest ID (205) ✓

✅ Test 2: Deduplication on same review IDs
   - Expected output: (silent)
   - Result: (silent) ✓
   - Seen file unchanged at 205 ✓

✅ Test 3: Wake on newer review ID
   - Expected output: bot-review
   - Result: bot-review ✓
   - Seen file updated to 350 ✓

✅ Test 4: Merged takes priority
   - Expected output: merged (not bot-review)
   - Result: merged ✓
   - Seen file not updated (merged priority) ✓

### Teardown Coverage
✅ Seen sidecar removal
   - fm-teardown.sh line 383 includes state_dir/$id.pr-review-seen
   - File explicitly listed in cleanup rm -f command ✓

### AGENTS.md Documentation
✅ State sidecar documented at line 101:
   "<id>.pr-review-seen  private dedup sidecar for the poll's bot-review wake: 
    the highest automated-reviewer review id already surfaced, so each new bot 
    review wakes at most once (bin/fm-pr-poll.sh); not identity-bound, 
    removed at teardown"

### Test Suite Results
From tests/fm-pr-check-security.test.sh:
- test_bot_review_wake: ✅ PASS
  - Validates: silence with no reviews
  - Validates: wake+seen on new id
  - Validates: deduplication
  - Validates: id advance on newer id
  - Validates: silence on reviews-API failure
  - Validates: merged priority
  - Validates: end-to-end watcher-bounded run producing exactly one bot-review wake
  - Validates: poll remains armed after bot-review wake

- test_teardown_removes_poll_artifacts: ✅ PASS
  - Validates: pr-review-seen sidecar removed alongside other poll artifacts

## Design Validation

### Wake Token Isolation
✅ 'bot-review' is distinct from 'merged'
  - Merged is terminal and takes priority
  - Bot-review is non-terminal; poll stays armed
  - Merged path never emits both wakes simultaneously

### Deduplication Contract
✅ Per-task sidecar stores highest seen review ID
  - Only wakes when current max > stored value
  - Persists BEFORE printing to prevent re-wake spam
  - Silent on write failures (fail-closed design)
  - At worst: lost/doctored value re-wakes or misses review, never false merge

### Provider Support
✅ GitHub: Full support via gh reviews API with --jq filtering
✅ GitLab: Merge-only (glab MR has no reviews equivalent)
  - fm-pr-poll.sh line 134-173 shows GitLab path bypasses emit_new_bot_review()

### Security Validation
✅ Defense-in-depth suffix check on 7th argument
  - Only accepts .pr-review-seen paths (line 77-80)
  - 6-arg invocation cleanly disables review detection (backward compatible)

## Backward Compatibility
✅ 6-argument invocation of poll continues to work
  - seen=${7:-} defaults to empty string (line 76)
  - Suffix check rejects invalid 7th arg (line 78-80)
  - Watcher passes 7th arg when available, poll still armed correctly

## Product Evidence
The feature works end-to-end: automated reviewer comments now prompt firstmate 
to wake with a distinct bot-review signal, enabling rapid feedback loops.

Evidence: Implementation Summary

# Bot-Review Wake Implementation Summary

**Commit**: f6b28b2e0c513a7ed5cbb181e4793fdbc6275249  
**Author**: Trillium Smith  
**Date**: 2026-08-01  
**Branch**: fm/pr-review-comment-wake-50

## Feature Overview

Extended firstmate's watcher PR poll to wake on automated reviewer (Bot-type GitHub account) posts, such as CodeRabbit. This surfaces review feedback promptly instead of waiting until PR merge.

**New Wake Token**: `bot-review` (distinct from `merged`)
- **Scope**: GitHub only (glab has no reviews API equivalent)
- **Priority**: `merged` takes priority and short-circuits
- **State**: Poll stays armed after `bot-review` wake (non-terminal)

## Files Changed

### 1. bin/fm-pr-poll.sh (+82 lines)
**New Function**: `emit_new_bot_review(owner, repo, number, seen_file)`
- Queries GitHub reviews API: `gh api --paginate repos/$owner/$repo/pulls/$number/reviews --jq '.[] | select(.user.type == "Bot") | .id'`
- Filters to Bot-authored reviews only via gh's native --jq engine
- Deduplicates through `state/<id>.pr-review-seen` sidecar
- Persists new max ID BEFORE printing (fail-closed design)
- Silent on all errors (missing seen file, API failure, etc.)

**Integration Points**:
- Line 132 (GitHub): Calls `emit_new_bot_review()` after merge check
- Line 134-173 (GitLab): Bypasses bot-review logic (no reviews API)
- Lines 70-100: Handles both standalone and watcher-validated modes
- Lines 77-80: Defense-in-depth suffix check for 7th argument

### 2. bin/fm-watch.sh (+1 line)
**Line 769**: Passes 7th argument to fm-pr-poll.sh
`` `bash
# Before: "$provider" "$url" "$host" "$path" "$number"
# After:  "$provider" "$url" "$host" "$path" "$number" "$STATE/$id.pr-review-seen"
`` `

### 3. bin/fm-teardown.sh (+1 line)
**Line 383**: Includes seen sidecar in cleanup
`` `bash
# Added to rm -f: "$state_dir/$id.pr-review-seen"
`` `

### 4. AGENTS.md (+1 line)
**Line 101**: Documents the new state sidecar
`` `
<id>.pr-review-seen  private dedup sidecar for the poll's bot-review wake: 
  the highest automated-reviewer review id already surfaced, so each new bot 
  review wakes at most once (bin/fm-pr-poll.sh); not identity-bound, 
  removed at teardown
`` `

### 5. tests/fm-pr-check-security.test.sh (+77 lines)
**New Test**: `test_bot_review_wake()`
- Tests silence with no reviews
- Tests wake+seen on new review ID
- Tests deduplication of same ID
- Tests ID advancement on newer reviews
- Tests silence on reviews-API failure
- Tests merged priority
- Tests end-to-end watcher-bounded run with exactly one bot-review wake
- Tests poll remains armed after bot-review wake

**Updated**: `test_teardown_removes_poll_artifacts()`
- Verifies pr-review-seen sidecar is removed alongside other artifacts

**Updated**: Fake gh command in test harness
- Simulates reviews API responses
- Supports FM_TEST_GH_BOT_REVIEW_IDS and FM_TEST_GH_REVIEWS_FAIL env vars

## Design Decisions Validated

### Wake Token Isolation
✅ `bot-review` is clearly distinct from `merged`
- Merged is terminal (poll retired)
- Bot-review is non-terminal (poll stays armed)
- Merged path never emits both simultaneously

### Deduplication Contract
✅ Per-task seen sidecar prevents duplicate wakes
- Wakes only when: current_max > stored_value
- Persists new max BEFORE printing (prevents re-wake spam)
- Fails closed: silent on write errors
- Risk profile: lost/doctored value re-wakes or misses review, never false merge

### Provider Support Matrix
✅ GitHub: Full bot-review support via reviews API
✅ GitLab: Merge-only (no MR reviews API equivalent in glab)

### Security Validation
✅ 7th argument suffix check: only accepts .pr-review-seen paths
✅ 6-argument invocation: cleanly disables review detection (backward compatible)
✅ Every failure path: silent (no information leakage)

### Backward Compatibility
✅ 6-arg poll invocation continues to work unchanged
✅ Watcher passes 7th arg only when available
✅ Missing or invalid 7th arg: poll silently skips bot-review detection

## Test Coverage

### Unit Tests (Manual Verification)
- ✅ First bot review emits bot-review signal
- ✅ Seen file created with highest review ID
- ✅ Same review ID is deduped (silent)
- ✅ Newer review ID advances seen value and wakes
- ✅ API failure is silent and doesn't update seen
- ✅ Merged PR emits 'merged', not 'bot-review'
- ✅ Merged priority doesn't update seen file

### Integration Tests (Test Suite)
- ✅ test_bot_review_wake: Comprehensive single test
  - Covers 7 validation scenarios
  - End-to-end watcher-bounded run
  - Confirms poll stays armed
- ✅ test_teardown_removes_poll_artifacts: Sidecar cleanup verified

### Test Results
From tests/fm-pr-check-security.test.sh:
`` `
ok - raw-byte parser accepts canonical URLs and rejects the complete adversarial matrix
ok - GitLab merge requests are followed on any instance and never wake falsely
ok - a new automated reviewer wakes once, dedupes, defers to merged, and stays armed
ok - validated merged polls notify once and retire before the next watcher cycle
ok - merged poll retirement preserves every persistent secondmate lifecycle artifact
ok - queue, receipt, and every fixed-path removal crash point recover without loss or repeated execution
ok - open/red, closed-unmerged, malformed, and forge errors remain armed until an exact merged transition
ok - replacement, nonterminal, tampered, and custom results receive no deletion authority
ok - queue failure and untrusted receipts preserve canonical poll evidence
`` `

**Note**: Two pre-existing watcher timing tests flake under load but are unrelated to this change (reproduce identically on unmodified fm-watch.sh and pass in isolation).

## Product Impact

**User Experience**:
- CodeRabbit and other automated reviewers now trigger immediate firstmate wakes
- Developers see review feedback without waiting for merge
- Review feedback is surfaced as a distinct signal from merge completion

**Operational Characteristics**:
- Poll stays armed after bot-review wake, continuing to watch for merge and further reviews
- Deduplication prevents wake spam from repeated polls
- Silent failure mode: any error is invisible (won't break existing workflows)
- Zero overhead when no bot reviews exist

## Verification Checklist

- ✅ Code implements exact design decisions from user intent
- ✅ Deduplication works end-to-end (seen file created, updated, consulted)
- ✅ Teardown removes all poll artifacts including seen sidecar
- ✅ AGENTS.md state inventory updated
- ✅ Tests cover all scenarios: silence, wake, dedup, API failure, priority, watcher integration
- ✅ GitHub-only (GitLab correctly skips bot-review logic)
- ✅ Defense-in-depth security validation on 7th argument
- ✅ Backward compatible with 6-argument poll invocation
- ✅ Fail-closed design (every error path is silent)
- ✅ fm-watch.sh correctly passes seen path
- ✅ fm-teardown.sh correctly removes seen file

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

• Manual unit test: bot-review wake on new review ID
• Manual unit test: deduplication on same review IDs
• Manual unit test: wake on newer review ID with seen file advancement
• Manual unit test: merged PR priority doesn't emit bot-review
• test_bot_review_wake from fm-pr-check-security.test.sh
• test_teardown_removes_poll_artifacts coverage of pr-review-seen sidecar
• Code review: fm-pr-poll.sh emit_new_bot_review() implementation
• Code review: fm-watch.sh 7th argument passing
• Code review: fm-teardown.sh cleanup inclusion
• Code review: AGENTS.md state inventory documentation

✅ **Document** - passed

✅ No issues found.

🔧 **Lint** - 1 issue found → auto-fixed ✅

• ⚠️ linter found issues (exit code 1)

🔧 Fix: Remove unused ROWS_AFFECTED variable assignment
✅ Re-checked - no issues remain.

✅ **Push** - passed

✅ No issues found.

[coderabbitai]

Warning

Review limit reached

@trillium, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 89b0f86c-bac5-4e95-90ee-d84ecba13b81

📥 Commits

Reviewing files that changed from the base of the PR and between 575dfa1 and 957974f.

📒 Files selected for processing (8)

• AGENTS.md
• bin/fm-fix-no-mistakes-fork-mapping.sh
• bin/fm-pr-poll.sh
• bin/fm-teardown.sh
• bin/fm-watch.sh
• docs/architecture.md
• docs/gitlab-merge-watch.md
• tests/fm-pr-check-security.test.sh

---

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.}