edercnj/x-review-pr

Skill: Review PR (Master Orchestrator — ADR-0012)

Purpose

Orchestrate the full three-layer PR review pipeline:

1. Specialist Reviews (x-review-specialist) — all applicable domain specialists in parallel
2. Developer Reviews (x-review-developer) — all detected language specialists in parallel
3. Tech Lead Review (x-review-tech-lead) — dynamic dimensions, reads all reports, issues GO/NO-GO

If the Tech Lead issues a NO-GO, an automatic remediation agent is dispatched to fix all reported issues. The cycle repeats (max 2 auto-remediation cycles) until GO is achieved or the gate is exhausted.

This skill replaces the previous inline Tech Lead rubric. The Tech Lead logic now lives in x-review-tech-lead, which loads dynamic dimensions from the architecture plan and tech context.

Triggers

• /x-review-pr — review current branch against main
• /x-review-pr NNN — review PR #NNN
• /x-review-pr STORY-ID — review by story ID
• /x-review-pr --resume-review <pr> --interactive — resume an exhausted-retry gate session

Prerequisites

• Code must be committed
• Branch should have changes relative to main

Parameters

| Parameter | Required | Default | Description | |-----------|----------|---------|-------------| | Positional | No | current branch | PR-NNN, story-XXXX-YYYY, or empty (current branch vs main) | | --no-auto-remediation | No | false | Skip auto-remediation agents on NO-GO; route directly to Step 8.4 | | --interactive | No | false | Enable the Step 8.4 AskUserQuestion 3-option gate (default: HALT non-interactively) | | --resume-review <pr> | No | — | Resume gate from plans/review/<pr>/state.json |

Output Contract

| Artifact | Path | Produced By | |----------|------|-------------| | Per-specialist reports | .aikittools/features/feature-XXXX/reviews/review-{specialist}-story-XXXX-YYYY.md | Phase 1 | | Per-developer reports | .aikittools/features/feature-XXXX/reviews/review-developer-{lang}-story-XXXX-YYYY.md | Phase 2 | | Tech Lead report | .aikittools/features/feature-XXXX/reviews/review-tech-lead-story-XXXX-YYYY.md | Phase 3 | | Updated dashboard | .aikittools/features/feature-XXXX/reviews/dashboard-story-XXXX-YYYY.md | Phase 3 | | Updated remediation tracker | .aikittools/features/feature-XXXX/reviews/remediation-story-XXXX-YYYY.md | Phase 3 | | Frontmatter on Tech Lead report | YAML block per schema | Phase 3 (MANDATORY — Rule 24 §Camada-1) | | Gate state (opt-in) | plans/review/<pr-number>/state.json | Step 8.4 FIX-PR slot only |

CRITICAL EXECUTION RULE

6 phases (Rule 25 REGRA-001). Phases 1+2 run in parallel when possible. Phase 6 (frontmatter emission, done by x-review-tech-lead) is NON-NEGOTIABLE per Rule 24 §Camada-1 — absent frontmatter aborts with REVIEW_FRONTMATTER_INVALID (no fallback).

Workflow Overview

Phase 0: CONTEXT      -> Idempotency check + diff detect + context load (Steps 0–3)
Phase 1: SPECIALISTS  -> x-review-specialist (all applicable specialists in parallel)
Phase 2: DEVELOPERS   -> x-review-developer (all detected languages in parallel)
Phase 3: TECH-LEAD    -> x-review-tech-lead (reads all reports, dynamic rubric, GO/NO-GO)
Phase 4: REMEDIATION  -> Iterative auto-remediation cycles (max 2; then gate); update tracker
Phase 5: APPROVAL     -> Final approval, console summary, FINAL phase gate
Phase 6: FRONTMATTER  -> Emitted by x-review-tech-lead (MANDATORY — Rule 24 §Camada-1)

Phases 1 and 2 run concurrently in a SINGLE batch message when the runtime supports it. Each phase opens with TaskCreate + --mode pre gate and closes with TaskUpdate(completed) + POST/FINAL gate. Phase 4 iterates sub-tasks per cycle (max 2 per Rule 20 with addBlockedBy).

Phases 0–6

The detailed inline protocol lives in references/full-protocol.md:

• Phase 0 — Context & Idempotency (Steps 0–3): pre-check via mtime(report) >= mtime(latest commit) (skip to Phase 3 on reuse); detect PR vs STORY context; template gate (_TEMPLATE-TECH-LEAD-REVIEW.md per RULE-012 fallback); persist interactiveMode to execution-state.json.

  Canonical context selection (MANDATORY):

  Skill(skill: "x-internal-select-context-packs",
        args: "--phase review --capabilities <detected_capabilities_csv>")
  

• Phase 1 — Specialist Reviews (parallel):

  Skill(skill: "x-review-specialist", args: "{STORY_ID} [--agent {agent_name}]")
  

  Invokes all applicable specialist skills in a single batch. Each specialist saves its report to .aikittools/features/feature-XXXX/reviews/review-{specialist}-story-XXXX-YYYY.md. Security + QA always run; others conditional on tech-context.

• Phase 2 — Developer Reviews (parallel with Phase 1 when possible):

  Skill(skill: "x-review-developer", args: "{STORY_ID}")
  

  Detects active languages and invokes x-internal-review-developer-{lang} for each. Each language saves its report to .aikittools/features/feature-XXXX/reviews/review-developer-{lang}-story-XXXX-YYYY.md.

