mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(agent): resumable checkpoint at tool-call cap + tolerant tool parsing/resolution (#2683)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
sanil-23
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
de7d8bb0ec
commit
6736467ba3
@@ -26,7 +26,49 @@ pub(crate) fn parse_arguments_value(raw: Option<&serde_json::Value>) -> serde_js
|
||||
}
|
||||
}
|
||||
|
||||
/// Object keys that may carry the tool **arguments**, in priority order.
|
||||
/// Models drift from the canonical `arguments` to `args`/`parameters`/etc.;
|
||||
/// accepting these recovers an otherwise well-formed call (with a correct
|
||||
/// `name`) instead of dropping it and burning an agent iteration
|
||||
/// (bug-report-2026-05-26 A3). The tool **name** is deliberately left
|
||||
/// strict — widening it would risk misreading a plain JSON answer as a
|
||||
/// tool call in the whole-response parse path.
|
||||
const TOOL_ARG_KEYS: &[&str] = &["arguments", "args", "parameters", "params", "input"];
|
||||
|
||||
/// Normalized arguments for the first present key among [`TOOL_ARG_KEYS`]
|
||||
/// (via [`parse_arguments_value`], which tolerates both stringified and
|
||||
/// object JSON). Empty-object default when none are present.
|
||||
fn first_args_by_keys(obj: &serde_json::Value) -> serde_json::Value {
|
||||
for key in TOOL_ARG_KEYS {
|
||||
if let Some(v) = obj.get(*key) {
|
||||
return parse_arguments_value(Some(v));
|
||||
}
|
||||
}
|
||||
parse_arguments_value(None)
|
||||
}
|
||||
|
||||
pub(crate) fn parse_tool_call_value(value: &serde_json::Value) -> Option<ParsedToolCall> {
|
||||
// Default to the permissive (tagged) behaviour: callers that reach a
|
||||
// value through an explicit tool-call marker (`tool_calls` array,
|
||||
// `<tool_call>` tags, ```tool_call blocks) accept the arg-key aliases.
|
||||
parse_tool_call_value_aliased(value, true)
|
||||
}
|
||||
|
||||
/// Parse a single JSON value as a tool call.
|
||||
///
|
||||
/// `allow_arg_aliases` controls whether the generic argument-key aliases in
|
||||
/// [`TOOL_ARG_KEYS`] (notably the very generic `input`) are honoured for a
|
||||
/// **bare** `{ "name": .., .. }` object. The whole-response fallback path
|
||||
/// (`parse_tool_calls` on a top-level JSON object) passes `false`: there, a
|
||||
/// normal model reply such as `{"name":"Alice","input":"hi"}` must not have
|
||||
/// its `input` slurped into tool arguments and routed to execution
|
||||
/// (bug-report-2026-05-26 A3 follow-up). The `function`-wrapped shape stays
|
||||
/// permissive regardless — the `function` key is an unambiguous tool-call
|
||||
/// marker.
|
||||
fn parse_tool_call_value_aliased(
|
||||
value: &serde_json::Value,
|
||||
allow_arg_aliases: bool,
|
||||
) -> Option<ParsedToolCall> {
|
||||
if let Some(function) = value.get("function") {
|
||||
let name = function
|
||||
.get("name")
|
||||
@@ -35,7 +77,7 @@ pub(crate) fn parse_tool_call_value(value: &serde_json::Value) -> Option<ParsedT
|
||||
.trim()
|
||||
.to_string();
|
||||
if !name.is_empty() {
|
||||
let arguments = parse_arguments_value(function.get("arguments"));
|
||||
let arguments = first_args_by_keys(function);
|
||||
return Some(ParsedToolCall {
|
||||
name,
|
||||
arguments,
|
||||
@@ -55,7 +97,22 @@ pub(crate) fn parse_tool_call_value(value: &serde_json::Value) -> Option<ParsedT
|
||||
return None;
|
||||
}
|
||||
|
||||
let arguments = parse_arguments_value(value.get("arguments"));
|
||||
let arguments = if allow_arg_aliases {
|
||||
first_args_by_keys(value)
|
||||
} else {
|
||||
// Whole-response bare-object fallback: require the canonical
|
||||
// `arguments` key as an explicit tool-call marker. A plain JSON reply
|
||||
// that merely carries a `name` (e.g. {"name":"Alice","input":…}) must
|
||||
// stay plain text, not be dispatched as a tool call just because its
|
||||
// name happens to match a registered tool (CodeRabbit, #2683). Tagged
|
||||
// contexts (`<tool_call>`/`<invoke>`, `tool_calls` array, `function`
|
||||
// wrapper) reach this fn with `allow_arg_aliases = true` and keep the
|
||||
// permissive behaviour.
|
||||
match value.get("arguments") {
|
||||
Some(args) => parse_arguments_value(Some(args)),
|
||||
None => return None,
|
||||
}
|
||||
};
|
||||
Some(ParsedToolCall {
|
||||
name,
|
||||
arguments,
|
||||
@@ -64,11 +121,25 @@ pub(crate) fn parse_tool_call_value(value: &serde_json::Value) -> Option<ParsedT
|
||||
}
|
||||
|
||||
pub(crate) fn parse_tool_calls_from_json_value(value: &serde_json::Value) -> Vec<ParsedToolCall> {
|
||||
// Tagged contexts (callers reach here via an explicit tool-call marker)
|
||||
// accept the argument-key aliases.
|
||||
parse_tool_calls_from_json_value_aliased(value, true)
|
||||
}
|
||||
|
||||
/// Like [`parse_tool_calls_from_json_value`], but lets the caller forbid
|
||||
/// generic arg-key aliases on a **bare** singleton/array object. The
|
||||
/// `tool_calls`-keyed envelope always stays permissive — that key is an
|
||||
/// unambiguous tool-call marker even on the whole-response path.
|
||||
pub(crate) fn parse_tool_calls_from_json_value_aliased(
|
||||
value: &serde_json::Value,
|
||||
allow_arg_aliases: bool,
|
||||
) -> Vec<ParsedToolCall> {
|
||||
let mut calls = Vec::new();
|
||||
|
||||
if let Some(tool_calls) = value.get("tool_calls").and_then(|v| v.as_array()) {
|
||||
for call in tool_calls {
|
||||
if let Some(parsed) = parse_tool_call_value(call) {
|
||||
// `tool_calls` entries are explicitly tool-call shaped → widen.
|
||||
if let Some(parsed) = parse_tool_call_value_aliased(call, true) {
|
||||
calls.push(parsed);
|
||||
}
|
||||
}
|
||||
@@ -80,14 +151,14 @@ pub(crate) fn parse_tool_calls_from_json_value(value: &serde_json::Value) -> Vec
|
||||
|
||||
if let Some(array) = value.as_array() {
|
||||
for item in array {
|
||||
if let Some(parsed) = parse_tool_call_value(item) {
|
||||
if let Some(parsed) = parse_tool_call_value_aliased(item, allow_arg_aliases) {
|
||||
calls.push(parsed);
|
||||
}
|
||||
}
|
||||
return calls;
|
||||
}
|
||||
|
||||
if let Some(parsed) = parse_tool_call_value(value) {
|
||||
if let Some(parsed) = parse_tool_call_value_aliased(value, allow_arg_aliases) {
|
||||
calls.push(parsed);
|
||||
}
|
||||
|
||||
@@ -350,7 +421,12 @@ pub(crate) fn parse_tool_calls(response: &str) -> (String, Vec<ParsedToolCall>)
|
||||
// First, try to parse as OpenAI-style JSON response with tool_calls array
|
||||
// This handles providers like Minimax that return tool_calls in native JSON format
|
||||
if let Ok(json_value) = serde_json::from_str::<serde_json::Value>(response.trim()) {
|
||||
calls = parse_tool_calls_from_json_value(&json_value);
|
||||
// Whole-response parse: a bare top-level object/array is NOT an
|
||||
// explicit tool-call marker, so forbid the generic arg-key aliases
|
||||
// here (a plain `{"name":..,"input":..}` answer must stay text).
|
||||
// The `tool_calls`-keyed envelope is still honoured (it carries its
|
||||
// own marker) — handled inside the `_aliased` helper.
|
||||
calls = parse_tool_calls_from_json_value_aliased(&json_value, false);
|
||||
if !calls.is_empty() {
|
||||
// If we found tool_calls, extract any content field as text
|
||||
if let Some(content) = json_value.get("content").and_then(|v| v.as_str()) {
|
||||
@@ -388,7 +464,15 @@ pub(crate) fn parse_tool_calls(response: &str) -> (String, Vec<ParsedToolCall>)
|
||||
}
|
||||
|
||||
if !parsed_any {
|
||||
tracing::warn!("Malformed <tool_call> JSON: expected tool-call object in tag body");
|
||||
// body_chars only (never the body itself — it may carry tool
|
||||
// arguments with user data). Stable `[agent_parse]` prefix so
|
||||
// it aggregates with the other harness log families. Surfaces
|
||||
// how often the model emits an unparseable tool-call tag
|
||||
// (bug-report-2026-05-26 A3).
|
||||
tracing::warn!(
|
||||
body_chars = inner.chars().count(),
|
||||
"[agent_parse] malformed <tool_call> JSON: expected tool-call object in tag body"
|
||||
);
|
||||
}
|
||||
|
||||
remaining = &after_open[close_idx + close_tag.len()..];
|
||||
|
||||
@@ -69,6 +69,65 @@ fn parse_tool_call_value_supports_function_shape_flat_shape_and_invalid_names()
|
||||
assert!(parse_tool_call_value(&serde_json::json!({ "function": {} })).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_tool_call_value_accepts_argument_key_aliases() {
|
||||
// Correct name but the model used `args`/`parameters` instead of the
|
||||
// canonical `arguments` — recover the call rather than drop it and burn
|
||||
// an agent iteration (bug-report-2026-05-26 A3).
|
||||
let with_args = serde_json::json!({ "name": "echo", "args": { "value": "hi" } });
|
||||
let parsed = parse_tool_call_value(&with_args).expect("args alias should parse");
|
||||
assert_eq!(parsed.name, "echo");
|
||||
assert_eq!(parsed.arguments, serde_json::json!({ "value": "hi" }));
|
||||
|
||||
let with_parameters = serde_json::json!({
|
||||
"function": { "name": "shell", "parameters": "{\"command\":\"ls\"}" }
|
||||
});
|
||||
let parsed = parse_tool_call_value(&with_parameters).expect("parameters alias should parse");
|
||||
assert_eq!(parsed.name, "shell");
|
||||
assert_eq!(parsed.arguments, serde_json::json!({ "command": "ls" }));
|
||||
|
||||
// Name stays strict: an arg alias without a recognized name key is not
|
||||
// a tool call (guards the whole-response JSON parse path).
|
||||
assert!(parse_tool_call_value(&serde_json::json!({ "tool": "echo", "args": {} })).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn whole_response_singleton_ignores_generic_arg_aliases() {
|
||||
// A plain JSON answer that happens to carry a `name` plus a generic,
|
||||
// object-valued `input`. Tagged contexts widen `input` into arguments…
|
||||
let answer = serde_json::json!({ "name": "Alice", "input": { "value": "hi" } });
|
||||
let tagged = parse_tool_calls_from_json_value(&answer);
|
||||
assert_eq!(tagged.len(), 1);
|
||||
assert_eq!(tagged[0].arguments, serde_json::json!({ "value": "hi" }));
|
||||
|
||||
// …but the whole-response (bare singleton) path must treat this as plain
|
||||
// text, not a tool call: it carries no canonical `arguments` marker, only
|
||||
// a `name` that happens to match a tool (CodeRabbit, #2683).
|
||||
let whole = parse_tool_calls_from_json_value_aliased(&answer, false);
|
||||
assert!(
|
||||
whole.is_empty(),
|
||||
"bare whole-response object without canonical `arguments` must not dispatch a tool call"
|
||||
);
|
||||
|
||||
// A bare object WITH the canonical `arguments` key is still recognized on
|
||||
// the whole-response path — `arguments` is the explicit tool-call marker.
|
||||
let bare_call = serde_json::json!({ "name": "echo", "arguments": { "value": "hi" } });
|
||||
let calls = parse_tool_calls_from_json_value_aliased(&bare_call, false);
|
||||
assert_eq!(calls.len(), 1);
|
||||
assert_eq!(calls[0].name, "echo");
|
||||
assert_eq!(calls[0].arguments, serde_json::json!({ "value": "hi" }));
|
||||
|
||||
// The `tool_calls`-keyed envelope is an explicit marker and stays
|
||||
// permissive even when aliases are forbidden for bare objects.
|
||||
let envelope = serde_json::json!({
|
||||
"tool_calls": [ { "name": "echo", "input": { "value": "hi" } } ]
|
||||
});
|
||||
let calls = parse_tool_calls_from_json_value_aliased(&envelope, false);
|
||||
assert_eq!(calls.len(), 1);
|
||||
assert_eq!(calls[0].name, "echo");
|
||||
assert_eq!(calls[0].arguments, serde_json::json!({ "value": "hi" }));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_tool_calls_from_json_value_handles_tool_calls_array_arrays_and_singletons() {
|
||||
let wrapped = serde_json::json!({
|
||||
|
||||
@@ -36,7 +36,7 @@ use crate::openhuman::context::prompt::{LearnedContextData, PromptContext, Promp
|
||||
use crate::openhuman::context::{ReductionOutcome, ARCHIVIST_EXTRACTION_PROMPT};
|
||||
use crate::openhuman::inference::model_context::context_window_for_model;
|
||||
use crate::openhuman::inference::provider::{
|
||||
ChatMessage, ChatRequest, ConversationMessage, ProviderDelta,
|
||||
ChatMessage, ChatRequest, ConversationMessage, ProviderDelta, UsageInfo,
|
||||
};
|
||||
use crate::openhuman::memory::MemoryCategory;
|
||||
use crate::openhuman::tools::traits::ToolCallOptions;
|
||||
@@ -50,6 +50,41 @@ use anyhow::Result;
|
||||
use std::hash::{Hash, Hasher};
|
||||
use std::sync::Arc;
|
||||
|
||||
/// Instruction appended (as a synthetic user turn) to the provider
|
||||
/// messages when a turn hits the tool-call iteration cap. Asks the model
|
||||
/// to wrap up with a resumable checkpoint instead of letting the turn die.
|
||||
/// Native tools are disabled for this call so the model produces prose,
|
||||
/// not yet another tool call. See bug-report-2026-05-26 A1.
|
||||
const MAX_ITER_CHECKPOINT_INSTRUCTION: &str = "\
|
||||
You have reached the maximum number of tool calls allowed for this single turn, so you cannot call any more tools right now. \
|
||||
Do not attempt another tool call. Instead, write a short progress checkpoint for the user with two clearly labelled parts:\n\
|
||||
1. **Done so far** — what you have accomplished in this turn, grounded in the tool results above.\n\
|
||||
2. **Next steps** — exactly what you plan to do next.\n\
|
||||
Write it so you can pick up seamlessly where you left off when the user replies. Be concise.";
|
||||
|
||||
/// Build a deterministic checkpoint summary from this turn's tool-call
|
||||
/// records. Used only as a safety net when the model-written checkpoint
|
||||
/// call fails or returns empty, so a capped turn can never be left without
|
||||
/// a well-formed assistant message — which is what silently wedged the
|
||||
/// thread before (bug-report-2026-05-26 A1).
|
||||
fn build_deterministic_checkpoint(records: &[ToolCallRecord], max_iterations: usize) -> String {
|
||||
let mut out = format!(
|
||||
"I reached the tool-call limit for this turn ({max_iterations} steps), so I paused here.\n\n**Done so far:**\n"
|
||||
);
|
||||
if records.is_empty() {
|
||||
out.push_str("- (no tools completed yet)\n");
|
||||
} else {
|
||||
for r in records {
|
||||
let status = if r.success { "ok" } else { "failed" };
|
||||
out.push_str(&format!("- `{}` — {}\n", r.name, status));
|
||||
}
|
||||
}
|
||||
out.push_str(
|
||||
"\n**Next steps:** I'll continue from here — just reply (e.g. \"continue\") and I'll pick up where I left off.",
|
||||
);
|
||||
out
|
||||
}
|
||||
|
||||
impl Agent {
|
||||
/// Executes a single interaction "turn" with the agent.
|
||||
///
|
||||
@@ -443,6 +478,17 @@ impl Agent {
|
||||
// Collect tool call records across all iterations for post-turn hooks
|
||||
let mut all_tool_records: Vec<ToolCallRecord> = Vec::new();
|
||||
|
||||
// Trim-robust digest of THIS turn's tool calls + results, compiled as
|
||||
// the loop runs. Used as the *only* context for the max-iteration
|
||||
// checkpoint summary, so it compiles "what I did this turn" without
|
||||
// the prior conversation or system prompt bleeding in — and it's
|
||||
// immune to history trimming (which drops/reorders from the front).
|
||||
// The persisted transcript is unaffected (bug-report-2026-05-26 A1).
|
||||
// Bounded: each entry truncates the result to 800 chars, so at the
|
||||
// default 10-iteration cap the digest is ~8 KB — revisit if
|
||||
// `max_tool_iterations` is raised substantially.
|
||||
let mut turn_tool_digest = String::new();
|
||||
|
||||
// Capture the last `Vec<ChatMessage>` sent to the provider so we
|
||||
// can persist it as a session transcript after the turn completes.
|
||||
let mut last_provider_messages: Option<Vec<ChatMessage>> = None;
|
||||
@@ -745,6 +791,21 @@ impl Agent {
|
||||
} else {
|
||||
text
|
||||
};
|
||||
// Defense-in-depth (bug-report-2026-05-26 A1): a
|
||||
// completion with no text *and* no tool calls is never a
|
||||
// valid final answer — it's a degenerate/poisoned
|
||||
// response. Surfacing it as an error is visible; the old
|
||||
// behaviour returned `Ok("")`, which rendered as a blank
|
||||
// reply and silently wedged the thread.
|
||||
if final_text.trim().is_empty() {
|
||||
log::warn!(
|
||||
"[agent_loop] provider returned an empty final response (i={}, no text, no tool calls) — surfacing as error instead of a silent blank reply",
|
||||
iteration + 1
|
||||
);
|
||||
return Err(anyhow::anyhow!(
|
||||
"The model returned an empty response. Please try again."
|
||||
));
|
||||
}
|
||||
log::info!(
|
||||
"[agent] no tool calls — returning final response after {} iteration(s)",
|
||||
iteration + 1
|
||||
@@ -910,6 +971,14 @@ impl Agent {
|
||||
r.name,
|
||||
truncate_with_ellipsis(&r.output, 300)
|
||||
);
|
||||
// Record this call in the turn digest (output truncated to
|
||||
// bound size) for a possible max-iteration checkpoint.
|
||||
turn_tool_digest.push_str(&format!(
|
||||
"- {} [{}]: {}\n",
|
||||
r.name,
|
||||
if r.success { "ok" } else { "failed" },
|
||||
truncate_with_ellipsis(&r.output, 800)
|
||||
));
|
||||
}
|
||||
log::info!(
|
||||
"[agent] all tools complete for iteration {} — looping back to provider",
|
||||
@@ -940,26 +1009,136 @@ impl Agent {
|
||||
);
|
||||
}
|
||||
|
||||
// Tool-call iteration cap reached. Instead of aborting the turn
|
||||
// — which left the persisted transcript on an unterminated tool
|
||||
// cycle and silently wedged the thread on the next message
|
||||
// (bug-report-2026-05-26 A1) — emit a *resumable checkpoint*:
|
||||
// ask the model (tools disabled) to summarize what it did and
|
||||
// what comes next, persist that as the final assistant message,
|
||||
// and return it. The full tool-call history stays in the
|
||||
// transcript, so the user's next message naturally resumes the
|
||||
// task — no heuristic "continue" detection needed.
|
||||
log::warn!(
|
||||
"[agent] exceeded max tool iterations ({}) — aborting turn",
|
||||
"[agent_loop] reached max tool iterations max={} — emitting resumable checkpoint instead of aborting",
|
||||
self.config.max_tool_iterations
|
||||
);
|
||||
log::warn!(
|
||||
"[agent_loop] exceeded maximum tool iterations max={}",
|
||||
self.config.max_tool_iterations
|
||||
|
||||
let base_messages = last_provider_messages
|
||||
.clone()
|
||||
.unwrap_or_else(|| self.tool_dispatcher.to_provider_messages(&self.history));
|
||||
// Summarize ONLY this turn's work: feed the compiled tool-call
|
||||
// digest (no system prompt, no prior conversation), not the full
|
||||
// conversation. `base_messages` above is still used for the
|
||||
// transcript persist below, so the saved transcript is unchanged
|
||||
// (bug-report-2026-05-26 A1). `user_message` below is the
|
||||
// `turn(&mut self, message: &str)` parameter (the turn's request).
|
||||
let turn_summary_input = vec![ChatMessage::user(format!(
|
||||
"You were working on this user request:\n{user_message}\n\nHere are the tool calls you made this turn and their results — compile your checkpoint from these:\n{}",
|
||||
if turn_tool_digest.is_empty() {
|
||||
"(no tool calls recorded)"
|
||||
} else {
|
||||
turn_tool_digest.as_str()
|
||||
}
|
||||
))];
|
||||
let checkpoint_iteration = (self.config.max_tool_iterations + 1) as u32;
|
||||
let (mut checkpoint, checkpoint_usage) = self
|
||||
.summarize_iteration_checkpoint(
|
||||
&turn_summary_input,
|
||||
&effective_model,
|
||||
checkpoint_iteration,
|
||||
)
|
||||
.await;
|
||||
|
||||
// Fold the checkpoint call's usage into the turn's cumulative
|
||||
// accounting. The provider call happens regardless of whether we
|
||||
// keep its prose, so dropping its tokens would undercount the
|
||||
// turn and mis-attribute the prior iteration's usage to the
|
||||
// checkpoint message (mirrors the normal final-response path).
|
||||
if let Some(ref usage) = checkpoint_usage {
|
||||
self.context.record_usage(usage);
|
||||
cumulative_input_tokens += usage.input_tokens;
|
||||
cumulative_output_tokens += usage.output_tokens;
|
||||
cumulative_cached_input_tokens += usage.cached_input_tokens;
|
||||
cumulative_charged_usd += usage.charged_amount_usd;
|
||||
last_turn_usage = Some(transcript::TurnUsage {
|
||||
model: effective_model.clone(),
|
||||
usage: transcript::MessageUsage {
|
||||
input: usage.input_tokens,
|
||||
output: usage.output_tokens,
|
||||
cached_input: usage.cached_input_tokens,
|
||||
cost_usd: usage.charged_amount_usd,
|
||||
},
|
||||
ts: chrono::Utc::now().to_rfc3339(),
|
||||
});
|
||||
} else {
|
||||
// No usage on the checkpoint call: don't attribute a stale
|
||||
// prior-iteration snapshot to the checkpoint assistant message.
|
||||
last_turn_usage = None;
|
||||
}
|
||||
|
||||
if checkpoint.trim().is_empty() {
|
||||
log::warn!("[agent_loop] checkpoint summary empty — using deterministic fallback");
|
||||
checkpoint = build_deterministic_checkpoint(
|
||||
&all_tool_records,
|
||||
self.config.max_tool_iterations,
|
||||
);
|
||||
}
|
||||
log::info!(
|
||||
"[agent_loop] max-iter checkpoint emitted chars={}",
|
||||
checkpoint.chars().count()
|
||||
);
|
||||
// Return the typed `AgentError::MaxIterationsExceeded` variant
|
||||
// (boxed through `anyhow::Error`) so `Agent::run_single` can
|
||||
// downcast at the Sentry funnel and suppress emission — this is
|
||||
// a deterministic agent-state outcome, not a bug (OPENHUMAN-
|
||||
// TAURI-99 / -98). The `Display` text is preserved verbatim so
|
||||
// the user-visible chat-rendered "Error: Agent exceeded
|
||||
// maximum tool iterations" string is unchanged.
|
||||
Err(anyhow::Error::new(
|
||||
crate::openhuman::agent::error::AgentError::MaxIterationsExceeded {
|
||||
max: self.config.max_tool_iterations,
|
||||
},
|
||||
))
|
||||
|
||||
self.emit_progress(AgentProgress::TurnCompleted {
|
||||
iterations: self.config.max_tool_iterations as u32,
|
||||
})
|
||||
.await;
|
||||
|
||||
self.history
|
||||
.push(ConversationMessage::Chat(ChatMessage::assistant(
|
||||
checkpoint.clone(),
|
||||
)));
|
||||
self.trim_history();
|
||||
|
||||
// Persist the checkpoint so the transcript ends on a
|
||||
// well-formed assistant message (never a dangling tool cycle).
|
||||
// Note: `base_messages` ends before the final (capped) iteration's
|
||||
// tool results — those landed after the last `last_provider_messages`
|
||||
// snapshot — so the persisted transcript omits them. That's fine:
|
||||
// the checkpoint prose covers the work done, and the transcript
|
||||
// stays structurally correct (ends on an assistant message).
|
||||
let mut checkpoint_messages = base_messages;
|
||||
checkpoint_messages.push(ChatMessage::assistant(checkpoint.clone()));
|
||||
self.persist_session_transcript(
|
||||
&checkpoint_messages,
|
||||
cumulative_input_tokens,
|
||||
cumulative_output_tokens,
|
||||
cumulative_cached_input_tokens,
|
||||
cumulative_charged_usd,
|
||||
last_turn_usage.as_ref(),
|
||||
);
|
||||
|
||||
self.context.record_tool_calls(all_tool_records.len());
|
||||
|
||||
// Fire post-turn hooks with the checkpoint as the assistant
|
||||
// response (mirrors the normal final-response path).
|
||||
if !self.post_turn_hooks.is_empty() {
|
||||
let ctx = TurnContext {
|
||||
user_message: user_message.to_string(),
|
||||
assistant_response: checkpoint.clone(),
|
||||
tool_calls: all_tool_records,
|
||||
turn_duration_ms: turn_started.elapsed().as_millis() as u64,
|
||||
session_id: Some(self.event_session_id.clone())
|
||||
.filter(|session_id| !session_id.trim().is_empty()),
|
||||
agent_id: Some(self.agent_definition_id.clone())
|
||||
.filter(|agent_id| !agent_id.trim().is_empty()),
|
||||
entrypoint: Some(self.event_channel.clone())
|
||||
.filter(|entrypoint| !entrypoint.trim().is_empty()),
|
||||
iteration_count: self.config.max_tool_iterations,
|
||||
};
|
||||
hooks::fire_hooks(&self.post_turn_hooks, ctx);
|
||||
}
|
||||
|
||||
Ok(checkpoint)
|
||||
}; // end of `turn_body` async block
|
||||
|
||||
// Run the turn body inside the parent-execution-context scope so
|
||||
@@ -1875,6 +2054,104 @@ impl Agent {
|
||||
}
|
||||
}
|
||||
|
||||
/// Ask the provider for a resumable checkpoint summary when a turn
|
||||
/// hits the tool-call iteration cap, with native tools **disabled** so
|
||||
/// the model returns prose rather than another tool call. Streams text
|
||||
/// deltas to the progress sink (when attached) so the checkpoint
|
||||
/// appears in the UI like any other reply.
|
||||
///
|
||||
/// Returns the summary text (empty when the provider call fails or
|
||||
/// yields nothing — the caller then falls back to
|
||||
/// [`build_deterministic_checkpoint`] so the thread is never left on an
|
||||
/// unterminated tool cycle, bug-report-2026-05-26 A1) **paired with the
|
||||
/// provider usage** for this extra call, so the caller can fold it into
|
||||
/// the turn's cumulative token/cost accounting instead of silently
|
||||
/// dropping it.
|
||||
async fn summarize_iteration_checkpoint(
|
||||
&self,
|
||||
base_messages: &[ChatMessage],
|
||||
effective_model: &str,
|
||||
iteration_for_stream: u32,
|
||||
) -> (String, Option<UsageInfo>) {
|
||||
let mut messages = base_messages.to_vec();
|
||||
messages.push(ChatMessage::user(MAX_ITER_CHECKPOINT_INSTRUCTION));
|
||||
|
||||
// Mirror the main loop's streaming sink so the checkpoint renders
|
||||
// incrementally. Only text deltas are relevant here (tools are
|
||||
// disabled for this call).
|
||||
let (delta_tx_opt, delta_forwarder) = if self.on_progress.is_some() {
|
||||
let (tx, mut rx) = tokio::sync::mpsc::channel::<ProviderDelta>(128);
|
||||
let progress_tx = self.on_progress.clone();
|
||||
let forwarder = tokio::spawn(async move {
|
||||
while let Some(event) = rx.recv().await {
|
||||
let Some(ref sink) = progress_tx else {
|
||||
continue;
|
||||
};
|
||||
if let ProviderDelta::TextDelta { delta } = event {
|
||||
if sink
|
||||
.send(AgentProgress::TextDelta {
|
||||
delta,
|
||||
iteration: iteration_for_stream,
|
||||
})
|
||||
.await
|
||||
.is_err()
|
||||
{
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
(Some(tx), Some(forwarder))
|
||||
} else {
|
||||
(None, None)
|
||||
};
|
||||
|
||||
let result = self
|
||||
.provider
|
||||
.chat(
|
||||
ChatRequest {
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: delta_tx_opt.as_ref(),
|
||||
},
|
||||
effective_model,
|
||||
self.temperature,
|
||||
)
|
||||
.await;
|
||||
drop(delta_tx_opt);
|
||||
if let Some(handle) = delta_forwarder {
|
||||
let _ = handle.await;
|
||||
}
|
||||
|
||||
match result {
|
||||
Ok(resp) => {
|
||||
let usage = resp.usage.clone();
|
||||
// Strip any stray tool-call XML a text-mode model may have
|
||||
// emitted; keep only the prose.
|
||||
let (text, calls) = self.tool_dispatcher.parse_response(&resp);
|
||||
let checkpoint = if !text.trim().is_empty() {
|
||||
text
|
||||
} else if calls.is_empty() {
|
||||
// No tool-call markup was present, so the raw text (if
|
||||
// any) is genuine prose — safe to use.
|
||||
resp.text.unwrap_or_default()
|
||||
} else {
|
||||
// `parse_response` stripped tool-call markup and left no
|
||||
// prose. Do NOT re-emit `resp.text` here: it would persist
|
||||
// the raw `<tool_call>…` markup verbatim as the checkpoint.
|
||||
// Return empty so the caller uses the deterministic
|
||||
// fallback instead (bug-report-2026-05-26 A1).
|
||||
String::new()
|
||||
};
|
||||
(checkpoint, usage)
|
||||
}
|
||||
Err(e) => {
|
||||
log::warn!("[agent_loop] checkpoint summary call failed: {e:#}");
|
||||
(String::new(), None)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Persist the exact provider messages as a session transcript.
|
||||
///
|
||||
/// Writes JSONL as source of truth and re-renders the companion `.md`
|
||||
|
||||
@@ -7,7 +7,7 @@ use crate::openhuman::agent::tool_policy::{
|
||||
GeneratedToolRuntimeContext, GeneratedToolRuntimeRisk, ToolPolicy, ToolPolicyDecision,
|
||||
ToolPolicyRequest,
|
||||
};
|
||||
use crate::openhuman::inference::provider::{ChatRequest, ChatResponse, Provider};
|
||||
use crate::openhuman::inference::provider::{ChatRequest, ChatResponse, Provider, UsageInfo};
|
||||
use crate::openhuman::memory::Memory;
|
||||
use crate::openhuman::tools::ToolResult;
|
||||
use crate::openhuman::tools::{PermissionLevel, Tool};
|
||||
@@ -829,13 +829,28 @@ async fn turn_uses_cached_transcript_prefix_on_first_iteration() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_errors_when_max_tool_iterations_are_exceeded() {
|
||||
async fn turn_emits_checkpoint_when_max_tool_iterations_are_exceeded() {
|
||||
// First response forces a tool call (consuming the single allowed
|
||||
// iteration); the second is the model-written checkpoint the harness
|
||||
// requests (tools disabled) once the cap is hit. The turn must NOT
|
||||
// error anymore — it returns a resumable checkpoint so the thread stays
|
||||
// well-formed and the user can continue on their next message
|
||||
// (bug-report-2026-05-26 A1).
|
||||
let provider: Arc<dyn Provider> = Arc::new(SequenceProvider {
|
||||
responses: AsyncMutex::new(vec![Ok(ChatResponse {
|
||||
text: Some("<tool_call>{\"name\":\"echo\",\"arguments\":{}}</tool_call>".into()),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
})]),
|
||||
responses: AsyncMutex::new(vec![
|
||||
Ok(ChatResponse {
|
||||
text: Some("<tool_call>{\"name\":\"echo\",\"arguments\":{}}</tool_call>".into()),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
}),
|
||||
Ok(ChatResponse {
|
||||
text: Some(
|
||||
"**Done so far:** ran echo.\n**Next steps:** I'll continue from here.".into(),
|
||||
),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
}),
|
||||
]),
|
||||
requests: AsyncMutex::new(Vec::new()),
|
||||
});
|
||||
let mut agent = make_agent_with_builder(
|
||||
@@ -852,17 +867,179 @@ async fn turn_errors_when_max_tool_iterations_are_exceeded() {
|
||||
crate::openhuman::config::ContextConfig::default(),
|
||||
);
|
||||
|
||||
let err = agent
|
||||
let reply = agent
|
||||
.turn("hello")
|
||||
.await
|
||||
.expect_err("turn should stop at configured iteration budget");
|
||||
assert!(err
|
||||
.to_string()
|
||||
.contains("Agent exceeded maximum tool iterations (1)"));
|
||||
.expect("turn should emit a checkpoint at the iteration cap, not error");
|
||||
assert!(
|
||||
reply.contains("Next steps"),
|
||||
"checkpoint should summarize next steps, got: {reply}"
|
||||
);
|
||||
// The tool-call history from the capped iteration is preserved...
|
||||
assert!(agent.history.iter().any(|message| matches!(
|
||||
message,
|
||||
ConversationMessage::AssistantToolCalls { tool_calls, .. } if tool_calls.len() == 1
|
||||
)));
|
||||
// ...and the transcript ends on a well-formed assistant message (the
|
||||
// checkpoint), never a dangling tool cycle — this is what stops the
|
||||
// next message from silently wedging the thread.
|
||||
assert!(
|
||||
matches!(
|
||||
agent.history.last(),
|
||||
Some(ConversationMessage::Chat(msg))
|
||||
if msg.role == "assistant" && msg.content.contains("Next steps")
|
||||
),
|
||||
"history should end on the assistant checkpoint, got: {:?}",
|
||||
agent.history.last()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_errors_on_empty_provider_response() {
|
||||
// A completion with no text and no tool calls is never a valid final
|
||||
// answer — surface it as an error instead of accepting a blank reply,
|
||||
// which previously rendered as silence and wedged the thread
|
||||
// (bug-report-2026-05-26 A1, defect B).
|
||||
let provider: Arc<dyn Provider> = Arc::new(SequenceProvider {
|
||||
responses: AsyncMutex::new(vec![Ok(ChatResponse {
|
||||
text: Some(String::new()),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
})]),
|
||||
requests: AsyncMutex::new(Vec::new()),
|
||||
});
|
||||
let mut agent = make_agent_with_builder(
|
||||
provider,
|
||||
vec![],
|
||||
Box::new(FixedMemoryLoader {
|
||||
context: String::new(),
|
||||
}),
|
||||
vec![],
|
||||
crate::openhuman::config::AgentConfig::default(),
|
||||
crate::openhuman::config::ContextConfig::default(),
|
||||
);
|
||||
|
||||
let err = agent
|
||||
.turn("hello")
|
||||
.await
|
||||
.expect_err("an empty provider response should surface as an error");
|
||||
assert!(
|
||||
err.to_string().contains("empty response"),
|
||||
"expected an empty-response error, got: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_checkpoint_falls_back_to_deterministic_summary_when_model_summary_empty() {
|
||||
// Tool call consumes the single iteration; the checkpoint request then
|
||||
// comes back empty. The harness must fall back to a deterministic
|
||||
// done/next summary so the turn never returns blank — the safety net
|
||||
// that guarantees the thread can't re-wedge (bug-report-2026-05-26 A1).
|
||||
let provider: Arc<dyn Provider> = Arc::new(SequenceProvider {
|
||||
responses: AsyncMutex::new(vec![
|
||||
Ok(ChatResponse {
|
||||
text: Some("<tool_call>{\"name\":\"echo\",\"arguments\":{}}</tool_call>".into()),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
}),
|
||||
Ok(ChatResponse {
|
||||
text: Some(String::new()),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
}),
|
||||
]),
|
||||
requests: AsyncMutex::new(Vec::new()),
|
||||
});
|
||||
let mut agent = make_agent_with_builder(
|
||||
provider,
|
||||
vec![Box::new(EchoTool)],
|
||||
Box::new(FixedMemoryLoader {
|
||||
context: String::new(),
|
||||
}),
|
||||
vec![],
|
||||
crate::openhuman::config::AgentConfig {
|
||||
max_tool_iterations: 1,
|
||||
..crate::openhuman::config::AgentConfig::default()
|
||||
},
|
||||
crate::openhuman::config::ContextConfig::default(),
|
||||
);
|
||||
|
||||
let reply = agent
|
||||
.turn("hello")
|
||||
.await
|
||||
.expect("empty model checkpoint should fall back, not error");
|
||||
assert!(
|
||||
reply.contains("tool-call limit"),
|
||||
"deterministic fallback summary expected, got: {reply}"
|
||||
);
|
||||
assert!(
|
||||
reply.contains("echo"),
|
||||
"fallback should list the tool that ran, got: {reply}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_checkpoint_usage_is_folded_into_transcript_accounting() {
|
||||
// The extra checkpoint provider call costs tokens; those must land in
|
||||
// the persisted transcript's cumulative accounting rather than being
|
||||
// silently dropped (CodeRabbit review on bug-report-2026-05-26 A1).
|
||||
let provider: Arc<dyn Provider> = Arc::new(SequenceProvider {
|
||||
responses: AsyncMutex::new(vec![
|
||||
// Tool iteration — provider reports no usage.
|
||||
Ok(ChatResponse {
|
||||
text: Some("<tool_call>{\"name\":\"echo\",\"arguments\":{}}</tool_call>".into()),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
}),
|
||||
// Checkpoint call — reports usage that must be accounted for.
|
||||
Ok(ChatResponse {
|
||||
text: Some("**Done so far:** ran echo.\n**Next steps:** continue.".into()),
|
||||
tool_calls: vec![],
|
||||
usage: Some(UsageInfo {
|
||||
input_tokens: 11,
|
||||
output_tokens: 4,
|
||||
cached_input_tokens: 2,
|
||||
charged_amount_usd: 0.05,
|
||||
..UsageInfo::default()
|
||||
}),
|
||||
}),
|
||||
]),
|
||||
requests: AsyncMutex::new(Vec::new()),
|
||||
});
|
||||
let mut agent = make_agent_with_builder(
|
||||
provider,
|
||||
vec![Box::new(EchoTool)],
|
||||
Box::new(FixedMemoryLoader {
|
||||
context: String::new(),
|
||||
}),
|
||||
vec![],
|
||||
crate::openhuman::config::AgentConfig {
|
||||
max_tool_iterations: 1,
|
||||
..crate::openhuman::config::AgentConfig::default()
|
||||
},
|
||||
crate::openhuman::config::ContextConfig::default(),
|
||||
);
|
||||
|
||||
agent
|
||||
.turn("hello")
|
||||
.await
|
||||
.expect("turn should emit a checkpoint at the iteration cap");
|
||||
|
||||
let transcript = transcript::read_transcript(
|
||||
agent
|
||||
.session_transcript_path
|
||||
.as_ref()
|
||||
.expect("checkpoint turn should persist a transcript"),
|
||||
)
|
||||
.expect("transcript should be readable");
|
||||
// Only the checkpoint call reported usage, so the turn totals must equal
|
||||
// exactly its numbers — proof the extra call is accounted for, not lost.
|
||||
assert_eq!(
|
||||
transcript.meta.input_tokens, 11,
|
||||
"checkpoint input tokens should be folded into the turn total"
|
||||
);
|
||||
assert_eq!(transcript.meta.output_tokens, 4);
|
||||
assert_eq!(transcript.meta.cached_input_tokens, 2);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -34,6 +34,7 @@ use crate::openhuman::context::prompt::{
|
||||
use crate::openhuman::inference::provider::{ChatMessage, ChatRequest, Provider, ToolCall};
|
||||
use crate::openhuman::memory_conversations::ConversationMessage;
|
||||
use crate::openhuman::tools::{Tool, ToolCategory, ToolSpec};
|
||||
use crate::openhuman::util::truncate_with_ellipsis;
|
||||
|
||||
/// Prompt suffix injected into every typed sub-agent run.
|
||||
///
|
||||
@@ -199,7 +200,7 @@ struct LazyToolkitResolver {
|
||||
|
||||
impl LazyToolkitResolver {
|
||||
fn resolve(&self, name: &str) -> Option<Box<dyn Tool>> {
|
||||
let action = self.actions.iter().find(|a| a.name == name)?;
|
||||
let action = self.find_action(name)?;
|
||||
Some(Box::new(
|
||||
crate::openhuman::composio::ComposioActionTool::new(
|
||||
self.config.clone(),
|
||||
@@ -210,6 +211,63 @@ impl LazyToolkitResolver {
|
||||
))
|
||||
}
|
||||
|
||||
/// Match a model-supplied tool name to a real toolkit action, tolerant
|
||||
/// of the near-miss slugs models routinely emit — case differences and
|
||||
/// separator/prefix drift (bug-report-2026-05-26 A2). Tries, in order:
|
||||
/// exact, case-insensitive, then a normalized alphanumeric match
|
||||
/// (accepted only when **unique**, so a fabricated slug can't silently
|
||||
/// resolve to the wrong action — those still fall through to the
|
||||
/// "tool not available" error, which lists `known_slugs` for the model
|
||||
/// to self-correct).
|
||||
fn find_action(
|
||||
&self,
|
||||
name: &str,
|
||||
) -> Option<&crate::openhuman::context::prompt::ConnectedIntegrationTool> {
|
||||
if let Some(action) = self.actions.iter().find(|a| a.name == name) {
|
||||
return Some(action);
|
||||
}
|
||||
if let Some(action) = self
|
||||
.actions
|
||||
.iter()
|
||||
.find(|a| a.name.eq_ignore_ascii_case(name))
|
||||
{
|
||||
tracing::debug!(
|
||||
requested = %name,
|
||||
matched = %action.name,
|
||||
"[subagent_runner] resolved tool by case-insensitive match"
|
||||
);
|
||||
return Some(action);
|
||||
}
|
||||
let norm = normalize_slug(name);
|
||||
if !norm.is_empty() {
|
||||
let mut matches = self
|
||||
.actions
|
||||
.iter()
|
||||
.filter(|a| normalize_slug(&a.name) == norm);
|
||||
if let Some(action) = matches.next() {
|
||||
if matches.next().is_none() {
|
||||
tracing::info!(
|
||||
requested = %name,
|
||||
matched = %action.name,
|
||||
"[subagent_runner] resolved tool by normalized-slug match"
|
||||
);
|
||||
return Some(action);
|
||||
}
|
||||
// Ambiguous: 2+ actions normalize to the same slug (e.g.
|
||||
// `read_file` and `ReadFile` → `readfile`). We deliberately
|
||||
// refuse to guess. Warn (not debug): a slug collision is a
|
||||
// toolkit configuration anomaly that should surface in normal
|
||||
// operator logs, not stay hidden behind debug filtering.
|
||||
tracing::warn!(
|
||||
requested = %name,
|
||||
norm = %norm,
|
||||
"[subagent_runner] ambiguous normalized-slug match — multiple actions resolve to the same slug; not resolving"
|
||||
);
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Slugs from the bound toolkit, for inclusion in unknown-tool
|
||||
/// errors so the model can self-correct without burning a turn.
|
||||
fn known_slugs(&self) -> Vec<&str> {
|
||||
@@ -217,6 +275,17 @@ impl LazyToolkitResolver {
|
||||
}
|
||||
}
|
||||
|
||||
/// Lowercased, non-alphanumerics stripped — collapses separator/prefix
|
||||
/// drift (`GOOGLESLIDES_BATCH_UPDATE` vs `googleslides_batch_update`) so
|
||||
/// near-miss tool slugs still resolve, while genuinely different slugs
|
||||
/// (e.g. a hallucinated `GMAIL_GET_LAST_3_MESSAGES`) stay distinct.
|
||||
fn normalize_slug(s: &str) -> String {
|
||||
s.chars()
|
||||
.filter(|c| c.is_ascii_alphanumeric())
|
||||
.map(|c| c.to_ascii_lowercase())
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Run a sub-agent based on its definition and a task prompt.
|
||||
///
|
||||
/// This is the primary entry point for agent delegation. It performs the following:
|
||||
@@ -1161,6 +1230,12 @@ async fn run_inner_loop(
|
||||
) -> Result<(String, usize, AggregatedUsage), SubagentRunError> {
|
||||
let max_iterations = max_iterations.max(1);
|
||||
|
||||
// Compiled digest of this sub-agent run's tool calls + results, for a
|
||||
// graceful checkpoint if it hits the iteration cap (mirrors the main
|
||||
// agent — bug-report-2026-05-26 A1). Accumulated as the loop runs so it's
|
||||
// robust to history trimming.
|
||||
let mut run_tool_digest = String::new();
|
||||
|
||||
// Sub-agent transcript stem — mirrors what
|
||||
// `persist_subagent_transcript` used to compute on one-shot
|
||||
// post-loop writes. We compute it once up front so **every
|
||||
@@ -1694,6 +1769,15 @@ async fn run_inner_loop(
|
||||
let call_output_chars = result_text.chars().count();
|
||||
let call_elapsed_ms = call_started.elapsed().as_millis() as u64;
|
||||
|
||||
// Record this call in the run digest (output truncated to bound
|
||||
// size) for a possible max-iteration checkpoint.
|
||||
run_tool_digest.push_str(&format!(
|
||||
"- {} [{}]: {}\n",
|
||||
call.name,
|
||||
if call_success { "ok" } else { "failed" },
|
||||
truncate_with_ellipsis(&result_text, 800)
|
||||
));
|
||||
|
||||
// Repeated-failure circuit breaker (shared guard). `call.arguments`
|
||||
// is the stable signature; on a trip we stash the root-cause summary
|
||||
// and bail after this iteration's tool results are recorded.
|
||||
@@ -1785,7 +1869,70 @@ async fn run_inner_loop(
|
||||
}
|
||||
}
|
||||
|
||||
Err(SubagentRunError::MaxIterationsExceeded(max_iterations))
|
||||
// Iteration cap reached. Instead of erroring — which discards all of the
|
||||
// sub-agent's partial work (the parent just sees "delegate failed") —
|
||||
// compile a graceful checkpoint of what it accomplished and return it as
|
||||
// the result, so the calling agent can continue from the partial progress
|
||||
// (mirrors the main-agent checkpoint — bug-report-2026-05-26 A1).
|
||||
let digest = if run_tool_digest.is_empty() {
|
||||
"(no tool calls completed)"
|
||||
} else {
|
||||
run_tool_digest.as_str()
|
||||
};
|
||||
let deterministic = format!(
|
||||
"I reached my tool-call limit ({max_iterations} steps) before finishing this task. \
|
||||
Progress so far (tool calls + results):\n{digest}\n\nThe task is incomplete — the above is \
|
||||
what I accomplished; continue from here."
|
||||
);
|
||||
let summary_input = vec![ChatMessage::user(format!(
|
||||
"You are sub-agent `{agent_id}` and reached your tool-call limit before finishing. Here are \
|
||||
the tool calls you made and their results — compile a brief progress checkpoint (what you \
|
||||
accomplished, what still remains) for the agent that delegated to you. Do not call tools.\n\n{digest}"
|
||||
))];
|
||||
let checkpoint = match provider
|
||||
.chat(
|
||||
ChatRequest {
|
||||
messages: &summary_input,
|
||||
tools: None,
|
||||
stream: None,
|
||||
},
|
||||
model,
|
||||
temperature,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(resp) => {
|
||||
if let Some(ref u) = resp.usage {
|
||||
usage.input_tokens += u.input_tokens;
|
||||
usage.output_tokens += u.output_tokens;
|
||||
usage.cached_input_tokens += u.cached_input_tokens;
|
||||
usage.charged_amount_usd += u.charged_amount_usd;
|
||||
}
|
||||
// Strip any stray tool-call markup a text-mode model emits; if no
|
||||
// prose survives, fall back to the deterministic digest.
|
||||
let raw = resp.text.unwrap_or_default();
|
||||
let (prose, _) = super::super::parse::parse_tool_calls(&raw);
|
||||
if prose.trim().is_empty() {
|
||||
deterministic
|
||||
} else {
|
||||
prose
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
agent_id = %agent_id,
|
||||
task_id = %task_id,
|
||||
error = %e,
|
||||
"[subagent_runner] checkpoint summary call failed — using deterministic fallback"
|
||||
);
|
||||
deterministic
|
||||
}
|
||||
};
|
||||
// NB: unlike the main-agent path, this checkpoint is intentionally NOT
|
||||
// written to a sub-agent transcript — the calling agent's transcript
|
||||
// captures the delegated result, so there's no data loss. Don't "fix"
|
||||
// this by adding a `persist_subagent_transcript` call.
|
||||
Ok((checkpoint, max_iterations, usage))
|
||||
}
|
||||
|
||||
fn parse_tool_arguments(arguments: &str) -> serde_json::Value {
|
||||
|
||||
@@ -1,6 +1,44 @@
|
||||
use super::*;
|
||||
use crate::openhuman::agent::harness::definition::{ModelSpec, ToolScope};
|
||||
|
||||
#[test]
|
||||
fn lazy_resolver_tolerates_near_miss_slugs() {
|
||||
use crate::openhuman::context::prompt::ConnectedIntegrationTool;
|
||||
let mk = |name: &str| ConnectedIntegrationTool {
|
||||
name: name.into(),
|
||||
description: "d".into(),
|
||||
parameters: None,
|
||||
};
|
||||
let resolver = LazyToolkitResolver {
|
||||
config: std::sync::Arc::new(crate::openhuman::config::Config::default()),
|
||||
actions: vec![mk("GOOGLESLIDES_BATCH_UPDATE"), mk("GMAIL_LIST_MESSAGES")],
|
||||
};
|
||||
// Exact, case-insensitive, and separator/prefix drift all resolve
|
||||
// (bug-report-2026-05-26 A2).
|
||||
assert!(resolver.resolve("GMAIL_LIST_MESSAGES").is_some());
|
||||
assert!(resolver.resolve("gmail_list_messages").is_some());
|
||||
assert!(resolver.resolve("googleslides_batch_update").is_some());
|
||||
// A fabricated slug stays unresolved → routed to the "available tools"
|
||||
// error so the model self-corrects, not silently mis-dispatched.
|
||||
assert!(resolver.resolve("GMAIL_GET_LAST_3_MESSAGES").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normalize_slug_collapses_separators_and_case() {
|
||||
assert_eq!(
|
||||
normalize_slug("GOOGLESLIDES_BATCH_UPDATE"),
|
||||
"googleslidesbatchupdate"
|
||||
);
|
||||
assert_eq!(
|
||||
normalize_slug("googleslides_batch_update"),
|
||||
"googleslidesbatchupdate"
|
||||
);
|
||||
assert_ne!(
|
||||
normalize_slug("GMAIL_GET_LAST_3_MESSAGES"),
|
||||
normalize_slug("GMAIL_LIST_MESSAGES")
|
||||
);
|
||||
}
|
||||
|
||||
fn make_def_named_tools(names: &[&str]) -> AgentDefinition {
|
||||
AgentDefinition {
|
||||
id: "test".into(),
|
||||
@@ -597,6 +635,68 @@ async fn runner_errors_outside_parent_context() {
|
||||
assert!(matches!(result, Err(SubagentRunError::NoParentContext)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn subagent_emits_checkpoint_at_iteration_cap_instead_of_erroring() {
|
||||
// A sub-agent that keeps calling tools and never finishes must hit its
|
||||
// cap and return a graceful partial-progress checkpoint (Ok), not a bare
|
||||
// MaxIterationsExceeded that discards its work — so the delegating agent
|
||||
// can continue from what it got (bug-report-2026-05-26 A1, mirrors the
|
||||
// main agent). Two tool rounds (max_iterations=2), then the summarize
|
||||
// call returns prose which becomes the checkpoint.
|
||||
let provider = ScriptedProvider::new(vec![
|
||||
tool_response("file_read", "{}"),
|
||||
tool_response("file_read", "{}"),
|
||||
text_response("Progress so far: read the file. Remaining: keep going."),
|
||||
]);
|
||||
let parent = make_parent(provider.clone(), vec![stub("file_read")]);
|
||||
let mut def = make_def_named_tools(&["file_read"]);
|
||||
def.max_iterations = 2;
|
||||
|
||||
let outcome = with_parent_context(parent, async {
|
||||
run_subagent(&def, "keep reading forever", SubagentRunOptions::default()).await
|
||||
})
|
||||
.await
|
||||
.expect("hitting the iteration cap should return a checkpoint, not error");
|
||||
|
||||
assert!(
|
||||
outcome.output.contains("Progress so far"),
|
||||
"expected the model-written checkpoint, got: {}",
|
||||
outcome.output
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn subagent_checkpoint_falls_back_to_deterministic_when_summary_empty() {
|
||||
// Same cap, but the summarize call yields nothing (response queue
|
||||
// exhausted → empty). The runner must fall back to a deterministic
|
||||
// partial-progress digest so the parent still gets a usable result
|
||||
// (bug-report-2026-05-26 A1).
|
||||
let provider = ScriptedProvider::new(vec![
|
||||
tool_response("file_read", "{}"),
|
||||
tool_response("file_read", "{}"),
|
||||
]);
|
||||
let parent = make_parent(provider.clone(), vec![stub("file_read")]);
|
||||
let mut def = make_def_named_tools(&["file_read"]);
|
||||
def.max_iterations = 2;
|
||||
|
||||
let outcome = with_parent_context(parent, async {
|
||||
run_subagent(&def, "keep reading forever", SubagentRunOptions::default()).await
|
||||
})
|
||||
.await
|
||||
.expect("empty summary should fall back, not error");
|
||||
|
||||
assert!(
|
||||
outcome.output.contains("tool-call limit"),
|
||||
"expected the deterministic fallback checkpoint, got: {}",
|
||||
outcome.output
|
||||
);
|
||||
assert!(
|
||||
outcome.output.contains("file_read"),
|
||||
"deterministic checkpoint should list the tool work done, got: {}",
|
||||
outcome.output
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn runner_allows_spawn_at_max_depth() {
|
||||
let provider = ScriptedProvider::new(vec![text_response("ok")]);
|
||||
|
||||
@@ -437,12 +437,18 @@ async fn turn_handles_multi_step_tool_chain() {
|
||||
}
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
// 4. Max-iteration bailout
|
||||
// 4. Max-iteration checkpoint (resumable, not a hard bailout)
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_bails_out_at_max_iterations() {
|
||||
// Create more tool calls than max_tool_iterations allows.
|
||||
async fn turn_emits_checkpoint_at_max_iterations() {
|
||||
// Create more tool calls than max_tool_iterations allows. Hitting the
|
||||
// cap must NOT error anymore: the harness emits a resumable checkpoint
|
||||
// and returns it Ok, so the transcript ends on a well-formed assistant
|
||||
// message instead of a dangling tool cycle that wedges the next turn
|
||||
// (bug-report-2026-05-26 A1). Every scripted response here is a tool
|
||||
// call, so the checkpoint summary call also yields no prose and the
|
||||
// deterministic fallback summary is used.
|
||||
let max_iters = 3;
|
||||
let mut responses = Vec::new();
|
||||
for i in 0..max_iters + 5 {
|
||||
@@ -462,12 +468,24 @@ async fn turn_bails_out_at_max_iterations() {
|
||||
|
||||
let (mut agent, _tmp) = build_agent_with_config(provider, vec![Box::new(EchoTool)], config);
|
||||
|
||||
let result = agent.turn("infinite loop").await;
|
||||
assert!(result.is_err());
|
||||
let err = result.unwrap_err().to_string();
|
||||
let reply = agent
|
||||
.turn("infinite loop")
|
||||
.await
|
||||
.expect("hitting the iteration cap should return a checkpoint, not error");
|
||||
assert!(
|
||||
err.contains("maximum tool iterations"),
|
||||
"Expected max iterations error, got: {err}"
|
||||
reply.contains("tool-call limit") && reply.contains("Next steps"),
|
||||
"Expected a resumable checkpoint summary, got: {reply}"
|
||||
);
|
||||
// The transcript ends on the assistant checkpoint (well-formed), which
|
||||
// is what lets the user's next message resume the task cleanly.
|
||||
assert!(
|
||||
matches!(
|
||||
agent.history().last(),
|
||||
Some(ConversationMessage::Chat(msg))
|
||||
if msg.role == "assistant" && msg.content.contains("Next steps")
|
||||
),
|
||||
"history should end on the assistant checkpoint, got: {:?}",
|
||||
agent.history().last()
|
||||
);
|
||||
}
|
||||
|
||||
@@ -714,7 +732,11 @@ async fn xml_dispatcher_does_not_send_tool_specs() {
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_handles_empty_text_response() {
|
||||
async fn turn_errors_on_empty_text_response() {
|
||||
// A completion with no text *and* no tool calls is never a valid final
|
||||
// answer. The old behaviour returned `Ok("")`, which rendered as a blank
|
||||
// reply and silently wedged the thread; now it surfaces as a visible
|
||||
// error the user can retry on (bug-report-2026-05-26 A1).
|
||||
let provider = Box::new(ScriptedProvider::new(vec![ChatResponse {
|
||||
text: Some(String::new()),
|
||||
tool_calls: vec![],
|
||||
@@ -723,12 +745,18 @@ async fn turn_handles_empty_text_response() {
|
||||
|
||||
let (mut agent, _tmp) = build_agent_with(provider, vec![], Box::new(NativeToolDispatcher));
|
||||
|
||||
let response = agent.turn("hi").await.unwrap();
|
||||
assert!(response.is_empty());
|
||||
let err = agent
|
||||
.turn("hi")
|
||||
.await
|
||||
.expect_err("an empty provider response should surface as an error");
|
||||
assert!(
|
||||
err.to_string().contains("empty response"),
|
||||
"expected an empty-response error, got: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_handles_none_text_response() {
|
||||
async fn turn_errors_on_none_text_response() {
|
||||
let provider = Box::new(ScriptedProvider::new(vec![ChatResponse {
|
||||
text: None,
|
||||
tool_calls: vec![],
|
||||
@@ -737,9 +765,14 @@ async fn turn_handles_none_text_response() {
|
||||
|
||||
let (mut agent, _tmp) = build_agent_with(provider, vec![], Box::new(NativeToolDispatcher));
|
||||
|
||||
// Should not panic — falls back to empty string
|
||||
let response = agent.turn("hi").await.unwrap();
|
||||
assert!(response.is_empty());
|
||||
let err = agent
|
||||
.turn("hi")
|
||||
.await
|
||||
.expect_err("a null-text provider response should surface as an error");
|
||||
assert!(
|
||||
err.to_string().contains("empty response"),
|
||||
"expected an empty-response error, got: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
||||
Reference in New Issue
Block a user