Skip to content

Commit deb4669

Browse files
alanshurafaclaude
alanshurafa
and
claude
committed
fix: agent_auth_failed no longer false-positives on echoed auth text
The execute/verify auth gate scanned both the output and the full stderr log for auth substrings unconditionally, so any run whose working log echoed auth strings — plan text, or the auth-detection source itself — was misread as "authentication failed" and aborted with no verdict (observed building claude-build via codex-build). Mirror lib's robust validate_agent_artifact discriminator: an auth banner in the output counts only when the output is short (<50 words), and a banner in stderr counts only when the output is empty (a real work product means the CLI authenticated). Adds reliability sim S5a-S5d covering the regression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 7934bcd commit deb4669

2 files changed

Lines changed: 99 additions & 6 deletions

File tree

‎dev-review/codex/dev-review.sh‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -315,18 +315,36 @@ write_text_file() {
315315

316316
agent_auth_failed() {
317317
local agent="$1"
318-
shift
319-
local file_path
320-
local cli_name
318+
local output_file="${2:-}"
319+
local stderr_file="${3:-}"
320+
local cli_name words
321321

322322
cli_name=$(agent_cli_name "$agent")
323323

324-
for file_path in "$@"; do
325-
if file_contains_auth_failure "$file_path"; then
324+
# A genuine auth failure means the CLI bailed BEFORE doing work, so its banner
325+
# is short and stands alone. Mirror validate_agent_artifact's discriminator so
326+
# a substantial work product that merely echoes auth strings — e.g. plan text,
327+
# or the auth-detection source itself — is never misread as an auth failure.
328+
#
329+
# (1) Auth banner IN THE OUTPUT, but only when the output is short (< 50
330+
# words). A long output that mentions "Unauthorized"/"Not logged in" is
331+
# real work, not the CLI's own banner.
332+
if [[ -n "$output_file" ]] && file_contains_auth_failure "$output_file"; then
333+
words=$(wc -w < "$output_file" | tr -d '\r\n ')
334+
if (( words < 50 )); then
326335
log "WARNING: ${cli_name} authentication failed. Refresh the ${cli_name} CLI session and rerun."
327336
return 0
328337
fi
329-
done
338+
fi
339+
340+
# (2) Auth banner in STDERR counts only when the agent produced NO output. A
341+
# non-empty work product means the CLI authenticated and ran; auth strings
342+
# in its (possibly huge) working log are echoed content, not the banner.
343+
if [[ ! -s "$output_file" && -n "$stderr_file" && -s "$stderr_file" ]] \
344+
&& file_contains_auth_failure "$stderr_file"; then
345+
log "WARNING: ${cli_name} authentication failed. Refresh the ${cli_name} CLI session and rerun."
346+
return 0
347+
fi
330348

331349
return 1
332350
}

‎tests/reliability-simulation.sh‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,81 @@ else
100100
fail "S5: empty output -> expected rc 1, got $rc"
101101
fi
102102

103+
# ---------------------------------------------------------------------------
104+
# R-2b: agent_auth_failed (the dev-review.sh execute/verify auth gate, distinct
105+
# from lib's validate_agent_artifact). Regression for the false positive where a
106+
# large working log echoes auth strings (plan text, or the auth-detection source
107+
# itself) and trips a naive substring scan. Extract the real functions from the
108+
# runner via sed (dev-review.sh has no main guard, so it is not safe to source
109+
# whole — same discipline as tests/revise-loop-simulation.sh).
110+
# ---------------------------------------------------------------------------
111+
AUTH_FN_SRC="$TEST_DIR/agent_auth_failed.sh"
112+
sed -n '/^agent_cli_name() {/,/^}$/p' "$REPO_ROOT/dev-review/codex/dev-review.sh" > "$AUTH_FN_SRC"
113+
sed -n '/^agent_auth_failed() {/,/^}$/p' "$REPO_ROOT/dev-review/codex/dev-review.sh" >> "$AUTH_FN_SRC"
114+
# shellcheck disable=SC1090
115+
source "$AUTH_FN_SRC"
116+
117+
if ! declare -F agent_auth_failed >/dev/null; then
118+
TOTAL=$((TOTAL + 1))
119+
fail "R-2b: agent_auth_failed not sourced — cannot test"
120+
else
121+
# Scenario 5a: empty output + auth banner in stderr -> detected (rc 0).
122+
TOTAL=$((TOTAL + 1))
123+
out="$TEST_DIR/s5a-out.md"; err="$TEST_DIR/s5a-err.log"
124+
: > "$out"; printf 'Not logged in. Please run /login\n' > "$err"
125+
rc=0; agent_auth_failed codex "$out" "$err" >/dev/null 2>&1 || rc=$?
126+
if [[ "$rc" -eq 0 ]]; then
127+
pass "S5a: agent_auth_failed: empty output + stderr banner -> detected"
128+
else
129+
fail "S5a: expected detection (rc 0), got $rc"
130+
fi
131+
132+
# Scenario 5b: short auth banner in the OUTPUT -> detected (rc 0).
133+
TOTAL=$((TOTAL + 1))
134+
out="$TEST_DIR/s5b-out.md"; err="$TEST_DIR/s5b-err.log"
135+
printf 'Failed to authenticate. Please run `claude login`.\n' > "$out"; : > "$err"
136+
rc=0; agent_auth_failed claude "$out" "$err" >/dev/null 2>&1 || rc=$?
137+
if [[ "$rc" -eq 0 ]]; then
138+
pass "S5b: agent_auth_failed: short banner in output -> detected"
139+
else
140+
fail "S5b: expected detection (rc 0), got $rc"
141+
fi
142+
143+
# Scenario 5c (REGRESSION): substantial output + a huge stderr working log that
144+
# echoes auth strings deep inside (plan text / the auth-detection source) ->
145+
# NOT an auth failure (rc 1). This is the codex-build self-build false positive.
146+
TOTAL=$((TOTAL + 1))
147+
out="$TEST_DIR/s5c-out.md"; err="$TEST_DIR/s5c-err.log"
148+
{
149+
printf 'Implemented the feature as planned. Files changed and tests pass.\n'
150+
for i in $(seq 1 80); do printf 'word%d ' "$i"; done; printf '\n'
151+
} > "$out"
152+
{
153+
for i in $(seq 1 3000); do printf 'log line %d: working...\n' "$i"; done
154+
printf ' 66: - If the output contains `Not logged in` (or `/login`): degrade\n'
155+
printf " 576: grep -qiE 'Not logged in|Please run /login|Unauthorized' file\n"
156+
for i in $(seq 1 3000); do printf 'log line %d: more work...\n' "$i"; done
157+
} > "$err"
158+
rc=0; agent_auth_failed codex "$out" "$err" >/dev/null 2>&1 || rc=$?
159+
if [[ "$rc" -eq 1 ]]; then
160+
pass "S5c: agent_auth_failed: big working log echoing auth strings -> NOT flagged (regression)"
161+
else
162+
fail "S5c: expected no detection (rc 1), got $rc — false positive regressed"
163+
fi
164+
165+
# Scenario 5d: empty output + genuine stderr banner -> still detected (rc 0).
166+
# The regression guard must not mask a real failure whose only signal is stderr.
167+
TOTAL=$((TOTAL + 1))
168+
out="$TEST_DIR/s5d-out.md"; err="$TEST_DIR/s5d-err.log"
169+
: > "$out"; printf 'authentication_error: OAuth token expired\n' > "$err"
170+
rc=0; agent_auth_failed codex "$out" "$err" >/dev/null 2>&1 || rc=$?
171+
if [[ "$rc" -eq 0 ]]; then
172+
pass "S5d: agent_auth_failed: empty output + genuine stderr banner -> detected"
173+
else
174+
fail "S5d: expected detection (rc 0), got $rc"
175+
fi
176+
fi
177+
103178
# ---------------------------------------------------------------------------
104179
# C-2/S-2: fill_template metacharacter pinning (false-positive verification)
105180
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)