fix(agent): relax recoverable tool-loop breaker (#4027)

src/openhuman/agent/harness/tool_loop.rs

@@ -30,9 +30,18 @@ pub(crate) const EXTENDED_MAX_TOOL_ITERATIONS: usize = 50;
 /// If the SAME `(tool, args)` call fails this many times, the agent is repeating a
 /// known-failed action verbatim — stop.
 pub(crate) const REPEAT_FAILURE_THRESHOLD: u32 = 3;
-/// If this many tool calls fail back-to-back with no success in between (even with
-/// varied args), the agent is making no progress — stop.
+/// Recoverable/transient failures (timeouts, connection resets, rate limits, ...)
+/// are still bounded, but need more room than deterministic terminal failures so
+/// the model can adapt (change timeout, narrow work, split a command, retry a
+/// flaky network call) before the breaker stops the turn.
+pub(crate) const RECOVERABLE_REPEAT_FAILURE_THRESHOLD: u32 = 8;
+/// If this many non-recoverable tool calls fail back-to-back with no success in
+/// between (even with varied args), the agent is making no progress — stop.
 pub(crate) const NO_PROGRESS_FAILURE_THRESHOLD: u32 = 6;
+/// Recoverable failures get a separate, larger no-progress headroom. The
+/// iteration cap and cost budget still bound the turn, while a handful of
+/// timeouts no longer stops an otherwise adaptable agent.
+pub(crate) const RECOVERABLE_NO_PROGRESS_FAILURE_THRESHOLD: u32 = 12;
 /// Hard policy rejections (a security block or a gate denial) are deterministic:
 /// the identical `(tool, args)` call provably cannot succeed. Halt on the FIRST
 /// verbatim repeat — i.e. the second identical attempt — rather than letting the
@@ -186,6 +195,41 @@ pub(crate) fn terminal_inference_failure_kind(result: &str) -> Option<TerminalIn
     }
 }
 
+/// Failures that are informative and plausibly recoverable by changing the next
+/// action (longer timeout, smaller batch, different network retry/fallback)
+/// rather than by immediately abandoning the turn.
+///
+/// Keep this deliberately marker-based and conservative: it only controls
+/// breaker headroom, never converts a failure into success. Hard policy rejects
+/// and permanent provider/account failures are classified before this function.
+pub(crate) fn is_recoverable_tool_failure(result: &str) -> bool {
+    let lower = result.to_ascii_lowercase();
+    [
+        "timed out",
+        "timeout",
+        "deadline exceeded",
+        "temporarily unavailable",
+        "temporary failure",
+        "connection reset",
+        "connection refused",
+        "connection closed",
+        "connection aborted",
+        "network is unreachable",
+        "host is unreachable",
+        "dns error",
+        "failed to lookup address",
+        "failed to resolve",
+        "rate limit",
+        "too many requests",
+        "retry after",
+        "503 service unavailable",
+        "502 bad gateway",
+        "504 gateway timeout",
+    ]
+    .iter()
+    .any(|marker| lower.contains(marker))
+}
+
 /// Shared repeated-failure circuit breaker, used by BOTH agent loops
 /// (`run_tool_call_loop` here and `run_inner_loop` in `subagent_runner`) so they
 /// can't drift. Tracks per-`(tool,args)`-signature failure counts and a
@@ -195,6 +239,7 @@ pub(crate) fn terminal_inference_failure_kind(result: &str) -> Option<TerminalIn
 pub(crate) struct RepeatFailureGuard {
     sig_counts: std::collections::HashMap<String, u32>,
     consecutive: u32,
+    consecutive_recoverable: u32,
 }
 
 impl RepeatFailureGuard {
@@ -215,9 +260,9 @@ impl RepeatFailureGuard {
     ) -> Option<String> {
         if success {
             self.consecutive = 0;
+            self.consecutive_recoverable = 0;
             return None;
         }
-        self.consecutive += 1;
         let count = {
             let c = self
                 .sig_counts
@@ -257,11 +302,27 @@ impl RepeatFailureGuard {
                 ),
             });
         }
