amelnagdy/review-skills/babysit-pr

Babysit a PR

Goal: carry a pull request (GitHub) or merge request (GitLab) from "just opened" to "nothing left unanswered," without the human having to sit and refresh the page. "PR" below means either.

Review bots are diff-anchored samplers. Every push mints a fresh round, and a fix in one place can light up commentary somewhere adjacent. Left alone, a PR accumulates half-answered threads that nobody resolves, and the real bug in round three gets buried under nitpicks from rounds one and two. Your job is to be the person who reads every finding, decides what is actually true, fixes what blocks, and closes every loop in writing.

You know how to drive gh (GitHub), glab (GitLab), and git. What follows is only the judgment this loop needs and the few API calls that are easy to get wrong. The harvest script picks the forge from the cwd's git origin; everything it returns has the same shape on both, with a capabilities block naming what that forge cannot tell you.

The three rules that matter most

Verify before you believe. A bot's severity badge is a guess made without running anything. Treat every finding, including the P1s, as a claim to check against the code. Bots are frequently right (that is why this loop is worth running), and they are also confidently wrong often enough that shipping their suggestions unexamined will introduce bugs. Read the actual code path before you agree or disagree.

Every thread gets an answer. A finding you fixed, rejected, or deferred is only closed once you have said so in that thread and resolved it. Silence reads as "ignored" to the next human who opens the PR, and it is how a real bug gets lost.

Publish before you answer. A "fixed" reply is only true once the remote branch carries the fix. Never post a confirmed reply, or resolve its thread, while the fix exists only locally. Rejections need no push. Reply with evidence and resolve immediately.

Harvest the round

Findings arrive on two different surfaces, and a round that reads only one silently misses half of them. This is the single most common way a babysit loop goes wrong:

• Inline review threads. This is where debate-review and Codex post their findings (Codex attaches

P1/P2-badged inline comments to an otherwise boilerplate review body; an empty-looking body proves nothing). Each thread carries a thread_id (to resolve) and a reply_to (to reply inside the thread). On GitHub these are GraphQL review threads; on GitLab they are discussions.

• Top-level review bodies. This is where Greptile summarizes, Codex sometimes posts a numbered list,

and debate-review posts its round summary. These have no thread to resolve; answer them with one PR comment per round. On GitHub they are review objects; on GitLab they are plain notes.

The bundled script returns both in one call, already correlated (<skill-dir> is the folder that holds this SKILL.md):

"<skill-dir>/scripts/threads.sh" <N> > /tmp/pr-<N>-round-<k>.json

Never trust a filtered count without its unfiltered twin. Before applying any jq filter to the harvest, print the raw totals (jq '{threads: (.threads|length), reviews: (.reviews|length)}') and compare. A filter that eliminates 100% of items is presumed broken until the field names are verified against the actual schema (jq '.threads[0] | keys'). jq selects on a misspelled field fail silently-empty, and a "clean round" built on one is how a P1 gets a merge-gate mention posted over it. That has happened. GitHub tooling fails by returning less data, not by erroring; pair this with the pagination rule.

Never describe an object you did not fetch. If a query for a specific id returns empty, that is a stop signal. Say "I can't see it" and fetch it another way (gh api .../reviews/<id>), never narrate its presumed content. Related trap: every inline thread reply arrives wrapped in a zero-byte COMMENTED review object, so a watcher's "new review" event may be just a reply wrapper, not a new round. threads.sh's .reviews does not include these wrappers, so a review id from an event that is missing from the harvest means "wrapper", not "gone".

Diff it against the previous round's file to see what is genuinely new. outdated: true on a thread means the line moved underneath it. The finding may already be fixed, so check it against current code before spending the round on it. A comment_count bump on a thread you already handled means a bot followed up inside it.

Has this reviewer seen the current push? Only trust a field that names a sha. On GitHub each review carries commit_id; compare it to head. For debate-review on either forge, the round body's debate_head is the sha it reviewed. On GitLab other reviewers' notes carry no sha (capabilities. review_commit_id: false); a note's timestamp being later than your push does not prove it reviewed that push, so say "coverage unknown" rather than guessing.

Two kinds of author count as a reviewer. First, a bot: author_bot: true in the harvest. On GitHub that comes from the API's own author type and is reliable (chatgpt-codex-connector and greptile-apps are the usual ones; don't hardcode a whitelist). On GitLab the API only sometimes says, so author_bot can be null; treat null as unknown, look at the thread, and say in your report that you could not confirm it. Second, any thread whose first comment carries a <!-- debate-review:... --> marker. debate-review posts from the user's own account, so the author is the PR author (author_is_pr_author: true), but the thread is a reviewer thread. The harvest flags these as debate_review: true with debate_id, debate_status, and debate_severity parsed from the marker; its round body shows up in .reviews with debate_head (the sha it reviewed) and debate_agreed / debate_contested. Treat them like any other bot thread. Anything else from the PR author, and any human's comment without that marker, is never in scope for autonomous fixing. Surface it to the user instead.

