Add review-planning.sh for independent planning review

.agents/prompts/planning-reviewer.md

@@ -0,0 +1,35 @@
+# Independent Planning Reviewer Contract
+
+You did not write this planning. Review only. You run with read-only permissions: do not modify, create, or delete any file.
+
+The calling script supplies the planning documents of a project bootstrap and their diff against `origin/main`:
+
+- `docs/PROJECT_DESCRIPTION.md`, when present: the human's original idea.
+- `docs/PROJECT_REQUIREMENTS.md`: the requirements the human approved.
+- `docs/architecture.md`, `docs/roadmap.md`, and the ADRs under `docs/decisions/`: the planner's work.
+
+Read `AGENTS.md`, `.agents/prompts/project-planner.md`, and the rest of the repository for context.
+
+## What to check
+
+- The architecture, roadmap, and ADRs follow from the approved requirements and are consistent with each other.
+- Every requirement, including MVP scope, non-goals, security, privacy, data, and deployment constraints, is covered by the architecture or a roadmap feature, or is explicitly out of scope.
+- Technology and architecture choices are justified by the requirements, not assumed.
+- Each roadmap feature has a stable ID, a clear outcome, in-scope and out-of-scope behavior, dependencies on existing feature IDs only, testable acceptance criteria, and a risk.
+- Features are independently deliverable vertical slices where that fits, in an order that respects their dependencies.
+- ADRs exist for decisions that need a durable record, and none are ceremonial.
+- Risks, unresolved questions, and assumptions are visible rather than hidden.
+
+The approved requirements are authoritative. When the plan should differ from them, or when the requirements themselves look wrong or incomplete, report it as a finding and say in its title that it concerns the approved requirements. Do not treat the planner's deviation from the requirements as acceptable.
+
+## Result
+
+Return the review as JSON that matches the schema supplied by the calling script (`.agents/schemas/review.schema.json`). Do not write it to a file and do not add text around it.
+
+- `findings`: one entry per finding; an empty array when there are none.
+  - `severity`: `critical` (the plan cannot be built or contradicts the requirements), `major` (a significant gap, inconsistency, or unjustified decision), `minor`, or `suggestion`.
+  - `title`, `evidence` (document and section), `impact`, and `recommendation`.
+- `verdict`: `PASS` without findings, `PASS_WITH_MINOR_FINDINGS` with only minor or suggestion findings, and `CHANGES_REQUIRED` with at least one critical or major finding.
+- `limitations`: anything you could not verify; an empty string otherwise.
+
+Do not number the findings; the calling script assigns the identifiers.

docs/development.md

