From a1984a319caeee5ff1932d3c91c29382fc5d86ac Mon Sep 17 00:00:00 2001 From: Rex Lorenzo Date: Wed, 12 Aug 2026 22:54:55 -0700 Subject: [PATCH 1/2] fix(review-loop): use --sandbox workspace-write, not the removed --full-auto codex-cli dropped --full-auto from the `exec` subcommand (gone as of 0.147.0). An unknown flag is a hard parse error, so codex exited 2 before the agent started, no agent-code-review.md was written, and the loop ended with "codex did not produce a review file. Changes are NOT reviewed." Every codex-reviewed run had been failing this way -- and because the editor pass runs first and succeeds, the run looked half-successful rather than broken. workspace-write is the equivalent policy: `exec` is already non-interactive, so there were never approvals to auto-accept. The flag's real job is widening the sandbox far enough for the reviewer to write its report. Verified against codex-cli 0.147.0 by running a full review cycle to a clean verdict. The flags run_codex passes are a compatibility surface with another tool's CLI, not an internal detail, but nothing tested them -- which is how a stale flag shipped silently. Adds three tests pinning that contract: no --full-auto, a sandbox that can actually write the review file, and the exec/stdin/mcp_servers shape the loop depends on. Also updates the stale --full-auto reference in run_antigravity's comment. One known rough edge, left alone: workspace-write denies writes outside the workspace, so a project whose tests bind a socket or whose toolchain caches under $HOME (uv, pip) makes the reviewer work around it per run. Worth revisiting if it costs real review turns, but it is the faithful translation of what --full-auto granted. --- lib/lib-review-loop | 12 +++++++-- test/lib-review-loop.bats | 54 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/lib/lib-review-loop b/lib/lib-review-loop index 94b49e0..37400c7 100644 --- a/lib/lib-review-loop +++ b/lib/lib-review-loop @@ -151,17 +151,25 @@ run_claude() { # Run Codex CLI with a prompt. Wraps the prompt with a preamble that prevents # conversational output, and disables MCP servers to avoid artifact creation. +# +# --sandbox workspace-write, not --full-auto: codex-cli dropped --full-auto from +# the `exec` subcommand (gone as of 0.147.0), and an unknown flag is a hard +# parse error, so the agent never starts and the loop exits with no review. +# workspace-write is the equivalent policy -- `exec` is already non-interactive, +# so there are no approvals to auto-accept, only the sandbox to widen far enough +# that the reviewer can write agent-code-review.md. # $1: prompt text run_codex() { local prompt="$1" local wrapped="Execute the following task now. Do not introduce yourself. Begin immediately. $prompt" - echo "$wrapped" | codex exec --full-auto --config 'mcp_servers={}' - + echo "$wrapped" | codex exec --sandbox workspace-write --config 'mcp_servers={}' - } # Run Antigravity CLI with a prompt in non-interactive headless mode. -# Uses --dangerously-skip-permissions for auto-approval (equivalent to Codex --full-auto). +# Uses --dangerously-skip-permissions for auto-approval (the Codex equivalent is +# --sandbox workspace-write; see run_codex). # The prompt MUST be piped via stdin without -p: since agy 1.1.1, passing any # -p value disables stdin reading, and whitespace-only -p values are rejected # as an empty prompt. A piped stdin prompt alone still enters print mode. diff --git a/test/lib-review-loop.bats b/test/lib-review-loop.bats index 0f0987f..307beea 100644 --- a/test/lib-review-loop.bats +++ b/test/lib-review-loop.bats @@ -558,3 +558,57 @@ make_run_dir() { # make_run_dir run is_inside_dir "logs/a.log" ""; assert_failure run is_inside_dir "logs/a.log" "$root/nope"; assert_failure } + +# ========================================================================= +# Codex invocation +# +# The flags run_codex passes are a compatibility surface, not an internal +# detail: codex rejects an unknown flag at parse time, so a stale one means +# the agent never starts and the loop ends with no review written. That +# failure looked like a codex problem rather than ours, so pin the contract. +# ========================================================================= + +stub_codex() { # records the argv it was invoked with, then succeeds + local dir="$BATS_TEST_TMPDIR/bin" + mkdir -p "$dir" + { + echo '#!/usr/bin/env bash' + echo 'cat >/dev/null' + printf 'printf "%%s\\n" "$@" > %q\n' "$BATS_TEST_TMPDIR/codex-argv" + } > "$dir/codex" + chmod +x "$dir/codex" + PATH="$dir:$PATH" +} + +@test "run_codex does not pass --full-auto, which codex exec no longer accepts" { + source_lib + stub_codex + run run_agent codex "a prompt" "Read" + assert_success + run cat "$BATS_TEST_TMPDIR/codex-argv" + refute_output --partial "--full-auto" +} + +@test "run_codex requests a sandbox that can write the review file" { + source_lib + stub_codex + run run_agent codex "a prompt" "Read" + assert_success + run cat "$BATS_TEST_TMPDIR/codex-argv" + # read-only would let the agent run but silently fail to write + # agent-code-review.md, which the loop reports as a dead codex. + assert_output --partial "--sandbox" + assert_output --partial "workspace-write" +} + +@test "run_codex still runs exec non-interactively with MCP servers disabled" { + source_lib + stub_codex + run run_agent codex "a prompt" "Read" + assert_success + run cat "$BATS_TEST_TMPDIR/codex-argv" + assert_output --partial "exec" + assert_output --partial "mcp_servers={}" + # The trailing "-" is what makes codex read the prompt from stdin. + assert_output --partial "-" +} From 7d800756b9134314e31d8fb328a16b53dd27ada3 Mon Sep 17 00:00:00 2001 From: Rex Lorenzo Date: Wed, 12 Aug 2026 23:27:26 -0700 Subject: [PATCH 2/2] test(review-loop): assert the standalone stdin "-" argument, not any hyphen assert_output --partial "-" was satisfied by the hyphens in --sandbox and --config, so the test passed with the trailing "-" removed and never protected the stdin invocation contract it documents. assert_line matches a whole argument line, which fails when the "-" goes missing. --- test/lib-review-loop.bats | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/lib-review-loop.bats b/test/lib-review-loop.bats index 307beea..8c6ac94 100644 --- a/test/lib-review-loop.bats +++ b/test/lib-review-loop.bats @@ -610,5 +610,7 @@ stub_codex() { # records the argv it was invoked with, then succeeds assert_output --partial "exec" assert_output --partial "mcp_servers={}" # The trailing "-" is what makes codex read the prompt from stdin. - assert_output --partial "-" + # assert_line matches a whole argument: --partial would also be satisfied + # by the hyphens in --sandbox, so it would pass with the "-" removed. + assert_line "-" }