feat(review-loop): log agent output so a failed run can be diagnosed

README.md

@@ -190,6 +190,26 @@ code-review-loop --editor claude --reviewer codex
 
 **Outputs (project root):** `agent-code-review.md` (latest findings), `agent-review-summary.md` (narrative).
 
+**When a run fails, read the logs.** Each agent's full output is written to a per-run directory, printed in the banner at startup and again whenever an agent exits non-zero:
+
+```text
+ Logs           : ~/.cache/code-review-loop/20260807-142516
+```
+
+One file per step, named for the step and the agent that ran it:
+
+```text
+1-refinement.claude.log
+3-review-initial.antigravity.log
+4.1-response.claude.log
+6.1-review.antigravity.log
+final-summary.claude.log
+```
+
+Each records the agent, the tools it was allowed, its combined stdout and stderr, and its exit code. This is the difference between "it failed" and knowing why: a loop that stops with a bare `Execution error` on the terminal leaves nothing else behind, and a run started in the background does not even have the scrollback. Note that an agent failing does not stop the loop; it logs the failure and carries on, so the log is often the only sign a step went wrong.
+
+Logs are kept for **one day** and older runs are pruned at startup. Retention is by age rather than by count because the loop tends to be run several times in a sitting, and what you come back for is today's failure. Override with `REVIEW_LOOP_LOG_DAYS`, or set `CODE_REVIEW_LOOP_LOG_DIR` to keep logs somewhere of your own, which opts out of pruning entirely.
+
 ### plan-review-loop
 
 Iteratively improves a **plan document** through review feedback:
