jankneumann/parallel-review-plan

Parallel Review Plan

Receive plan artifacts as read-only input and produce structured findings conforming to review-findings.schema.json. Designed for vendor-diverse dispatch — any LLM agent can execute this skill.

Arguments

$ARGUMENTS - OpenSpec change-id to review (e.g., "add-user-authentication")

Optional flags:

• --adversarial — Use adversarial review mode: challenges design decisions instead of standard review

Prerequisites

• OpenSpec proposal exists at openspec/changes/<change-id>/
• Proposal has been generated by /parallel-plan-feature or /linear-plan-feature

Provider-Neutral Dispatch

Plan review uses the provider-neutral dispatch adapter/configuration path as the canonical cross-provider mechanism. Claude Code, Codex, and Gemini/Jules are first-class reviewers when configured; provider-specific CLI or harness details stay inside their adapters.

Input (Read-Only)

The reviewer receives these artifacts as context but MUST NOT modify them:

• openspec/changes/<change-id>/proposal.md
• openspec/changes/<change-id>/design.md
• openspec/changes/<change-id>/tasks.md
• openspec/changes/<change-id>/specs/**/spec.md
• openspec/changes/<change-id>/contracts/ (if present)
• openspec/changes/<change-id>/work-packages.yaml (if present)

Five-Axis Finding Schema

Every finding produced by this skill MUST be classified into BOTH dimensions below. The JSON Schema at openspec/schemas/review-findings.schema.json enforces both fields as required — output that omits either is rejected by the validator in Step 4.

Five Axes (the axis field)

Adopted from the code-review-and-quality reference skill. Pick exactly one:

| Axis | What it covers | |---|---| | correctness | Does the plan, when implemented, produce the right answer? Bugs, off-by-one, missing requirements, ambiguous SHALL clauses. | | readability | Will a future reader understand intent? Naming, structure, spec clarity, ambiguous prose. | | architecture | Does the design fit the system? Module boundaries, layering, coupling, dependency direction, contract shape. | | security | Does the plan introduce or fail to prevent security risk? Auth gaps, input-validation holes, secret handling, OWASP categories. | | performance | Will the implementation be fast and scalable enough? N+1, unbounded queries, missing pagination, hot-path allocations. |

The legacy type enum (spec_gap, contract_mismatch, etc. — see Step 3) is preserved for backward compatibility; axis is the new mandatory categorization that all reviewers — human or vendor — must agree on.

Five Severity Prefixes (the severity field)

Every finding's description MUST begin with one of these markers. The severity enum value MUST match the prefix.

| Prefix | Severity value | Meaning | |---|---|---| | Critical | critical | Blocks merge. Must be fixed before implementation begins (or before the PR merges). | | Nit | nit | Should fix but does not block. Quality, naming, minor structure. | | Optional | optional | Consider it. Author may accept or reject without further discussion. | | FYI | fyi | Informational. Surfaces context the author may not have known; no action required. | | none | none | Positive observation. Names what the plan got right so good patterns survive review. |

Example finding (note prefix and matching severity):

{
  "id": 1,
  "axis": "security",
  "severity": "critical",
  "type": "security",
  "criticality": "high",
  "description": "Critical: Requirement R3 admits unauthenticated DELETE on /v1/users/{id} — missing auth precondition.",
  "resolution": "Add a SHALL clause requiring an authenticated session with role=admin before the DELETE handler executes.",
  "disposition": "fix"
}

Reviewers MUST NOT collapse multiple severities onto one finding (split them). Reviewers MUST NOT use a severity that contradicts the disposition (e.g., severity: critical with disposition: accept is incoherent — escalate instead).

Steps

1. Load Review Context

Read all plan artifacts listed above. Build a mental model of:

• What the feature does (proposal.md)
• How it will be built (design.md)
• What requirements it satisfies (specs/)
• How work is decomposed (tasks.md, work-packages.yaml)

2. Review Checklist

Evaluate the plan against these dimensions:

Specification Completeness

• [ ] All requirements use SHALL/MUST language
• [ ] Requirements are testable and verifiable
• [ ] No ambiguous or vague terms

Contract Consistency

• [ ] OpenAPI schemas match spec requirements
• [ ] Database schemas support all declared operations
• [ ] Event schemas cover all async flows
• [ ] Generated types match OpenAPI definitions

Architecture Alignment

• [ ] Design follows existing codebase patterns
• [ ] No unnecessary dependencies introduced
• [ ] Error handling strategy is complete
• [ ] Migration path is reversible

Security Review

