pr-review

name

pr-review

description

Use when reviewing pull requests, fixing CI failures, addressing review comments, resolving threads, cleaning commit history, getting a PR to mergeable state, or managing GitHub issues. Triggers include "check PR", "fix CI", "failing tests", "review comments", "resolve threads", "squash commits", "rebase", "DIRTY merge", "open issues", "close issue", or any request to review, merge, or track work in GitHub.

PR Review

Gets a PR to mergeable state: CI green, all threads resolved, clean history, rebased.

This repo: owner catatafishen, repo agentbridge, default branch master.

Run configurations (preferred — use run_configuration tool)

Edit the SCRIPT_OPTIONS field to set the PR number, then run:

Config	Purpose
PR CI Check	CI status + failing test names + error messages
PR Threads	Unresolved review threads with reply/resolve IDs
PR Squash	Squash all branch commits into one

edit_run_configuration("PR CI Check", script_parameters="628")
run_configuration("PR CI Check")

Run configurations are terminal-attached (output visible in IDE). The PR number defaults to 628 — always update it before running.

Runnable scripts (fallback — copy path, run directly)

Script	Purpose
.agents/skills/pr-review/pr-ci.sh <PR>	CI status + failing test names + error messages
.agents/skills/pr-review/pr-threads.sh <PR>	Unresolved review threads with reply/resolve IDs
.agents/skills/pr-review/pr-threads.sh <PR> all	All threads (resolved + unresolved)
.agents/skills/pr-review/pr-squash.sh	Squash all branch commits into one
.agents/skills/pr-review/pr-issues.sh list	List open GitHub issues
.agents/skills/pr-review/pr-issues.sh view <N>	Show issue details + comments
.agents/skills/pr-review/pr-issues.sh close <N>	Close issue with optional reason comment

---

GitHub Issues

Manage issues that are related to the work you are doing.

# List all open issues
bash .agents/skills/pr-review/pr-issues.sh list

# View an issue with comments
bash .agents/skills/pr-review/pr-issues.sh view 624

# Add a comment to an issue
bash .agents/skills/pr-review/pr-issues.sh comment 624 "Fixed in PR #630."

# Close with a reason
bash .agents/skills/pr-review/pr-issues.sh close 624 "Fixed in PR #630 — Shell Script run config XML builder."

# Link issue to a PR (adds cross-reference comment on the issue)
bash .agents/skills/pr-review/pr-issues.sh link-pr 624 630

# Add a label
bash .agents/skills/pr-review/pr-issues.sh label 624 "bug"

# Open (reopen) a closed issue
bash .agents/skills/pr-review/pr-issues.sh open 624

When to close an issue from a PR:

• When the bug or feature addressed by a PR was tracked in an issue, close the issue when the PR is merged.
• Always comment first with what was done and which PR fixed it.
• Use link-pr to add a cross-reference, then close to close.

---

Mergeable Checklist

• CI passing (all checks green)
• No unresolved review threads
• Rebased onto latest master (always do this — don't just report BEHIND)
• Clean commit history (fold review-fix noise into logical parent commits; keep atomic increments)

---

Step 1 — Check CI

bash .agents/skills/pr-review/pr-ci.sh <PR>

This is a single-shot command: gets CI checks, finds failing job IDs from the URLs, then extracts exact test names and error messages from the job logs. No second call needed.

For manual inspection:

gh pr checks <PR> --repo catatafishen/agentbridge

---

Step 2 — Fix Failing Tests

After pr-ci.sh identifies the failing test and error:

1. Find the test class and reproduce locally: run_tests in the IDE
2. Fix the root cause
3. Re-run locally to confirm green
4. Commit and push — CI re-runs automatically

---

Step 3 — Get Review Threads

bash .agents/skills/pr-review/pr-threads.sh <PR>             # unresolved only
bash .agents/skills/pr-review/pr-threads.sh <PR> all         # all threads

Output includes both IDs needed for the next step:

• Thread ID (for resolve) — the PRRT_... node ID
• Comment DB ID (for reply) — the integer databaseId

---

Step 4 — Reply + Resolve Threads

Always reply first, then resolve. Never resolve silently — reviewers need to see the decision.

Reply to a thread

# Use `Comment DB ID` from pr-threads.sh output (note: PR number required in path)
gh api repos/catatafishen/agentbridge/pulls/<PR>/comments/<COMMENT_DB_ID>/replies \
  --method POST -f body="Fixed in <commit/file>: <one sentence describing what changed and why>"

Resolve a thread

# Use `Thread ID` (PRRT_...) from pr-threads.sh output
gh api graphql -f query='mutation {
  resolveReviewThread(input: {threadId: "<PRRT_...>"}) {
    thread { id isResolved }
  }
}'

Key distinction: databaseId (integer) → replies API. id (PRRT_...) → GraphQL resolve. The scripts output both; pick the right one for the operation.

---

Step 5 — Rebase onto Latest Master

Always rebase as part of every sweep — don't just report that a PR is behind.

git_fetch(remote="origin")
git_rebase(branch="origin/master")
git_push(force=true)

Conflicts during rebase:

