Skip to content

pr-review: mention-triggered runs silently lost to ci-pending skip (self-referential check runs) #469

Closed

Description

@don-petry

Summary

Mention-triggered (FORCE_REVIEW=true) PR-review runs are silently swallowed by the CI gate in scripts/review-one-pr.sh. The bot acks "I'm on it — starting a fresh review now" and then the dispatched run exits 100 with skip: CI checks still in progress — no review, no retry, no follow-up comment.

Related: #463 (general ci-pending skip with no CI-completion re-trigger). This issue is the mention-path slice plus the self-referential check problem that makes it much more likely to fire.

Evidence (2026-06-07)

After #468 merged, @donpetry-bot review was posted simultaneously on 6 open PRs (#458, #383, #226, #152, #465, #455). All 6 dispatch runs skipped:

The pending entries in statusCheckRollup were largely caused by the mention burst itself: the mention-ack, ci-relay, and the PR Review Agent's own review/dispatch check runs attach to the PR head SHA and appear in its own rollup. The dispatched run starts ~30s after the mention, sees its sibling check runs still queued, classifies CI as pending, and exits.

A second mention round ~10 minutes later (after the rollup settled) worked and produced APPROVED verdicts — confirming the first round was lost purely to transient rollup churn.

Why this matters

  1. Same false-promise shape as the (fixed) cycle-cap defect pr-review: cycle cap counts approvals, ignores human re-requests, and escalation is unrecoverable #467: the ack promises a review that never arrives, and exit 100 means nothing retries.
  2. The CI gate runs before the FORCE_REVIEW handling, so an explicit human request — the strongest possible trigger — is the easiest to lose.

Proposed fix (for dev-lead)

In scripts/review-one-pr.sh:

  1. Exclude the agent's own check runs from CI classification — filter statusCheckRollup entries named review/dispatch/mention-ack (the PR Review Agent / Mention Trigger workflows) before computing CI_STATUS, so the cascade never blocks on itself.
  2. When FORCE_REVIEW=true and CI is pending: poll the rollup briefly (e.g. up to 5 min with 30s interval) instead of skipping immediately. If still pending after the window, post a visible reply ("CI still running — re-mention when green" or auto-requeue) instead of exiting silently.
  3. (Optional, overlaps PR Review Agent never reviews at green CI — skips on ci-pending with no CI-completion re-trigger (clean PRs stuck at REVIEW_REQUIRED) #463) on ci-pending skip, schedule/dispatch a delayed re-check so non-mention runs also recover.

Acceptance criteria

  • A @donpetry-bot review mention during check churn produces either a review or a visible explanatory comment — never a silent no-op.
  • The PR Review Agent's own check runs never cause ci-pending/ci-failing classification.
  • Bats coverage for the rollup-filtering logic.

Activity

  1. don-petry commented on Jun 8, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead Implementation Plan

    Issue: #469 — pr-review: mention-triggered runs silently lost to ci-pending skip (self-referential check runs)

    Scope

    Filter the PR Review Agent's own check runs from the CI gate and add FORCE_REVIEW polling so mention-triggered runs never silently no-op on self-induced CI churn.

    Implementation Checklist

    • Create scripts/lib/ci-status.sh — extract CI_STATUS jq logic into a testable compute_ci_status function that filters self-referential check runs (review, dispatch, ci-relay, pr-review-mention, mention-ack and their workflow / job variants)
    • Update scripts/review-one-pr.sh — source ci-status.sh, use compute_ci_status, add FORCE_REVIEW polling (up to FORCE_REVIEW_POLL_ATTEMPTS × FORCE_REVIEW_POLL_INTERVAL_SEC, default 10×30s = 5 min) when CI is pending, post visible comment if still pending after polling
    • Add tests/test_ci_status.bats — BATS coverage for the rollup-filtering logic
    • Add tests/test_ci_status.bats to the bats job in .github/workflows/lint.yml

    Tests to Write

    • compute_ci_status: empty rollup → "passing"
    • compute_ci_status: own check named review filtered — does not cause pending
    • compute_ci_status: own check named dispatch filtered
    • compute_ci_status: own check named ci-relay filtered
    • compute_ci_status: own check in workflow / job format filtered (e.g. PR Review Agent / review)
    • compute_ci_status: external pending check → "pending"
    • compute_ci_status: external failing check → "failing"
    • compute_ci_status: mix — own pending + external passing → "passing"
    • compute_ci_status: mix — own pending + external pending → "pending"
    • compute_ci_status: mix — all own checks only → "passing"
    • compute_ci_status: status-context checks not accidentally filtered

    Test Command

    bats tests/test_ci_status.bats tests/test_review_cycle.bats

  2. don-petry commented on Jun 8, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead: Implementation Complete

    Plan Execution

    • Created scripts/lib/ci-status.sh with compute_ci_status function (own-check filter + CI classification)
    • Wrote failing BATS tests in tests/test_ci_status.bats (31 tests, TDD red phase)
    • Implemented compute_ci_status — all 31 tests pass (green phase)
    • Updated scripts/review-one-pr.sh: sourced ci-status.sh, replaced inline jq, added FORCE_REVIEW polling block
    • Added tests/test_ci_status.bats to .github/workflows/lint.yml bats job

    Test Results

    1..124
    ok 94 empty rollup returns passing
    ok 95 single passing external check returns passing
    ok 101 own check named 'review' (IN_PROGRESS) is filtered out → passing
    ok 102 own check named 'dispatch' (IN_PROGRESS) is filtered out → passing
    ok 103 own check named 'ci-relay' (IN_PROGRESS) is filtered out → passing
    ok 106 'PR Review Agent / review' (IN_PROGRESS) is filtered out → passing
    ok 107 'PR Review Reusable / review' (IN_PROGRESS) is filtered out → passing
    ok 108 'PR Review — Mention Trigger / pr-review-mention' (IN_PROGRESS) is filtered out → passing
    ok 113 mix: own 'review' pending + external passing → passing
    ok 116 mix: own 'review' pending + external pending → pending
    ok 117 mix: own 'review' pending + external failing → failing
    ok 118 rollup with only own checks returns passing
    ok 124 null JSON input returns passing (safe default)
    ... (124/124 pass, 0 failures)
    

    Lint: shellcheck on 35 files + agent profile frontmatter — all checks passed.

    Files Changed

    • scripts/lib/ci-status.sh (new): compute_ci_status function — filters self-referential check runs named review, dispatch, ci-relay, pr-review-mention, mention-ack (bare and workflow / job formats) before computing pending/passing/failing
    • scripts/review-one-pr.sh: source ci-status.sh; replace 14-line inline jq with compute_ci_status; add FORCE_REVIEW polling block (defaults: 10 × 30s = 5 min) — polls for CI to settle, posts visible "CI still running — re-mention when done" comment if poll window exhausted, handles ci-failing detected during polling, refreshes REVIEW_DECISION after CI settles
    • tests/test_ci_status.bats (new): 31 BATS tests covering own-check filtering (both name forms), external check classification, mix scenarios, malformed input
    • .github/workflows/lint.yml: added tests/test_ci_status.bats to bats job

    Notes

  3. added
    dev-leadFor dev-lead agent pickup
    and removed
    dev-leadFor dev-lead agent pickup
    on Jun 8, 2026
  4. don-petry commented on Jun 8, 2026

    @don-petry
    CollaboratorAuthor
    Already working on this: an open PR exists for issue #469.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBug reportsdev-leadFor dev-lead agent pickup

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions