Back to Cherry Studio

Teams Review

.agents/skills/gh-pr-review/references/teams-review.md

1.9.413.6 KB
Original Source

Teams Review

You are the coordinator. Dispatch reviewer, verifier, and fixer agents using the Agent tool. Never modify files directly. Read code only for arbitration, diagnosis, and fix verification.

Always process all auto-fixable issues before involving the user. Do NOT pause to ask the user anything until Confirm (Phase 5) or Report (Phase 6).

The reviewer–verifier adversarial pair is the core quality mechanism: reviewers find issues, verifiers challenge them. This two-party check significantly reduces false positives. Reviewers and verifiers MUST NOT see each other's output or share conversation history.

Input from SKILL.md

  • FIX_MODE: low | low_medium | full

References

FilePurpose
code-checklist.mdCode review checklist
doc-checklist.mdDocument review checklist
judgment-matrix.mdRisk levels, worth-fixing criteria, special rules
checklist-evolution.mdChecklist update flow and rules

Flow

Scope → Review → Filter → Fix/Validate → Confirm → Report
  • Filter routes auto-fixable issues to Fix/Validate; remaining go to Confirm. If nothing to fix or confirm, skip directly to Report.
  • ConfirmFix/Validate loop until no pending issues remain.

Phase 1: Scope

Determine the diff to review based on $ARGUMENTS:

  • Empty arguments: find the base branch by checking common base branches in order: main, master. Use the first one that exists. Fetch the branch diff:
    git merge-base origin/{base_branch} HEAD
    git diff <merge-base-sha>
    
  • Commit hash (e.g., abc123): validate with git rev-parse --verify, then git show.
  • Commit range (e.g., abc123..def456 or abc123...def456): validate both endpoints. Fetch the diff including both endpoints:
    git diff A~1..B
    
  • File/directory paths: verify all paths exist on disk, then read file contents.

If diff is empty → show usage examples and exit: /gh-pr-review (uncommitted changes or current branch), /gh-pr-review a1b2c3d, /gh-pr-review a1b2c3d..e4f5g6h, /gh-pr-review src/foo.ts, /gh-pr-review 123, /gh-pr-review https://github.com/.../pull/123.

Associated PR comments

If gh is available, check whether the current branch has an open PR:

gh pr view --json number,state --jq 'select(.state == "OPEN") | .number' 2>/dev/null

If an open PR exists, fetch its line-level review comments:

gh api repos/{owner}/{repo}/pulls/{number}/comments

Store as PR_COMMENTS for verification in the review step.

Build baseline

Skip if doc-only. Run pnpm lint && pnpm test as build baseline. If no build/test commands can be determined, warn that fix validation will be skipped. Fail → abort.

Module partition

Partition files in scope into review modules for parallel review. Each module is a self-contained logical unit. Split large files by section/function group; group related small files together. Classify each module as code, doc, or mixed.

Suggested module boundaries for this project:

  • src/main/ — Main process services
  • src/renderer/src/ — React UI components, hooks, pages
  • packages/aiCore/ — AI SDK middleware & providers
  • packages/shared/ — Cross-process types and constants
  • src/preload/ — IPC bridge

Issue tracking

The coordinator tracks all issues in memory throughout the session. Each issue has:

  • Brief description
  • Status: pending | approved | fixed | failed | skipped
  • Risk: low | medium | high
  • File: file path:line
  • Proposed fix (medium/high risk only)

Phase 2: Review

Agent setup

Launch agents using the Agent tool:

  • One reviewer agent per module (use subagent_type: "general-purpose" or "feature-dev:code-reviewer").
  • One verifier agent (subagent_type: "general-purpose"), launched after all reviewers complete.

Module merging: if the total diff is ≤1000 changed lines AND ≤20 files, merge all modules into a single reviewer. The overhead of multiple agents (startup, coordination, forwarding) outweighs the parallelism benefit at this scale.

Launch all reviewer agents in parallel using multiple Agent tool calls in a single message.

Reviewer prompt

Stance: thorough — discover as many real issues as possible, self-verify before submitting.

