|
| 1 | +# Review Protocol Workflow |
| 2 | + |
| 3 | +Standardized 3-stage review process producing schema-compliant JSON output. |
| 4 | +See `${CLAUDE_PLUGIN_ROOT}/docs/REVIEW-SCHEMA.md` for the output schema. |
| 5 | + |
| 6 | +## Parameters |
| 7 | + |
| 8 | +Callers configure the review by providing: |
| 9 | + |
| 10 | +| Parameter | Required | Description | |
| 11 | +|-----------|----------|-------------| |
| 12 | +| `subject` | Yes | What is being reviewed (used in schema output) | |
| 13 | +| `artifact` | Yes | The content to review (text, JSON, code, design spec) | |
| 14 | +| `perspectives` | Yes | Array of perspective definitions (see below) | |
| 15 | +| `pass_threshold` | No | Minimum dimension score to pass. Default: 4 | |
| 16 | +| `start_stage` | No | `1` (default), `2`, or `3`. Use `2` to skip mental pre-review when caller already self-validated | |
| 17 | +| `max_revision_cycles` | No | Default: 3 | |
| 18 | +| `caller_id` | No | If the caller has a registered schema (see `scripts/validate-review-output.sh --list-callers`), pass the caller ID here to enable per-caller validation of perspectives, dimensions, and reviewer-specific finding fields. Known IDs: `roadmap`, `design-wireframe`, `implementation-plan`, `retro`, `design-review`, `dev-onboarding`, `preplanning`. | |
| 19 | + |
| 20 | +### Perspective Definition |
| 21 | + |
| 22 | +Each perspective in the `perspectives` array: |
| 23 | + |
| 24 | +``` |
| 25 | +{ |
| 26 | + "name": "Security", |
| 27 | + "dimensions": { |
| 28 | + "auth_coverage": "All endpoints require appropriate authentication", |
| 29 | + "input_validation": "All user inputs are validated and sanitized" |
| 30 | + }, |
| 31 | + "context": "Optional additional context for this perspective" |
| 32 | +} |
| 33 | +``` |
| 34 | + |
| 35 | +- `name`: Short label (appears in output as `perspective`) |
| 36 | +- `dimensions`: Map of dimension names to descriptions of what "passing" looks like |
| 37 | +- `context`: Optional extra context the reviewer should consider |
| 38 | + |
| 39 | +For complex reviewers with separate prompt files (e.g., `/dso:design-wireframe`'s reviewer prompts), the caller reads the file and passes its content as `context`. |
| 40 | + |
| 41 | +--- |
| 42 | + |
| 43 | +## Stage 1: Mental Pre-Review |
| 44 | + |
| 45 | +**Skip when**: `start_stage >= 2`. Use this when the calling skill has already performed its own self-validation (e.g., `/dso:design-wireframe` Phase 4 artifact consistency check). |
| 46 | + |
| 47 | +The calling agent reviews the artifact against each perspective's dimensions. For each dimension, ask: "Would this score below {pass_threshold}?" |
| 48 | + |
| 49 | +If obvious issues are found: |
| 50 | +1. Fix them directly in the artifact |
| 51 | +2. Note what was changed (for transparency in the review log) |
| 52 | +3. Proceed to Stage 2 with the revised artifact |
| 53 | + |
| 54 | +This is cheap (no sub-agent) and catches low-hanging issues before spending tokens on a sub-agent. |
| 55 | + |
| 56 | +--- |
| 57 | + |
| 58 | +## Stage 2: Sub-Agent Structured Review |
| 59 | + |
| 60 | +Dispatch a **single sub-agent** with a structured multi-perspective rubric. |
| 61 | + |
| 62 | +### Sub-Agent Prompt Template |
| 63 | + |
| 64 | +``` |
| 65 | +## Structured Multi-Perspective Review |
| 66 | +
|
| 67 | +Review the following artifact from multiple perspectives. For each perspective, |
| 68 | +score every dimension 1-5 (or null if not applicable). For any score below |
| 69 | +{pass_threshold}, provide a finding with severity, description, and specific |
| 70 | +suggestion. |
| 71 | +
|
| 72 | +### Artifact |
| 73 | +{artifact content} |
| 74 | +
|
| 75 | +### Perspectives and Dimensions |
| 76 | +
|
| 77 | +{For each perspective:} |
| 78 | +#### {perspective.name} |
| 79 | +{perspective.context if provided} |
| 80 | +
|
| 81 | +Dimensions (score each 1-5, null if N/A): |
| 82 | +{For each dimension: "- {name}: {description of passing}"} |
| 83 | +
|
| 84 | +### Conflict Detection |
| 85 | +
|
| 86 | +After scoring all dimensions, scan your findings for contradictions: |
| 87 | +- Group findings by target (the component, file, or section they address) |
| 88 | +- Within each group, check if any two suggestions pull in opposite directions: |
| 89 | + - add vs remove |
| 90 | + - more detail vs less complexity |
| 91 | + - stricter vs more flexible |
| 92 | + - expand scope vs reduce scope |
| 93 | +- Only flag contradictions where both findings have severity "critical" or "major" |
| 94 | +- Minor-vs-anything is not a conflict (minor finding yields to the other) |
| 95 | +
|
| 96 | +### Output Format |
| 97 | +
|
| 98 | +Return valid JSON matching this structure: |
| 99 | +{REVIEW-SCHEMA.md schema} |
| 100 | +``` |
| 101 | + |
| 102 | +#### Determine Review Model |
| 103 | + |
| 104 | +Check whether the artifact under review contains high-blast-radius content. For code reviews, use the pattern list in `REVIEW-WORKFLOW.md` Step 3. For non-code reviews (plans, designs), the caller specifies the model (e.g., `/dso:plan-review` uses opus for designs, sonnet for implementation plans). |
| 105 | + |
| 106 | +If **any** high-blast-radius pattern matches → `model="opus"`. Otherwise → `model="sonnet"`. |
| 107 | + |
| 108 | +Launch with: |
| 109 | +``` |
| 110 | +subagent_type: "general-purpose" |
| 111 | +model: "{opus or sonnet per detection above}" |
| 112 | +``` |
| 113 | + |
| 114 | +### Parse and Validate |
| 115 | + |
| 116 | +After the sub-agent returns: |
| 117 | +1. Save the JSON output to a temp file |
| 118 | +2. Run the schema validator (schema-hash: 3053fa9a43e12b79): |
| 119 | + ```bash |
| 120 | + REPO_ROOT=$(git rev-parse --show-toplevel) |
| 121 | + source "${CLAUDE_PLUGIN_ROOT}/hooks/lib/deps.sh" |
| 122 | + REVIEW_OUT="$(get_artifacts_dir)/review-protocol-output.json" |
| 123 | + cat > "$REVIEW_OUT" <<'EOF' |
| 124 | + <sub-agent JSON output> |
| 125 | + EOF |
| 126 | +
|
| 127 | + # Base schema check (always) |
| 128 | + ".claude/scripts/dso validate-review-output.sh" review-protocol "$REVIEW_OUT" |
| 129 | +
|
| 130 | + # Per-caller check (when caller_id is provided) |
| 131 | + # ".claude/scripts/dso validate-review-output.sh" review-protocol "$REVIEW_OUT" --caller <caller_id> |
| 132 | + ``` |
| 133 | +3. If `SCHEMA_VALID: no` — retry the sub-agent once with an explicit format correction prompt; do not proceed with invalid output |
| 134 | +4. If `SCHEMA_VALID: yes` — proceed to Stage 3 |
| 135 | +
|
| 136 | +**When `caller_id` is provided**, pass `--caller <caller_id>` to the validator. This additionally checks that: |
| 137 | +- All expected perspectives are present (or marked `not_applicable`) |
| 138 | +- Each perspective's `dimensions` map contains the required dimension keys |
| 139 | +- Each finding contains the reviewer-specific fields required by that perspective (e.g., `wcag_criterion` for Accessibility, `owasp_category` for Security) |
| 140 | +- Enum-typed fields use valid values (e.g., `complexity_estimate` must be `"low"`, `"medium"`, or `"high"`) |
| 141 | +- Conditional fields are checked only when the finding's `dimension` matches (e.g., `stale_location` is only required for `freshness` findings) |
| 142 | +
|
| 143 | +--- |
| 144 | +
|
| 145 | +## Stage 3: Conflict Escalation |
| 146 | +
|
| 147 | +**Trigger**: `conflicts[]` array is non-empty after Stage 2. |
| 148 | +
|
| 149 | +For each conflict: |
| 150 | +- If one finding is `critical` and the other is `minor` → resolve in favor of the critical finding, no escalation |
| 151 | +- If both are `critical` or `major` → **escalate to user** via AskUserQuestion, presenting both suggestions as options |
| 152 | +- If both are `minor` → caller chooses either direction without escalation |
| 153 | +
|
| 154 | +**Multi-agent escalation** (optional, caller decides): If the caller prefers sub-agent resolution over user escalation, dispatch one sub-agent per conflicting perspective to debate the tradeoff. Each receives the other's finding and must propose a resolution that addresses both concerns. Use only when user escalation is impractical (e.g., batch processing). |
| 155 | +
|
| 156 | +--- |
| 157 | +
|
| 158 | +## Revision Protocol |
| 159 | +
|
| 160 | +When the review does not pass (any dimension below `pass_threshold`): |
| 161 | +
|
| 162 | +1. **Triage by severity**: Address `critical` findings first, then `major`, then `minor`. Skip `minor` if they don't affect pass/fail. |
| 163 | +
|
| 164 | +2. **Check conflicts first**: If `conflicts[]` is non-empty, resolve conflicts (Stage 3) before spending a revision cycle. |
| 165 | +
|
| 166 | +3. **Revise**: For each finding being addressed, modify the specific artifact it targets. The calling skill documents what changed. |
| 167 | +
|
| 168 | +4. **Re-submit**: Send the full revised artifact back through Stage 2. Do not send partial updates. |
| 169 | +
|
| 170 | +5. **Cycle limit**: Maximum `max_revision_cycles` automated cycles (default 3). After exhausting cycles: |
| 171 | + - Do NOT revise automatically |
| 172 | + - Present the user with: current artifact state, all unresolved findings across all cycles, and specific questions about direction |
| 173 | + - Apply user input, then run one final Stage 2 review |
| 174 | +
|
| 175 | +6. **Cycle tracking**: The calling skill is responsible for tracking cycle count and logging review history in whatever format it uses (ticket notes, review-log.md, etc.). |
| 176 | +
|
| 177 | +--- |
| 178 | +
|
| 179 | +## Quick Reference |
| 180 | +
|
| 181 | +``` |
| 182 | +Stage 1 (Mental) → Caller self-reviews, fixes obvious issues |
| 183 | + Skip if start_stage=2 |
| 184 | +Stage 2 (Single) → One sub-agent, multi-perspective rubric |
| 185 | + Returns REVIEW-SCHEMA.md JSON |
| 186 | +Stage 3 (Conflict) → Triggered by non-empty conflicts[] |
| 187 | + Escalate critical+critical to user |
| 188 | + Minor yields to other finding |
| 189 | +Revision → Triage by severity, resolve conflicts first |
| 190 | + Max 3 cycles, then escalate to user |
| 191 | +``` |
| 192 | +
|
| 193 | +## Common Mistakes |
| 194 | +
|
| 195 | +| Mistake | Fix | |
| 196 | +|---------|-----| |
| 197 | +| Skipping Stage 1 without reason | Only skip if caller already self-validated. Set `start_stage: 2` explicitly. | |
| 198 | +| Speculative conflict detection | Don't guess whether a suggestion "might" worsen another dimension. Only flag direct contradictions in suggestions. | |
| 199 | +| Revising before resolving conflicts | Always resolve Stage 3 conflicts before spending a revision cycle. | |
| 200 | +| Ignoring N/A dimensions | `null` scores are valid. Don't treat them as failures. | |
| 201 | +| Retrying failed review parse indefinitely | One retry with format correction. After that, report the raw output to the caller. | |
0 commit comments