pr-review: don't skip labeled PRs that are already approved

Open
  • Is something wrong?
#11 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
Half a day
Newbie friendliness
45/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
bash, github, json
Domain
devtools

Research direction

The filter that drops approved PRs is the jq select(.reviewDecision != "APPROVED") in skills/repoman/scripts/pr-review-scanner.sh. Read how the scanner decides eligibility and how state.json and the <!-- reviewed: <sha> --> marker prevent duplicates. Done when a labeled, non-draft approved PR appears as a candidate in latest.json, an already-reviewed head SHA is still skipped, and the eligibility rule is documented in SKILL.md. The issue also asks whether a skip_approved config flag is wanted, which is an open design question.

Written by the indexing model from the issue text.

Description

Problem

pr-review-scanner.sh drops PRs whose reviewDecision is APPROVED, even if they carry ready-for-ai-review:

# Filter out bot's own PRs, drafts, and already-approved PRs
select(.reviewDecision != "APPROVED")

(skills/repoman/scripts/pr-review-scanner.sh, at b3f128c)

So whether a labeled PR gets an AI review depends on timing, not intent:

  • If the scanner reaches it before a human approves it, it gets an AI review, and later a human approval: 2 reviews.
  • If a human approves first, the scanner never picks it up: 1 review.

Real case: rossoctl/cortex#1535. Labeled at 15:37Z, approved at 15:42Z, merged at 16:44Z, with no AI review. The reviewer was healthy: the OpenClaw scan logs show "0 eligible" across that window. OpenClaw has the same filter, so moving to RepoMan won't fix this by itself.

An AI review from a different model is useful even after a human approval. The author added the label to ask for one.

Proposal

Treat ready-for-ai-review as an explicit opt-in. Labeled PRs are eligible no matter what reviewDecision is. Keep the other filters (bot-authored, draft).

Duplicates are already handled by state.json (in_progress / reviewed) and the <!-- reviewed: <sha> --> marker. This change still gives one review per labeled head SHA. (Note: rossoctl/repoman#9 did get a second review on an unchanged head, 81f19ab, so that path may need a look too.)

Optional: if a skip-approved default is still wanted for other setups (e.g., future org-wide or unlabeled enrollment), make it a per-program config flag, e.g. skip_approved in ~/.repoman/programs/pr-review.json. Default it to false when labels_required is set.

Acceptance criteria

  • A labeled, non-draft PR with reviewDecision == APPROVED shows up in latest.json as a candidate
  • A PR already reviewed at its current head SHA is still skipped (no duplicate)
  • A new push to an approved, labeled PR gets reviewed at the new head
  • SKILL.md / docs describe the eligibility rule

Context

Raised by @huang195 in #rosso-automation on 2026-10-29. Root-caused with scan logs from @rubambiza.

Generated by RepoMan

Dominant language
Shell
Stars
7
Forks
1
Avg merge
5d 10h
Merged PRs (30d)
2

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from rossoctl/repoman

All issues in rossoctl/repoman

Similar issues

More Shell/Bash issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.