Each reviewer receives:

  • Scope: file list + changed line ranges for its module. Reviewers fetch diffs and read additional context themselves as needed — coordinator does NOT pass raw diff or file contents.
  • Checklist: code-checklist.md for code, doc-checklist.md for doc, both for mixed. Include the checklist content verbatim in the reviewer prompt. For React/performance-heavy modules, also include relevant rules from vercel-react-best-practices skill as supplementary checks.
  • Evidence requirement: every issue must have a code citation (file:line + snippet) from the current tree.
  • Checklist exclusion: see the exclusion section in the corresponding checklist. Project rules loaded in context take priority.
  • Self-check: before submitting, re-read the relevant code and verify each issue. Mark as confirmed or withdrawn. Only submit confirmed issues. If a cited path/line no longer exists, locate the correct file/path via git diff --name-only or file search before reporting.
  • Output format: [file:line] [A/B/C] — [description] — [key lines]

PR comment reviewer (when PR_COMMENTS exist): one additional agent to verify PR review comments against current code. Same output format, same verification pipeline.

Verification

Stance: adversarial — default to doubting the reviewer, actively look for reasons each issue is wrong. Reject with real evidence, confirm if it holds up. This step is mandatory — the coordinator MUST NOT skip it or perform verification itself. Exception: if every reviewer explicitly reports zero issues (LGTM / no issues found), skip verification and proceed directly to Phase 3.

After all reviewer agents complete, collect their findings. Launch a single verifier agent with ALL findings combined. Include the following verbatim in the verifier's prompt:

You are a code review verifier. Your stance is adversarial — default to doubting the
reviewer's conclusion and actively look for reasons why the issue might be wrong. Your
job is to stress-test each issue so that only real problems survive.

For each issue you receive:

1. Read the cited code (file:line) and sufficient surrounding context.
2. Actively try to disprove the issue: Is the reviewer's reasoning flawed? Is there
   context that makes this a non-issue (e.g., invariants guaranteed by callers, platform
   constraints, intentional design)? Does the code actually behave as the reviewer
   claims? Look for the strongest counter-argument you can find.
3. Output for each issue:
   - Verdict: REJECT or CONFIRM
   - Reasoning: for REJECT, state the concrete counter-argument. For CONFIRM, briefly
     note what you checked and why no valid counter-argument exists.

Important constraints:
- Your counter-arguments must be grounded in real evidence from the code. Do not
  fabricate hypothetical defenses or invent caller guarantees that are not visible in
  the codebase.
- A CONFIRM verdict is not a failure — it means the reviewer found a real issue and
  your challenge validated it.

After review

Before entering Phase 3, confirm: (1) all reviewers have submitted their final reports; (2) the verifier has given a CONFIRM/REJECT verdict for every finding, OR all reviewers reported zero issues and verification was skipped.


Phase 3: Filter — coordinator only

Your stance here is neutral — trust no single party. Treat reviewer reports and verifier rebuttals as equally weighted inputs. Use your project-wide view to consider cross-module impact, conventions, and architectural intent that local reviewers may miss.

3.1 De-dup

Remove cross-reviewer duplicates (same location, same topic).

3.2 Existence check

Verifier verdictAction
CONFIRMPlausibility check — verify description matches cited code. Read code if anything looks off.
REJECTRead code. Evaluate both arguments. Drop only if counter-argument is sound.

3.3 Risk level

Consult judgment-matrix.md for risk level assessment, worth-fixing criteria, handling by risk level, and special rules.

Fix approach (Medium/High only): specify the chosen approach and reasoning. Record in the issue's Proposed field. Low risk: single obvious fix, no guidance.

3.4 Route

All confirmed issues are recorded with risk level.

Risk vs FIX_MODE
At or below thresholdauto-fix queue
Above thresholdpending (for Phase 5 Confirm)
  • Cross-module impact: if a fix requires updates outside the fixer's module, add it to the current fix queue and assign to the appropriate fixer.

