Claude Skill

subagent-review-cycles

Structure a multi-round Leader / Developer / Code-Reviewer review of a document or code change. Use when planning how many review rounds to run, dispatching a Code Reviewer subagent and deciding which tools and review criteria it carries, adjudicating a reviewer's BLOCKER / MAJOR

LLM Mart · 0 points · 0 views 0 listing impressions 0 install-command copies
Virus-scanned Reviewed automatically before listing.

Full trust report

Download wei18-apple-dev-skills-collaboration-skills_skills_subagent-review-cycles-7ea7e61.zip · 5 KB
Part of wei18/apple-dev-skills — 37 skills

Install

skills CLI npx skills add https://github.com/wei18/apple-dev-skills/tree/main/collaboration-skills/skills/subagent-review-cycles
Claude Code claude plugin marketplace add https://llmmart.ai/marketplace.json && claude plugin install wei18-apple-dev-skills@llmmart
Git git clone https://github.com/wei18/apple-dev-skills.git

The skills CLI installs just this skill, for any of its supported agents. Claude Code installs the whole wei18/apple-dev-skills collection as a plugin from our marketplace. Git is the plain clone.

Skill manifest

Subagent Review Cycles

When to invoke

  • About to dispatch a sub-agent to draft a technical document / design section.
  • The first version of a document is ready and a Code Reviewer should audit technical correctness.
  • User asks "how many review rounds", "should Code Reviewer run CLI", "what counts as rejection".

The triad

Role Job
Leader (main agent) Dispatch, integrate review results, accept / reject, communicate with the user
Developer (subagent) Draft / revise document sections; works after Leader replies with rejection. (May be dispatched as any implementer-type subagent when the task is architectural in nature; the role name in this skill remains "Developer".)
Code Reviewer (subagent) Audits Developer output for technical / API correctness and logic gaps; CLI is forbidden for probing API/runtime behavior (build, run, simctl, trial-and-error); read-only search (grep, rg, git log, git show) is allowed

The Leader never writes the implementation / drafts a section directly — that's the Developer's job. The Leader never does the review either — that's the Code Reviewer's job.

Round structure

round N:
  Leader → Developer: dispatch (scope + skills + inputs + return format + criteria)
  Developer → Leader: draft
  Leader → Code Reviewer: dispatch (review criteria + must use WebSearch, no CLI)
  Code Reviewer → Leader: BLOCKER / MAJOR / MINOR list
  Leader: ACCEPT / REJECT each item with explicit reason
  if accepted_count == total: done
  if N == limit: pause, report to user
  else: round N+1 with feedback

limit(N) is typically 3–5. If the limit is reached without convergence: pause, report to the user, wait for direction.

The "round-1 cosmetic" pragmatic rule

Signal Leader inline-edits Dispatch round N+1 to Developer
Fix scope Spelling / formatting / paragraph order / string typo only Any semantic or content change
Time to fix ≤ 5 minutes > 5 minutes
New decisions required None Any

Still record inline fixes in the meeting log. Reasoning: "limit is an upper bound, not a requirement"; burning a whole round just for typos is uneconomical.

Dispatch contract for Code Reviewer

Every Code Reviewer dispatch prompt must include:

  1. Target file / section scope (explicit file + section)
  2. Forbidden tools: CLI is forbidden for probing API/runtime behavior (build, run, simctl, trial-and-error); read-only search (grep, rg, git log, git show) is allowed
  3. Allowed tools: WebSearch / WebFetch (for verifying Apple APIs, library behaviour)
  4. Review criteria (4 dimensions + domain-specific checklist):
    • Technical correctness (API name, behaviour, version)
    • Logical consistency (internal contradictions, cross-section conflicts)
    • Completeness (missing edge case, error handling, prerequisite)
    • Efficiency (algorithm, CI / build / runtime cost)
  5. Return format: BLOCKER / MAJOR / MINOR three-level classification; each item with location (file + section) + suggestion. An absence claim ("the spec doesn't define this", "nothing covers this case") must include the grep/search command run and its zero-hit output — an absence claim with no evidence attached doesn't count as a finding.

Accept / Reject reply style

