Tie reviews to the exact reviewed version with a fingerprint

docs/development.md

@@ -336,6 +336,28 @@ fields the scripts own, so editing a stored artifact cannot weaken a decision.
 Documents that people maintain, such as the roadmap, requirements, and
 architecture, keep Markdown as their source.
 
+### When a review becomes stale
+
+Every review records a fingerprint of what it reviewed (`reviewed_tree`): the
+Git tree hash of the file contents at review time, including uncommitted and
+untracked changes. Review and triage artifacts and ignored files are not part
+of it, except files Git already tracks. A review may instead cover an explicit
+list of files (`reviewed_paths`); then only those files count.
+
+The fingerprint depends on content, not on commits. Committing the reviewed
+content keeps the review current; changing, adding, or deleting a covered file
+makes it stale. `triage-review.sh` and `apply-triage.sh` refuse a stale review
+before they start an agent. After `apply-triage.sh` changes the code, the
+review is stale by design: run a new review when the fixes need confirmation.
+
+Check a review yourself with:
+
+```bash
+./scripts/check-review.sh .agents/reviews/feature-12-player-movement-review-01.json
+```
+
+It exits 0 when the review is current, 1 when it is stale, and 2 on an error.
+
 Verify again after review fixes, then commit and push:
 
 ```bash

scripts/apply-triage.sh

@@ -5,6 +5,7 @@ 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"
 
 usage() {
   echo "Usage: $0 <triage-json> [--agent <agent>] [--model <model>]"
@@ -82,6 +83,24 @@ review_errors="$(review_artifact_errors "$source_review_path")"
 triage_errors="$(triage_artifact_errors "$triage_path" "$source_review_path")"
 [[ -z "$triage_errors" ]] || report_errors "invalid or unapproved triage artifact:" "$triage_errors"
 
+tmp_work="$(mktemp -d "${TMPDIR:-/tmp}/apply-triage.XXXXXX")"
+trap 'rm -rf "$tmp_work"' EXIT
+review_path="$source_review_path"
+review_relative="$source_review_relative"
+fail() {
+  echo "Error: $*" >&2
+  exit 1
+}
+
+# Triage and fixes apply only to the content that was reviewed.
+stale_status=0
+review_is_current "$root" "$tmp_work" "$review_path" || stale_status=$?
+case "$stale_status" in
+  0) ;;
+  1) fail "the review is stale: the reviewed content changed after $review_relative was written. Run a new review." ;;
+  *) fail "could not compute the current fingerprint of the working tree." ;;
+esac
+
 source_issue="$(jq -r '.issue' "$triage_path")"
 
 branch="$(git branch --show-current)"

scripts/check-review.sh

@@ -0,0 +1,54 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)"
+source "$script_dir/lib/review-data.sh"
+source "$script_dir/lib/fingerprint.sh"
+
+if [[ $# -ne 1 ]]; then
+  echo "Usage: $0 <review-json>"
+  echo
+  echo "Reports whether the content a review covers is unchanged since the review."
+  echo "Exits 0 when the review is current, 1 when it is stale, and 2 on an error."
+  exit 2
+fi
+
+fail() {
+  echo "Error: $*" >&2
+  exit 2
+}
+
+root="$(git rev-parse --show-toplevel)"
+[[ -f "$1" ]] || fail "review artifact not found: $1"
+review_path="$(cd "$(dirname "$1")" && pwd -P)/$(basename "$1")"
+[[ "$review_path" == "$root/.agents/reviews/"*.json ]] ||
+  fail "review artifact must match .agents/reviews/*.json. Received: $review_path"
+review_relative="${review_path#"$root"/}"
+
+review_data_require_jq
+review_errors="$(review_artifact_errors "$review_path")"
+if [[ -n "$review_errors" ]]; then
+  echo "Error: invalid review artifact:" >&2
+  printf '%s\n' "$review_errors" | sed 's/^/  - /' >&2
+  exit 2
+fi
+
+tmp_work="$(mktemp -d "${TMPDIR:-/tmp}/check-review.XXXXXX")"
+trap 'rm -rf "$tmp_work"' EXIT
+
+status=0
+review_is_current "$root" "$tmp_work" "$review_path" || status=$?
+case "$status" in
+  0)
+    echo "Current: $review_relative still matches the reviewed content."
+    ;;
+  1)
+    echo "Stale: the reviewed content changed after $review_relative was written."
+    echo "Run a new review before triage, fixes, or approval."
+    exit 1
+    ;;
+  *)
+    fail "could not compute the current fingerprint."
+    ;;
+esac

scripts/lib/fingerprint.sh

@@ -0,0 +1,113 @@
+#!/usr/bin/env bash
+
+# Content fingerprints of a working tree or of a set of files.
+# Source this file; do not execute it.
+#
+# A fingerprint is the Git tree hash of the current file contents, including
+# uncommitted and untracked changes. It depends on content only, so committing
+# unchanged content keeps it valid, and any change to a covered file changes
+# it. Review and triage artifacts are never part of a fingerprint. The real
+# Git index is not touched; objects may be written to the object database.
+
+FINGERPRINT_EXCLUDES=(".agents/reviews" ".agents/triage")
+
+# fingerprint_worktree <root> <scratch-dir>
+# Prints the fingerprint of the whole working tree. Ignored files are left
+# out unless Git already tracks them.
+fingerprint_worktree() {
+  local root="$1"
+  local index="$2/fingerprint.index"
+  local real_index
+  local exclude_specs=()
+  local path
+
+  for path in "${FINGERPRINT_EXCLUDES[@]}"; do
+    exclude_specs+=(":(exclude)$path")
+  done
+
+  # Start from the real index so tracked files that match an ignore rule stay
+  # covered. The path may be relative to the repository root.
+  rm -f "$index"
+  real_index="$(cd "$root" && git rev-parse --git-path index)"
+  if (cd "$root" && [[ -f "$real_index" ]]); then
+    (cd "$root" && cp "$real_index" "$index") || return 1
+  fi
+
+  GIT_INDEX_FILE="$index" git -C "$root" rm -r -q --cached --ignore-unmatch -- "${FINGERPRINT_EXCLUDES[@]}" >/dev/null &&
+    GIT_INDEX_FILE="$index" git -C "$root" add -A -- . "${exclude_specs[@]}" >/dev/null &&
+    GIT_INDEX_FILE="$index" git -C "$root" write-tree
+}
+
+# fingerprint_files <root> <scratch-dir> <path>...
+# Prints the fingerprint of exactly the given repository-relative paths, which
+# may be files, symlinks, or directories. A missing path is part of the
+# fingerprint as absent. Review and triage artifacts are excluded here too.
+fingerprint_files() {
+  local root="$1"
+  local index="$2/fingerprint-files.index"
+  local specs=()
+  local path
+
+  shift 2
+  for path in "$@"; do
+    if [[ -e "$root/$path" || -L "$root/$path" ]]; then
+      specs+=("$path")
+    fi
+  done
+
+  rm -f "$index"
+  if [[ "${#specs[@]}" -gt 0 ]]; then
+    for path in "${FINGERPRINT_EXCLUDES[@]}"; do
+      specs+=(":(exclude)$path")
+    done
+    GIT_INDEX_FILE="$index" git -C "$root" add -f -- "${specs[@]}" >/dev/null || return 1
+  fi
+  GIT_INDEX_FILE="$index" git -C "$root" write-tree
+}
+
+# fingerprint_artifacts <root>
+# Prints a hash of the review and triage artifacts, which every other
+# fingerprint leaves out, so a run can detect that an agent changed them.
+fingerprint_artifacts() {
+  (
+    cd "$1" || exit 1
+    # A missing artifact directory is an empty set, not an error.
+    { find "${FINGERPRINT_EXCLUDES[@]}" \( -type f -o -type l \) 2>/dev/null || true; } | LC_ALL=C sort |
+      while IFS= read -r path; do
+        if [[ -L "$path" ]]; then
+          printf '%s symlink %s\n' "$path" "$(readlink "$path")"
+        else
+          printf '%s %s\n' "$path" "$(git hash-object -- "$path")"
+        fi
+      done
+  ) | git hash-object --stdin
+}
+
+# fingerprint_review <root> <scratch-dir> <review-json>
+# Prints the current fingerprint of what the review covers: the paths in
+# reviewed_paths when the review lists them, otherwise the whole working tree.
+fingerprint_review() {
+  local root="$1"
+  local scratch="$2"
+  local review="$3"
+  local paths=()
+  local path
+
+  if jq -e '(.reviewed_paths | type) == "array"' "$review" >/dev/null; then
+    while IFS= read -r path; do
+      paths+=("$path")
+    done < <(jq -r '.reviewed_paths[]' "$review")
+    fingerprint_files "$root" "$scratch" ${paths[@]+"${paths[@]}"}
+  else
+    fingerprint_worktree "$root" "$scratch"
+  fi
+}
+
+# review_is_current <root> <scratch-dir> <review-json>
+# Succeeds when the reviewed content is unchanged since the review.
+review_is_current() {
+  local current
+
+  current="$(fingerprint_review "$1" "$2" "$3")" || return 2
+  [[ "$current" == "$(jq -r '.reviewed_tree' "$3")" ]]
+}

scripts/lib/review-data.sh

@@ -169,7 +169,10 @@ review_artifact_errors() {
       "the file is not a review/v1 artifact"
     else
       unexpected(["base", "branch", "created_at", "findings", "head", "issue", "limitations",
-                  "merge_base", "reviewed_tree", "reviewer", "round", "schema", "verdict"]; "the review"),
+                  "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 (.round | positive_integer) then empty else "round must be a positive integer" end),
       (("branch", "base", "merge_base", "head", "reviewed_tree", "created_at") as $field |

scripts/review-feature.sh

@@ -5,6 +5,7 @@ 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"
 
 fail() {
   echo "Error: $*" >&2
@@ -76,25 +77,13 @@ issue_context="$(gh issue view "$issue" \
   --template 'Title: {{.title}}{{"\n\n"}}{{.body}}')" ||
   fail "could not read GitHub Issue #$issue."
 
-# Tree of the complete working tree, including uncommitted and untracked
-# files, built in a temporary index so the real index is not touched.
-snapshot_tree() {
-  local index="$tmp_work/index.$1"
-  local real_index
-
-  # Start from the real index so tracked files that match an ignore rule stay
-  # in the snapshot. The path may be relative to the repository root.
-  real_index="$(cd "$root" && git rev-parse --git-path index)"
-  if (cd "$root" && [[ -f "$real_index" ]]); then
-    (cd "$root" && cp "$real_index" "$index") ||
-      fail "could not copy the Git index for the review snapshot."
-  fi
-  GIT_INDEX_FILE="$index" git -C "$root" add -A >/dev/null
-  GIT_INDEX_FILE="$index" git -C "$root" write-tree
-}
-
 head_before="$(git -C "$root" rev-parse HEAD)"
-tree_before="$(snapshot_tree before)"
+# The fingerprint of the reviewed content, including uncommitted and untracked
+# files; the diff below is built from the same snapshot.
+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"
@@ -190,7 +179,8 @@ for attempt in 1 2; do
   agent_status=$?
   set -e
 
-  if [[ "$(git -C "$root" rev-parse HEAD)" != "$head_before" || "$(snapshot_tree "after-$attempt")" != "$tree_before" ]]; then
+  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
@@ -235,6 +225,7 @@ review_result_with_ids "$report_file" | jq \
     merge_base: $merge_base,
     head: $head,
     reviewed_tree: $tree,
+    reviewed_paths: null,
     reviewer: {agent: $agent, model: $model},
     created_at: $created_at,
     verdict: .verdict,

scripts/triage-review.sh

@@ -5,6 +5,7 @@ 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"
 
 usage() {
   echo "Usage: $0 <review-json> [--agent <agent>] [--model <model>]"
@@ -85,6 +86,15 @@ provenance+="[$review_ref]"
 tmp_work="$(mktemp -d "${TMPDIR:-/tmp}/triage-review.XXXXXX")"
 trap 'rm -rf "$tmp_work"' EXIT
 
+# Triage and fixes apply only to the content that was reviewed.
+stale_status=0
+review_is_current "$root" "$tmp_work" "$review_path" || stale_status=$?
+case "$stale_status" in
+  0) ;;
+  1) fail "the review is stale: the reviewed content changed after $review_relative was written. Run a new review." ;;
+  *) fail "could not compute the current fingerprint of the working tree." ;;
+esac
+
 decisions_file="$tmp_work/decisions.json"
 context_file="$tmp_work/context.md"
 
@@ -120,6 +130,7 @@ every finding exactly once, using its 'id' as 'finding_id', and return JSON
 that matches the supplied schema."
 
   review_hash_before="$(git hash-object "$review_path")"
+  artifacts_before="$(fingerprint_artifacts "$root")"
   status_before="$(git -C "$root" status --porcelain=v1 --untracked-files=all)"
 
   # Invalid output is retried once; a failed agent or a modified tree is not.
@@ -134,7 +145,8 @@ that matches the supplied schema."
     if [[ "$(git hash-object "$review_path")" != "$review_hash_before" ]]; then
       fail "triage agent modified the source review artifact."
     fi
-    if [[ "$(git -C "$root" status --porcelain=v1 --untracked-files=all)" != "$status_before" ]]; then
+    if [[ "$(git -C "$root" status --porcelain=v1 --untracked-files=all)" != "$status_before" ||
+          "$(fingerprint_artifacts "$root")" != "$artifacts_before" ]]; then
       echo "Error: triage agent modified the working tree." >&2
       git -C "$root" status --short >&2
       exit 1

scripts/verify.conf

@@ -27,6 +27,7 @@ shell-syntax: for file in scripts/*.sh scripts/lib/*.sh tests/*.sh; do bash -n "
 verify: ./tests/verify-test.sh
 doctor: ./tests/doctor-test.sh
 agent: ./tests/agent-test.sh
+fingerprint: ./tests/fingerprint-test.sh
 review-feature: ./tests/review-feature-test.sh
 triage-review: ./tests/triage-review-test.sh
 apply-triage: ./tests/apply-triage-test.sh

tests/apply-triage-test.sh

@@ -74,6 +74,7 @@ setup_repo() {
   git -C "$repo" add .
   git -C "$repo" commit -qm "Seed project"
   git -C "$repo" switch -q -c feature/13-apply-test
+  record_reviewed_tree "$repo" "$repo/$review" "$repo/$triage"
 
   printf '%s\n' "$repo"
 }
@@ -211,6 +212,12 @@ expect_rejected "$repo" "the input is the generated report" ".agents/triage/repo
 expect_rejected "$repo" "the triage does not exist" ".agents/triage/missing.json"
 expect_rejected "$repo" "the agent is unsupported" "$triage" --agent copilot --model model-x
 
+# Fixes apply only to the reviewed content; a stale review is refused.
+repo="$(setup_repo stale)"
+printf 'changed after the review\n' >>"$repo/AGENTS.md"
+expect_rejected "$repo" "the review is stale" "$triage"
+grep -Fq "stale" "$repo.out" || fail "a stale review was not reported as stale"
+
 # A failing agent is reported after verification still ran.
 repo="$(setup_repo agent-fails)"
 if MOCK_AGENT_EXIT=7 run_apply "$repo" y "$triage"; then
@@ -235,6 +242,7 @@ fi
 repo="$(setup_repo verification-fails)"
 printf 'broken: false\n' >"$repo/scripts/verify.conf"
 git -C "$repo" commit -qam "Break verification"
+record_reviewed_tree "$repo" "$repo/$review" "$repo/$triage"
 if run_apply "$repo" y "$triage"; then
   fail "a failed verification returned success"
 fi