pr-review

# 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. ```bash # 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 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: ```bash 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 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 ```bash # 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 ```bash # 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.** ```bash 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) ```bash 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 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. ```bash 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) ```bash cp -r .agents/skills/pr-review ~/.copilot/skills/ ``` ### Install project-locally (shown in IDE panel) ```bash 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 | --- > Source: [catatafishen/agentbridge](https://github.com/catatafishen/agentbridge) — distributed by [TomeVault](https://tomevault.io). <!-- tomevault:4.0:skill_md:2026-07-15 -->