[5uck1ess]

Cause

ClaudeRunner.Run (src/runners/claude.go) parsed is_error and the exit code from claude -p --output-format json but never checked them. A rate limit, auth failure or --max-turns exhaustion came back as a nil error, so the engine marked the step kept and fed the error text into later {{step}} prompts. Codex and Gemini runners already fail on a non-zero exit.

Fix

• Return an error when is_error is true or the exit code is non-zero. Output, cost and exit code stay on the result.
• Max-turns responses carry no result; the error message falls back to errors[], then stderr, and is prefixed with subtype.
• runStep / runLoop record and charge the cost a failed run reports, so a loop of failing runs still hits the budget cap (this was implicitly counted before, when failures looked like successes).

Tests

• claude_run_test.go: fake claude on PATH covering is_error, non-zero exit with valid JSON, success, and max-turns errors[].
• TestRunWorkflowFailedRunsCountTowardBudget.

[5uck1ess]

🔍 REVIEW — automated merge-readiness

Review only — a verdict on this PR; no code was changed. Corrections, when applied, arrive as a separate ## 🔧 Corrections applied comment.

gpt-6.1-sol · high effort · fast speed · reviewed at 77381a7

Reviewed head: 77381a77712a35272f97d9c457f6bb0270bee3b5.

Both findings survive repository and merge-base inspection:

1. [P1] Failed Claude iterations can commit edits without passing the gate — claude.go:114. With max: 2, until: DONE, and gate test "$(cat state.txt)" = good, let iteration one pass without DONE. Iteration two writes bad to tracked state.txt, then emits {"is_error":true,"subtype":"error_max_turns"} and exits 1. The new error return reaches engine.go:437, which continues without gate execution or rollback. The earlier success makes the loop return nil; engine.go:229 then commits bad. Expected: revert or prevent successful finalization. At the specified base, identical JSON returned nil, reached the failing gate, and reverted the edit. Existing gate tests and TestClaudeRun_MaxTurnsUsesErrorsField do not cover this combined scenario.

2. [P2] The new parallel test races on its shared counter — engine_test.go:1725. TestRunWorkflowParallelFailedRunsCountCost supplies one erroringCostRunner to children a and b. Their goroutines call Run at engine.go:607, outside the mutex, allowing concurrent unsynchronized r.calls++ operations. Expected: race-free tests; actual: a data race that CI’s -race invocation can report. The helper and test are absent at the specified base. The existing mock’s mutex protects only that separate mock.

These are static code-path confirmations; runtime tests were not executed under the read-only filesystem restriction.

VERDICT: CHANGES_REQUESTED - failed-loop edits can be committed, and the new parallel test races

---

Automated pre-merge review by pr-merge-brief. A full-context READY on the current head is auto-merged; a diff-only or failover review waits for the maintainer.

[5uck1ess]

🔍 REVIEW — automated merge-readiness

Review only — a verdict on this PR; no code was changed. Corrections, when applied, arrive as a separate ## 🔧 Corrections applied comment.

gpt-6.1-sol · high effort · fast speed · reviewed at dd1d2e9

Reviewed head: dd1d2e9ae333ccbae5f48591361d7e1c70e6e51a.

One blocking finding survives; the repository does not support refuting it.

1. [P1] Failed-loop rollback deletes earlier successful work — engine.go:450, also line 466.

   The supported CLI calls RunWorkflow, which executes ordinary steps through runStep. A successful command marks its result kept and returns without committing (engine.go:353). There is no checkpoint before entering runLoop.

   With the reported input—printf good > state.txt, followed by a two-attempt loop whose first Codex invocation exits nonzero and whose second returns DONE—Codex returns an error, triggering the newly added RevertAll. That method executes git checkout -- . followed by git clean -fd (git.go:56), deleting the preceding command’s untracked state.txt. The final verification therefore fails; expected behavior is to preserve the successful preceding step. The initial clean-checkout requirement does not prevent files created during the workflow.

   Introduced by this PR: git show dc024cdead038a12a972b4f1dd5242378f35f185:src/engine/engine.go confirms that runner-error and required-output failures previously retried without rollback. Although gate-failure rollback already existed, this ungated Codex-error scenario preserved the file at base.

   Tests do not contradict it: TestRunWorkflowLoopRunnerErrorRevertsEdits starts with a successful loop iteration, which commits before the subsequent failure. It does not cover uncommitted files from preceding ordinary steps.

