Skip to content

Ask an After-Checks Side Reviewer Once CI Settles, and Document the Exit 44 Holds #3885

Closed

Description

@ptr727

#3881 merged with these findings from its last recorded local strict pass still open, since that push's edit budget was spent. Each was on code #3881 introduced.

  1. An explicit after-checks CodeRabbit or Qodo is never asked when CI outlasts Copilot's round. side.ask in scripts/pr_review_side.py reads the checks once, after the Copilot poll. Where a round already covers the head, or Copilot's review lands first, the read gives PENDING and only a note prints. wait exits 0, and the merge gate's run_wait passes may_request=False, so nothing asks later. Repro: give Qodo {"trigger": "explicit", "when": "after-checks"}, let a Copilot round cover the head, and keep one check in progress. wait exits 0 and posts no /review. The held path polls checks first and is not affected.
  2. The DIRTY and unreadable-rollup holds exit 44 undocumented. checks_hold returns DIRTY or UNREAD, and checks_status exits 44 for both. The wait usage text in scripts/pr_review.py and the exit table in scripts/README.md give 44 only for a check stuck in a shape no wait clears. The held path documents a DIRTY head as 45. Name both cases at 44.
  3. Design: PAYLOAD_KEY = POLICY_KEY in scripts/pr_review_policy.py gives one constant a second name. pr_review.py and the tests can read pr_review_payload.POLICY_KEY directly. Drop the alias, the test asserting the two agree, and the docstring sentences explaining the two names.
  4. Design: request_side_reviews in scripts/pr_review_side.py repeats the post-and-confirm that comment_on_pr in scripts/pr_review.py does. One helper posting a comment and confirming its URL and body would serve both.

pr_review.py sits one line under its 2200-line ceiling and pr_review_rounds.py four under its 700. So a fix for item 1 goes into pr_review_side.py. There an after-checks ask could poll checks with the held path's await_green, bounded by the wait's timeout.

The duplicated trusted-comment filter in side_asks is noted on #3880 instead.

Refs #3877.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions