plan-reviewer

name	plan-reviewer
description	Review SDD task plans for execution blockers and return [OKAY] or [REJECT].
metadata	{"author":"thoth-agents","version":"1.0"}

Plan Reviewer Skill

Verify that an SDD task plan is executable, references valid files, and is safe to hand to implementation.

Shared Conventions

• ../_shared/openspec-convention.md
• ../_shared/persistence-contract.md
• ../_shared/thoth-mem-convention.md

Purpose

Review the tasks artifact for true execution blockers. Retrieve it according to the persistence mode: read openspec/changes/{change-name}/tasks.md for openspec/hybrid modes, use the thoth-mem recall funnel (mem_recall(mode="compact") -> mem_recall(mode="context") -> mem_get(...)) for thoth-mem/hybrid modes, or read from inline context for none mode.

The artifact governance validator is not part of this review. Plan-reviewer is only the pre-execution approval gate for the task plan; it does not run the validator, enforce its findings, or manage the future pre-sdd-apply handoff. Plan-reviewer remains read-only: it returns the durable review payload, while the orchestrator or quick persistence helper writes plan-review.md and/or sdd/{change-name}/plan-review when the selected persistence mode requires it. Invoke the semantic oracle role through the available role dispatch surface.

Focus on whether the plan can be executed as written, not whether you would have designed it differently.

Inputs

• change-name
• pipeline-type (accelerated or full)
• persistence-mode (thoth-mem, openspec, hybrid, or none)
• Tasks artifact (retrieved per persistence mode: filesystem for openspec/hybrid, thoth-mem for thoth-mem/hybrid, inline for none)
• Full pipeline: related spec and design artifacts when needed for dependency checks
• Accelerated pipeline: proposal artifact when needed for dependency checks

Task State Awareness

Recognize these checklist states in tasks.md:

• - [ ] pending
• - [~] in progress
• - [x] completed
• - [-] skipped with reason

Review executability of the remaining work. If a task is marked - [-], ensure the skip reason is explicit and does not hide a blocker.

Review Checklist

Check only what affects executability:

1. Referenced file paths exist when they are supposed to already exist.
2. New-file tasks use exact intended paths.
3. Tasks reference exact file paths instead of vague areas.
4. Each task includes a Verification section.
5. Each Verification section includes both:
   • Run:
   • Expected:
6. Dependency order is valid.
7. The sequence is workable without hidden prerequisite steps.
8. In-scope success criteria from the accepted proposal/spec are represented by executable tasks.
9. Deferred or unresolved affected areas from the accepted proposal/spec have required discovery, coordination, follow-up tasks, or an explicit execution warning.
10. Task boundaries or non-goals do not contradict accepted proposal/spec scope.

Cross-Artifact Consistency Analysis

In addition to the executability review above, perform cross-artifact
consistency analysis across proposal <-> spec <-> design <-> tasks. This is NOT
a new pipeline phase; it rides inside this plan-review gate. All checks below
are gated by the corresponding `openspec/config.yaml rules:` sections; a section
that disables enforcement downgrades its block to a report.

### Severity model

Every consistency finding carries one severity:

- `CRITICAL` — a spec requirement with no mapping in tasks, an orphan task with
  no spec/proposal basis, scope drift contradicting the accepted proposal, or a
  direct contradiction between artifacts.
- `HIGH` / `MEDIUM` / `LOW` — weaker inconsistencies, ambiguity, or
  presentation issues that are worth reporting but are not on their own
  blocking.

Detect and report at least: requirements present in the spec but absent from
design or tasks; tasks with no spec basis; scope drift from the proposal; and
contradictory statements between artifacts.

### Requirement-coverage percentage

Compute and REPORT a requirement-coverage percentage in the review output
(gated by `rules.consistency.require_coverage_percentage`):

```
coverage % = (distinct spec requirements named by >=1 task `Spec:` tag)
             / (total `### Requirement:` headings across all delta specs)