• [ ] Input validation at system boundaries
• [ ] Authentication/authorization for new endpoints
• [ ] No secrets in configuration or code
• [ ] OWASP top-10 considerations addressed

Performance Review

• [ ] No unbounded queries or loops in design
• [ ] List operations have pagination or size limits
• [ ] Synchronous operations that should be async are identified
• [ ] Caching strategy defined for hot paths (if applicable)
• [ ] Rate limiting considered for public endpoints

Observability Review

• [ ] Monitoring requirements defined for new services/endpoints
• [ ] Structured logging requirements for key operations
• [ ] Alerting criteria specified for failure conditions
• [ ] Health/readiness endpoint requirements for new services

Compatibility Review

• [ ] Breaking changes to existing APIs are identified and justified
• [ ] Data migration plan is reversible (rollback path exists)
• [ ] Consumer impact analysis for changed interfaces
• [ ] Deprecation notices for removed or changed APIs

Resilience Review

• [ ] Retry/timeout/fallback requirements for external dependencies
• [ ] Failure mode analysis for critical paths
• [ ] Idempotency requirements for operations that may be retried
• [ ] Graceful degradation strategy when dependencies are unavailable

Work Package Validity (if work-packages.yaml exists)

• [ ] DAG has no cycles
• [ ] Parallel packages have non-overlapping write scopes
• [ ] Lock keys follow canonicalization rules
• [ ] Verification steps are appropriate for each package tier
• [ ] Integration package depends on all implementation packages

3. Produce Findings

Generate findings as a JSON array conforming to review-findings.schema.json:

{
  "review_type": "plan",
  "target": "<change-id>",
  "reviewer_vendor": "<model-name>",
  "findings": [
    {
      "id": 1,
      "type": "spec_gap",
      "criticality": "high",
      "description": "Requirement R3 lacks error handling specification for 429 rate limit responses",
      "resolution": "Add a requirement specifying retry-after header handling",
      "disposition": "fix"
    }
  ]
}

Finding Types

• spec_gap — Missing or incomplete requirements
• contract_mismatch — Inconsistency between contracts and specs
• architecture — Design pattern or structural concern
• security — Security vulnerability or missing protection
• performance — Potential performance issue
• style — Code style or convention violation
• correctness — Logical error in the plan
• observability — Missing monitoring, logging, or alerting requirements
• compatibility — Breaking change to existing API or missing migration plan
• resilience — Missing retry, timeout, or fallback requirements

Dispositions

• fix — Author should fix before implementation
• regenerate — Artifact needs regeneration (e.g., contract schema mismatch)
• accept — Minor issue, acceptable as-is
• escalate — Requires human decision or cross-team coordination

4. Validate Output

Validate the findings JSON against openspec/schemas/review-findings.schema.json:

# Quick validation
python3 -c "
import json, jsonschema
schema = json.load(open('openspec/schemas/review-findings.schema.json'))
findings = json.load(open('<findings-output-path>'))
jsonschema.validate(findings, schema)
print('Valid')
"

5. Submit Findings

Write findings to openspec/changes/<change-id>/review-findings-plan.json.

If CAN_HANDOFF=true, write a review handoff with:

• Summary of critical/high findings
• Overall disposition recommendation (proceed/revise/block)

6. Dispatch Multi-Vendor Reviews

After writing your own findings, dispatch reviews to other vendor CLIs and synthesize consensus.

Write the review prompt to openspec/changes/<change-id>/reviews/review-prompt.md — include instructions to read the plan artifacts and output only valid JSON conforming to review-findings.schema.json.

Adversarial mode: If --adversarial flag was passed, wrap the review prompt with adversarial framing before dispatch:

from adversarial_prompt import wrap_adversarial
prompt = wrap_adversarial(prompt)  # Prepends contrarian persona instructions

The dispatch still uses --mode review (unchanged) — only the prompt content differs. Adversarial findings flow through the same consensus pipeline with equal weight (Design Decision D1).