Always auto-fix eligible issues first — do NOT present pending issues to the user before all auto-fixable issues have been processed and validated. Phase 4 if auto-fix queue is non-empty. Otherwise jump to Phase 5 if pending issues exist, or Phase 6 if none.


Phase 4: Fix/Validate

Fix

Stance: precise — apply each fix completely and correctly, never expand scope. The coordinator MUST NOT apply fixes directly.

Agent assignment: launch fixer agents using the Agent tool. Prefer reusing reviewer context by describing which files were already reviewed:

  • Issue in a file that a reviewer already analyzed → include that context in the fixer prompt.
  • Cross-module issues → single fixer agent with all relevant file paths.
  • Multi-file renames → single atomic task assigned to one agent.

One agent may receive multiple fix tasks if it covers several files. Avoid assigning the same file to multiple agents to prevent concurrent edit conflicts.

Each fixer receives (include verbatim in every fixer prompt):

Fix rules:
1. After fixing each issue, immediately: git commit --only <files> -m "message"
2. Only modify files explicitly assigned by the coordinator. Never use git add .
3. If a fix requires changes to unassigned files, stop and report to the coordinator
   for re-assignment.
4. Commit message: English, under 120 characters, ending with a period.
5. When in doubt, skip the fix rather than risk a wrong change.
6. Do not run build or tests.
7. Do not modify public API function signatures or class definitions (comments are OK),
   unless the coordinator's issue description explicitly requires an API signature fix.
8. After each fix, check whether the change affects related comments or documentation
   within your assigned files (function/class doc-comments, inline comments describing
   the changed logic). If so, update them in the same commit as the fix.
   Cross-module documentation updates (README, spec files, other modules) are handled
   separately by the coordinator.
9. When done, report the commit hash for each fix and list any skipped issues with
   the reason for skipping.

Each fixer commits per issue (one commit per fix — never combine multiple issues into a single commit).

Verify fixes (coordinator)

Wait for all fixers. Before running build + test, the coordinator reads each fixer's commit diff and verifies:

  1. The fix correctly addresses the original issue
  2. No new issues introduced (naming inconsistencies, missing updates in surrounding code, logic errors)
  3. Fix scope matches the issue — no unintended changes

If a problem is found, launch a correction agent with specific details (max 1 retry). If the retry fails, revert and mark failed.

Build/test validate

Run pnpm lint && pnpm test.

Revert scope: only revert commits produced by fixers in this phase. Never revert commits unrelated to the current fixes. Identify fixer commits by the commit hashes reported by fixers; any other commits on the branch are out of scope.

  • Skip if no build/test commands available or doc-only modules.
  • Pass → mark issues fixed.
  • Fail → bisect among fixer commits only to find the failing commit, revert it, re-validate remaining before blaming others (one bad commit may cause cascading failures). Per failing issue: retry via a new fixer agent with failure details (max 2 retries), or revert and mark failed.

After validation

Condition
pending or failed issues existPhase 5 (Confirm)
OtherwisePhase 6 (Report)

If Phase 5 approves further fixes, launch new fixer agents and re-enter Phase 4.


Phase 5: Confirm

Present pending + failed issues grouped by risk (high → low), sorted by file path within each group: [number] [file:line] [risk] [reason] — [description]

Then present issues via multi-select. Each option label is the issue summary (e.g., [risk] file:line — description). Checked → approved, unchecked → skipped.

If the user replies with a bulk instruction (e.g., "fix all", "skip the rest"), apply it only to issues at or below the current FIX_MODE threshold. Issues above the threshold still require individual confirmation.

  • All skipped → Phase 6.
  • Any approved → Phase 4 (Fix/Validate). After validation, if more pending/failed remain, return here (Phase 5). If nothing remains, proceed to Phase 6.

Phase 6: Report

Summary:

  • Issues found / fixed / skipped / failed
  • Rolled-back issues and reasons
  • Final test result
  • Issues from PR comments (when PR_COMMENTS existed)
  • Note: "To verify fix quality, run /gh-pr-review again."

Checklist evolution

Review all confirmed issues from this session. If any represent a recurring pattern not covered by the current checklist, read checklist-evolution.md and follow its steps.