$npx -y skills add tody-agent/codymaster --skill cm-code-reviewFull review lifecycle — request reviews, handle feedback with technical rigor, and complete branch integration. Use when completing tasks, receiving feedback, or finishing feature branches.
| 1 | # Code Review — Request + Receive + Complete |
| 2 | |
| 3 | ## TL;DR |
| 4 | - **Use when** task complete, before/during PR review |
| 5 | - **Pre-review**: lint, tests, diff scan, blast radius |
| 6 | - **Reads**: handoff/exec.json — **Writes**: handoff/review.json |
| 7 | - **Severity**: info | warn | error | critical |
| 8 | - **Next**: cm-quality-gate |
| 9 | |
| 10 | > **Full review lifecycle in one skill:** Request → Receive → Integrate. |
| 11 | |
| 12 | ## Part A: Requesting Code Review |
| 13 | |
| 14 | ### When to Request |
| 15 | |
| 16 | **Mandatory:** |
| 17 | - After each task in `cm-execution` |
| 18 | - After completing major features |
| 19 | - Before merge to main |
| 20 | |
| 21 | **Optional but valuable:** |
| 22 | - When stuck (fresh perspective) |
| 23 | - Before refactoring (baseline check) |
| 24 | - After fixing complex bugs |
| 25 | |
| 26 | ### How to Request |
| 27 | |
| 28 | 1. **Get git SHAs:** |
| 29 | ```bash |
| 30 | BASE_SHA=$(git rev-parse HEAD~1) |
| 31 | HEAD_SHA=$(git rev-parse HEAD) |
| 32 | ``` |
| 33 | |
| 34 | 2. **Dispatch reviewer subagent** with: |
| 35 | - What was implemented |
| 36 | - Plan/requirements reference |
| 37 | - Base and head SHAs |
| 38 | - Brief description |
| 39 | |
| 40 | 3. **Act on feedback:** |
| 41 | - Fix Critical issues immediately |
| 42 | - Fix Important issues before proceeding |
| 43 | - Note Minor issues for later |
| 44 | - Push back if reviewer is wrong (with reasoning) |
| 45 | |
| 46 | --- |
| 47 | |
| 48 | ## Part B: Receiving Code Review |
| 49 | |
| 50 | ### When to Use |
| 51 | When receiving feedback — whether from human reviewers, AI reviewers, or code review subagents. |
| 52 | |
| 53 | ### The Protocol |
| 54 | |
| 55 | ``` |
| 56 | 1. READ feedback completely before responding |
| 57 | 2. UNDERSTAND the technical reasoning |
| 58 | 3. VERIFY if the feedback is technically correct |
| 59 | 4. RESPOND with evidence, not agreement |
| 60 | ``` |
| 61 | |
| 62 | ### Response Framework |
| 63 | |
| 64 | | Feedback Type | Response | |
| 65 | |--------------|----------| |
| 66 | | **Technically correct** | Fix it. Thank reviewer. | |
| 67 | | **Unclear intent** | Ask for clarification with specific questions | |
| 68 | | **Technically questionable** | Challenge with evidence (code, tests, docs) | |
| 69 | | **Stylistic preference** | Discuss trade-offs, defer to team convention | |
| 70 | |
| 71 | ### Red Flags — STOP |
| 72 | |
| 73 | - Blindly implementing all suggestions without verification |
| 74 | - "Performative agreement" — saying yes without understanding |
| 75 | - Implementing a suggestion that breaks existing tests |
| 76 | - Making changes you can't justify technically |
| 77 | |
| 78 | ### Anti-Pattern: Performative Agreement |
| 79 | |
| 80 | ``` |
| 81 | ❌ "Good catch! Fixed." (without verifying it's actually a problem) |
| 82 | ✅ "I verified this: [evidence]. The suggestion is correct because [reason]. Fixed." |
| 83 | ✅ "I investigated this: [evidence]. The current code is correct because [reason]." |
| 84 | ``` |
| 85 | |
| 86 | --- |
| 87 | |
| 88 | ## Part C: Finishing a Development Branch |
| 89 | |
| 90 | ### When to Use |
| 91 | When implementation is complete and all tests pass. |
| 92 | |
| 93 | ### The Process |
| 94 | |
| 95 | 1. **Verify current state:** |
| 96 | ```bash |
| 97 | npm run test:gate # All tests must pass |
| 98 | git status # Working tree should be clean |
| 99 | ``` |
| 100 | |
| 101 | 2. **Present options to user:** |
| 102 | |
| 103 | | Option | When | Command | |
| 104 | |--------|------|---------| |
| 105 | | **Merge to main** | Feature ready | `git checkout main && git merge feature-branch` | |
| 106 | | **Create PR** | Needs team review | `git push origin feature-branch` | |
| 107 | | **Keep working** | More tasks remain | Continue on branch | |
| 108 | | **Cleanup only** | Abandoned/merged | `git worktree remove path` | |
| 109 | |
| 110 | 3. **Execute chosen option** |
| 111 | |
| 112 | 4. **Cleanup:** |
| 113 | - Remove worktree if using `cm-git-worktrees` |
| 114 | - Delete feature branch if merged |
| 115 | - Update task tracking |
| 116 | |
| 117 | ### Rules |
| 118 | - Never merge with failing tests |
| 119 | - Never force push main/production |
| 120 | - Always use `cm-identity-guard` before git push |
| 121 | |
| 122 | --- |
| 123 | |
| 124 | ### Step FINAL: Record Review Learnings |
| 125 | |
| 126 | After processing review feedback, ALWAYS update `.cm/CONTINUITY.md`: |
| 127 | |
| 128 | - **Key Decisions:** If reviewer changed architecture approach, record with scope: |
| 129 | `[Decision]: [Rationale] — scope: [global|module:{name}]` |
| 130 | - **Mistakes & Learnings:** If reviewer caught a pattern mistake, record with scope: |
| 131 | - What Failed: [the pattern that was wrong] |
| 132 | - How to Prevent: [correct pattern going forward] |
| 133 | - Scope: [global | module:{name} | file:{path}] |
| 134 | |
| 135 | **Anti-duplicate:** If similar learning exists, reinforce it instead of creating new. |
| 136 | |
| 137 | > **Token savings:** Future code reviews in same project avoid repeating |
| 138 | > the same feedback. Reviewer patterns become accumulated knowledge. |
| 139 | |
| 140 | --- |
| 141 | |
| 142 | ## Integration |
| 143 | |
| 144 | | Skill | Relationship | |
| 145 | |-------|-------------| |
| 146 | | `cm-execution` | Reviews after each task in execution | |
| 147 | | `cm-quality-gate` | Tests must pass before finishing branch | |
| 148 | | `cm-identity-guard` | Before git push | |
| 149 | | `cm-git-worktrees` | Cleanup worktree after completion | |
| 150 | |
| 151 | ## Karpathy Discipline — Review Checklist |
| 152 | |
| 153 | Block any PR/diff that violates these regardless of green tests: |
| 154 | - **Scope creep:** changed line that doesn't trace to the task → request removal. |
| 155 | - **Bloat:** new abstraction with one caller, premature config, error paths for impossible inputs → request simplification. |
| 156 | - **Side-effect edits:** u |