-        // Hard policy rejections trip on the first verbatim repeat; everything
-        // else uses the generic identical-retry threshold.
+        // Hard policy rejections trip on the first verbatim repeat; recoverable
+        // failures get extra headroom; everything else uses the generic
+        // identical-retry threshold.
         let hard = hard_reject_kind(result);
+        let recoverable = hard.is_none() && is_recoverable_tool_failure(result);
+        if recoverable {
+            self.consecutive_recoverable += 1;
+            tracing::debug!(
+                tool,
+                count,
+                consecutive_recoverable = self.consecutive_recoverable,
+                "[agent_loop] recoverable tool failure recorded with extended circuit-breaker headroom"
+            );
+        } else {
+            self.consecutive += 1;
+            self.consecutive_recoverable = 0;
+        }
         let repeat_threshold = if hard.is_some() {
             HARD_REJECT_REPEAT_THRESHOLD
+        } else if recoverable {
+            RECOVERABLE_REPEAT_FAILURE_THRESHOLD
         } else {
             REPEAT_FAILURE_THRESHOLD
         };
@@ -283,13 +344,32 @@ impl RepeatFailureGuard {
                 None => format!(
                     "Stopping: the `{tool}` call was retried {count} times with identical \
                      arguments and kept failing — repeating it will not help. Last error:\n{}\n\n\
-                     This looks unrecoverable in the current environment (e.g. a missing \
-                     tool/dependency that cannot be installed here). Report this back instead of \
-                     retrying.",
+                     {} Report this back instead of retrying.",
                     truncate_for_halt(result),
+                    if recoverable {
+                        "This looked recoverable at first, but the same call exhausted the \
+                         extended transient-failure headroom."
+                    } else {
+                        "This looks unrecoverable in the current environment (e.g. a missing \
+                         tool/dependency that cannot be installed here)."
+                    },
                 ),
             });
         }
