From 20d4b495b9b89ba605d4ea5d7e842fe89bead655 Mon Sep 17 00:00:00 2001 From: Steven Enamakel <31011319+senamakel@users.noreply.github.com> Date: Tue, 23 Jun 2026 21:54:25 -0700 Subject: [PATCH] fix(agent): relax recoverable tool-loop breaker (#4027) --- src/openhuman/agent/harness/tool_loop.rs | 96 +++++++++++- .../agent/harness/tool_loop_tests.rs | 137 +++++++++++++++++- 2 files changed, 217 insertions(+), 16 deletions(-) diff --git a/src/openhuman/agent/harness/tool_loop.rs b/src/openhuman/agent/harness/tool_loop.rs index 9a0f264f9..dfbc74e72 100644 --- a/src/openhuman/agent/harness/tool_loop.rs +++ b/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 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, consecutive: u32, + consecutive_recoverable: u32, } impl RepeatFailureGuard { @@ -215,9 +260,9 @@ impl RepeatFailureGuard { ) -> Option { 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 \ diff --git a/src/openhuman/agent/harness/tool_loop_tests.rs b/src/openhuman/agent/harness/tool_loop_tests.rs index 60a1b8bca..57a6b632f 100644 --- a/src/openhuman/agent/harness/tool_loop_tests.rs +++ b/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