Validation was by source tracing and base comparison. Runtime reproduction was unavailable under the read-only filesystem restriction.

VERDICT: CHANGES_REQUESTED - failed-loop rollback deletes earlier successful work

---

Automated pre-merge review by pr-merge-brief. A full-context READY on the current head is auto-merged; a diff-only or failover review waits for the maintainer.

[5uck1ess]

🔍 REVIEW — automated merge-readiness

Review only — a verdict on this PR; no code was changed. Corrections, when applied, arrive as a separate ## 🔧 Corrections applied comment.

gpt-6.1-sol · high effort · fast speed · reviewed at d136f58

Reviewed head: d136f58e641db7ef319f49e4e36d793b5fb9fc46.

Previous finding — fixed. The checkpoint preserves preceding steps’ work before rollback. TestRunWorkflowLoopRollbackKeepsEarlierStepWork directly covers the reported scenario.

One new blocker:

1. [P2] Checkpoint prevents repair loops from fixing work rejected by commit hooks — src/engine/engine.go:415.

   The CLI calls RunWorkflow → runLoop → CommitAll, which executes ordinary git commit through src/lib/git.go:52.

   Concrete scenario: start with tracked state.txt containing good and a pre-commit hook running test "$(cat state.txt)" = good. A successful preparation command writes bad; the following repair loop would restore good and return DONE.

   Expected: execute the repair loop and complete successfully. Actual: the checkpoint invokes the hook against bad, then returns checkpoint commit failed before calling the runner. This also affects the supplied bugfix workflow when test hooks reject failures that its fix-tests loop should repair.

   Introduced here: base 90c2b8d has no pre-loop commit; it executes the repair before attempting its iteration commit. The checkpoint regression test installs no commit hook, so it does not contradict this failure.

Reviewed the entire PR diff and affected call paths. Validation was by source tracing and base comparison; tests were not run under the read-only filesystem restriction. CI status supplied: none.

The rollback correction and its checkpoint follow-up have now each introduced a workflow regression, meeting the requested human-escalation threshold.

VERDICT: NEEDS_HUMAN - repeated corrections still leave a repair-loop regression

---

Automated pre-merge review by pr-merge-brief. A full-context READY on the current head is auto-merged; a diff-only or failover review waits for the maintainer.

[5uck1ess]

🔍 REVIEW — automated merge-readiness

Review only — a verdict on this PR; no code was changed. Corrections, when applied, arrive as a separate ## 🔧 Corrections applied comment.

gpt-6.1-sol · high effort · fast speed · reviewed at 950a714

Reviewed head: 950a7141651cf23d7c5d3315fdfc3e07a3a43d16.

Previous findings:

• Earlier-step work lost during rollback — fixed. Snapshots preserve preceding steps’ changes. TestRunWorkflowLoopRollbackKeepsEarlierStepWork covers this behavior.
• Pre-commit hooks prevent repair loops from starting — fixed. Startup now creates a tree snapshot without committing. TestRunWorkflowRepairLoopRunsDespiteRejectingPreCommitHook covers the reported scenario.

Reviewed the entire base-to-head diff and affected call paths, with particular attention to changes since d136f58. No additional demonstrated merge-blocking defects found.

CI is GREEN as supplied. Tests were inspected but not rerun because the filesystem is read-only.

The supplied review history already meets your threshold of two failed correction cycles, requiring human sign-off despite the fixes above.

VERDICT: NEEDS_HUMAN - two prior correction cycles failed

---

Automated pre-merge review by pr-merge-brief. A full-context READY on the current head is auto-merged; a diff-only or failover review waits for the maintainer.