fix(agent): halt on first permanent inference failure to stop agent cascade (#3104) (#3779)

Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
This commit is contained in:
oxoxDev
2026-06-22 12:16:59 -07:00
committed by GitHub
co-authored by Steven Enamakel
parent 333d1bb282
commit 9198444ffd
4 changed files with 1025 additions and 3 deletions
+67 -3
View File
@@ -824,6 +824,17 @@ pub(crate) async fn run_turn_engine(
"[agent_loop] circuit breaker tripped — halting with root cause"
);
halt_reason = Some(reason);
// Stop executing the rest of this assistant message's tool-call
// batch (#3104). Native-tool providers can emit multiple tool
// calls in one message; without this break the loop would drain
// the remaining calls — and on a permanent inference failure
// (out of budget / provider-config) that means launching the
// *next* paid sub-agent delegation right after the first one
// proved the wall is unrecoverable. Breaking here makes the
// "halt on the first occurrence" guarantee hold for batched
// calls too. The tool results recorded so far are still threaded
// into history below, so the caller keeps full context.
break;
}
// Early-exit when a sub-agent calls ask_user_clarification:
@@ -840,6 +851,55 @@ pub(crate) async fn run_turn_engine(
}
}
// A circuit-breaker / early-exit `break` can stop the batch before every
// tool call ran, so `individual_results` (one entry per EXECUTED call)
// may be shorter than `native_tool_calls` (every call the model emitted).
// The persisted assistant message must reference ONLY the executed calls:
// a native-mode assistant turn carrying N `tool_call` ids followed by
// fewer than N `role: tool` results is rejected by OpenAI-compatible
// providers ("an assistant message with tool_calls must be followed by
// tool messages responding to each tool_call_id") on the next request —
// exactly the raw `ChatMessage` histories used by run_tool_call_loop /
// the sub-agent paths (Codex review #3779). Trim the persisted tool-call
// list to the executed prefix so call-ids and tool-results stay in
// lockstep. `tool_calls` is a 1:1, same-order map of `native_tool_calls`
// (see `parse_structured_tool_calls`), so the executed prefix is simply
// the first `individual_results.len()` native calls.
let executed = individual_results.len();
let executed_native_calls = &native_tool_calls[..executed.min(native_tool_calls.len())];
// The parsed list is a 1:1, same-order map of the native list, so the
// executed prefix lines up. Trim it too: the typed-history observer
// (`turn_engine_adapter::persisted_tool_calls`) builds the `Agent::turn`
// `AssistantToolCalls` entry from these, and would otherwise persist a
// tool-call for every emitted call while only collecting results for the
// executed prefix — the same orphaned-id mismatch in the raw-ChatMessage
// path (Codex review #3779).
let executed_parsed_calls = &tool_calls[..executed.min(tool_calls.len())];
let assistant_history_content = if executed < native_tool_calls.len() {
tracing::debug!(
iteration,
emitted = native_tool_calls.len(),
executed,
"[agent_loop] batch truncated before all tool calls ran — trimming \
persisted assistant tool-calls to the executed prefix so tool_call_ids \
match tool-results (no orphaned id)"
);
// Rebuild from the executed prefix. Empty prefix (a break before the
// first call could ever produce one) degrades to the plain
// response-text assistant message, mirroring the no-tool-call path.
if executed_native_calls.is_empty() {
response_text.clone()
} else {
build_native_assistant_history(
&response_text,
reasoning_content.as_deref(),
executed_native_calls,
)
}
} else {
assistant_history_content
};
// Add assistant message with tool calls + tool results to history.
// Native mode: JSON-structured messages so convert_messages() can
// reconstruct OpenAI-format tool_calls + tool result messages. Prompt
@@ -850,8 +910,8 @@ pub(crate) async fn run_turn_engine(
&display_text,
&response_text,
reasoning_content.as_deref(),
&native_tool_calls,
&tool_calls,
executed_native_calls,
executed_parsed_calls,
iteration,
false,
)
@@ -861,7 +921,11 @@ pub(crate) async fn run_turn_engine(
observer.on_results_batch(&content, iteration);
history.push(ChatMessage::user(content));
} else {
for (native_call, result) in native_tool_calls.iter().zip(individual_results.iter()) {
// Zip over the executed prefix only — one `role: tool` result per
// executed `tool_call_id`, matching the trimmed assistant message
// above so the next provider request has no orphaned tool-call id.
for (native_call, result) in executed_native_calls.iter().zip(individual_results.iter())
{
let tool_msg = serde_json::json!({
"tool_call_id": native_call.id,
"content": result,
@@ -262,6 +262,355 @@ async fn engine_autocompacts_history_when_guard_trips() {
);
}
// -- #3104 / Codex #3779 Finding B: batch-break leaves no orphaned tool-call id -
/// Provider that emits a single assistant response carrying MULTIPLE native tool
/// calls in one message (the native-mode batch the reviewer flagged), then a
/// plain final text on any later call so the loop can terminate.
struct BatchToolCallProvider {
served: Mutex<bool>,
calls: Vec<ToolCall>,
}
impl BatchToolCallProvider {
fn new(calls: Vec<ToolCall>) -> Arc<Self> {
Arc::new(Self {
served: Mutex::new(false),
calls,
})
}
}
#[async_trait]
impl Provider for BatchToolCallProvider {
async fn chat_with_system(
&self,
_system: Option<&str>,
_message: &str,
_model: &str,
_temperature: f64,
) -> anyhow::Result<String> {
Ok("noop".into())
}
async fn chat(
&self,
_request: ChatRequest<'_>,
_model: &str,
_temperature: f64,
) -> anyhow::Result<ChatResponse> {
let mut served = self.served.lock().unwrap();
if *served {
// Loop should have already halted on the first batch; this is only a
// safety net so the engine can never block waiting for more input.
return Ok(ChatResponse {
text: Some("FINAL".into()),
tool_calls: vec![],
usage: None,
reasoning_content: None,
});
}
*served = true;
Ok(ChatResponse {
text: Some(String::new()),
tool_calls: self.calls.clone(),
usage: None,
reasoning_content: None,
})
}
fn supports_native_tools(&self) -> bool {
true
}
async fn effective_context_window(&self, _model: &str) -> Option<u64> {
None
}
}
/// Tool source whose FIRST executed call returns a terminal (budget-exhausted)
/// delegated-inference failure carrying the sub-agent dispatch wrapper, so the
/// shared `RepeatFailureGuard` halts on the first occurrence and the batch
/// breaks before the remaining call(s) run. Any later call would succeed — but
/// must never be reached.
struct FailFirstToolSource {
specs: Vec<crate::openhuman::tools::ToolSpec>,
executed: Vec<String>,
}
#[async_trait]
impl ToolSource for FailFirstToolSource {
fn request_specs(&self) -> &[crate::openhuman::tools::ToolSpec] {
&self.specs
}
async fn execute_call(
&mut self,
call: &ParsedToolCall,
_iteration: usize,
_progress: &dyn super::ProgressReporter,
_progress_call_id: &str,
) -> ToolRunResult {
self.executed.push(call.name.clone());
if self.executed.len() == 1 {
// Sub-agent dispatch wrapper (`failed and did not complete`) + a
// budget body → terminal inference failure → halt on first failure.
ToolRunResult {
text: "run_code failed and did not complete — no work was performed. \
Error: {\"error\":\"insufficient balance — add credits\"}"
.into(),
success: false,
}
} else {
ToolRunResult {
text: "ok".into(),
success: true,
}
}
}
}
#[allow(clippy::too_many_arguments)]
async fn run_with_source(
provider: &dyn Provider,
history: &mut Vec<ChatMessage>,
tool_source: &mut dyn ToolSource,
) -> TurnEngineOutcome {
let progress = NullProgress;
let mut observer = NullObserver;
let checkpoint = ErrorCheckpoint;
let parser = DefaultParser;
let multimodal = MultimodalConfig::default();
let multimodal_files = MultimodalFileConfig::default();
run_turn_engine(
provider,
history,
tool_source,
&progress,
&mut observer,
&checkpoint,
&parser,
"test-provider",
"ctx-test-model-xyz",
0.0,
true,
&multimodal,
&multimodal_files,
8,
None,
&[],
None,
None,
)
.await
.expect("turn engine should complete")
}
/// Collect the `tool_call` ids referenced by an assistant message's native-mode
/// JSON content (`{"content":…,"tool_calls":[{"id":…}, …]}`). Returns empty for
/// non-native / non-tool-call assistant messages.
fn assistant_tool_call_ids(content: &str) -> Vec<String> {
serde_json::from_str::<serde_json::Value>(content)
.ok()
.and_then(|v| {
v.get("tool_calls").and_then(|tc| tc.as_array()).map(|arr| {
arr.iter()
.filter_map(|c| c.get("id").and_then(|i| i.as_str()).map(str::to_string))
.collect()
})
})
.unwrap_or_default()
}
/// Collect the `tool_call_id`s of every `role: tool` result message in history.
fn tool_result_ids(history: &[ChatMessage]) -> Vec<String> {
history
.iter()
.filter(|m| m.role == "tool")
.filter_map(|m| {
serde_json::from_str::<serde_json::Value>(&m.content)
.ok()
.and_then(|v| {
v.get("tool_call_id")
.and_then(|i| i.as_str())
.map(str::to_string)
})
})
.collect()
}
#[tokio::test]
async fn batch_break_trims_assistant_tool_calls_to_executed_no_orphan_id() {
// The model emits THREE native tool calls in one message. The first returns a
// terminal budget failure → the shared guard halts on the first occurrence
// and the batch breaks, so calls #2 and #3 never run. The persisted assistant
// message must reference ONLY the executed call id, and there must be exactly
// one matching `role: tool` result — no orphaned tool-call id that an
// OpenAI-compatible provider would reject on the next request.
let provider = BatchToolCallProvider::new(vec![
ToolCall {
id: "call-A".into(),
name: "run_code".into(),
arguments: "{\"prompt\":\"a\"}".into(),
extra_content: None,
},
ToolCall {
id: "call-B".into(),
name: "run_code".into(),
arguments: "{\"prompt\":\"b\"}".into(),
extra_content: None,
},
ToolCall {
id: "call-C".into(),
name: "run_code".into(),
arguments: "{\"prompt\":\"c\"}".into(),
extra_content: None,
},
]);
let mut tool_source = FailFirstToolSource {
specs: Vec::new(),
executed: Vec::new(),
};
let mut history = vec![ChatMessage::system("SYSTEM"), ChatMessage::user("TASK")];
let outcome = run_with_source(provider.as_ref(), &mut history, &mut tool_source).await;
// Only the first call ran; the batch broke before #2/#3.
assert_eq!(
tool_source.executed.as_slice(),
["run_code"],
"only the first tool call should have executed before the terminal halt"
);
// The turn returned the root-cause halt summary (budget), not a generic stop.
assert!(
outcome.text.contains("out of inference budget"),
"expected the budget root-cause halt summary, got: {}",
outcome.text
);
// The persisted assistant message references ONLY the executed call id.
let assistant = history
.iter()
.find(|m| m.role == "assistant" && m.content.contains("tool_calls"))
.expect("an assistant message carrying tool_calls must be in history");
let asst_ids = assistant_tool_call_ids(&assistant.content);
assert_eq!(
asst_ids,
vec!["call-A".to_string()],
"assistant tool-call list must be trimmed to the executed prefix \
(no orphaned call-B/call-C): {asst_ids:?}"
);
// Exactly one tool result, matching that id — perfect id ↔ result pairing.
let result_ids = tool_result_ids(&history);
assert_eq!(
result_ids,
vec!["call-A".to_string()],
"exactly one tool-result, matching the single executed tool-call id: {result_ids:?}"
);
// The invariant the provider enforces: every persisted assistant tool-call id
// has a corresponding tool-result, and vice-versa (no orphans either way).
assert_eq!(
asst_ids, result_ids,
"tool-call ids and tool-result ids must be in lockstep after a batch break"
);
}
#[tokio::test]
async fn single_failing_call_pairs_id_with_result_boundary() {
// Boundary: a SINGLE native tool call that fails terminally. There is no
// un-executed suffix to trim, but the executed-prefix path must still produce
// exactly one assistant tool-call id paired with one tool-result (the
// degenerate case must not regress to zero results or an orphan).
let provider = BatchToolCallProvider::new(vec![ToolCall {
id: "only-1".into(),
name: "run_code".into(),
arguments: "{}".into(),
extra_content: None,
}]);
let mut tool_source = FailFirstToolSource {
specs: Vec::new(),
executed: Vec::new(),
};
let mut history = vec![ChatMessage::system("SYSTEM"), ChatMessage::user("TASK")];
run_with_source(provider.as_ref(), &mut history, &mut tool_source).await;
let assistant = history
.iter()
.find(|m| m.role == "assistant" && m.content.contains("tool_calls"))
.expect("assistant message with tool_calls");
assert_eq!(assistant_tool_call_ids(&assistant.content), vec!["only-1"]);
assert_eq!(tool_result_ids(&history), vec!["only-1"]);
}
#[tokio::test]
async fn full_batch_success_keeps_all_ids_paired() {
// Success path: when NO break happens (all calls run), every emitted tool-call
// id must be preserved and paired 1:1 with its result — proving the trim is
// additive (only fires on truncation) and never drops calls on the happy path.
struct AllOkToolSource {
specs: Vec<crate::openhuman::tools::ToolSpec>,
}
#[async_trait]
impl ToolSource for AllOkToolSource {
fn request_specs(&self) -> &[crate::openhuman::tools::ToolSpec] {
&self.specs
}
async fn execute_call(
&mut self,
_call: &ParsedToolCall,
_iteration: usize,
_progress: &dyn super::ProgressReporter,
_progress_call_id: &str,
) -> ToolRunResult {
ToolRunResult {
text: "ok".into(),
success: true,
}
}
}
let provider = BatchToolCallProvider::new(vec![
ToolCall {
id: "x1".into(),
name: "noop".into(),
arguments: "{}".into(),
extra_content: None,
},
ToolCall {
id: "x2".into(),
name: "noop".into(),
arguments: "{}".into(),
extra_content: None,
},
]);
let mut tool_source = AllOkToolSource { specs: Vec::new() };
let mut history = vec![ChatMessage::system("SYSTEM"), ChatMessage::user("TASK")];
run_with_source(provider.as_ref(), &mut history, &mut tool_source).await;
let assistant = history
.iter()
.find(|m| m.role == "assistant" && m.content.contains("tool_calls"))
.expect("assistant message with tool_calls");
let asst_ids = assistant_tool_call_ids(&assistant.content);
let result_ids = tool_result_ids(&history);
assert_eq!(
asst_ids,
vec!["x1".to_string(), "x2".to_string()],
"both emitted tool-call ids must be preserved on the all-success path"
);
assert_eq!(
result_ids,
vec!["x1".to_string(), "x2".to_string()],
"both tool results must be present and ordered on the all-success path"
);
assert_eq!(asst_ids, result_ids);
}
#[tokio::test]
async fn engine_does_not_autocompact_when_opted_out() {
// Same guard-tripping scenario, but `autocompact = None` (the main-agent /
+149
View File
@@ -68,6 +68,124 @@ pub(crate) fn hard_reject_kind(result: &str) -> Option<HardReject> {
}
}
/// A permanent, non-retryable inference failure surfaced by a tool result —
/// typically a delegated sub-agent (`run_code` / `tools_agent` / `plan`) whose
/// provider call hit a user-state wall. Unlike a transient error, re-issuing the
/// call cannot succeed even under a *different* delegation tool or varied args:
/// the budget is account-wide and the model/provider configuration is shared by
/// every sub-agent. See [`terminal_inference_failure_kind`].
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub(crate) enum TerminalInferenceFailure {
/// Out of inference budget / credits — every retry hits the same wall.
/// Detected via
/// [`is_budget_exhausted_message`](crate::openhuman::inference::provider::is_budget_exhausted_message).
BudgetExhausted,
/// The configured model/provider rejected the request for a reason the user
/// must fix (unknown model, non-chat/embedding model, missing credential,
/// region block, …). Detected via
/// [`is_provider_config_rejection_message`](crate::openhuman::inference::provider::is_provider_config_rejection_message).
ProviderConfig,
}
/// Inference/delegation **envelope** markers that prove a tool result came from
/// a delegated inference call (a sub-agent / provider round-trip) rather than
/// from arbitrary tool stderr.
///
/// The two provider classifiers ([`is_budget_exhausted_message`] /
/// [`is_provider_config_rejection_message`]) match on short message substrings
/// (`"insufficient balance"`, `"invalid temperature"`, `"model field is
/// required"`, …) that can legitimately appear in a *recoverable* tool's output
/// — e.g. a `shell`/`run_code` script printing `ValueError: invalid temperature`
/// or a test asserting on `"model field is required"`. Applying the terminal
/// halt to those would misreport a fixable script failure as "fix the model or
/// API key" and stop after a single attempt.
///
/// Gating on these envelope markers scopes the classifier to genuinely
/// delegated inference failures. Every marker here is **harness-generated** —
/// produced by our own reliable-chain rollup or sub-agent dispatch wrapper, NOT
/// by a provider HTTP body that arbitrary tool stderr could forge:
/// * the reliable-chain exhaustion rollup (`"All providers/models failed"` /
/// `"may not be available on your provider"`, reliable.rs), and
/// * the sub-agent dispatch wrapper (`"failed and did not complete"`, see
/// [`crate::openhuman::agent_orchestration::tools::dispatch::format_subagent_failure`]).
///
/// **Why the bare provider envelope is NOT a marker (Codex review #3779):** the
/// raw provider-HTTP shape (`"<provider> API error (…)"`, `"<provider> Responses
/// API error: …"`) is reproducible verbatim by a *recoverable* tool that is
/// debugging its own API client — e.g. a `shell`/`run_code` script printing
/// `OpenAI API error (400): invalid temperature` or `… model field is required`.
/// Matching the bare `"api error"` substring there would let that script trip
/// the broad provider-config classifier and HALT the whole turn after a single
/// failed command with a misleading "fix your model in Settings → AI" message,
/// even though the agent should just recover. Every *genuine* delegated
/// inference failure additionally surfaces through one of the harness wrappers
/// above (a sub-agent provider error reaches the orchestrator only via
/// `dispatch::format_subagent_failure`; a direct reliable-chain exhaustion via
/// the rollup), so dropping the bare provider envelope loses no real detection
/// while closing the false-positive on tool stderr.
///
/// [`is_budget_exhausted_message`]: crate::openhuman::inference::provider::is_budget_exhausted_message
/// [`is_provider_config_rejection_message`]: crate::openhuman::inference::provider::is_provider_config_rejection_message
const INFERENCE_FAILURE_ENVELOPE_MARKERS: &[&str] = &[
// Reliable-chain exhaustion rollup (reliable.rs::format_failure_aggregate).
"all providers/models failed",
"may not be available on your provider",
// Sub-agent delegation failure wrapper (dispatch.rs::format_subagent_failure).
"failed and did not complete",
];
/// True if `result` carries one of the inference/delegation envelope markers —
/// i.e. the failure demonstrably came from a delegated provider round-trip, not
/// from an arbitrary tool's stderr. See [`INFERENCE_FAILURE_ENVELOPE_MARKERS`].
fn has_inference_failure_envelope(result: &str) -> bool {
let lower = result.to_ascii_lowercase();
INFERENCE_FAILURE_ENVELOPE_MARKERS
.iter()
.any(|marker| lower.contains(marker))
}
/// Recognize a permanent (non-retryable) inference failure from a tool result.
///
/// Two-stage gate so a *recoverable* tool failure can't be misclassified:
/// 1. The result must carry a delegated-inference **envelope**
/// ([`has_inference_failure_envelope`]) — proving it came from a sub-agent
/// / provider round-trip and not from arbitrary tool stderr that merely
/// happens to contain a classifier substring (e.g. a `shell` script
/// printing `ValueError: invalid temperature` or a test asserting on
/// `"model field is required"`). Without this guard a fixable script/test
/// failure would be misreported as "fix the model or API key" and stopped
/// after a single attempt (Codex review #3779).
/// 2. The (then-trusted) body is matched against the two deliberately-tight
/// provider classifiers, which stay in lockstep with the Sentry-demotion
/// phrase sets: a transient / 5xx / generic 4xx body matches NEITHER, so
/// genuinely retryable failures still get the normal consecutive-failure
/// grace ([`NO_PROGRESS_FAILURE_THRESHOLD`]) and are never halted early.
/// Budget takes precedence if both somehow match.
///
/// The orchestrator otherwise re-emits a failed delegation under *varied* tool
/// names (Plan → `run_code` → `tools_agent`), so the identical-`(tool,args)`
/// [`REPEAT_FAILURE_THRESHOLD`] never trips and the chain grinds through ~6-8
/// doomed, paid delegations before [`NO_PROGRESS_FAILURE_THRESHOLD`] finally
/// halts with an opaque "Something went wrong" (#3104). Tripping on the FIRST
/// permanent failure stops that cascade and surfaces the root cause.
pub(crate) fn terminal_inference_failure_kind(result: &str) -> Option<TerminalInferenceFailure> {
use crate::openhuman::inference::provider::{
is_budget_exhausted_message, is_provider_config_rejection_message,
};
// Require the delegated-inference envelope first: the message-only
// classifiers are too broad to apply to arbitrary tool stderr.
if !has_inference_failure_envelope(result) {
return None;
}
if is_budget_exhausted_message(result) {
Some(TerminalInferenceFailure::BudgetExhausted)
} else if is_provider_config_rejection_message(result) {
Some(TerminalInferenceFailure::ProviderConfig)
} else {
None
}
}
/// 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
@@ -108,6 +226,37 @@ impl RepeatFailureGuard {
*c += 1;
*c
};
// Permanent inference failures (out of budget / provider-config rejection)
// cannot be recovered by retrying — the budget is account-wide and the
// model/provider config is shared by every (sub-)agent. Halt on the FIRST
// occurrence with an actionable root cause instead of letting the
// orchestrator re-emit the step under varied delegation-tool names until
// NO_PROGRESS_FAILURE_THRESHOLD (#3104: Plan → Run Code ×6 → Tools Agent
// ×2). Checked before the count-based thresholds precisely because those
// never trip in time when the tool name keeps changing.
if let Some(kind) = terminal_inference_failure_kind(result) {
tracing::warn!(
tool,
kind = ?kind,
"[agent_loop] permanent inference failure — halting on first occurrence with root cause"
);
return Some(match kind {
TerminalInferenceFailure::BudgetExhausted => format!(
"Stopping: the `{tool}` step failed because the account is out of inference \
budget/credits — every retry hits the same wall. Add credits to your account \
(or, when using a custom/BYO provider, top up that provider's own account) \
and try again. Details:\n{}",
truncate_for_halt(result),
),
TerminalInferenceFailure::ProviderConfig => format!(
"Stopping: the `{tool}` step failed because the configured model/provider \
rejected the request (e.g. an unknown model, a non-chat/embedding model, a \
missing credential, or a region block) — retrying will not help. Fix the model \
or API key in Settings → AI. Details:\n{}",
truncate_for_halt(result),
),
});
}
// Hard policy rejections trip on the first verbatim repeat; everything
// else uses the generic identical-retry threshold.
let hard = hard_reject_kind(result);
@@ -109,6 +109,62 @@ impl Tool for ErrorResultTool {
}
}
/// Simulates a delegated sub-agent (`run_code` / `tools_agent`) whose provider
/// call hit a permanent, non-retryable wall: `dispatch_subagent` converts that
/// sub-agent `Err` into a `ToolResult::error` carrying the budget-exhaustion
/// body. Used to prove the #3104 cascade halts on the FIRST such failure.
struct BudgetExhaustedDelegationTool;
#[async_trait]
impl Tool for BudgetExhaustedDelegationTool {
fn name(&self) -> &str {
"run_code"
}
fn description(&self) -> &str {
"delegate to the code executor (always 400s on budget here)"
}
fn parameters_schema(&self) -> serde_json::Value {
serde_json::json!({"type":"object"})
}
async fn execute(&self, _args: serde_json::Value) -> Result<ToolResult> {
// Mirrors dispatch.rs::format_subagent_failure wrapping the upstream body.
Ok(ToolResult::error(
"run_code failed and did not complete — no work was performed. \
Error: OpenHuman API error (400): {\"error\":\"Insufficient budget\"}",
))
}
}
/// Records whether it ever executed. Used to prove that a tool placed AFTER a
/// terminal-inference failure in the SAME assistant-message batch never runs
/// (#3104 — Codex review #3779, batched tool calls).
struct RanTrackerTool {
ran: std::sync::Arc<std::sync::atomic::AtomicBool>,
}
#[async_trait]
impl Tool for RanTrackerTool {
fn name(&self) -> &str {
"ran_tracker"
}
fn description(&self) -> &str {
"records that it executed"
}
fn parameters_schema(&self) -> serde_json::Value {
serde_json::json!({"type":"object"})
}
async fn execute(&self, _args: serde_json::Value) -> Result<ToolResult> {
self.ran.store(true, std::sync::atomic::Ordering::SeqCst);
Ok(ToolResult::success("ran-tracker-out"))
}
}
struct FailingTool;
#[async_trait]
@@ -1071,6 +1127,148 @@ async fn run_tool_call_loop_halts_when_no_progress() {
);
}
/// #3104 end-to-end repro through the real engine: a delegation that fails with
/// a PERMANENT inference condition (out of budget) must halt the orchestrator on
/// the FIRST failure with an actionable root cause — NOT grind to the
/// 6-consecutive no-progress backstop (the Plan → Run Code ×6 → Tools Agent ×2
/// cascade the user saw). Before the fix this loop consumed 6 LLM turns and
/// ended in a generic message; after it, exactly 1.
#[tokio::test]
async fn run_tool_call_loop_halts_on_first_budget_exhausted_delegation() {
let mut responses = Vec::new();
for i in 0..10 {
// Varied args so the per-signature repeat guard can NEVER trip — only the
// terminal-inference check (or, pre-fix, the 6-consecutive backstop) can.
responses.push(Ok(ChatResponse {
text: Some(format!(
"<tool_call>{{\"name\":\"run_code\",\"arguments\":{{\"step\":{i}}}}}</tool_call>"
)),
tool_calls: vec![],
usage: None,
reasoning_content: None,
}));
}
let provider = ScriptedProvider {
responses: Mutex::new(responses),
native_tools: false,
vision: false,
};
let mut history = vec![ChatMessage::user("build me a dashboard")];
let tools: Vec<Box<dyn Tool>> = vec![Box::new(BudgetExhaustedDelegationTool)];
let result = run_tool_call_loop(
&provider,
&mut history,
&tools,
"test-provider",
"model",
0.0,
true,
"channel",
&crate::openhuman::config::MultimodalConfig::default(),
&crate::openhuman::config::MultimodalFileConfig::default(),
10, // max_iterations — must NOT be reached; terminal halt fires at 1
None,
None,
&[],
None,
None,
&crate::openhuman::tools::policy::DefaultToolPolicy,
)
.await
.expect("budget-exhausted halt returns Ok with an actionable summary");
assert!(
result.contains("Stopping") && result.contains("out of inference budget"),
"expected an actionable budget halt, got: {result}"
);
// The decisive assertion: only the FIRST scripted turn was consumed (9 of 10
// remain). Proves the cascade is killed on the first permanent failure
// instead of burning 6 doomed, paid delegations.
assert_eq!(
provider.responses.lock().len(),
9,
"loop must halt after exactly 1 turn on a permanent inference failure"
);
}
/// #3104 batched-tool-call guarantee (Codex review #3779): when a single
/// assistant message emits MULTIPLE tool calls and the FIRST records a terminal
/// inference failure (out of budget / provider-config), the loop must STOP
/// executing the rest of the batch — so a second delegated call in the same
/// message can never launch a paid sub-agent after the first proved the wall is
/// unrecoverable. Pre-fix the loop set `halt_reason` but drained the batch first.
#[tokio::test]
async fn run_tool_call_loop_stops_batch_after_first_terminal_failure() {
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;
// One assistant message with TWO tool calls: the budget-exhausted delegation
// first, a tracker second. Native mode emits both as `tool_calls` in a single
// ChatResponse so they share one batch iteration.
let provider = ScriptedProvider {
responses: Mutex::new(vec![Ok(ChatResponse {
text: Some(String::new()),
tool_calls: vec![
crate::openhuman::inference::provider::ToolCall {
id: "call-budget".into(),
name: "run_code".into(),
arguments: "{}".into(),
extra_content: None,
},
crate::openhuman::inference::provider::ToolCall {
id: "call-tracker".into(),
name: "ran_tracker".into(),
arguments: "{}".into(),
extra_content: None,
},
],
usage: None,
reasoning_content: None,
})]),
native_tools: true,
vision: false,
};
let mut history = vec![ChatMessage::user("build me a dashboard")];
let ran = Arc::new(AtomicBool::new(false));
let tools: Vec<Box<dyn Tool>> = vec![
Box::new(BudgetExhaustedDelegationTool),
Box::new(RanTrackerTool { ran: ran.clone() }),
];
let result = run_tool_call_loop(
&provider,
&mut history,
&tools,
"test-provider",
"model",
0.0,
true,
"channel",
&crate::openhuman::config::MultimodalConfig::default(),
&crate::openhuman::config::MultimodalFileConfig::default(),
5,
None,
None,
&[],
None,
None,
&crate::openhuman::tools::policy::DefaultToolPolicy,
)
.await
.expect("budget-exhausted halt returns Ok with an actionable summary");
assert!(
result.contains("Stopping") && result.contains("out of inference budget"),
"expected an actionable budget halt, got: {result}"
);
// The decisive assertion: the SECOND call in the batch must NOT have run.
assert!(
!ran.load(Ordering::SeqCst),
"a tool placed after a terminal inference failure in the same batch must NOT execute"
);
}
// -- RepeatFailureGuard (shared by run_tool_call_loop + run_inner_loop) --------
#[test]
@@ -1197,6 +1395,268 @@ fn hard_reject_distinct_args_do_not_trip_repeat() {
);
}
// -- Terminal inference failures (#3104: budget / provider-config cascade) -------
#[test]
fn terminal_inference_failure_kind_classifies_budget_and_config() {
// Budget wins precedence; provider-config covers the user-state model/key
// rejections; transient / 5xx / generic-4xx match NEITHER (so retryable
// failures keep their normal consecutive-failure grace).
assert_eq!(
terminal_inference_failure_kind(
"run_code failed and did not complete. Error: OpenHuman API error (400): \
{\"error\":\"insufficient budget — add credits\"}"
),
Some(TerminalInferenceFailure::BudgetExhausted)
);
assert_eq!(
terminal_inference_failure_kind(
"tools_agent failed and did not complete. Error: ollama API error (400 Bad Request): \
{\"error\":{\"message\":\"\\\"bge-m3:latest\\\" does not support chat\"}}"
),
Some(TerminalInferenceFailure::ProviderConfig)
);
for transient in [
"Error: connection reset by peer",
"Error: 503 Service Unavailable",
"Error: rate limit exceeded, retry after 1s",
"run_code failed and did not complete. Error: timed out",
] {
assert_eq!(
terminal_inference_failure_kind(transient),
None,
"{transient:?} must NOT classify as a terminal inference failure"
);
}
}
#[test]
fn terminal_inference_failure_requires_delegated_inference_envelope() {
// Codex review #3779: the message-only provider classifiers match short
// substrings (`invalid temperature`, `model field is required`,
// `insufficient balance`, …) that can legitimately appear in a RECOVERABLE
// tool's stderr. Those must NOT trip the terminal halt — only a result that
// carries a delegated-inference/provider envelope may.
// 1) Recoverable `shell`/`run_code` SCRIPT stderr that merely *contains* a
// classifier substring — no provider envelope → must NOT classify, so the
// normal consecutive-failure grace still applies and the script can be
// retried/fixed.
for recoverable in [
// A Python test raising on a `temperature` arg — not an inference call.
"ValueError: invalid temperature: only 1 is allowed for this model",
// A user script validating config and printing the provider-ish phrase.
"AssertionError: model field is required",
// A finance script echoing an account-balance string.
"RuntimeError: insufficient balance in wallet 0xabc",
// A test asserting on the literal remediation copy.
"FAILED test_models.py::test_unknown - assert 'model_not_found' in resp",
] {
assert_eq!(
terminal_inference_failure_kind(recoverable),
None,
"recoverable tool stderr without an inference envelope must NOT classify \
as a terminal inference failure: {recoverable:?}"
);
}
// 2) The SAME phrases, but now wrapped in a genuine delegated-inference
// envelope (provider HTTP error / reliable rollup / sub-agent dispatch
// wrapper) → MUST classify, because these only come from a delegated
// provider round-trip.
assert_eq!(
terminal_inference_failure_kind(
"run_code failed and did not complete. Error: custom_openai API error \
(400 Bad Request): {\"error\":{\"message\":\"invalid temperature: only 1 is \
allowed for this model\"}}"
),
Some(TerminalInferenceFailure::ProviderConfig),
"a delegated provider config-rejection (with envelope) must classify"
);
assert_eq!(
terminal_inference_failure_kind(
"tools_agent failed and did not complete. Error: OpenHuman API error (402): \
{\"error\":\"Insufficient balance\"}"
),
Some(TerminalInferenceFailure::BudgetExhausted),
"a delegated budget-exhaustion (with envelope) must classify"
);
// The reliable-chain rollup envelope (no `API error` token) also qualifies.
assert_eq!(
terminal_inference_failure_kind(
"All providers/models failed. Attempts:\n provider=custom_openai model=gpt-5.5 \
attempt 1/3: non_retryable; error=insufficient balance"
),
Some(TerminalInferenceFailure::BudgetExhausted),
"a reliable-chain exhaustion rollup carrying a budget body must classify"
);
}
#[test]
fn bare_provider_api_error_in_tool_stderr_does_not_classify() {
// Codex review #3779 (follow-up): a recoverable `shell`/`run_code` task that
// is debugging its OWN API client can print the verbatim provider-HTTP
// envelope shape (`<provider> API error (status): …`). Without a
// harness-generated wrapper (`failed and did not complete` /
// `all providers/models failed` / `may not be available on your provider`)
// that output must NOT be treated as a delegated inference failure — else the
// whole turn halts after a single failed command with a misleading "fix your
// model in Settings → AI" message.
for tool_stderr in [
// The exact shape called out in the review: a config-rejection body that
// a debugging script could print to stdout/stderr.
"OpenAI API error (400): invalid temperature",
"OpenAI API error (400): model field is required",
// Budget-flavoured body, same forged-envelope risk.
"custom_openai API error (402 Payment Required): {\"error\":\"Insufficient balance\"}",
// Responses/streaming envelope variants, still bare (no harness wrapper).
"custom_openai Responses API error: {\"error\":{\"message\":\"model 'x' not found\"}}",
"custom_openai streaming API error (404 Not Found): model 'llama3.3' not found",
] {
assert_eq!(
terminal_inference_failure_kind(tool_stderr),
None,
"a bare provider-HTTP envelope without a harness wrapper must NOT \
classify as a terminal inference failure: {tool_stderr:?}"
);
}
}
#[test]
fn bare_provider_api_error_keeps_consecutive_failure_grace() {
// The behavioural consequence of the above at the guard level: a `run_code`
// task printing a bare provider API-error body on each attempt must keep its
// normal consecutive-failure grace (it is recoverable user code, not a
// delegated wall), and only trip the 6-in-a-row no-progress backstop — never
// halt on the FIRST failure.
let mut g = RepeatFailureGuard::new();
let bare = "OpenAI API error (400): invalid temperature";
for i in 0..(NO_PROGRESS_FAILURE_THRESHOLD - 1) {
assert!(
g.record("run_code", &format!("debug-attempt{i}"), false, bare)
.is_none(),
"a bare provider API-error in tool stderr must NOT halt on failure #{}",
i + 1
);
}
// The Nth consecutive failure trips the generic no-progress backstop, as for
// any other recoverable failure — proving the grace was preserved, not that
// the terminal classifier fired.
let halt = g.record(
"run_code",
&format!("debug-attempt{}", NO_PROGRESS_FAILURE_THRESHOLD - 1),
false,
bare,
);
let msg = halt.expect("the consecutive no-progress backstop still trips");
assert!(
msg.contains("in a row failed with no progress"),
"must halt via the generic no-progress path, not the terminal-inference \
path: {msg}"
);
assert!(
!msg.contains("Settings → AI") && !msg.contains("out of inference budget"),
"must NOT use the terminal-inference remediation copy: {msg}"
);
}
#[test]
fn delegated_provider_failure_still_halts_first_after_narrowing() {
// Boundary guard for the #3779 narrowing: a GENUINE delegated provider
// failure — the same config-rejection body, but wrapped by the sub-agent
// dispatch (`failed and did not complete`) — MUST still halt on the first
// occurrence. Narrowing the envelope must not regress the #3104 fix.
let mut g = RepeatFailureGuard::new();
let delegated = "run_code failed and did not complete — no work was performed. \
Error: OpenAI API error (400): invalid temperature";
let halt = g.record("run_code", "{\"prompt\":\"x\"}", false, delegated);
let msg = halt.expect("a wrapped delegated provider-config rejection must halt first");
assert!(msg.contains("rejected the request"), "got: {msg}");
assert!(
msg.contains("Settings → AI"),
"actionable remediation: {msg}"
);
}
#[test]
fn terminal_budget_failure_halts_on_first_occurrence() {
let mut g = RepeatFailureGuard::new();
let budget = "run_code failed and did not complete — no work was performed. \
Error: OpenHuman API error (400): {\"error\":\"Insufficient budget\"}";
let halt = g.record("run_code", "{\"prompt\":\"write a file\"}", false, budget);
assert!(
halt.is_some(),
"a budget-exhausted delegation must halt on the FIRST failure, not grind to 6"
);
let msg = halt.unwrap();
assert!(msg.contains("out of inference budget"), "got: {msg}");
assert!(msg.contains("`run_code`"), "names the failing step: {msg}");
}
#[test]
fn terminal_config_rejection_halts_on_first_occurrence() {
let mut g = RepeatFailureGuard::new();
let cfg = "tools_agent failed and did not complete. Error: OpenHuman API error \
(400 Bad Request): Model 'gpt-5.5' is not available. Use GET \
/openai/v1/models to list available models.";
let halt = g.record("tools_agent", "{\"prompt\":\"do the thing\"}", false, cfg);
assert!(
halt.is_some(),
"a provider-config rejection must halt on the FIRST failure"
);
let msg = halt.unwrap();
assert!(msg.contains("rejected the request"), "got: {msg}");
assert!(
msg.contains("Settings → AI"),
"actionable remediation: {msg}"
);
}
#[test]
fn terminal_failure_halts_first_even_across_varied_delegation_tools() {
// The #3104 cascade reproduction at the guard level: the orchestrator
// re-emits a doomed step under VARIED tool names (plan → run_code →
// tools_agent), so neither the identical-(tool,args) repeat guard nor the
// 6-consecutive no-progress guard would catch it in time. The terminal
// classifier must halt on the very first permanent failure regardless of
// which delegation tool surfaced it.
let mut g = RepeatFailureGuard::new();
// Carries the sub-agent dispatch envelope (`failed and did not complete`)
// so it is recognised as a *delegated* inference failure, not arbitrary
// tool stderr — see `has_inference_failure_envelope`.
let budget = "plan failed and did not complete. Error: {\"error\":\"insufficient balance\"}";
let halt = g.record("plan", "{\"goal\":\"x\"}", false, budget);
assert!(
halt.is_some(),
"first permanent failure under ANY delegation tool must halt immediately"
);
}
#[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.
let mut g = RepeatFailureGuard::new();
for i in 0..5 {
assert!(
g.record(
"run_code",
&format!("attempt{i}"),
false,
"Error: timed out"
)
.is_none(),
"transient failures must NOT halt before the 6-consecutive threshold"
);
}
assert!(
g.record("run_code", "attempt5", false, "Error: timed out")
.is_some(),
"6 consecutive transient failures still trip the no-progress guard"
);
}
/// Provider that records the tool-spec names of every `chat()` request
/// it sees, then returns the next scripted response.
struct CapturingProvider {