pr-polish skill

PR Polish

Goal. Drive a PR to merge-ready by alternating /pr-review and /pr-address until all of the following hold:

1. The most recent /pr-review produces zero new findings (no new inline comments, no new top-level reviews with a non-empty body).
2. Every inline review thread reachable via GraphQL reports isResolved: true.
3. Every non-bot, non-author top-level review has been acknowledged (replied-to) OR resolved via a thread it spawned.
4. Every non-bot, non-author issue comment has been acknowledged (replied-to).
5. Every CI check is conclusion: "success" or "skipped" / "neutral" — none "failure" or still pending.
6. Two consecutive post-CI polls (≥60s apart) stay clean — no new threads, no new non-empty reviews, no new issue comments. Bots (coderabbitai, sentry, autogpt-pr-reviewer) frequently post late after CI settles; a single green snapshot is not sufficient.
7. No requested-but-unanswered review. If a /review comment was posted on the PR and the review bot has not replied yet, quiet polls do not count toward condition 6 — keep polling until the review lands or the wait budget expires. See Waiting on a requested bot review.

Do not stop at a fixed number of rounds. If round N introduces new comments, round N+1 is required. Cap at MAXROUNDS = 10 as a safety valve, but expect 2–5 in practice.

TodoWrite

Before starting, write two todos so the user can see the loop progression:

• Round {current}: /pr-review + /pr-address on PR #{N} — current iteration.
• Final polish polling: 2 consecutive clean polls, CI green, 0 unresolved — runs after the last non-empty review round.

Update the current round counter at the start of each iteration; mark completed only when the round's address step finishes (all new threads addressed + resolved).

Find the PR

