From 4e08ccf0c79de203b751cc4681c6e3a6fb0f2ac4 Mon Sep 17 00:00:00 2001 From: Ahmet Taspinar Date: Fri, 2 Oct 2026 20:44:09 +0200 Subject: [PATCH] Add review-planning.sh for independent planning review review-planning.sh runs in a planning worktree with role planning-reviewer and the read-only profile. It supplies the planning documents and their diff against origin/main, including deletions, and stores each round as a planning review in the shared review format, without an Issue. The review covers the planning scope, so adding, changing, or deleting a planning document makes it stale. Running and storing a reviewer is now shared with review-feature.sh in scripts/lib/review-run.sh. Planning reviews are not triaged. Closes #45 Co-Authored-By: Claude Opus 5.5 --- .agents/prompts/planning-reviewer.md | 35 ++++++ docs/development.md | 23 ++++ scripts/lib/review-data.sh | 16 ++- scripts/lib/review-run.sh | 100 +++++++++++++++ scripts/review-feature.sh | 60 ++------- scripts/review-planning.sh | 182 +++++++++++++++++++++++++++ scripts/triage-review.sh | 4 + scripts/verify.conf | 1 + tests/review-planning-test.sh | 174 +++++++++++++++++++++++++ 9 files changed, 538 insertions(+), 57 deletions(-) create mode 100644 .agents/prompts/planning-reviewer.md create mode 100644 scripts/lib/review-run.sh create mode 100755 scripts/review-planning.sh create mode 100755 tests/review-planning-test.sh diff --git a/.agents/prompts/planning-reviewer.md b/.agents/prompts/planning-reviewer.md new file mode 100644 index 0000000..72dd16a --- /dev/null +++ b/.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. diff --git a/docs/development.md b/docs/development.md index 5606125..197d18a 100644 --- a/docs/development.md +++ b/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--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. diff --git a/scripts/lib/review-data.sh b/scripts/lib/review-data.sh index 6ebf54c..b2f4ce6 100644 --- a/scripts/lib/review-data.sh +++ b/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); "\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" + diff --git a/scripts/lib/review-run.sh b/scripts/lib/review-run.sh new file mode 100644 index 0000000..5d7b80e --- /dev/null +++ b/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 +# 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 +# Stores .json from the script-owned metadata and the validated +# result, renders .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)" +} diff --git a/scripts/review-feature.sh b/scripts/review-feature.sh index c2c6997..91be1cf 100755 --- a/scripts/review-feature.sh +++ b/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)" diff --git a/scripts/review-planning.sh b/scripts/review-planning.sh new file mode 100755 index 0000000..9517edf --- /dev/null +++ b/scripts/review-planning.sh @@ -0,0 +1,182 @@ +#!/usr/bin/env bash + +set -euo pipefail + +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 + exit 1 +} + +agent_parse_args "$@" +if [[ "${#AGENT_POSITIONAL[@]}" -ne 0 ]]; then + echo "Usage: $0 [--agent ] [--model ]" + echo + echo "Run in a planning worktree (branch planning/). The agent and model come" + echo "from role 'planning-reviewer' in .agents/agents.conf unless --agent and" + echo "--model are given." + exit 1 +fi + +root="$(git rev-parse --show-toplevel)" +branch="$(git branch --show-current)" +[[ "$branch" == planning/* ]] || + fail "review-planning.sh must run in a planning worktree (branch planning/). Current branch: $branch" +name="${branch#planning/}" + +prompt_file="$root/.agents/prompts/planning-reviewer.md" +schema_file="$root/.agents/schemas/review.schema.json" +reviews_dir="$root/.agents/reviews" +[[ -f "$prompt_file" ]] || fail "planning reviewer prompt not found: $prompt_file" +[[ -f "$schema_file" ]] || fail "review schema not found: $schema_file" +review_data_require_jq + +for required in docs/PROJECT_REQUIREMENTS.md docs/architecture.md docs/roadmap.md; do + [[ -f "$root/$required" && ! -L "$root/$required" ]] || fail "required planning document is missing: $required" +done +grep -Fqx "Status: Approved" "$root/docs/PROJECT_REQUIREMENTS.md" || + fail "the project requirements are not approved. Finish start-planning.sh first." + +agent_resolve "$root" planning-reviewer "$AGENT_CLI_PROVIDER" "$AGENT_CLI_MODEL" +agent="$AGENT_PROVIDER" +model="$AGENT_MODEL" + +# The reviewed scope: the planning documents, and nothing else. It names the +# optional description and the decisions directory even when they are absent +# or empty, so adding, changing, or deleting any planning document makes the +# review stale. +scope=(docs/PROJECT_DESCRIPTION.md docs/PROJECT_REQUIREMENTS.md docs/architecture.md docs/roadmap.md docs/decisions) + +# The documents whose contents the reviewer receives. +paths=() +[[ ! -f "$root/docs/PROJECT_DESCRIPTION.md" ]] || paths+=(docs/PROJECT_DESCRIPTION.md) +paths+=(docs/PROJECT_REQUIREMENTS.md docs/architecture.md docs/roadmap.md) +while IFS= read -r decision; do + paths+=("docs/decisions/$decision") +done < <(cd "$root" && find docs/decisions -mindepth 1 -maxdepth 1 -name '*.md' -type f 2>/dev/null | + sed 's#^docs/decisions/##' | LC_ALL=C sort) + +if git -C "$root" rev-parse --verify --quiet "origin/main^{commit}" >/dev/null; then + base_ref="origin/main" +elif git -C "$root" rev-parse --verify --quiet "main^{commit}" >/dev/null; then + base_ref="main" +else + fail "neither origin/main nor main exists." +fi +merge_base="$(git -C "$root" merge-base HEAD "$base_ref")" + +tmp_work="$(mktemp -d "${TMPDIR:-/tmp}/review-planning.XXXXXX")" +trap 'rm -rf "$tmp_work"' EXIT + +tree="$(fingerprint_files "$root" "$tmp_work" "${scope[@]}")" || + fail "could not compute the fingerprint of the planning documents." +head_before="$(git -C "$root" rev-parse HEAD)" + +context_file="$tmp_work/context.md" +result_file="$tmp_work/result.json" +{ + echo "# Planning review context ($branch)" + echo + for path in "${paths[@]}"; do + echo "## $path" + echo + echo '````markdown' + cat "$root/$path" + echo '````' + echo + done + echo "## Diff of the planning documents against $base_ref, including uncommitted changes and deletions" + echo + GIT_INDEX_FILE="$tmp_work/fingerprint-files.index" git -C "$root" diff --cached --no-color "$merge_base" -- "${scope[@]}" +} >"$context_file" + +review_number=1 +previous_review="" +while true; do + candidate="$reviews_dir/planning-${name}-review-$(printf "%02d" "$review_number")" + if [[ ! -e "$candidate.json" && ! -e "$candidate.md" ]]; then + out="$candidate" + break + fi + if [[ -f "$candidate.json" ]]; then + previous_review="$candidate.json" + fi + review_number=$((review_number + 1)) +done +review_relative=".agents/reviews/$(basename "$out")" + +echo "Preparing independent planning review:" +echo " Branch: $branch" +echo " Base: $base_ref" +echo " Agent: $agent" +echo " Model: $model" +echo " Files: ${paths[*]}" +echo " Output: $review_relative.json" +if [[ -n "$previous_review" ]]; then + echo " Previous: .agents/reviews/$(basename "$previous_review")" +fi +echo + +prompt="Read and follow .agents/prompts/planning-reviewer.md. + +Standard input contains the planning documents of $branch and their diff +against ${base_ref}. Review those documents." + +if [[ -n "$previous_review" ]]; then + prompt+=" + +This is a re-review. Read the previous review (JSON): +.agents/reviews/$(basename "$previous_review") +and any revision decisions next to it (*-revision.json). Check whether its +findings have been resolved, but review the complete current planning." +fi + +prompt+=" + +You have read-only access. Do not modify, create, or delete any file. +Return the review as JSON that matches the supplied schema. The calling +script validates and stores it." + +echo "Starting $agent planning reviewer ($model) with read-only permissions..." +echo + +review_run_reviewer "$agent" "$model" "$root" "$prompt" "$context_file" "$schema_file" "$result_file" "$tmp_work" + +paths_json="$(printf '%s\n' "${scope[@]}" | jq -R . | jq -s .)" +metadata="$(jq -n \ + --argjson round "$review_number" \ + --arg branch "$branch" \ + --arg base "$base_ref" \ + --arg merge_base "$merge_base" \ + --arg head "$head_before" \ + --arg tree "$tree" \ + --argjson paths "$paths_json" \ + --arg agent "$agent" \ + --arg model "$model" \ + --arg created_at "$(date -u +'%Y-%m-%dT%H:%M:%SZ')" \ + '{ + schema: "review/v1", + kind: "planning", + issue: null, + round: $round, + branch: $branch, + base: $base, + merge_base: $merge_base, + head: $head, + reviewed_tree: $tree, + reviewed_paths: $paths, + reviewer: {agent: $agent, model: $model}, + created_at: $created_at + }')" +review_store "$result_file" "$out" "$metadata" +echo " $review_relative.json (source of truth)" +echo " $review_relative.md (generated report)" +echo +echo "Next: revise the planning documents in this worktree for the findings you" +echo "accept, then run ./scripts/review-planning.sh again. When the review passes," +echo "commit the planning and open the planning PR as described in docs/development.md." diff --git a/scripts/triage-review.sh b/scripts/triage-review.sh index 38c2b4f..8704f28 100755 --- a/scripts/triage-review.sh +++ b/scripts/triage-review.sh @@ -48,6 +48,10 @@ fi [[ -f "$schema_file" ]] || fail "triage schema not found: $schema_file" review_data_require_jq +if [[ "$(jq -r '.kind // "feature"' "$review_path" 2>/dev/null)" == "planning" ]]; then + fail "this is a planning review; it is not triaged. Revise the planning documents and run ./scripts/review-planning.sh again." +fi + agent_resolve "$root" triage "$AGENT_CLI_PROVIDER" "$AGENT_CLI_MODEL" agent="$AGENT_PROVIDER" model="$AGENT_MODEL" diff --git a/scripts/verify.conf b/scripts/verify.conf index ba8a93b..8f7a0a7 100644 --- a/scripts/verify.conf +++ b/scripts/verify.conf @@ -30,6 +30,7 @@ agent: ./tests/agent-test.sh fingerprint: ./tests/fingerprint-test.sh create-feature-issue: ./tests/create-feature-issue-test.sh review-feature: ./tests/review-feature-test.sh +review-planning: ./tests/review-planning-test.sh triage-review: ./tests/triage-review-test.sh apply-triage: ./tests/apply-triage-test.sh start-planning: ./tests/start-planning-test.sh diff --git a/tests/review-planning-test.sh b/tests/review-planning-test.sh new file mode 100755 index 0000000..aae65d0 --- /dev/null +++ b/tests/review-planning-test.sh @@ -0,0 +1,174 @@ +#!/usr/bin/env bash + +set -euo pipefail + +source_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P)" +tmp="$(mktemp -d "${TMPDIR:-/tmp}/review-planning-test.XXXXXX")" + +cleanup() { + rm -rf "$tmp" +} +trap cleanup EXIT + +fail() { + echo "review-planning test failed: $*" >&2 + exit 1 +} + +source "$source_root/tests/lib-fakes.sh" +make_fake_agents "$tmp/bin" + +valid_result='{"verdict": "CHANGES_REQUIRED", "limitations": "", "findings": [ + {"severity": "major", "title": "Roadmap omits offline use", "evidence": "docs/roadmap.md, F02", "impact": "A requirement is unplanned.", "recommendation": "Add a feature for offline use."} +]}' +export MOCK_OUTPUT="$valid_result" + +# Creates a repository whose origin/main holds the template documents and +# whose planning branch holds uncommitted planning work. +setup_repo() { + local label="$1" + local seed="$tmp/$label-seed" + local remote="$tmp/$label-remote.git" + local repo="$tmp/$label" + + mkdir -p "$seed/docs/decisions" + copy_workflow "$seed" + printf 'planning-reviewer: claude model-p\n' >"$seed/.agents/agents.conf" + printf '# Agents\n' >"$seed/AGENTS.md" + printf '# Architecture\n\nTemplate.\n' >"$seed/docs/architecture.md" + printf '# Roadmap\n\nTemplate.\n' >"$seed/docs/roadmap.md" + printf '# Project Requirements\n\nStatus: Draft\nApproved at: Not approved\n' >"$seed/docs/PROJECT_REQUIREMENTS.md" + git -C "$seed" init -q -b main + git -C "$seed" config user.name "Planning Review Test" + git -C "$seed" config user.email "planning-review-test@example.com" + git -C "$seed" add . + git -C "$seed" commit -qm "Seed" + git init -q --bare -b main "$remote" + git -C "$seed" push -q "$remote" main + git clone -q "$remote" "$repo" + git -C "$repo" switch -q -c planning/project-bootstrap + + printf 'A recipe organizer that works offline.\n' >"$repo/docs/PROJECT_DESCRIPTION.md" + printf '# Project Requirements\n\nStatus: Approved\nApproved at: 2026-01-01T00:00:00Z\n\n## MVP scope\n\nOffline recipes.\n' \ + >"$repo/docs/PROJECT_REQUIREMENTS.md" + printf '# Architecture\n\nLocal storage.\n' >"$repo/docs/architecture.md" + printf '# Roadmap\n\n## F01 — Recipes\n\n- Goal: list recipes.\n' >"$repo/docs/roadmap.md" + mkdir -p "$repo/docs/decisions" + printf '# ADR 001: Local storage\n' >"$repo/docs/decisions/001-local-storage.md" + + printf '%s\n' "$repo" +} + +run_review() { + local repo="$1" + shift + + ( + cd "$repo" + PATH="$tmp/bin:/usr/bin:/bin" MOCK_AGENT_LOG="$repo.log" ./scripts/review-planning.sh "$@" + ) >"$repo.out" 2>&1 +} + +expect_no_review() { + local repo="$1" + local description="$2" + shift 2 + + if run_review "$repo" "$@"; then + cat "$repo.out" >&2 + fail "expected failure: $description" + fi + if compgen -G "$repo/.agents/reviews/*.json" >/dev/null; then + fail "a review was stored although: $description" + fi + [[ ! -e "$repo.log" ]] || fail "a reviewer was started although: $description" +} + +# A planning review covers exactly the planning documents, runs read-only, and +# is stored as a planning review without an Issue. +repo="$(setup_repo review)" +run_review "$repo" || { + cat "$repo.out" >&2 + fail "planning review failed" +} +artifact="$repo/.agents/reviews/planning-project-bootstrap-review-01" +[[ -f "$artifact.json" && -f "$artifact.md" ]] || fail "the planning review and its report were not stored" +[[ "$(jq -c '[.kind, .issue, .branch, .round]' "$artifact.json")" == '["planning",null,"planning/project-bootstrap",1]' ]] || + fail "the planning review metadata is wrong" +[[ "$(jq -c '.reviewed_paths' "$artifact.json")" == \ + '["docs/PROJECT_DESCRIPTION.md","docs/PROJECT_REQUIREMENTS.md","docs/architecture.md","docs/roadmap.md","docs/decisions"]' ]] || + fail "the reviewed scope is wrong" +grep -Fq "# ADR 001: Local storage" "$repo.log.stdin" || fail "the ADR was not supplied" +grep -Fq -- "-Template." "$repo.log.stdin" || fail "the diff does not show replaced template content" +grep -Fq -- "--tools Read,Glob,Grep" "$repo.log" || fail "the planning reviewer was not read-only" +grep -Fq -- "--model model-p" "$repo.log" || fail "the configured planning reviewer was not used" +grep -Fq "A recipe organizer that works offline." "$repo.log.stdin" || fail "the description was not supplied" +grep -Fq "+Local storage." "$repo.log.stdin" || fail "the diff against origin/main was not supplied" +grep -Fq "Planning Review" "$artifact.md" || fail "the report does not identify a planning review" + +# Only planning documents make the review stale. +(cd "$repo" && ./scripts/check-review.sh "$artifact.json" >/dev/null) || fail "a fresh planning review is not current" +printf 'Unrelated change.\n' >>"$repo/AGENTS.md" +(cd "$repo" && ./scripts/check-review.sh "$artifact.json" >/dev/null) || fail "an unrelated change made the planning review stale" +expect_stale() { + local status=0 + (cd "$repo" && ./scripts/check-review.sh "$artifact.json" >/dev/null) || status=$? + [[ "$status" -eq 1 ]] || fail "$1 did not make the planning review stale" +} +cp "$repo/docs/roadmap.md" "$tmp/roadmap.saved" +printf '\n## F02 — Offline use\n' >>"$repo/docs/roadmap.md" +expect_stale "a roadmap change" +cp "$tmp/roadmap.saved" "$repo/docs/roadmap.md" +printf '# ADR 002: Sync\n' >"$repo/docs/decisions/002-sync.md" +expect_stale "a new ADR" +rm "$repo/docs/decisions/002-sync.md" +mv "$repo/docs/PROJECT_DESCRIPTION.md" "$tmp/description.saved" +expect_stale "a removed description" +mv "$tmp/description.saved" "$repo/docs/PROJECT_DESCRIPTION.md" +(cd "$repo" && ./scripts/check-review.sh "$artifact.json" >/dev/null) || + fail "restoring the planning documents did not make the review current again" +printf '\n## F02 — Offline use\n' >>"$repo/docs/roadmap.md" + +# A second round references the previous one. +run_review "$repo" --agent codex --model model-c || { + cat "$repo.out" >&2 + fail "planning re-review failed" +} +[[ "$(jq '.round' "$repo/.agents/reviews/planning-project-bootstrap-review-02.json")" -eq 2 ]] || + fail "the re-review was not numbered" +grep -Fq "planning-project-bootstrap-review-01.json" "$repo.log" || fail "the re-review did not reference the previous review" + +# A planning review is not triaged as a feature review. +if (cd "$repo" && PATH="$tmp/bin:/usr/bin:/bin" ./scripts/triage-review.sh "$artifact.json" "$repo.triage.out" 2>&1; then + fail "a planning review was accepted by triage-review.sh" +fi +grep -Fq "review-planning.sh" "$repo.triage.out" || fail "triage-review.sh did not point to the planning review" + +# A planning document deleted relative to the base appears in the reviewer's diff. +repo="$(setup_repo deleted-adr)" +git -C "$repo" add -A +git -C "$repo" commit -qm "Planning so far" +git -C "$repo" push -q origin HEAD:main +git -C "$repo" fetch -q origin +git -C "$repo" rm -q docs/decisions/001-local-storage.md +run_review "$repo" || { + cat "$repo.out" >&2 + fail "planning review after deleting an ADR failed" +} +grep -Fq "deleted file mode" "$repo.log.stdin" || fail "a deleted ADR was not shown to the reviewer" +grep -Fq -- "-# ADR 001: Local storage" "$repo.log.stdin" || fail "the deleted ADR content was not shown" + +# Preconditions fail before a reviewer starts. +repo="$(setup_repo not-planning)" +git -C "$repo" switch -q -c feature/1-x +expect_no_review "$repo" "the branch is not a planning branch" + +repo="$(setup_repo unapproved)" +printf '# Project Requirements\n\nStatus: Draft\n' >"$repo/docs/PROJECT_REQUIREMENTS.md" +expect_no_review "$repo" "the requirements are not approved" + +repo="$(setup_repo missing-roadmap)" +rm "$repo/docs/roadmap.md" +expect_no_review "$repo" "the roadmap is missing" + +echo "review-planning tests passed"