@@ -223,6 +243,14 @@ Two caveats for `kimi`: it takes its prompt as a command-line argument (there is
 
 Both loops write their working files (`agent-code-review.md`, `agent-review-summary.md`, `feedback-plan.md`, `plan-review-summary.md`) to the target project's root. Consider adding those names to that project's `.gitignore` (or your global gitignore) so an agent never commits them by accident.
 
+Environment variables:
+
+| Variable | Default | Effect |
+| --- | --- | --- |
+| `CODE_REVIEW_LOOP_LOG_DIR` | `~/.cache/code-review-loop/<timestamp>` | Where `code-review-loop` writes its run logs. Setting it also turns off log pruning, on the grounds that a directory you named is yours to manage. |
+| `REVIEW_LOOP_LOG_DAYS` | `1` | Delete run logs older than this many days. Only applies to the default location. |
+| `AI_CODING_SETUP_PROMPTS_DIR` | `~/.local/share/ai-coding-setup/prompts` | Where the loops read their prompts from. |
+
 ### Shared prompts
 
 Both loops are driven by agent-agnostic prompts in [prompts/](prompts/), not interactive commands. They're listed here so you can audit or tweak the behavior:

bin/code-review-loop

@@ -121,6 +121,8 @@ stage_review_changes() {
     if [[ -n "$new_untracked" ]]; then
         while IFS= read -r file; do
             [[ -z "$file" ]] && continue
+            # A log records the run; it is not part of the change.
+            is_inside_dir "$file" "${RUN_LOG_DIR:-}" && continue
             git add -- "$file"
             write_status "Staged new file: $file" "$DIM"
         done <<< "$new_untracked"
@@ -166,6 +168,18 @@ fi
 setup_temp "code-review-loop"
 trap cleanup_temp EXIT
 
+# ---- run logs -------------------------------------------------------------
+# Outside TMPDIR_REVIEW, which the EXIT trap wipes, and outside the project,
+# where stage_review_changes would commit them. CODE_REVIEW_LOOP_LOG_DIR names
+# the directory runs go under, not the run itself.
+RUN_LOG_ROOT="${CODE_REVIEW_LOOP_LOG_DIR:-$HOME/.cache/code-review-loop}"
+RUN_LOG_DIR=$(claim_run_log_dir "$RUN_LOG_ROOT")
+
+# Only the default location is pruned; a directory the user named is theirs.
+if [[ -z "${CODE_REVIEW_LOOP_LOG_DIR:-}" ]]; then
+    pruned=$(prune_run_logs "$RUN_LOG_ROOT") || true
+fi
+
 echo ""
 echo -e "${MAGENTA}========================================${NC}"
 echo -e "${MAGENTA} Code Review Loop${NC}"
@@ -174,6 +188,8 @@ echo " Max iterations : $MAX_ITERATIONS"
 echo " Skip refinement: $SKIP_REFINEMENT"
 echo " Editor         : $EDITOR_AGENT"
 echo " Reviewer       : $REVIEWER_AGENT"
+echo " Logs           : $RUN_LOG_DIR"
+[[ -n "${pruned:-}" ]] && echo " Pruned         : $pruned run log(s) older than a day"
 
 start_time=$(date +%s)
 
@@ -219,11 +235,13 @@ if [[ "$SKIP_REFINEMENT" == false ]]; then
     snapshot_untracked > "$pre_snapshot"
 
     local_exit=0
-    run_agent "$EDITOR_AGENT" "$refinement_prompt" "$EDITOR_TOOLS" || local_exit=$?
+    AGENT_LOG="$RUN_LOG_DIR/1-refinement.$EDITOR_AGENT.log" \
+        run_agent "$EDITOR_AGENT" "$refinement_prompt" "$EDITOR_TOOLS" || local_exit=$?
     cleanup_agent_artifacts "$EDITOR_AGENT" "$pre_snapshot" "editor"
 
     if [[ $local_exit -ne 0 ]]; then
         write_status "$EDITOR_AGENT code-refinement exited with code $local_exit" "$YELLOW"
+        write_status "Log: $RUN_LOG_DIR/1-refinement.$EDITOR_AGENT.log" "$YELLOW"
     else
         write_status "Code refinement complete" "$GREEN"
     fi
@@ -262,15 +280,18 @@ pre_snapshot="$TMPDIR_REVIEW/pre-review-initial.txt"
 snapshot_untracked > "$pre_snapshot"
 
 local_exit=0
-run_agent "$REVIEWER_AGENT" "$review_task" "$REVIEWER_TOOLS" || local_exit=$?
+AGENT_LOG="$RUN_LOG_DIR/3-review-initial.$REVIEWER_AGENT.log" \
+    run_agent "$REVIEWER_AGENT" "$review_task" "$REVIEWER_TOOLS" || local_exit=$?
 cleanup_agent_artifacts "$REVIEWER_AGENT" "$pre_snapshot" "reviewer"
 
 if [[ $local_exit -ne 0 ]]; then
     write_status "$REVIEWER_AGENT code review exited with code $local_exit" "$YELLOW"
+    write_status "Log: $RUN_LOG_DIR/3-review-initial.$REVIEWER_AGENT.log" "$YELLOW"
 fi
 
 if [[ ! -f "$REVIEW_FILE" ]]; then
     write_status "No review file created; $REVIEWER_AGENT may have failed" "$RED"
+    write_status "Log: $RUN_LOG_DIR/3-review-initial.$REVIEWER_AGENT.log" "$RED"
     echo ""
     echo -e "${MAGENTA}========================================${NC}"
     echo -e "${MAGENTA} Code Review Loop Complete${NC}"
@@ -315,7 +336,8 @@ Read it, evaluate each finding, implement valid fixes, and respond inline per th
     snapshot_untracked > "$pre_snapshot"
 
     local_exit=0
-    run_agent "$EDITOR_AGENT" "$context_prompt" "$EDITOR_TOOLS" || local_exit=$?
+    AGENT_LOG="$RUN_LOG_DIR/4.$iteration-response.$EDITOR_AGENT.log" \
+        run_agent "$EDITOR_AGENT" "$context_prompt" "$EDITOR_TOOLS" || local_exit=$?
     cleanup_agent_artifacts "$EDITOR_AGENT" "$pre_snapshot" "editor"
 
     if [[ $local_exit -ne 0 ]]; then
@@ -343,7 +365,8 @@ IMPORTANT: You MUST overwrite agent-code-review.md with your updated findings."
     snapshot_untracked > "$pre_snapshot"
 
     local_exit=0
-    run_agent "$REVIEWER_AGENT" "$followup_task" "$REVIEWER_TOOLS" || local_exit=$?
+    AGENT_LOG="$RUN_LOG_DIR/6.$iteration-review.$REVIEWER_AGENT.log" \
+        run_agent "$REVIEWER_AGENT" "$followup_task" "$REVIEWER_TOOLS" || local_exit=$?
     cleanup_agent_artifacts "$REVIEWER_AGENT" "$pre_snapshot" "reviewer"
 
     if [[ $local_exit -ne 0 ]]; then
@@ -437,7 +460,8 @@ Write agent-review-summary.md with this structure:
 Be concise and focus on the substance of review-driven improvements, not the original feature work or process details."
 
 local_exit=0
-run_agent "$EDITOR_AGENT" "$summary_prompt" "Read,Write,Grep,Glob" || local_exit=$?
+AGENT_LOG="$RUN_LOG_DIR/final-summary.$EDITOR_AGENT.log" \
+    run_agent "$EDITOR_AGENT" "$summary_prompt" "Read,Write,Grep,Glob" || local_exit=$?
 
 if [[ $local_exit -ne 0 ]]; then
     write_status "$EDITOR_AGENT summary generation exited with code $local_exit" "$YELLOW"
@@ -487,4 +511,6 @@ if [[ -n "$STASH_REF" ]]; then
         echo ""
     fi
 fi
+echo -e " Logs     : $RUN_LOG_DIR"
+echo ""
 echo -e "${CYAN}Waiting for manual review.${NC}"

lib/lib-review-loop

@@ -227,7 +227,7 @@ agent_command() {
 #   $1: agent name ("claude", "codex", "copilot", "antigravity", or "kimi")
 #   $2: prompt text
 #   $3: allowed tools (only used by Claude; ignored by others)
-run_agent() {
+_dispatch_agent() {
     local agent="$1" prompt="$2" tools="${3:-}"
     case "$agent" in
         claude)      run_claude "$prompt" "$tools" ;;
@@ -239,6 +239,30 @@ run_agent() {
     esac
 }
 
+# Mirrors the agent's combined output to AGENT_LOG when set. Without it an agent
+# that dies leaves only an exit code and whatever is still in scrollback.
+run_agent() {
+    local agent="$1" prompt="$2" tools="${3:-}"
+    local rc=0
+
+    if [[ -z "${AGENT_LOG:-}" ]]; then
+        _dispatch_agent "$agent" "$prompt" "$tools"
+        return $?
+    fi
+
+    mkdir -p "$(dirname "$AGENT_LOG")"
+    printf '=== %s  agent=%s  tools=%s ===\n' \
+        "$(date '+%Y-%m-%d %H:%M:%S')" "$agent" "${tools:-default}" >> "$AGENT_LOG"
+
+    # PIPESTATUS, not the pipeline's status: tee succeeds even when the agent
+    # does not, and the caller's `|| local_exit=$?` has to see the agent's code.
+    _dispatch_agent "$agent" "$prompt" "$tools" 2>&1 | tee -a "$AGENT_LOG"
+    rc=${PIPESTATUS[0]}
+
+    printf '=== exit=%s ===\n\n' "$rc" >> "$AGENT_LOG"
+    return "$rc"
+}
+
 # ---- shared validation ----------------------------------------------------
 
 # Validate that the configured editor/reviewer agents are installed.
@@ -395,6 +419,70 @@ cleanup_temp() {
     [[ -n "$TMPDIR_REVIEW" ]] && rm -rf "$TMPDIR_REVIEW"
 }
 
+# ---- paths ----------------------------------------------------------------
+
+# True when $1 is a file inside directory $2. Both sides are resolved, so a
+# relative, trailing-slashed, or symlinked argument compares correctly.
+#   $1: file path
+#   $2: directory path
+is_inside_dir() {
+    local file="$1" dir="$2" file_abs dir_abs
+    [[ -n "$dir" && -d "$dir" ]] || return 1
+    dir_abs=$(cd "$dir" && pwd -P) || return 1
+    file_abs="$(cd "$(dirname "$file")" 2>/dev/null && pwd -P)/$(basename "$file")"
+    [[ "$file_abs" == "$dir_abs"/* ]]
+}
+
+# ---- run log directories --------------------------------------------------
+
+# Claim a run directory under $1 and print its path.
+# Claimed by creating it: mkdir is atomic, so two loops starting in the same
+# second cannot share one. The loser falls back to mktemp, also atomic (a pid is
+# not enough: in a subshell $$ is the parent's, so every loser picks alike).
+#   $1: directory to create the run directory under
+claim_run_log_dir() {
+    local root="$1" stamp dir
+    mkdir -p "$root" || return 1
+    stamp=$(date '+%Y%m%d-%H%M%S')
+    dir="$root/$stamp"
+    if mkdir "$dir" 2>/dev/null; then
+        printf '%s\n' "$dir"
+        return 0
+    fi
+    # Suffixed, so it still starts with the stamp and prune_run_logs sees it.
+    dir=$(mktemp -d "$root/$stamp-XXXXXX") || return 1
+    printf '%s\n' "$dir"
+}
+
+# ---- run log retention ----------------------------------------------------
+
+# Delete run directories under $1 older than $2 days. Prints how many.
+# By age, not count: the loop is run repeatedly in a sitting, so ten runs in an
+# afternoon should not push out yesterday's. Only matches the YYYYmmdd-HHMMSS
+# directories the loops create, so anything kept alongside them survives, and
+# -mtime +0 spares today's.
+#   $1: directory holding the run directories
+#   $2: delete runs older than this many days (default 1, or REVIEW_LOOP_LOG_DAYS)
+prune_run_logs() {
+    local root="$1" days="${2:-${REVIEW_LOOP_LOG_DAYS:-1}}"
+    [[ -d "$root" ]] || return 0
+    [[ "$days" =~ ^[0-9]+$ ]] && (( days >= 1 )) || return 0
+
+    local -a old=()
+    local d
+    while IFS= read -r d; do
+        [[ -n "$d" ]] && old+=("$d")
+    done < <(find "$root" -mindepth 1 -maxdepth 1 -type d \
+        -name '[0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]-[0-9][0-9][0-9][0-9][0-9][0-9]*' \
+        -mtime "+$(( days - 1 ))" 2>/dev/null | sort)
+
+    (( ${#old[@]} > 0 )) || return 0
+    for d in "${old[@]}"; do
+        rm -rf "$d"
+    done
+    printf '%s\n' "${#old[@]}"
+}
+
 # ---- elapsed time --------------------------------------------------------
 
 # Compute human-readable elapsed time from a start timestamp.
@@ -447,6 +535,9 @@ cleanup_agent_artifacts() {
             if [[ " $KNOWN_REVIEW_FILES " == *" $base "* ]]; then
                 continue
             fi
+            # A run log is not an agent artifact; it is only reachable here
+            # when the log dir sits inside the repo.
+            is_inside_dir "$file" "${RUN_LOG_DIR:-}" && continue
             rm -f "$file" 2>/dev/null || true
             write_status "Removed Codex artifact: $file" "$DIM"
         done <<< "$new_files"

test/code-review-loop.bats

@@ -33,3 +33,76 @@ BIN="$PROJECT_ROOT/bin/code-review-loop"
     assert_failure
     assert_output --partial "Unknown agent"
 }
+
+# =========================================================================
+# Run log staging exclusion
+#
+# stage_review_changes must never offer a run log up as a review change. The
+# log directory can sit inside the repo (CODE_REVIEW_LOOP_LOG_DIR allows it),
+# and the value can arrive relative, with a trailing slash, or as the project
+# root itself, each of which defeated an earlier string-prefix check.
+# =========================================================================
+
+# Runs the loop once against a throwaway repo with stub agents, and sets:
+#   log_root    absolute path the logs were written under
+#   log_count   how many .log files it produced
+#   staged_logs how many of them git ended up staging
+run_loop_with_logs() { # run_loop_with_logs <CODE_REVIEW_LOOP_LOG_DIR value> <abs log root>
+    cd "$BATS_TEST_TMPDIR" || return 1
+    rm -rf repo && mkdir repo && cd repo || return 1
+    git init -q . && git config user.email t@t && git config user.name t
+    echo tracked > f && git add . && git commit -qm init
+    echo changed > f && git add f
+
+    mkdir -p stub
+    printf '#!/usr/bin/env bash\ncat >/dev/null\necho ran\nprintf "# R\\n\\nHigh: 0\\nMedium: 0\\nLow: 0\\n\\nVerdict: good to go\\n" > agent-code-review.md\n' > stub/claude
+    cp stub/claude stub/agy
+    chmod +x stub/claude stub/agy
+    PATH="$PWD/stub:$PATH"
+    # The suite sandboxes HOME, so the installed prompts are not reachable and
+    # the loop would exit at validate_prompts before writing a single log.
+    # Point at the checkout's own prompts: the test should not depend on
+    # whether ./setup has been run on this machine.
+    export AI_CODING_SETUP_PROMPTS_DIR="$BATS_TEST_DIRNAME/../prompts"
+    export CODE_REVIEW_LOOP_LOG_DIR="$1"
+
+    # Agents named explicitly: without them the reviewer comes from
+    # ~/.ai-coding-setup.conf or the built-in default, so the test passes or
+    # fails on whether that machine happens to have codex installed. CI does
+    # not, and the loop exited at validate_tools before writing a log.
+    run "$BATS_TEST_DIRNAME/../bin/code-review-loop" -m 1 -e claude -r claude
+    log_root="$2"
+    log_count=$(find "$log_root" -name '*.log' -type f 2>/dev/null | wc -l | tr -d ' ')
+    staged_logs=$(git diff --staged --name-only | grep -cE '\.log$' || true)
+}
+
+@test "run logs are not staged when the log dir is a relative in-repo path" {
+    run_loop_with_logs "mylogs" "$BATS_TEST_TMPDIR/repo/mylogs"
+    # Assert logs were actually produced, or "none staged" proves nothing.
+    [ "$log_count" -gt 0 ]
+    [ "$staged_logs" -eq 0 ]
+}
+
+@test "run logs are not staged when the log dir has a trailing slash" {
+    run_loop_with_logs "$BATS_TEST_TMPDIR/repo/trailing/" "$BATS_TEST_TMPDIR/repo/trailing"
+    [ "$log_count" -gt 0 ]
+    [ "$staged_logs" -eq 0 ]
+}
+
+@test "run logs are not staged when the log dir is the project root" {
+    run_loop_with_logs "$BATS_TEST_TMPDIR/repo" "$BATS_TEST_TMPDIR/repo"
+    [ "$log_count" -gt 0 ]
+    [ "$staged_logs" -eq 0 ]
+}
+
+@test "each run gets its own log directory under a shared root" {
+    run_loop_with_logs "$BATS_TEST_TMPDIR/repo/shared" "$BATS_TEST_TMPDIR/repo/shared"
+    [ "$log_count" -gt 0 ]
+    git reset -q --hard HEAD
+    echo again > f && git add f
+    run "$BATS_TEST_DIRNAME/../bin/code-review-loop" -m 1 -e claude -r claude
+    # Two runs must not append into one set of step filenames.
+    local dirs
+    dirs=$(find "$BATS_TEST_TMPDIR/repo/shared" -mindepth 1 -maxdepth 1 -type d | wc -l | tr -d ' ')
+    [ "$dirs" -eq 2 ]
+}