For each review finding the Leader gives:

  • ACCEPT + reason: accepted; specify who fixes it this round
  • REJECT + reason: rejected with a technical reason (not just "no")
  • DEFER: acknowledged but deferred (goes to backlog / open items)

REJECT must cite specific evidence (API doc, prior decision, design constraint); pure preference is not acceptable.

Anti-patterns

  • Using CLI to probe Apple API behaviour: forbidden. Use official docs / WebSearch instead.
  • Repeatedly rejecting the same point in the same section: more than 2 identical rejections counts as a communication failure; pause and clarify with the user.
  • ACCEPT without a reason: every ACCEPT should still have a one-line note of why it adds value.
  • Leader drafting sections themselves: violates the role separation; only allowed for cosmetic-grade fixes.

Verification checklist

  • Every absence claim ("not defined", "no coverage") in a review finding cites the grep/search command and its zero-hit output — otherwise it doesn't count as a finding.
  • Each round has an explicit dispatch prompt (all 5 items of §Dispatch contract for Code Reviewer present, on top of the 6 elements from leader-developer-handoff-contract).
  • Each review finding has an explicit accept / reject label + reason.
  • When limit(N) is reached without convergence, pause; don't keep iterating indefinitely.
  • Cosmetic fixes are inline-edited by the Leader; don't burn a round on them.
  • The round-summary is recorded in the meeting log (not a verbatim copy of review content).

Phase TODO sweep checklist

A separate close-the-loop activity that fires once per phase (not once per review round), so it's not part of the round structure above. Before the Leader signs off on a phase-completion PR, run a sweep against the phase's diff scope to catch deferred-and-forgotten debt.

For the full sweep procedure, command, disposition rule, and anti-pattern, read references/phase-todo-sweep.md.

Related skills

  • leader-developer-handoff-contract: details the 6 required elements of every dispatch prompt.
  • spec-phase-orchestration: review cycles are usually embedded in the spec phase.
  • methodology-pattern-extractor: "round-1 cosmetic inline edit" is a codifiable pattern.
  • Official sources: when verifying or updating a factual or version-sensitive claim, read references/official-docs.md.