Bots post 5 to 10 minutes after a push, longer on a big diff. Don't poll tightly; background the wait and review the diff yourself meanwhile. A round is "in" once every reviewer you expect has either posted against the current head SHA or been marked unavailable after its own wait budget. An unavailable reviewer never blocks harvesting or acting on the ones that did post. Disclose the gap instead of reporting the PR clean.

Classify by real impact, not by badge

After verifying a finding, sort it by consequence rather than by the label the bot attached.

Blocking, meaning it would ship a defect or stop the merge:

• a real bug, wrong behavior, or broken edge case in the changed code
• security, authorization, data-integrity, or data-loss exposure
• a violation of the change's own stated contract, acceptance criteria, or spec
• a migration or schema hazard
• a failing or newly-flaky check

Non-blocking, meaning real but ships nothing broken: naming, structure, docs, test nitpicks, micro performance, "consider extracting this", style preference.

When a finding is genuinely ambiguous, hold it as blocking until you have read enough code to demote it. The asymmetry is deliberate. An over-cautious fix costs minutes, a missed P1 costs a production bug.

A debate-review thread with debate_status: contested means two models looked and disagreed. The main reviewer held the finding against a refutation, and the italic last line of the comment says what the challenge was. That is a claim with a known counter-argument, not a weaker claim. Verify it the same way, and say in your reply which side the code supports and why.

Fix the blockers, autonomously

Don't stop to ask about blockers. Verify, fix, push, keep watching, report what you did.

• Reproduce first where you can. A probe that fails before the fix and passes after is what separates

a real fix from a plausible edit. This matters most on findings you initially disagreed with. Those are the ones where being wrong is expensive.

• One push per round, not one per finding. Every push mints a new bot round, so per-finding pushes

multiply the rounds you have to sit through.

• Run the repo's own gate before pushing. A fix that breaks the suite costs a whole extra round.
• If a matching guard skill is installed (clean-code-guard, test-guard, wp-guard, woo-guard from

guard-skills), run it on your fix before pushing. The guards catch the failure modes a quick fix under review pressure tends to produce.

• When you disagree, prove it. Rejecting a finding is legitimate and common, but the reply has to

carry the evidence: the code path, the guard that already handles it, or the test that pins the behavior. "This is fine" is not a rejection.

Publish, then reply, then resolve

Work the round in one pass, not per finding: verify everything, reproduce confirmed blockers where practical, fix them all, run the gate, then commit and push once and confirm the remote SHA. Only then close the loops:

• Confirmed: reply naming the fix commit, then resolve.
• Rejected: reply with concrete evidence, then resolve. No push needed; these can close anytime.
• Deferred: create the agreed issue, reply with its link, then resolve.

Non-blocker fixes the user approves ride the next consolidated push, never a dedicated push of their own. There is no re-review-exempt push: every push, including a final docs-only or nit-only one, must be covered by a clean round from the merge-gate reviewer before merge (see "Before merge"). If publication or verification fails, leave the thread open and report the blocker.

Answer inside the thread the finding came from. A fresh top-level comment leaves the original thread open and forces the reader to correlate by hand. Use the harvest's reply_to to reply and thread_id to resolve. On GitHub those are two different identifiers (REST comment id, GraphQL thread id); on GitLab both are the discussion id.

GitHub:

gh api --method POST "repos/<owner>/<repo>/pulls/<N>/comments/<reply_to>/replies" \
  -f body="$(cat /tmp/reply.md)"
 
gh api graphql -f query='mutation($t:ID!){
  resolveReviewThread(input:{threadId:$t}){ thread{ isResolved } } }' -F t="<thread_id>"

GitLab (<project> is the URL-encoded group/path, --hostname your instance):

glab api --hostname <host> --method POST "projects/<project>/merge_requests/<N>/discussions/<reply_to>/notes" \
  --raw-field "body=$(cat /tmp/reply.md)"
 
glab api --hostname <host> --method PUT "projects/<project>/merge_requests/<N>/discussions/<thread_id>" \
  -F resolved=true

The GitLab reply and resolve calls are taken from the GitLab API docs and have not yet been exercised against a live instance from this skill. The first time you use them, check the response, and if either fails, stop and report rather than retrying variations.

Attribution. Open every reply by naming the model writing it and the person it writes for, so a reader never has to guess whether a human weighed in. Sign your own model name; this skill is model-neutral. The person is whoever owns the account the reply posts from. Get the name once per session, gh api user -q '.name // .login' on GitHub or glab api user --hostname <host> | jq -r '.name // .username' on GitLab, and reuse it:

I am \<model-slug\> writing on behalf of \<user\>.

Then the verdict, then the evidence, briefly:

I am <model-slug> writing on behalf of <user>.
 
Confirmed and fixed in `a1b2c3d`. You were right that `occurrence_time` was never
compared against `evidence.event_time`, so a mapping could bind proof from a
different occurrence. Reproduced with a failing test first
(`test_binds_proof_to_mapped_occurrence`), then fixed the composition check.

I am <model-slug> writing on behalf of <user>.