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

[rlorenzo]

The problem

A review loop that dies leaves nothing behind. Agent output went to the terminal and
nowhere else, so when a run stopped mid-refinement all it showed was:

[1] claude: Code Refinement (lint, KISS/DRY/YAGNI, tests)
------------------------------------------------------------
  This may take several minutes...
Execution error

No way to tell a crash from a timeout from an external kill, and no way to recover it
afterwards. A run started in the background does not even have the scrollback.

The change

run_agent mirrors the agent's combined output to AGENT_LOG when set, stamped with the
agent, the tools it was given, and the exit code. Dispatch moves to _dispatch_agent so
the wrapper lives in one place rather than being repeated per runner.

It returns PIPESTATUS[0], not the pipeline status. tee succeeds even when the agent
does not, and the caller's || local_exit=$? has to see the agent's real code, otherwise
every failure silently reads as success.

code-review-loop creates a per-run directory and names a log per step:

 Logs           : /Users/you/.cache/code-review-loop/20260807-142516

printed in the banner and again on any agent failure. It sits outside two places on
purpose:

• not TMPDIR_REVIEW, which the EXIT trap wipes, taking the most interesting run with it
• not the project, where stage_review_changes would offer the logs up for commit

That second point is also why staging now skips anything under the log directory.
CODE_REVIEW_LOOP_LOG_DIR can point inside a repo, and without the skip the first run
there staged its own logs. That was caught by testing the change rather than by reading it.

Tests

Five tests in test/lib-review-loop.bats cover the wrapper with a stubbed agent on
PATH:

• passes output through and writes no file when AGENT_LOG is unset
• mirrors output to the log with the agent name and exit code
• returns the agent's exit code, not tee's
• captures a failing agent's stderr, which is the whole point
• creates the log directory when it does not exist

The two that matter were confirmed to fail against rc=$? before the fix, so they are
not vacuous:

not ok 50 run_agent returns the agent's exit code, not tee's
not ok 51 run_agent captures a failing agent's stderr in the log

58 tests pass, shellcheck clean, pre-commit run green.

Not included

No timeout. claude has no timeout flag and gtimeout is not present on macOS by
default, so anything here would have been invented. run_antigravity already carries
--print-timeout 30m; the others do not.

Worth noting the original failure turned out not to be a bug in these scripts. The
tolerated-failure path works: with a stubbed agent that exits 1, the loop logs the warning
and continues through every remaining step. The run had been killed externally. The defect
was that this was not knowable, which is what this PR fixes.

[greptile-apps]

Greptile Summary

The PR records each review agent’s combined output and exit status in persistent, per-run log directories.

• Routes agent execution through a logging wrapper while preserving the agent’s actual pipeline exit code.
• Atomically claims unique run directories, prunes expired default-location logs, and prevents in-repository logs from being staged.
• Adds regression coverage for logging, path handling, retention, and concurrent directory claims.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
bin/code-review-loop	Integrates per-step logs, atomically claimed run directories, retention, failure-path log reporting, and resolved-path staging exclusion; the previously reported path and collision issues are fixed.
lib/lib-review-loop	Adds centralized output mirroring with correct agent exit propagation plus atomic run-directory claiming and suffix-aware pruning.
test/code-review-loop.bats	Exercises successful log production and non-staging for relative, trailing-slash, project-root, and shared-root configurations.
test/lib-review-loop.bats	Covers output capture, stderr capture, exit-code preservation, directory creation, concurrent uniqueness, and retention behavior.
README.md	Documents log locations, contents, retention, failure behavior, and configuration variables.

_{Reviews (3): Last reviewed commit: "fix(review-loop): claim run directories ..." | Re-trigger Greptile}

[rlorenzo]

Pushed 8d25b89 covering two follow-ups.

Retention. Logs outlived their run by design, which also meant nothing ever removed them: one directory per run, forever, in a cache directory. They are now pruned at startup by age rather than count, since the loop gets run several times in a sitting and what you come back for is today's failure. Ten runs in an afternoon should not push out yesterday's, and a quiet week should not keep last month's. One day by default, REVIEW_LOOP_LOG_DAYS to change it. Pruning only matches the YYYYmmdd-HHMMSS directories the loops create, and setting CODE_REVIEW_LOOP_LOG_DIR opts out entirely.

A bug the end-to-end test caught. Codex artifact cleanup sweeps files that appeared during a reviewer run, and when the log directory sits inside the repo, which CODE_REVIEW_LOOP_LOG_DIR permits, it deleted the reviewer's own log seconds after writing it:

Removed Codex artifact: .../20260807-193834/3-review-initial.codex.log

A log is a record of the run, not something the agent made, so cleanup_agent_artifacts now skips anything under RUN_LOG_DIR.

README now documents where the logs are, what each holds, and that an agent failing does not stop the loop, so the log is often the only sign a step went wrong. Environment variables are listed too.

64 tests pass. The six new ones cover retention and log preservation, and each was confirmed to fail against the previous behaviour.

[rlorenzo]

@greptileai review

[rlorenzo]

@greptileai review