Files (apple-dev-skills)
  • references
    • official-docs.md 843 B
      Official pages backing this skill's claims; read when verifying or updating a factual or version-sensitive claim.
      
      | Page | URL | Backs |
      |---|---|---|
      | Configure permissions | https://code.claude.com/docs/en/permissions#bash | Dispatch contract item 2 "Forbidden tools": a layered CLI ban can be enforced with Bash deny rules (which apply to subagents); the rule stays in prose where no rule is configured |
      | Create custom subagents | https://code.claude.com/docs/en/sub-agents#available-tools | Same item, caveat: `disallowedTools: Bash(...)` removes the whole Bash tool, including the allowed read-only `git log` / `git show` |
      | Git - git-diff Documentation | https://git-scm.com/docs/git-diff | references/phase-todo-sweep.md: `<base>...HEAD` (from merge-base) vs `..` (two tips); `--diff-filter=d` (lowercase excludes deleted); `-z` |
      
    • phase-todo-sweep.md 3.1 KB
      # Phase TODO sweep checklist
      
      A separate close-the-loop activity that fires **once per phase** (not once per review round): before the Leader signs off on a phase-completion PR, run a sweep against the phase's diff scope to catch deferred-and-forgotten debt.
      
      **Command** (Leader-run; against the phase's diff scope, not the full repo — `<base>` is
      the commit/branch the phase started from, e.g. `main` or the phase's starting SHA):
      
      ```
      git diff -z --name-only --diff-filter=d <base>...HEAD | xargs -0 -r rg -n --no-heading -e 'TODO|FIXME|XXX|HACK|stub|placeholder'
      ```
      
      If `rg` isn't installed (it isn't pinned in this repo's `.mise.toml`), use the `grep` fallback:
      
      ```
      git diff -z --name-only --diff-filter=d <base>...HEAD | xargs -0 -r grep -rnE 'TODO|FIXME|XXX|HACK|stub|placeholder'
      ```
      
      `<base>...HEAD` (three dots) diffs against the merge base, so commits landed on `<base>`
      after the phase branched off don't leak into the sweep. `--diff-filter=d` excludes deleted
      paths (a deleted file has nothing left to grep). `-z` / `xargs -0` keep filenames containing
      spaces intact.
      
      Add your own project's phase-marker string to the pattern if it has one (e.g. `|Phase [0-9]+ Part` for a project that labels work-in-progress chunks that way).
      
      Both forms scope to the files the diff actually touched — a file outside the phase's
      diff is out of scope for this sweep even if it contains a match.
      
      **Disposition rule** — every match must fall into exactly one bucket; otherwise the phase is not complete:
      
      1. **Resolved this phase** — fixed or implemented before merge.
      2. **Moved to §Backlog** — routed to the topic-appropriate document's §Backlog section (product → `design.md`, engineering → `foundations.md`, implementation step → `plan.md`, collaboration → `methodology.md`), and the §Backlog entry **cites the source `file:line`** so the debt is traceable.
      3. **Intentionally left** — documented in the phase meeting log as "intentionally left, see <follow-up issue / phase reference>".
      
      **Stub / placeholder code without a literal TODO comment still counts.** Identifier names like `xxxPlaceholder`, `xxxStub`, or scaffolding values that are not real implementations must be flagged in the phase log even if the regex didn't catch them via comment text.
      
      **Ownership**: the Leader runs the sweep. Subagents may flag debt during implementation (in their impl-notes open-questions section — see `agent-impl-notes-log`), but closing the loop before phase merge is non-delegable. A PR reviewer subagent dispatched for the phase-completion PR should also run the sweep against the diff and report findings as MAJOR (unless already documented per the disposition rule above).
      
      **When to invoke**:
      - Before declaring a phase complete.
      - When dispatching a Code Reviewer subagent on a phase-completion PR — include this command in the review brief's checklist.
      - When auditing whether a previously-declared "complete" phase actually was.
      
      **Anti-pattern**: marking a regex match as "obviously fine, ignore" without writing the disposition down. The point of the sweep is the paper trail; an undocumented justification is indistinguishable from forgetting.
      
  • SKILL.md 6.2 KB
    ---
    name: subagent-review-cycles
    description: Structure a multi-round Leader / Developer / Code-Reviewer review of a document or code change. Use when planning how many review rounds to run, dispatching a Code Reviewer subagent and deciding which tools and review criteria it carries, adjudicating a reviewer's BLOCKER / MAJOR / MINOR findings, or when asked "how many review rounds", "may the Code Reviewer run CLI", "what counts as a rejection". Does NOT own the dispatch prompt's required elements (leader-developer-handoff-contract) nor pre-dispatch worktree conflict checks (subagent-conflict-detection).
    ---
    
    # Subagent Review Cycles
    
    ## When to invoke
    
    - About to dispatch a sub-agent to draft a technical document / design section.
    - The first version of a document is ready and a Code Reviewer should audit technical correctness.
    - User asks "how many review rounds", "should Code Reviewer run CLI", "what counts as rejection".
    
    ## The triad
    
    | Role | Job |
    |---|---|
    | **Leader** (main agent) | Dispatch, integrate review results, accept / reject, communicate with the user |
    | **Developer** (subagent) | Draft / revise document sections; works after Leader replies with rejection. (May be dispatched as any implementer-type subagent when the task is architectural in nature; the role name in this skill remains "Developer".) |
    | **Code Reviewer** (subagent) | Audits Developer output for technical / API correctness and logic gaps; CLI is forbidden for probing API/runtime behavior (build, run, simctl, trial-and-error); read-only search (grep, rg, git log, git show) is allowed |
    
    The Leader **never** writes the implementation / drafts a section directly — that's the Developer's job. The Leader **never** does the review either — that's the Code Reviewer's job.
    
    ## Round structure
    
    ```
    round N:
      Leader → Developer: dispatch (scope + skills + inputs + return format + criteria)
      Developer → Leader: draft
      Leader → Code Reviewer: dispatch (review criteria + must use WebSearch, no CLI)
      Code Reviewer → Leader: BLOCKER / MAJOR / MINOR list
      Leader: ACCEPT / REJECT each item with explicit reason
      if accepted_count == total: done
      if N == limit: pause, report to user
      else: round N+1 with feedback
    ```
    
    `limit(N)` is typically 3–5. **If the limit is reached without convergence**: pause, report to the user, wait for direction.
    
    ## The "round-1 cosmetic" pragmatic rule
    
    | Signal | Leader inline-edits | Dispatch round N+1 to Developer |
    |---|---|---|
    | Fix scope | Spelling / formatting / paragraph order / string typo only | Any semantic or content change |
    | Time to fix | ≤ 5 minutes | > 5 minutes |
    | New decisions required | None | Any |
    
    Still record inline fixes in the meeting log. Reasoning: "limit is an upper bound, not a requirement"; burning a whole round just for typos is uneconomical.
    
    ## Dispatch contract for Code Reviewer
    
    Every Code Reviewer dispatch prompt must include:
    
    1. **Target file / section scope** (explicit file + section)
    2. **Forbidden tools**: CLI is forbidden for probing API/runtime behavior (build, run, simctl, trial-and-error); read-only search (grep, rg, git log, git show) is allowed
    3. **Allowed tools**: WebSearch / WebFetch (for verifying Apple APIs, library behaviour)
    4. **Review criteria** (4 dimensions + domain-specific checklist):
       - Technical correctness (API name, behaviour, version)
       - Logical consistency (internal contradictions, cross-section conflicts)
       - Completeness (missing edge case, error handling, prerequisite)
       - Efficiency (algorithm, CI / build / runtime cost)
    5. **Return format**: BLOCKER / MAJOR / MINOR three-level classification; each item with location (file + section) + suggestion. An **absence claim** ("the spec doesn't define this", "nothing covers this case") must include the grep/search command run and its zero-hit output — an absence claim with no evidence attached doesn't count as a finding.
    
    ## Accept / Reject reply style
    
    For each review finding the Leader gives:
    
    - **ACCEPT + reason**: accepted; specify who fixes it this round
    - **REJECT + reason**: rejected with a technical reason (not just "no")
    - **DEFER**: acknowledged but deferred (goes to backlog / open items)
    
    REJECT must cite specific evidence (API doc, prior decision, design constraint); pure preference is not acceptable.
    
    ## Anti-patterns
    
    - **Using CLI to probe Apple API behaviour**: forbidden. Use official docs / WebSearch instead.
    - **Repeatedly rejecting the same point in the same section**: more than 2 identical rejections counts as a communication failure; pause and clarify with the user.
    - **ACCEPT without a reason**: every ACCEPT should still have a one-line note of why it adds value.
    - **Leader drafting sections themselves**: violates the role separation; only allowed for cosmetic-grade fixes.
    
    ## Verification checklist
    
    - Every absence claim ("not defined", "no coverage") in a review finding cites the grep/search command and its zero-hit output — otherwise it doesn't count as a finding.
    - Each round has an explicit dispatch prompt (all 5 items of §Dispatch contract for Code Reviewer present, on top of the 6 elements from `leader-developer-handoff-contract`).
    - Each review finding has an explicit accept / reject label + reason.
    - When limit(N) is reached without convergence, pause; don't keep iterating indefinitely.
    - Cosmetic fixes are inline-edited by the Leader; don't burn a round on them.
    - The round-summary is recorded in the meeting log (not a verbatim copy of review content).
    
    ## Phase TODO sweep checklist
    
    A separate close-the-loop activity that fires **once per phase** (not once per review round), so it's not part of the round structure above. Before the Leader signs off on a phase-completion PR, run a sweep against the phase's diff scope to catch deferred-and-forgotten debt.
    
    For the full sweep procedure, command, disposition rule, and anti-pattern, read `references/phase-todo-sweep.md`.
    
    ## Related skills
    
    - `leader-developer-handoff-contract`: details the 6 required elements of every dispatch prompt.
    - `spec-phase-orchestration`: review cycles are usually embedded in the spec phase.
    - `methodology-pattern-extractor`: "round-1 cosmetic inline edit" is a codifiable pattern.
    - Official sources: when verifying or updating a factual or version-sensitive claim, read `references/official-docs.md`.
    

Comments (0)

Sign in to join the conversation.

No comments yet.

Reviews (0)

No reviews yet.

Related