mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(agent-harness): escalate user-actionable blockers to the user with a concrete ask (#4092) (#4482)
This commit is contained in:
@@ -1638,6 +1638,47 @@ impl RepeatedToolFailureMiddleware {
|
||||
}
|
||||
}
|
||||
|
||||
/// Recognise a **user-actionable** blocker in a failing tool result — one only
|
||||
/// the user can clear — and phrase the halt as a direct ask instead of the
|
||||
/// crate's generic "the goal looks unreachable in this environment, report this
|
||||
/// back" summary (issue #4092). Today that's a missing service connection (the
|
||||
/// issue's canonical example: acting on a service that isn't connected). Such a
|
||||
/// failure will never self-resolve by retrying, and the fix is the user's, so
|
||||
/// escalate with a concrete next step instead of looping or reporting a generic
|
||||
/// dead-end. Returns `None` for failures that are not user-actionable, leaving
|
||||
/// the crate's summary in place.
|
||||
fn user_actionable_escalation(tool: &str, error: &str) -> Option<String> {
|
||||
let lower = error.to_lowercase();
|
||||
let permission_or_scope_failure = lower.contains("[composio:error:insufficient_scope]")
|
||||
|| lower.contains("[composio:error:trigger_permission]")
|
||||
|| lower.contains("insufficient scope")
|
||||
|| lower.contains("insufficient authentication scopes")
|
||||
|| lower.contains("insufficient permissions")
|
||||
|| lower.contains("missing required permissions")
|
||||
|| lower.contains("permission to manage triggers");
|
||||
if permission_or_scope_failure {
|
||||
return None;
|
||||
}
|
||||
// Keep this narrow: some scope/permission failures legitimately tell the
|
||||
// user to reconnect in Settings, but they are not missing connections.
|
||||
let missing_connection = lower.contains("[composio:error:composio_platform]")
|
||||
|| lower.contains("not connected")
|
||||
|| lower.contains("isn't connected")
|
||||
|| lower.contains("is not connected")
|
||||
|| lower.contains("not enabled")
|
||||
|| lower.contains("token revoked")
|
||||
|| lower.contains("connection error, try to authenticate");
|
||||
if !missing_connection {
|
||||
return None;
|
||||
}
|
||||
Some(format!(
|
||||
"I can't continue without your input: the `{tool}` action needs a service that isn't \
|
||||
connected. {}\n\nConnect it (Settings \u{2192} Connections), then tell me to retry — or \
|
||||
tell me how you'd like to proceed instead.",
|
||||
crate::openhuman::util::truncate_with_ellipsis(error, 400),
|
||||
))
|
||||
}
|
||||
|
||||
/// A stable, bounded fingerprint of a tool call's arguments for the identical-
|
||||
/// repeat signature (hashed so a huge payload doesn't bloat the map/comparison).
|
||||
fn args_fingerprint(arguments: &serde_json::Value) -> String {
|
||||
@@ -1725,10 +1766,20 @@ impl Middleware<()> for RepeatedToolFailureMiddleware {
|
||||
)));
|
||||
}
|
||||
NoProgress::Halt(summary) => {
|
||||
// #4092: if the blocker is user-actionable (a missing connection),
|
||||
// escalate with a concrete ask instead of the crate's generic
|
||||
// "unreachable environment, report back" summary.
|
||||
let escalation = user_actionable_escalation(
|
||||
&result.name,
|
||||
result.error.as_deref().unwrap_or(result.content.as_str()),
|
||||
);
|
||||
let user_actionable = escalation.is_some();
|
||||
let summary = escalation.unwrap_or(summary);
|
||||
tracing::warn!(
|
||||
tool = %result.name,
|
||||
step,
|
||||
hard_reject,
|
||||
user_actionable,
|
||||
"[tinyagents::mw] repeated tool failure — halting run so the root cause surfaces"
|
||||
);
|
||||
if let Ok(mut slot) = self.halt_summary.lock() {
|
||||
@@ -2258,6 +2309,77 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_actionable_escalation_detects_missing_connection() {
|
||||
// A not-connected blocker → a user-directed ask with a concrete next step.
|
||||
let ask = user_actionable_escalation(
|
||||
"gmail_send",
|
||||
"Gmail is not connected. Ask the user to connect 'gmail' in Settings → Connections.",
|
||||
)
|
||||
.expect("a missing-connection failure is user-actionable");
|
||||
assert!(ask.contains("without your input"));
|
||||
assert!(ask.contains("Settings"));
|
||||
assert!(ask.to_lowercase().contains("connect"));
|
||||
assert!(ask.contains("gmail_send"));
|
||||
// The original tool text is relayed so the user sees which service.
|
||||
assert!(ask.to_lowercase().contains("gmail"));
|
||||
|
||||
// A plain environment failure is NOT user-actionable → keep crate summary.
|
||||
assert!(user_actionable_escalation("read_file", "file not found").is_none());
|
||||
assert!(user_actionable_escalation("shell", "exit code 1: segfault").is_none());
|
||||
assert!(user_actionable_escalation(
|
||||
"gmail_send",
|
||||
"[composio:error:insufficient_scope] `gmail_send` was rejected because the connected \
|
||||
gmail account is missing required permissions (insufficient authentication scopes). \
|
||||
Reconnect the integration in Settings → Connections → gmail and grant the scopes \
|
||||
requested during OAuth."
|
||||
)
|
||||
.is_none());
|
||||
assert!(user_actionable_escalation(
|
||||
"gmail_trigger",
|
||||
"[composio:error:trigger_permission] Couldn't enable this trigger: the connected \
|
||||
gmail account doesn't have permission to manage triggers. Reconnect gmail in \
|
||||
Settings → Connections → gmail and grant the permissions requested during OAuth, \
|
||||
then try again."
|
||||
)
|
||||
.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn halt_on_missing_connection_asks_the_user_instead_of_reporting_back() {
|
||||
// #4092: a repeated not-connected failure halts with a user-directed ask,
|
||||
// not the crate's generic "unreachable environment, report this back".
|
||||
let handle = SteeringHandle::allow_all();
|
||||
let slot = std::sync::Arc::new(std::sync::Mutex::new(None));
|
||||
let mw = RepeatedToolFailureMiddleware::new(handle.clone(), 3, slot.clone());
|
||||
// Three identical not-connected failures → halt.
|
||||
for _ in 0..3 {
|
||||
let mut r = failing_result(
|
||||
"slack_post",
|
||||
"Slack is not connected — connect it in Settings → Connections.",
|
||||
);
|
||||
mw.after_tool(&mut ctx(), &(), &mut r).await.unwrap();
|
||||
}
|
||||
let summary = slot
|
||||
.lock()
|
||||
.unwrap()
|
||||
.clone()
|
||||
.expect("halt records a summary");
|
||||
assert!(
|
||||
summary.contains("without your input") && summary.contains("Settings"),
|
||||
"the halt should ask the user to connect the service: {summary}"
|
||||
);
|
||||
assert!(
|
||||
!summary.contains("Report this back"),
|
||||
"a user-actionable blocker must not use the generic report-back summary: {summary}"
|
||||
);
|
||||
assert_eq!(
|
||||
drain_pause_count(&handle),
|
||||
1,
|
||||
"it still pauses the run to surface the ask"
|
||||
);
|
||||
}
|
||||
|
||||
/// Collect the nudge system-message texts drained from `handle`. The nudge
|
||||
/// rides the `InjectMessage` lane (not `Redirect`) so it is permitted on the
|
||||
/// user's interactive turn — see the test below.
|
||||
|
||||
Reference in New Issue
Block a user