jankneumann

parallel-review-plan

Independent plan review producing structured findings per review-findings.schema.json

jankneumann 4 1 Updated 2w ago
GitHub

Install

npx skillscat add jankneumann/agentic-coding-tools/parallel-review-plan

Install via the SkillsCat registry.

SKILL.md

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, Antigravity, Grok,
and Pi 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)

Eight-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.

Eight Axes (the axis field)

Five axes adopted from the code-review-and-quality reference skill, plus three fitness-function axes. 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.
observability Can the running system be understood from the outside? Missing logs/metrics/traces, unactionable errors, no way to detect the failure mode the plan introduces.
resilience Does it degrade safely? Missing retries/timeouts/backoff, unbounded blast radius, no fallback, undefined partial-failure behavior.
compatibility Does it stay compatible with what already exists? Breaking API/schema/CLI changes, missing migration path, version or platform assumptions.

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. The required-field list and enums MUST come from review_findings_schema.prompt_contract() / prompt_contract_block() — do not hand-copy the field list.

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: An explicit --agents-yaml wins. Otherwise the dispatcher searches from the reviewed --cwd for checkout-local agent-coordinator/agents.yaml, then tries the configured coordination MCP/HTTP roster, and finally the process/global disk fallback. SDK-only coordinator rosters count as configured and are not discarded. Dispatch, --check-vendors, and --list-agents use this same resolver. Use --list-agents --cwd <reviewed-checkout> to verify the exact roster.

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 seven 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.