ARG_PR="${ARG:-}"
# Normalize URL → numeric ID if the skill arg is a pull-request URL.
if [[ "$ARG_PR" =~ ^https?://github\.com/[^/]+/[^/]+/pull/([0-9]+) ]]; then
  ARG_PR="${BASH_REMATCH[1]}"
fi
PR="${ARG_PR:-$(gh pr list --head "$(git branch --show-current)" --repo Significant-Gravitas/AutoGPT --json number --jq '.[0].number')}"
if [ -z "$PR" ] || [ "$PR" = "null" ]; then
  echo "No PR found for current branch. Provide a PR number or URL as the skill arg."
  exit 1
fi
echo "Polishing PR #$PR"

The outer loop

round = 0
while round < _MAX_ROUNDS:
    round += 1
    baseline = snapshot_state(PR)   # see "Snapshotting state" below
    invoke_skill("pr-review", PR)   # posts findings as inline comments / top-level review
    findings = diff_state(PR, baseline)
    if findings.total == 0:
        break  # no new findings → go to polish polling
    invoke_skill("pr-address", PR)  # resolves every unresolved thread + CI failure
# Post-loop: polish polling (see below).
polish_polling(PR)

Optional: the review bot

invokeskill("pr-review", PR) is the agent's own** review and always runs every round. In addition to it, the repo's review bot can be summoned on demand by commenting /review on the PR (see the open-pr skill):

gh pr comment "${PR}" --body "/review"

This is an option, not a required step, and it is not a substitute for the /pr-review round — use it when you want a second opinion the outer loop cannot produce itself, or when the PR carries no bot findings at all yet. Bot findings land as inline threads and top-level reviews, exactly like the agent's own findings, so both feed the same invoke_skill("pr-address", PR) step — no separate handling is needed. If you do request one, the exit conditions must wait for it; see Waiting on a requested bot review.

Snapshotting state

Before each /pr-review, capture a baseline so the diff after the review reflects only what the review just added (not pre-existing threads):

# Inline threads — total count + latest databaseId per thread
gh api graphql -f query="
{
  repository(owner: \"Significant-Gravitas\", name: \"AutoGPT\") {
    pullRequest(number: ${PR}) {
      reviewThreads(first: 100) {
        totalCount
        nodes {
          id
          isResolved
          comments(last: 1) { nodes { databaseId } }
        }
      }
    }
  }
}" > /tmp/baseline_threads.json

# Top-level reviews — count + latest id per non-empty review
gh api "repos/Significant-Gravitas/AutoGPT/pulls/${PR}/reviews" --paginate \
  --jq '[.[] | select((.body // "") != "") | {id, user: .user.login, state, submitted_at}]' \
  > /tmp/baseline_reviews.json

# Issue comments — count + latest id per non-bot, non-author comment.
# Bots are filtered by User.type == "Bot" (GitHub sets this for app/bot
# accounts like coderabbitai, github-actions, sentry-io). The author is
# filtered by comparing login to the PR author — export it so jq can see it.
AUTHOR=$(gh api "repos/Significant-Gravitas/AutoGPT/pulls/${PR}" --jq '.user.login')
# Slash-command triggers are excluded here too: a `/review` posted by a
# maintainer is non-bot and non-author, so without this it enters the baseline
# 

Diffing after a review

After /pr-review runs, any of these counting as "new findings" means another address round is needed:

• New inline thread id not in the baseline.
• An existing thread whose latest comment databaseId is higher than the baseline's (new reply on an old thread).
• A new top-level review id with a non-empty body.
• A new issue comment id from a non-bot, non-author user.

If any of the four buckets is non-empty → not done; invoke /pr-address and loop.

Polish polling

Once /pr-review produces zero new findings, do not exit yet. Bots (coderabbitai, sentry, autogpt-pr-reviewer) commonly post late reviews after CI settles — 30–90 seconds after the final push. Poll at 60-second intervals:

NON_SUCCESS_TERMINAL = {"failure", "cancelled", "timed_out", "action_required", "startup_failure"}
clean_polls = 0
required_clean = 2
while clean_polls < required_clean:
    # 1. CI gate — any terminal non-success conclusion (not just "failure")
    # must trigger /pr-address. "success", "skipped", "neutral" are clean;
    # anything else (including cancelled, timed_out, action_required) is a
    # blocker that won't self-resolve.
    ci = fetch_check_runs(PR)
    if any ci.conclusion in NON_SUCCESS_TERMINAL:
        invoke_skill("pr-address", PR)  # address failures + any new comments
        baseline = snapshot_state(PR)   # reset — push during address invalidates old baseline
        clean_polls = 0
        continue
    if any ci.conclusion is None (still in_progress):
        sleep 60; continue  # wait without counting this as clean

    # 2. Comment / thread gate
    threads = fetch_unresolved_threads(PR)
    new_issue_comments = diff_against_baseline(issue_comments)
    new_reviews = diff_against_baseline(reviews)
    if threads or new_issue_comments or new_reviews:
        invoke_skill("pr-address", PR)
        baseline = snapshot_state(PR)   # reset — the address loop just 

Only after clean_polls == 2 do you report ORCHESTRATOR:DONE.

Concrete CI fetch (don't parse gh pr checks text columns)

The fetchcheckruns(PR) step above must use --json, not the default text output. Job names can contain spaces and parentheses (e.g. test (3.11), Analyze (python)), so gh pr checks $PR | awk '{print $2}' extracts (3.11) instead of the status — leading to a clean-poll firing while jobs are still pending.

# Reliable: use --json so columns are unambiguous.
ci_json=$(gh pr checks $PR --repo Significant-Gravitas/AutoGPT --json name,state,bucket)
pending=$(echo "$ci_json" | jq '[.[] | select(.bucket == "pending")] | length')
failed=$(echo "$ci_json" | jq '[.[] | select(.bucket == "fail" or .bucket == "cancel")] | length')

# Buckets are: pass | fail | pending | cancel | skipping
# (NOTE: gh pr checks does NOT expose `conclusion` as a JSON field —
# only `bucket`. Don't confuse with the GitHub REST API's check_runs
# endpoint, which DOES use conclusion.)

Map back to the pseudocode above: bucket == "pending" is ci.conclusion is None (still inprogress); bucket in {"fail", "cancel"} is ci.conclusion in NONSUCCESS_TERMINAL; bucket in {"pass", "skipping"} is clean.

Waiting on a requested bot review

autogpt-pr-reviewer[bot] typically takes ~30 minutes to answer a /review — far longer than the 60-second poll interval, so without this gate two clean polls would report ORCHESTRATOR:DONE while the requested review is still in flight.

reviewrequestedbut_unanswered(PR) is:

Recompute from the API on every poll — never track elapsed time in a loop variable, which is how this gate becomes an infinite wait. --paginate concatenates one array per page, so last picks the last item of the final page rather than the newest overall; use --slurp and maxby. An answer is either* a top-level review or an inline review comment, whichever is later — an inline-only reply is still an answer, and counting only top-level reviews left the gate waiting after its threads had already been addressed.

# `--slurp` cannot be combined with `--jq`; pipe to standalone jq instead.
REQUESTED_AT=$(gh api "repos/Significant-Gravitas/AutoGPT/issues/${PR}/comments" --paginate --slurp \
  | jq -r '[.[][] | select((.body // "") | test("^\\s*/review\\s*$"))] | max_by(.created_at) | .created_at // empty')
REVIEWED_AT=$(gh api "repos/Significant-Gravitas/AutoGPT/pulls/${PR}/reviews" --paginate --slurp \
  | jq -r '[.[][] | select(.user.login == "autogpt-pr-reviewer[bot]")] | max_by(.submitted_at) | .submitted_at // empty')
COMMENTED_AT=$(gh api "repos/Significant-Gravitas/AutoGPT/pulls/${PR}/comments" --paginate --slurp \
  | jq -r '[.[][] | select(.user.login == "autogpt-pr-reviewer[bot]")] | max_by(.created_at) | .created_at // empty')
ANSWERED_AT=$(printf '%s\n%s\n' "$REVIEWED_AT" "$COMMENTED_AT" | grep -v '^$' | sort | tail -1)

# Pending iff a request exists and no answer is strictly newer than it.
# Start every poll from `false`: a stale `true` from the previous poll would
# otherwise keep the gate waiting after the answer has already landed.
# `[ a \< b ]` is not portable (fails under zsh), so compare via sort.
pending=false
NEWEST=$(printf '%s\n%s\n' "$REQUESTED_AT" "$ANSWERED_AT" | gre

REVIEWWAITBUDGET = 60 minutes, measured as now - REQUESTED_AT:

# Only meaningful when a request exists: GNU date reads an empty string as
# "now", BSD date errors, and either way there is nothing to time out.
if [ -n "$REQUESTED_AT" ]; then
  WAITED=$(( $(date +%s) - $(date -u -j -f "%Y-%m-%dT%H:%M:%SZ" "$REQUESTED_AT" +%s 2>/dev/null || date -u -d "$REQUESTED_AT" +%s) ))
  [ "$WAITED" -ge 3600 ] && pending=false   # budget spent — stop waiting
fi

An observed reply on this repo took 46 minutes, so a 45-minute budget would have abandoned a review that was about to arrive. 60 gives headroom.

Past the budget, stop waiting, resume counting clean polls normally, and note in the final report that the requested bot review never arrived. The loop must never hang on a bot.

Never post a second /review while one is unanswered. Check reviewrequestedbut_unanswered(PR) before triggering, not just before exiting — otherwise each round requests another review and the bot answers a queue of duplicates.

Why 2 clean polls, not 1

A single green snapshot can be misleading — the final CI check often completes ~30s before a bot posts its delayed review. One quiet cycle does not prove the PR is stable; two consecutive cycles with no new threads, reviews, or issue comments arriving gives high confidence nothing else is incoming.

Why checking every source each poll

/pr-address polling inside a single round already re-checks its own comments, but /pr-polish sits a level above and must also catch:

• New top-level reviews (autogpt-pr-reviewer sometimes posts structured feedback only after several CI green cycles).
• Issue comments from human reviewers (not caught by inline thread polling).
• Sentry bug predictions that land on new line numbers post-push.
• Merge conflicts introduced by a race between your push and a merge to dev.

Invocation pattern

Delegate to existing skills with the Skill tool; do not re-implement the review or address logic inline. This keeps the polish loop focused on orchestration and lets the child skills evolve independently.

Skill(skill="pr-review",  args=pr_url)
Skill(skill="pr-address", args=pr_url)

After each child invocation, re-query GitHub state directly — never trust a summary for the stop condition. The orchestrator's ORCHESTRATOR:DONE is verified against actual GraphQL / REST responses per the rules in pr-address's "Verify actual count before outputting ORCHESTRATOR:DONE" section.

Auto-continue: do NOT end your response between child skills

/pr-polish is a single orchestration task — one invocation drives the PR all the way to merge-ready. When a child Skill() call returns control to you:

• Do NOT summarize and stop.
• Do NOT wait for user confirmation to continue.
• Immediately, in the same response, perform the next loop step: state diff → decide next action → next Skill() call or polling sleep.

The child skill returning is a loop iteration boundary, not a conversation turn boundary. You are expected to keep going until one of the exit conditions in the opening section is met (2 consecutive clean polls, MAXROUNDS hit, or an unrecoverable error).

If the user needs to approve a risky action mid-loop (e.g., a force-push or a destructive git operation), pause there — but not at the routine "round N finished, round N+1 needed" boundary. Those are silent transitions.

Run /pr-polish in the foreground — never in a background agent

Spawning /pr-polish inside an Agent(subagenttype="general-purpose") background task does not work**. Background agents don't inherit the parent's slash-command registry, so Skill(skill="pr-review") and Skill(skill="pr-address") calls aren't available — the agent has to manually replicate the child skills' logic, which is fragile and tends to stall on the first network or rate-limit hiccup. Symptom: the background task reports stalled: no progress for 600s mid-review.

Run /pr-polish inline in the foreground conversation. If the user asks for "/pr-polish + /pr-test in parallel", split them: foreground /pr-polish, and ONLY then can the test step go to a background agent (because /pr-test doesn't itself need to invoke skills).

You MUST invoke Skill(pr-review) every round — even when bot reviews already exist

A common failure mode: CodeRabbit / autogpt-pr-reviewer / Sentry have already posted findings on the PR, and the orchestrator skips the Skill(pr-review) step on the assumption that "review has been done." That's wrong — the outer loop's purpose is to layer the agent's own review on top of the bot reviews, catching issues the bots miss (architecture, naming, cross-file invariants, hidden coupling). If the orchestrator only addresses bot findings without ever running its own review, the loop converges to "bot-clean" but not "agent-reviewed-clean," and the user reasonably asks "did /pr-polish even read the diff?"