| 1 | # Appendix A: Wiring a Code Reviewer — Complete Walkthrough |
| 2 | |
| 3 | > End-to-end example of adding a code reviewer to your squad and wiring their gate so it actually gets enforced. This walkthrough addresses a common failure: a reviewer is on the roster but never reviews a single PR because the gate wasn't wired. |
| 4 | |
| 5 | ## The Problem This Solves |
| 6 | |
| 7 | Adding a reviewer to `team.md` gives them an identity. It does NOT: |
| 8 | - Tell the coordinator to route PRs to them |
| 9 | - Prevent PRs from being merged without their approval |
| 10 | - Prevent issues from being closed before review happens |
| 11 | |
| 12 | **What goes wrong without enforcement:** A reviewer can be on the roster as "Reviewer" from day one. Their charter says they review PRs. The routing table says "PR code review → {ReviewerName}." But PRs get merged and issues get closed without them ever being spawned. Why? |
| 13 | |
| 14 | Because the routing table says WHO handles what — it's for incoming requests ("review PR #42"). It does NOT say "after every agent completes work, route their output to {ReviewerName}." The coordinator routes work TO agents, but nothing tells it to route COMPLETED work to a reviewer. The "After Agent Work" flow in `squad.agent.md` says: collect results → present → spawn Scribe. No review step. |
| 15 | |
| 16 | **The fix has three layers:** |
| 17 | |
| 18 | | Layer | What it does | Where it lives | |
| 19 | |-------|-------------|----------------| |
| 20 | | Identity | Reviewer exists and knows how to review | `team.md` roster + `charter.md` | |
| 21 | | Routing | User can explicitly request "review this" | `routing.md` routing table | |
| 22 | | **Enforcement** | Coordinator MUST route every PR to reviewer before merge | `routing.md` Rules section + `issue-lifecycle.md` post-work steps | |
| 23 | |
| 24 | Most squads get layers 1 and 2 right. Layer 3 — enforcement — is what's usually missing. |
| 25 | |
| 26 | ## Step-by-Step Walkthrough |
| 27 | |
| 28 | ### Step 1: Create the reviewer's identity |
| 29 | |
| 30 | Create `.squad/agents/{name}/charter.md`: |
| 31 | |
| 32 | ```markdown |
| 33 | # {Name} — Code Reviewer |
| 34 | |
| 35 | ## Identity |
| 36 | - **Name:** {Name} |
| 37 | - **Role:** Code Reviewer |
| 38 | - **Expertise:** Code quality, correctness, test coverage, security, patterns |
| 39 | - **Style:** Thorough, fair, specific. Provides actionable feedback. |
| 40 | |
| 41 | ## What I Own |
| 42 | - Reviewing PRs for code quality, correctness, and test coverage |
| 43 | - Identifying bugs, security issues, and design problems |
| 44 | - Providing specific, actionable feedback (not vague suggestions) |
| 45 | |
| 46 | ## How I Review |
| 47 | 1. Read the PR diff completely |
| 48 | 2. Check: does it do what the issue asked for? |
| 49 | 3. Check: are there tests? Do they cover the important cases? |
| 50 | 4. Check: are there bugs, edge cases, or security issues? |
| 51 | 5. Check: does it follow project patterns and conventions? |
| 52 | 6. Verdict: APPROVE or REJECT with specific feedback |
| 53 | |
| 54 | ## Boundaries |
| 55 | **I handle:** Code review, PR review, quality gates |
| 56 | **I don't handle:** Implementation, design, research, documentation |
| 57 | |
| 58 | ## On REJECT |
| 59 | I provide specific feedback: what's wrong, why, and what to do instead. |
| 60 | The original author fixes their work. I re-review after fixes. |
| 61 | ``` |
| 62 | |
| 63 | Create `.squad/agents/{name}/history.md` seeded with project context. |
| 64 | |
| 65 | ### Step 2: Add to team.md roster |
| 66 | |
| 67 | ```markdown |
| 68 | | 👑 {Name} | Code Reviewer | `.squad/agents/{name}/charter.md` | ✅ Active | |
| 69 | ``` |
| 70 | |
| 71 | ### Step 3: Add routing table entry |
| 72 | |
| 73 | In `routing.md` → routing table: |
| 74 | |
| 75 | ```markdown |
| 76 | | PR code review | 👑 {Name} | — | "Review PR #42", code quality, finding reports | |
| 77 | ``` |
| 78 | |
| 79 | **⚠️ This is necessary but NOT sufficient.** This only handles explicit review requests. It does NOT enforce automatic review of every PR. |
| 80 | |
| 81 | ### Step 4: Add enforcement rule (THIS IS THE CRITICAL STEP) |
| 82 | |
| 83 | In `routing.md` → `## Rules` section, add a numbered rule: |
| 84 | |
| 85 | ```markdown |
| 86 | N. **{Name} PR Gate** — every PR created by any agent MUST be reviewed by {Name} |
| 87 | before merge. The coordinator spawns {Name} (sync) with the PR diff after |
| 88 | the author pushes and creates the PR. On REJECT, the original author addresses |
| 89 | feedback. On APPROVE, the coordinator merges via `gh pr merge`. No PR merges |
| 90 | without {Name}'s approval. |
| 91 | ``` |
| 92 | |
| 93 | **Why this works when the routing table alone didn't:** The routing table is for matching incoming work to agents. Rules are behavioral constraints the coordinator must follow AFTER work completes. The rule says "after a PR exists, you MUST do X before proceeding." The routing table says "if someone asks for a review, route to X." |
| 94 | |
| 95 | ### Step 5: Wire into issue-lifecycle.md |
| 96 | |
| 97 | In `.squad/templates/issue-lifecycle.md`, the "Coordinator Post-Work Steps" section should reference your reviewer by name: |
| 98 | |
| 99 | ```markdown |
| 100 | 4. **Route to reviewer.** Spawn {Name} (sync) with the PR diff for code review. |
| 101 | ``` |
| 102 | |
| 103 | This is the operational detail — the step-by-step instructions the coordinator follows after an agent completes issue work. The routing rule (Step 4) is the mandate; the lifecycle template is the procedure. |
| 104 | |
| 105 | ### Step 6: Add to casting registry |
| 106 | |
| 107 | Update `.squad/casting/registry.json` with the new entry. |
| 108 | |
| 109 | ### Step 7: Verify |
| 110 | |
| 111 | Ask yourself these questions: |
| 112 | |
| 113 | - [ ] If a clean session coordinator reads `routing.md` Rules, will it know to route PRs to this reviewer? → Check rule N exists. |
| 114 | - [ ] If an agent completes work and pushes a PR, does the coordinator's post-work flow include a review step? → Check `issue-lifecycle.md` step 4. |
| 115 | - [ ] Can the coordinator merge a PR without the reviewer's approval? → The rule should say "No PR merges without {Name}'s approval." |
| 116 | - [ ] Can the coordinator close an issue without a merged PR? → Check the issue closure rule exists. |
| 117 | |
| 118 | If any answer is wrong, you have a gap. |
| 119 | |
| 120 | ## What Each File Controls (Summary) |
| 121 | |
| 122 | | File | What it contributes to the reviewer gate | |
| 123 | |------|----------------------------------------| |
| 124 | | `charter.md` | WHO the reviewer is and HOW they review | |
| 125 | | `team.md` | That the reviewer EXISTS on the team | |
| 126 | | `routing.md` routing table | That explicit review requests go to this reviewer | |
| 127 | | `routing.md` Rules section | That the coordinator MUST route EVERY PR to this reviewer (enforcement) | |
| 128 | | `issue-lifecycle.md` | The step-by-step procedure for the post-work review flow | |
| 129 | | `casting/registry.json` | Persistent name tracking | |
| 130 | |
| 131 | **Remove any one of these and the gate has a hole.** The most commonly missed piece is the Rules section entry (Step 4). |