@@ -162,6 +162,29 @@ git commit -m "Plan project bootstrap"
 git push -u origin planning/project-bootstrap
 ```
 
+### Planning review
+
+Before committing the planning, let an independent agent review it from the
+planning worktree:
+
+```bash
+./scripts/review-planning.sh
+```
+
+It uses role `planning-reviewer` with the `read-only` profile; configure a
+different provider than for `project-planner`. The reviewed file set is the
+planning documents: `docs/PROJECT_DESCRIPTION.md` when present,
+`docs/PROJECT_REQUIREMENTS.md`, `docs/architecture.md`, `docs/roadmap.md`, and
+the ADRs in `docs/decisions/`. The script supplies their contents and their
+diff against `origin/main`. The requirements must be approved first.
+
+Each round is stored as `.agents/reviews/planning-<name>-review-NN.json` with
+a generated report, in the same format as feature reviews but without an Issue.
+The review covers only the planning documents, so it becomes stale when one of
+them is added, changed, or deleted, and stays current otherwise
+(`./scripts/check-review.sh`). A planning review is not triaged: revise the
+planning documents for the findings you accept and run the review again.
+
 Open and merge a planning PR before creating Issues for actionable roadmap
 features. The script never implements features, creates Issues, commits,
 pushes, opens or merges a PR, or deploys.

scripts/lib/review-data.sh

@@ -168,12 +168,19 @@ review_artifact_errors() {
     if type != "object" or .schema != "review/v1" then
       "the file is not a review/v1 artifact"
     else
-      unexpected(["base", "branch", "created_at", "findings", "head", "issue", "limitations",
+      unexpected(["base", "branch", "created_at", "findings", "head", "issue", "kind", "limitations",
                   "merge_base", "reviewed_paths", "reviewed_tree", "reviewer", "round", "schema", "verdict"]; "the review"),
       (if .reviewed_paths == null
           or ((.reviewed_paths | type) == "array" and (.reviewed_paths | length) > 0 and (.reviewed_paths | all(nonempty))) then empty
        else "reviewed_paths must be null or a non-empty list of paths" end),
-      (if (.issue | positive_integer) then empty else "issue must be a positive integer" end),
+      (if (.kind // "feature" | IN("feature", "planning")) then empty else "kind must be feature or planning" end),
+      (if (.kind // "feature") == "planning" then
+         (if .issue == null then empty else "a planning review has no issue" end),
+         (if (.branch | type) == "string" and (.branch | startswith("planning/")) then empty
+          else "a planning review must name its planning branch" end),
+         (if (.reviewed_paths | type) == "array" then empty else "a planning review must list its reviewed paths" end)
+       elif (.issue | positive_integer) then empty
+       else "issue must be a positive integer" end),
       (if (.round | positive_integer) then empty else "round must be a positive integer" end),
       (("branch", "base", "merge_base", "head", "reviewed_tree", "created_at") as $field |
         if (.[$field] | nonempty) then empty else "\($field) is missing" end),
@@ -212,8 +219,9 @@ review_render_markdown() {
            "**Recommended action:** \(.recommendation)\n"
          ) | join("\n")) end);
     "<!-- Generated from \($source). Do not edit; this file is never read by the scripts. -->\n\n" +
-    "# Independent Review — \(.branch)\n\n" +
-    "Issue: #\(.issue)\n\n" +
+    "# Independent \(if .kind == "planning" then "Planning " else "" end)Review — \(.branch)\n\n" +
+    (if .issue != null then "Issue: #\(.issue)\n\n" else "" end) +
+    (if (.reviewed_paths | type) == "array" then "Reviewed files: \(.reviewed_paths | map("`\(.)`") | join(", "))\n\n" else "" end) +
     "Round: \(.round)\n\n" +
     "Base: \(.base) (\(.merge_base))\n\n" +
     "HEAD at review start: \(.head)\n\n" +

scripts/lib/review-run.sh

@@ -0,0 +1,100 @@
+#!/usr/bin/env bash
+
+# Running a read-only reviewer and storing its result. Shared by
+# review-feature.sh and review-planning.sh.
+# Source this file after agent.sh, review-data.sh, and fingerprint.sh.
+
+# review_run_reviewer <agent> <model> <root> <prompt> <context-file> <schema-file> <result-file> <scratch-dir>
+# Runs the reviewer read-only. An invalid result is retried once, with the
+# reasons for the rejection. Exits the calling script when the reviewer fails,
+# changes the working tree or the review artifacts, creates a commit, or
+# returns an invalid result twice.
+review_run_reviewer() {
+  local agent="$1"
+  local model="$2"
+  local root="$3"
+  local prompt="$4"
+  local context_file="$5"
+  local schema_file="$6"
+  local result_file="$7"
+  local scratch="$8/review-run"
+  local head_before
+  local tree_before
+  local artifacts_before
+  local attempt_prompt="$prompt"
+  local attempt
+  local agent_status
+  local result_errors
+
+  mkdir -p "$scratch"
+  head_before="$(git -C "$root" rev-parse HEAD)"
+  tree_before="$(fingerprint_worktree "$root" "$scratch")" || {
+    echo "Error: could not compute the fingerprint of the working tree." >&2
+    exit 1
+  }
+  artifacts_before="$(fingerprint_artifacts "$root")"
+
+  for attempt in 1 2; do
+    : >"$result_file"
+    agent_status=0
+    agent_run read-only "$agent" "$model" "$root" "$attempt_prompt" "$result_file" "$context_file" "$schema_file" ||
+      agent_status=$?
+
+    if [[ "$(git -C "$root" rev-parse HEAD)" != "$head_before" ||
+          "$(fingerprint_worktree "$root" "$scratch")" != "$tree_before" ||
+          "$(fingerprint_artifacts "$root")" != "$artifacts_before" ]]; then
+      echo "Error: the reviewer modified the working tree or created a commit. No review was stored." >&2
+      git -C "$root" status --short >&2
+      exit 1
+    fi
+
+    if [[ "$agent_status" -ne 0 ]]; then
+      echo "Error: reviewer failed with status $agent_status. No review was stored." >&2
+      exit 1
+    fi
+
+    result_errors="$(review_result_errors "$result_file")"
+    [[ -n "$result_errors" ]] || return 0
+
+    echo "The reviewer returned an invalid result (attempt $attempt of 2):" >&2
+    printf '%s\n' "$result_errors" | sed 's/^/  - /' >&2
+    if [[ "$attempt" -eq 2 ]]; then
+      echo "Error: the reviewer's result is invalid. No review was stored." >&2
+      exit 1
+    fi
+    echo "Retrying once..." >&2
+    attempt_prompt="$prompt
+
+Your previous result was rejected for these reasons:
+$result_errors
+
+Return a corrected result."
+  done
+}
+
+# review_store <result-file> <artifact-base> <metadata-json>
+# Stores <artifact-base>.json from the script-owned metadata and the validated
+# result, renders <artifact-base>.md, and prints a summary. Exits the calling
+# script, storing nothing, when the artifact would be invalid.
+review_store() {
+  local result_file="$1"
+  local out="$2"
+  local metadata="$3"
+  local artifact_errors
+
+  mkdir -p "$(dirname "$out")"
+  review_result_with_ids "$result_file" | jq --argjson metadata "$metadata" '
+    $metadata + {verdict: .verdict, limitations: .limitations, findings: .findings}
+  ' >"$out.json"
+
+  artifact_errors="$(review_artifact_errors "$out.json")"
+  if [[ -n "$artifact_errors" ]]; then
+    rm -f "$out.json"
+    echo "Error: the stored review would be invalid; nothing was stored:" >&2
+    printf '%s\n' "$artifact_errors" | sed 's/^/  - /' >&2
+    exit 1
+  fi
+  review_render_markdown "$out.json" "$(basename "$out").json" >"$out.md"
+
+  echo "Review completed: $(jq -r '.verdict | gsub("_"; " ")' "$out.json") ($(jq '.findings | length' "$out.json") findings)"
+}

scripts/review-feature.sh

@@ -6,6 +6,7 @@ script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)"
 source "$script_dir/lib/agent.sh"
 source "$script_dir/lib/review-data.sh"
 source "$script_dir/lib/fingerprint.sh"
+source "$script_dir/lib/review-run.sh"
 
 fail() {
   echo "Error: $*" >&2
@@ -83,7 +84,6 @@ head_before="$(git -C "$root" rev-parse HEAD)"
 tree_before="$(fingerprint_worktree "$root" "$tmp_work")" ||
   fail "could not compute the fingerprint of the working tree."
 cp "$tmp_work/fingerprint.index" "$tmp_work/index.before"
-artifacts_before="$(fingerprint_artifacts "$root")"
 
 review_paths=(. ":(exclude).agents/reviews" ":(exclude).agents/triage")
 context_file="$tmp_work/context.md"
@@ -170,42 +170,9 @@ script validates and stores it."
 echo "Starting $agent reviewer ($model) with read-only permissions..."
 echo
 
-# Invalid output is retried once; a failed agent or a modified tree is not.
-attempt_prompt="$START_PROMPT"
-for attempt in 1 2; do
-  : >"$report_file"
-  set +e
-  agent_run read-only "$agent" "$model" "$root" "$attempt_prompt" "$report_file" "$context_file" "$schema_file"
-  agent_status=$?
-  set -e
-
-  if [[ "$(git -C "$root" rev-parse HEAD)" != "$head_before" || "$(fingerprint_worktree "$root" "$tmp_work")" != "$tree_before" ||
-        "$(fingerprint_artifacts "$root")" != "$artifacts_before" ]]; then
-    echo "Error: the reviewer modified the working tree or created a commit. No review was stored." >&2
-    git -C "$root" status --short >&2
-    exit 1
-  fi
-
-  [[ "$agent_status" -eq 0 ]] ||
-    fail "reviewer failed with status $agent_status. No review was stored."
-
-  result_errors="$(review_result_errors "$report_file")"
-  [[ -n "$result_errors" ]] || break
-
-  echo "The reviewer returned an invalid result (attempt $attempt of 2):" >&2
-  printf '%s\n' "$result_errors" | sed 's/^/  - /' >&2
-  [[ "$attempt" -lt 2 ]] || fail "the reviewer's result is invalid. No review was stored."
-  echo "Retrying once..." >&2
-  attempt_prompt="$START_PROMPT
-
-Your previous result was rejected for these reasons:
-$result_errors
+review_run_reviewer "$agent" "$model" "$root" "$START_PROMPT" "$context_file" "$schema_file" "$report_file" "$tmp_work"
 
-Return a corrected result."
-done
-
-mkdir -p "$reviews_dir"
-review_result_with_ids "$report_file" | jq \
+metadata="$(jq -n \
   --argjson issue "$issue" \
   --argjson round "$review_number" \
   --arg branch "$branch" \
@@ -218,6 +185,7 @@ review_result_with_ids "$report_file" | jq \
   --arg created_at "$(date -u +'%Y-%m-%dT%H:%M:%SZ')" \
   '{
     schema: "review/v1",
+    kind: "feature",
     issue: $issue,
     round: $round,
     branch: $branch,
@@ -227,22 +195,8 @@ review_result_with_ids "$report_file" | jq \
     reviewed_tree: $tree,
     reviewed_paths: null,
     reviewer: {agent: $agent, model: $model},
-    created_at: $created_at,
-    verdict: .verdict,
-    limitations: .limitations,
-    findings: .findings
-  }' >"$out.json"
-
-artifact_errors="$(review_artifact_errors "$out.json")"
-if [[ -n "$artifact_errors" ]]; then
-  rm -f "$out.json"
-  echo "Error: the stored review would be invalid; nothing was stored:" >&2
-  printf '%s\n' "$artifact_errors" | sed 's/^/  - /' >&2
-  exit 1
-fi
-review_render_markdown "$out.json" "$(basename "$out").json" >"$out.md"
-
-verdict="$(jq -r '.verdict | gsub("_"; " ")' "$out.json")"
-echo "Review completed: $verdict ($(jq '.findings | length' "$out.json") findings)"
+    created_at: $created_at
+  }')"
+review_store "$report_file" "$out" "$metadata"
 echo "  $review_relative.json  (source of truth)"
 echo "  $review_relative.md    (generated report)"