• Phase 3 — Tech Lead Review:

  Skill(skill: "x-review-tech-lead", args: "{STORY_ID_OR_PR_NUMBER} {interactiveFlag} [--agent {agent_name}]")
  

  Reads all reports from Phases 1 and 2. Applies dynamic rubric (base + conditional dimensions from arch plan + tech-context). Checks test coverage (≥95% line / ≥90% branch). Runs {{TEST_COMMAND}} + {{COVERAGE_COMMAND}} + {{SMOKE_COMMAND}}. Issues GO / NO-GO / GO-WITH-RESERVATIONS. Saves Tech Lead report and updates dashboard + remediation tracker.

• Phase 4 — Remediation Loop (only on NO-GO): iterative; max 2 auto-remediation cycles via Agent(subagent_type: "general-purpose", model: "sonnet", ...) dispatch. Agent reads ALL FAILED items from all specialist, developer, and Tech Lead reports. Applies fixes by classification (TEST_FAILURE / COVERAGE_GAP / CODE_QUALITY / SECURITY / ARCHITECTURE). After each fix cycle, re-runs Phases 1+2+3 to check if GO is achieved. Step 8.4 Exhausted-Retry Gate (Rule 20): non-interactive default HALT; interactive 3-option AskUserQuestion menu (Proceed / Fix-PR / Abort) with gateAttempts < 3 loop and REVIEW_FIX_LOOP_EXCEEDED guard-rail.

• Phase 5 — Final Approval: FINAL phase gate with --expected-artifacts covering all review artifacts (per-specialist, per-developer, tech-lead report, dashboard, remediation tracker).

• Phase 6 — Frontmatter (MANDATORY): emitted by x-review-tech-lead as its Phase 5. Validation via audit-review-frontmatter.sh → REVIEW_FRONTMATTER_INVALID on non-zero. No fallback (Rule 24 §Camada-1).

State File (opt-in)

Written only when the operator selects FIX-PR (slot 2) in Step 8.4. Enables resume via --resume-review <pr>. Path: plans/review/<pr-number>/state.json. Schema version "1.0" with fields phase, lastPhaseCompletedAt, lastGateDecision, fixAttempts[], schemaVersion. Full JSON schema in full-protocol §State File.

Error Envelope

Canonical orchestrator-wide error codes (COMMIT_FAILED, fail-open vs fail-closed conventions) live in _shared/error-handling-orchestrator.md. Review-specific patterns delegate to _shared/error-handling-review.md (empty input, RULE-012 fallback, specialist failure, idempotency). Rows below are skill-specific.

| Code / Scenario | Action | |------|---------| | REVIEW_REMEDIATION_EXHAUSTED | Operator selected ABORT in Step 8.4 gate | | REVIEW_FIX_LOOP_EXCEEDED | 3 consecutive PROCEED or FIX-PR attempts without converging to GO | | GATE_SCHEMA_INVALID | State file at plans/review/<pr>/state.json fails Rule 20 schema validation | | REVIEW_FRONTMATTER_INVALID | Phase 6 audit-review-frontmatter.sh returned exit ≠ 0 — no fallback (Rule 24 §Camada-1) | | No diff exists between branches | Abort: "No changes detected between current branch and base. Nothing to review." | | Template _TEMPLATE-TECH-LEAD-REVIEW.md missing | Log warning, use inline format as fallback (RULE-012). | | Test/Coverage/Smoke failure | Automatic NO-GO regardless of rubric score; record failures in report | | State file missing on --resume-review | Start gate fresh; emit warning | | x-review-specialist returns all FAILED | Log warning; Tech Lead proceeds with partial context | | x-review-developer detects no language | Log INFO; Tech Lead proceeds without developer reports |

Integration Notes

| Skill | Relationship | Context | |-------|-------------|---------| | x-implement-story | called-by | Produces the same artifacts as lifecycle Phase 6 | | x-review-specialist | calls (Phase 1) | Orchestrates all domain specialist reviews | | x-review-developer | calls (Phase 2) | Orchestrates all language developer reviews | | x-review-tech-lead | calls (Phase 3) | Issues final GO/NO-GO with dynamic dimensions | | x-review-specialist | deprecated alias | x-review-specialist now forwards to x-review-specialist | | x-fix-pr | calls (Step 8.4 FIX-PR slot) | Rule 20 interactive gate slot; x-fix-pr re-triggers x-review-pr | | x-internal-verify-phase-gates | calls (every phase) | Rule 25 Invariant 4 gate enforcement | | _TEMPLATE-TECH-LEAD-REVIEW.md | reads | Required output format; RULE-012 fallback to inline | | _TEMPLATE-CONSOLIDATED-REVIEW-DASHBOARD.md | updates | Cumulative across rounds (RULE-006) | | _TEMPLATE-REVIEW-REMEDIATION.md | updates | FIXED status tracking after Tech Lead review | | audit-review-frontmatter.sh | calls (Phase 6 via x-review-tech-lead) | Mandatory CI/runtime validation |

Full Protocol

Minimum viable contract above. Detailed phase-by-phase workflow (Step 0 idempotency mtime algorithm; Phase 1+2 parallel batch invocation pattern; Phase 3 Tech Lead execution with arch-plan loading and dynamic rubric; Phase 4 remediation Agent dispatch by classification with full prompts; Step 8.4 gate loop pseudocode with gateAttempts state machine and REVIEW_FIX_LOOP_EXCEEDED guard-rail) lives in references/full-protocol.md.