+        if recoverable {
+            if self.consecutive_recoverable >= RECOVERABLE_NO_PROGRESS_FAILURE_THRESHOLD {
+                return Some(format!(
+                    "Stopping: {} recoverable-looking tool failures happened in a row with no \
+                     successful progress. Last error (from `{tool}`):\n{}\n\nThe turn is still \
+                     bounded by the iteration/cost limits, but this many consecutive transient \
+                     failures means the goal is not currently reachable. Report this back instead \
+                     of retrying.",
+                    self.consecutive_recoverable,
+                    truncate_for_halt(result),
+                ));
+            }
+            return None;
+        }
         if self.consecutive >= NO_PROGRESS_FAILURE_THRESHOLD {
             return Some(format!(
                 "Stopping: {} tool calls in a row failed with no progress. Last error (from \

src/openhuman/agent/harness/tool_loop_tests.rs

@@ -1303,6 +1303,125 @@ fn repeat_failure_guard_halts_on_6_consecutive_varied() {
     );
 }
 
+#[test]
+fn recoverable_failure_classifier_recognizes_timeouts_and_transients() {
+    for recoverable in [
+        "Error: tool 'shell' timed out after 60 seconds",
+        "Command timed out after 60s and was killed",
+        "deadline exceeded while fetching",
+        "connection reset by peer",
+        "503 Service Unavailable",
+        "rate limit exceeded; retry after 1s",
+    ] {
+        assert!(
+            is_recoverable_tool_failure(recoverable),
+            "expected recoverable marker in {recoverable:?}"
+        );
+    }
+
+    for terminal in [
+        "externally-managed-environment",
+        "No such file or directory",
+        "permission denied",
+        "syntax error near unexpected token",
+    ] {
+        assert!(
+            !is_recoverable_tool_failure(terminal),
+            "non-transient failures should keep the generic breaker path: {terminal:?}"
+        );
+    }
+}
+
+#[test]
+fn recoverable_identical_failures_get_extended_headroom() {
+    let mut g = RepeatFailureGuard::new();
+    let timeout = "Error: tool 'shell' timed out after 60 seconds";
+
+    for i in 0..(REPEAT_FAILURE_THRESHOLD + 2) {
+        assert!(
+            g.record("shell", "python solve.py", false, timeout)
+                .is_none(),
+            "recoverable identical timeout should not halt at generic failure count {}",
+            i + 1
+        );
+    }
+
+    for i in (REPEAT_FAILURE_THRESHOLD + 2)..(RECOVERABLE_REPEAT_FAILURE_THRESHOLD - 1) {
+        assert!(
+            g.record("shell", "python solve.py", false, timeout)
+                .is_none(),
+            "recoverable identical timeout should keep headroom until count {}",
+            i + 1
+        );
+    }
+
+    let halt = g.record("shell", "python solve.py", false, timeout);
+    let msg = halt.expect("identical recoverable failures still eventually trip");
+    assert!(
+        msg.contains(&format!(
+            "retried {} times",
+            RECOVERABLE_REPEAT_FAILURE_THRESHOLD
+        )),
+        "got: {msg}"
+    );
+    assert!(
+        msg.contains("extended transient-failure headroom"),
+        "recoverable halt should explain why it waited longer: {msg}"
+    );
+}
+
+#[test]
+fn recoverable_varied_failures_do_not_trip_generic_no_progress() {
+    let mut g = RepeatFailureGuard::new();
+    let timeout = "Error: tool 'shell' timed out after 60 seconds";
+
+    for i in 0..(NO_PROGRESS_FAILURE_THRESHOLD + 2) {
+        assert!(
+            g.record(
+                "shell",
+                &format!("python solve.py --attempt={i}"),
+                false,
+                timeout
+            )
+            .is_none(),
+            "varied recoverable failures should not halt at generic no-progress count {}",
+            i + 1
+        );
+    }
+
+    for i in (NO_PROGRESS_FAILURE_THRESHOLD + 2)..(RECOVERABLE_NO_PROGRESS_FAILURE_THRESHOLD - 1) {
+        assert!(
+            g.record(
+                "shell",
+                &format!("python solve.py --attempt={i}"),
+                false,
+                timeout
+            )
+            .is_none(),
+            "varied recoverable failures should keep headroom until count {}",
+            i + 1
+        );
+    }
+
+    let halt = g.record(
+        "shell",
+        &format!(
+            "python solve.py --attempt={}",
+            RECOVERABLE_NO_PROGRESS_FAILURE_THRESHOLD - 1
+        ),
+        false,
+        timeout,
+    );
+    let msg = halt.expect("recoverable no-progress failures remain bounded");
+    assert!(
+        msg.contains(&format!(
+            "{} recoverable-looking tool failures",
+            RECOVERABLE_NO_PROGRESS_FAILURE_THRESHOLD
+        )),
+        "got: {msg}"
+    );
+}
+
 #[test]
 fn repeat_failure_guard_success_resets_consecutive() {
     let mut g = RepeatFailureGuard::new();
@@ -1635,10 +1754,10 @@ fn terminal_failure_halts_first_even_across_varied_delegation_tools() {
 #[test]
 fn terminal_classifier_does_not_short_circuit_transient_grace() {
     // A genuinely transient (retryable) failure must still flow through the
-    // existing no-progress backstop — proving the terminal halt is additive,
-    // not a blanket "halt on first failure" regression.
+    // recoverable-failure headroom — proving the terminal halt is additive, not
+    // a blanket "halt on first failure" regression.
     let mut g = RepeatFailureGuard::new();
-    for i in 0..5 {
+    for i in 0..(RECOVERABLE_NO_PROGRESS_FAILURE_THRESHOLD - 1) {
         assert!(
             g.record(
                 "run_code",
@@ -1647,14 +1766,16 @@ fn terminal_classifier_does_not_short_circuit_transient_grace() {
                 "Error: timed out"
             )
             .is_none(),
-            "transient failures must NOT halt before the 6-consecutive threshold"
+            "transient failures must NOT halt before the recoverable no-progress threshold"
         );
     }
-    assert!(
-        g.record("run_code", "attempt5", false, "Error: timed out")
-            .is_some(),
-        "6 consecutive transient failures still trip the no-progress guard"
+    let halt = g.record(
+        "run_code",
+        &format!("attempt{}", RECOVERABLE_NO_PROGRESS_FAILURE_THRESHOLD - 1),
+        false,
+        "Error: timed out",
     );
+    assert!(halt.is_some(), "recoverable failures still remain bounded");
 }
 
 /// Provider that records the tool-spec names of every `chat()` request