Dispatch to other vendors (excluding the current agent's vendor):

python3 "<skill-base-dir>/../parallel-infrastructure/scripts/review_dispatcher.py" \
  --review-type plan \
  --mode review \
  --prompt-file "openspec/changes/<change-id>/reviews/review-prompt.md" \
  --cwd "$(pwd)" \
  --output-dir "openspec/changes/<change-id>/reviews" \
  --exclude-vendor claude_code \
  --timeout 600

This dispatches to all available vendors configured in agents.yaml with cli sections. Each vendor runs independently and writes findings to reviews/findings-<vendor>-plan.json.

Agent discovery resolution chain: The dispatcher resolves agents via the coordination MCP server configured in ~/.claude.json → mcpServers.coordination. It extracts the agent-coordinator/ directory from the MCP server args and runs get_dispatch_configs.py to load agents.yaml. If the coordinator is not configured, pass --agents-yaml <path> explicitly as fallback. Use --list-agents to verify available agents.

Troubleshooting dispatch failures: Run python3 <script> --list-agents to verify agent discovery. Common issues: (1) ~/.claude.json has no mcpServers.coordination entry — run /setup-coordinator, (2) async/remote agents may time out — local agents are more reliable, (3) some vendors may return non-JSON output — check review-manifest.json for error details.

Synthesize consensus from all findings (yours + vendor results):

python3 "<skill-base-dir>/../parallel-infrastructure/scripts/consensus_synthesizer.py" \
  --review-type plan \
  --target "<change-id>" \
  --findings "openspec/changes/<change-id>/review-findings-plan.json" \
             "openspec/changes/<change-id>/reviews/findings-"*"-plan.json" \
  --output "openspec/changes/<change-id>/reviews/consensus-plan.json"

Present consensus summary to the user:

• Confirmed findings (2+ vendors agree) — high confidence, may block
• Unconfirmed findings (single vendor) — lower confidence, warnings
• Disagreements (vendors disagree on disposition) — escalate to human

If no other vendors are available (CLIs not installed), skip this step and proceed with single-vendor findings only.

Output

• openspec/changes/<change-id>/review-findings-plan.json — your findings
• openspec/changes/<change-id>/reviews/findings-<vendor>-plan.json — per-vendor findings
• openspec/changes/<change-id>/reviews/consensus-plan.json — synthesized consensus
• openspec/changes/<change-id>/reviews/review-manifest.json — dispatch metadata

Design for Vendor Diversity

This skill is intentionally simple and self-contained so it can be dispatched to any LLM agent:

• No coordinator dependencies required
• All input is file-based (read-only)
• Output is a single JSON file with a well-defined schema
• No side effects (no git commits, no lock acquisition)

When this skill is dispatched to another vendor by the orchestrator, only Steps 1-5 run (the vendor produces findings). Step 6 (multi-vendor dispatch) only runs when this skill is the primary reviewer — i.e., when invoked directly by the user or the orchestrating agent.

Common Rationalizations

| Rationalization | Why it's wrong | |---|---| | "I only found one issue, so I'll skip the axis/severity classification — the description is enough" | The schema rejects findings without axis and severity; cross-vendor consensus relies on these fields to match equivalent findings. Skipping = the dispatcher discards your review. | | "This finding spans two axes — I'll just pick one" | Pick the dominant axis and split the rest into separate findings. Mashing two axes into one description means consensus matching cannot deduplicate against another vendor who split them. | | "The plan looks fine — I'll just emit zero findings" | A review with zero findings is suspicious. At minimum, emit severity: none positive observations naming what the plan got right; this signals the review actually happened rather than timed out. | | "The orchestrator will catch contradictions between severity and disposition" | It won't — the orchestrator routes by disposition. Inconsistent severity/disposition pairs survive into the consensus and confuse downstream automation. Make them coherent at write time. |

Red Flags

• A review-findings-plan.json file with findings that lack the axis field — the schema validation step (Step 4) was skipped or its output ignored.
• Every finding has the same severity (e.g., everything is Critical). A real review covers a spectrum; uniform severity means the reviewer wasn't actually grading.
• Description prose does NOT start with the matching severity prefix (Critical: / Nit: / Optional: / FYI: / nothing-for-none). The prefix is the human-readable signal; if it disagrees with the enum value, the reviewer wrote the JSON without re-reading the prose.
• A security-axis finding with disposition: accept. Security findings are never silently accepted; if the risk is real, the disposition must be fix or escalate.
• Findings reference file paths or line ranges that don't exist in the plan artifacts (proposal/design/specs). The reviewer hallucinated context.

Verification

1. Run the JSON Schema validator from Step 4 and confirm Valid — this proves axis and severity are present on every finding.
2. Spot-check 3 findings: confirm the description text begins with the prefix matching the severity enum value (e.g., severity: critical ↔ description starts with Critical:).
3. Confirm at least two different axis values appear across the findings array (a single-axis review missed the other four dimensions of the schema).
4. Confirm disposition is coherent with severity: critical/nit → fix; optional/fyi → accept; none → accept; mismatches must be escalate with a justification in resolution.
5. Confirm reviewer_vendor is populated — anonymous findings cannot participate in consensus.