```

Parse `Spec:` trace tags from `tasks.md` and `### Requirement:` headings from
`openspec/changes/{change}/specs/*/spec.md`. Example: 8 of 10 requirements
covered -> report `80%`. When no `Spec:` tags are present (legacy tasks), report
coverage against what is present rather than failing.

### [NEEDS CLARIFICATION] cap enforcement

Flag any spec file containing more than `rules.clarification.max_markers_per_spec`
(default 3) `[NEEDS CLARIFICATION: ...]` markers. Exactly 3 is within the cap
and is NOT flagged; 4 or more is flagged as carrying too much unresolved
ambiguity to proceed cleanly.

### Constitution Check

Evaluate the design and plan against EACH principle in
`openspec/memory/constitution.md` (delegate-first coordination, read-only role
boundaries, governed persistence, multi-harness parity, evidence-led
verification). Report the specific violated principle(s). Gated by
`rules.constitution.enforce_check`; when disabled, note the skip in the output
and do not block.

### TDD ordering check

When `rules.tasks.tdd` is enabled, verify that within each phase every
implementation task that has a corresponding test task is PRECEDED by that test
task. Report a TDD-ordering finding for any implementation task that precedes
its test task. When `tasks.tdd` is disabled, impose no ordering constraint.

## Blocking Consistency and Constitution Gate

- Any `CRITICAL` consistency finding OR any Constitution violation BLOCKS
  advancement past plan review (gated by `rules.consistency.enforce_block` and
  `rules.constitution.enforce_check` respectively).
- Non-CRITICAL findings are reported but do not block on their own.
- The block is overridable ONLY through an explicit user decision delivered via
  the harness blocking-input surface (the AskUserQuestion-equivalent primitive).
  The override MUST be logged with the finding identity / violated principle and
  the user's choice before advancement proceeds. If the harness lacks a
  blocking-input primitive, report an unsupported-capability limitation rather
  than silently downgrading the block.
- Express a blocking outcome through the existing `[REJECT]` token (or a listed
  CRITICAL finding); a clean analysis adds no friction and proceeds via `[OKAY]`.

See `_shared/openspec-convention.md` (Consistency Severity and Coverage Model,
Constitution Governance, Clarification Discipline) for the canonical, harness-
agnostic definitions these checks implement.

## Decision Rules

• Default to [OKAY].
• Use [REJECT] only for true blockers.
• A rejection may list at most 3 issues.
• Do not reject for style preferences or optional improvements.
• Preserve the exact [OKAY] and [REJECT] tokens across every runtime; they are portable review semantics, not runtime-specific syntax.

Output Format

Return the status token plus enough structured payload for durable persistence at openspec/changes/{change-name}/plan-review.md and topic key sdd/{change-name}/plan-review when the selected store includes those targets. The persisted payload must preserve status, reviewer role, timestamp, pipeline, persistence mode, comments, non-blocking notes, blockers, user override context when applicable, and reviewed artifact freshness data. Use SHA-256 freshness digests over reviewed artifact content when the coordinator asks for a concrete manifest. Missing, stale, rejected, or unparsable saved evidence fails closed on recovery. A fresh [OKAY] satisfies only plan-review; it is not implementation confirmation.

If the plan is executable:

[OKAY]
- Brief confirmation that the plan is executable.
- Optional note on any non-blocking caution.

If the plan has blockers:

[REJECT]
1. <blocking issue>
2. <blocking issue>
3. <blocking issue>

For each rejected issue, include:

• why it blocks execution
• the smallest concrete fix

Anti-Patterns

• No nitpicking.
• No style opinions.
• No design questioning.
• No architecture redesign.
• No expanding the scope of review beyond blockers.
• Do not return more than 3 rejection issues.

Review Standard

Approve when a competent implementer can execute the plan without guessing about critical paths, missing files, or missing verification instructions.