• Clear conflicts (e.g., a line was deleted in master and also modified in the branch, or two unrelated changes touch the same area): resolve using your best judgment — pick the version that preserves all intended changes.
• Unclear conflicts (e.g., two diverging implementations of the same feature, or a large block rewritten in both): call prompt_user (do NOT end the turn with a question) — describe both sides and ask which is the right implementation, then continue resolving once the user replies.

If rebase drops commits already in master (duplicate changes from a merged sibling PR), that's correct — don't re-apply them.

---

Step 6 — Clean Up Commit History Before Merging

The git blame test

git blame shows the most recent commit that touched each line. If that commit is fix: address review comment, it tells you nothing — you have to blame again on its parent to find the actual reasoning. Every extra blame step is lost context.

The test: imagine reading this commit message as the first result of git blame on a line. Does it explain why the code is this way? If not — if the reader would need to dig further to understand the change — the commit is noise and should be folded into the commit that carries the real reasoning.

This is not about aesthetics. A shallow git blame that shows noise hides the original decision. The first blame result should always be the answer, not a pointer to dig deeper.

What to fold vs. keep

Fold (noise — hides reasoning when seen in git blame):

• fix: address review comment — the change belongs on the commit that introduced the code
• chore: fix import / chore: typo — invisible in git blame, should be invisible in history too
• Debug iterations: WIP, try X, revert X and try Y

Keep (atomic, carries reasoning on its own):

• feat: add ShellScript XML builder — explains why the builder exists
• fix: sanitizeConfigFileName was adding double .xml — explains a specific bug
• refactor: extract BillingCalculator from ChatConsolePanel — explains the separation

Interactive rebase (preferred)

git rebase -i origin/master

Or via the git_rebase tool:

git_rebase(interactive=true, branch="origin/master", operations=[
    {commit: "abc1234", action: "pick"},   # keep: original feature
    {commit: "def5678", action: "fixup"},  # noise: fold into prev, keep prev message
    {commit: "ghi9012", action: "pick"},   # keep: second logical change
    {commit: "jkl3456", action: "fixup"},  # noise: fold into prev
])

fixup discards the noise commit's message and folds the diff into the parent — exactly what you want. squash lets you edit the merged message, useful if the fix actually adds important context to the parent.

Full squash (only when ALL commits are noise)

When the entire branch is one logical change with no independent increments:

bash .agents/skills/pr-review/pr-squash.sh

Use sparingly — prefer interactive rebase for branches with multiple logical steps.

---

Step 7 — Verify Final State

Always re-check before concluding a PR is done. Automated scanners, review bots, and CI pipelines keep updating PRs during a session — new comments, new conflicts, and new check failures can appear at any time. Another PR may have been merged to master causing new merge conflicts on your branch. Never rely on a check you did earlier; always verify the current state.

gh pr view <PR> --repo catatafishen/agentbridge \
  --json mergeStateStatus,state,reviewDecision

Run pr-threads.sh <PR> one more time to confirm zero unresolved threads. Run pr-ci.sh <PR> to confirm CI is still green (a rebase or force-push triggers a new run).

---

Quick Reference

Task	Command
CI + failing tests	bash .agents/skills/pr-review/pr-ci.sh <PR>
Unresolved threads	bash .agents/skills/pr-review/pr-threads.sh <PR>
Clean history	git_rebase(interactive=true, branch="origin/master", ...) — fixup noise, pick logical commits
Full squash (rare)	bash .agents/skills/pr-review/pr-squash.sh — only when all commits are noise
PR status	gh pr view <PR> --repo catatafishen/agentbridge --json mergeStateStatus,state
Commit list	gh pr view <PR> --repo catatafishen/agentbridge --json commits --jq '[.commits[] | .oid[:8] + " " + .messageHeadline]'
File in master?	git show origin/master:<path> 2>&1 | head -3
Commits on branch	git rev-list origin/master..HEAD --count
Close PR with note	gh pr close <PR> --repo catatafishen/agentbridge --comment "<reason>"
Open issues	bash .agents/skills/pr-review/pr-issues.sh list
View issue	bash .agents/skills/pr-review/pr-issues.sh view <N>
Close issue	bash .agents/skills/pr-review/pr-issues.sh close <N> "<reason>"
Link issue to PR	bash .agents/skills/pr-review/pr-issues.sh link-pr <ISSUE> <PR>

---

Skill Installation

This skill lives in .agents/skills/pr-review/ (canonical repo source). For Copilot CLI to discover it, it must also exist at ~/.copilot/skills/pr-review/SKILL.md.

Install globally (Copilot CLI)

cp -r .agents/skills/pr-review ~/.copilot/skills/

Install project-locally (shown in IDE panel)

cp -r .agents/skills/pr-review .agent-work/copilot/skills/

The Copilot CLI scans ~/.copilot/skills/*/SKILL.md automatically — no CLI flag needed.

Other clients

Client	Where to install	Notes
Copilot	~/.copilot/skills/<name>/SKILL.md	Global; auto-discovered
Kiro	.agent-work/kiro/skills/<name>/SKILL.md	Project-local; shown in IDE panel
OpenCode	No native skill system	Embed skill content in startup instruction
Junie	No skill system	Prompt engineering only