fix(react): stuck-loop detection returns FAILED, not COMPLETED (#740)

[frankbria]

Closes #740

Problem

When the same tool signature repeats 3×, _react_loop returned AgentStatus.COMPLETED. Final verification then passed against the already-green repo, so a stuck agent that implemented nothing → task DONE → (with auto-close) the linked GitHub issue closed. A stuck loop is a failure, not completion.

Fix

• Loop detection now returns AgentStatus.FAILED (codeframe/core/react_agent.py).
• Records self._early_termination_reason = "loop_detected" (reset per run()) so the caller reports loop_detected in AGENT_FAILED, distinct from max_iterations_reached / stall_detected. The AGENT_EARLY_TERMINATION event already carried this reason.
• Because the loop returns FAILED, final verification no longer runs — so the task is not marked DONE and the linked issue is not auto-closed.

Acceptance criteria

• Loop detection returns FAILED — verified by test_loop_detected_returns_failed.
• Run result distinguishes "completed" from "terminated early due to loop" — _early_termination_reason == "loop_detected" + AGENT_EARLY_TERMINATION event.

Tests

test_loop_detected_returns_completed renamed to test_loop_detected_returns_failed; now asserts FAILED, the early-termination reason, and that gates.run is not called. No false-positive tests (different tools / different paths) unchanged and passing. Full react_agent + escalation + engine-integration suites (108 tests) green; ruff clean.

Review

Pre-PR third-party review: codex LGTM, no correctness/logic regressions (opencode was unavailable — server error).

Known limitations

None. Scope is the single return AgentStatus.COMPLETED → FAILED at the loop-detection site plus reason propagation.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 51 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c10a4013-0b4a-4c3e-b9f6-a102c6dc6005

📥 Commits

Reviewing files that changed from the base of the PR and between dada805 and 98adaed.

📒 Files selected for processing (2)

• codeframe/core/react_agent.py
• tests/core/test_react_agent.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/740-loop-detection-failed

---

_{Comment @coderabbitai help to get the list of available commands.}

[frankbria]

Pre-PR third-party review (codex; opencode was down with a server error):

LGTM. No correctness/logic regressions found in the diff.

Reviewed control flow around the loop-detection return and reason propagation; no issues.

[claude]

Review

Small, well-scoped fix that does exactly what the title says. Verified downstream: runtime.py's _status_map/fail_run path already treats AgentStatus.FAILED correctly (no DONE transition, no complete_run), so this change correctly prevents the auto-close-on-stuck-loop bug described in #740 without needing any changes outside react_agent.py.

Correctness

• The core change (react_agent.py:669, loop-detection site) is correct: returning FAILED instead of COMPLETED means run() skips _run_final_verification entirely (the if status == AgentStatus.FAILED: ... return status branch fires before verification), so gates never run against the already-green repo. Confirmed by the new mock_gates.run.assert_not_called() assertion.
• _early_termination_reason is reset at the top of run() (self._early_termination_reason = None) before each task, so it can't leak stale state across sequential run() calls on a reused ReactAgent instance.
• Reason precedence in the FAILED branch (_early_termination_reason → stall → max_iterations_reached) is sound: loop detection returns immediately from _react_loop, so by construction _stall_triggered can't also be set for that same return — no real ambiguity between the two reasons in practice.

Test coverage

• The renamed test (test_loop_detected_returns_failed) is a good comprehensiveness bump over the original — it now checks status, reason string, and that verification was skipped, not just status alone.
• The two false-positive tests (different tools, same tool/different paths) are untouched and still assert COMPLETED, confirming the loop-detection trigger logic itself wasn't touched — only what happens after it fires. Good regression coverage for the boundary.

Minor observations (non-blocking)

• AGENT_EARLY_TERMINATION (emitted just before the return AgentStatus.FAILED) and the subsequent AGENT_FAILED event both carry reason: "loop_detected" now — that's a nice, non-ambiguous signal for anything consuming the event stream (web UI notifications, CLI follow), and matches the "distinguish from max_iterations" acceptance criterion. No consumer currently special-cases the reason string, so this is forward-looking rather than something this PR needed to wire up — fine to leave as-is.
• Docstring on _early_termination_reason says "Reset per run()" — accurate and matches the actual reset at react_agent.py:198.

No blocking issues found. The scope matches what's described in the PR body, and the fix doesn't touch anything beyond the single return site plus reason propagation as claimed.