fix: fix: agent_run swallows all Claude failures silently via || true (#277) #286

Merged

dev-bot merged 1 commit from fix/issue-277 into main

2026-04-06 09:34:02 +00:00

dev-bot commented

2026-04-06 09:24:55 +00:00

Collaborator

Fixes #277

Changes

Fixes #277 ## Changes

dev-bot added 1 commit 2026-04-06 09:24:56 +00:00

fix: fix: agent_run swallows all Claude failures silently via || true (#277 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

26fa11efff

Capture exit code from claude invocations instead of suppressing with || true.
Log timeout (rc=124) and non-zero exits distinctly. Skip nudge when output is
empty (claude crashed or failed). Log empty output as a clear diagnostic message.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev-bot commented

2026-04-06 09:32:05 +00:00

Author

Collaborator

AI Review — Error\n\nReview failed.\n---\n`26fa11e`

## AI Review — Error\n\nReview failed.\n---\n*26fa11e*

review-bot commented

2026-04-06 09:33:16 +00:00

Collaborator

AI Review

Summary

The PR fixes issue #277 where agent_run silently swallowed all non-zero exit codes from claude via || true. The fix is minimal, correct, and follows existing conventions.

Review

What changed

|| true replaced with && rc=0 || rc=0 to capture the exit code without triggering set -e
Timeout (exit 124) and generic failure are now logged separately
Empty output is logged as a likely crash indicator
The nudge block is now guarded with [ -n "$output" ] to prevent spurious nudges when Claude crashed or timed out
Nudge invocation gets its own nudge_rc with identical logging pattern

Findings

None. The implementation is sound:

&& rc=0 || rc=0 pattern is the correct ShellCheck-clean idiom for capturing exit codes under set -e; using a bare || true would swallow the code, a bare test would abort on non-zero
Nudge guard [ -n "$output" ] correctly prevents resuming a dead session when the initial run produced no output — the previous code would have nudged even after a crash since _AGENT_SESSION_ID persists across calls
No propagation to caller — agent_run logs failures but still returns 0; this is intentional (the callers are agent loops that should continue and let the empty-output state be handled upstream), and matches the scope of issue #277 which asked only for logging, not for aborting
ShellCheck passes with no warnings
No security concerns — no unquoted variables introduced, no secret leakage in logs (only exit codes and session ID prefix are logged)

Verdict

APPROVE — Correctly captures and logs Claude exit codes; guards nudge against empty output; ShellCheck clean

Reviewed at 26fa11e | AGENTS.md

## AI Review  ### Summary The PR fixes issue #277 where `agent_run` silently swallowed all non-zero exit codes from `claude` via `|| true`. The fix is minimal, correct, and follows existing conventions. ### Review **What changed** - `|| true` replaced with `&& rc=0 || rc=0` to capture the exit code without triggering `set -e` - Timeout (exit 124) and generic failure are now logged separately - Empty output is logged as a likely crash indicator - The nudge block is now guarded with `[ -n "$output" ]` to prevent spurious nudges when Claude crashed or timed out - Nudge invocation gets its own `nudge_rc` with identical logging pattern ### Findings None. The implementation is sound: - **`&& rc=0 || rc=0` pattern** is the correct ShellCheck-clean idiom for capturing exit codes under `set -e`; using a bare `|| true` would swallow the code, a bare test would abort on non-zero - **Nudge guard `[ -n "$output" ]`** correctly prevents resuming a dead session when the initial run produced no output — the previous code would have nudged even after a crash since `_AGENT_SESSION_ID` persists across calls - **No propagation to caller** — `agent_run` logs failures but still returns 0; this is intentional (the callers are agent loops that should continue and let the empty-output state be handled upstream), and matches the scope of issue #277 which asked only for logging, not for aborting - **ShellCheck passes** with no warnings - **No security concerns** — no unquoted variables introduced, no secret leakage in logs (only exit codes and session ID prefix are logged) ### Verdict **APPROVE** — Correctly captures and logs Claude exit codes; guards nudge against empty output; ShellCheck clean --- *Reviewed at `26fa11e` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-06 09:33:16 +00:00

review-bot left a comment

Collaborator

AI Review: APPROVE — Correctly captures and logs Claude exit codes; guards nudge against empty output; ShellCheck clean

AI Review: **APPROVE** — Correctly captures and logs Claude exit codes; guards nudge against empty output; ShellCheck clean