Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 122 additions & 0 deletions src/openhuman/tinyagents/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Narrow not-connected escalation to user-owned integrations

This broad substring also matches agent-actionable MCP errors: mcp_clients_list_tools returns server_id=... is not connected; connect it first via mcp_clients_connect (src/openhuman/mcp_registry/ops.rs), and the tinyagents adapter turns that into a failed tool result. After repeated calls, this new branch replaces that recovery hint with “Connect it (Settings → Connections)”, even though the correct next step is for the agent to call the MCP connect tool, not for the user to reconnect a service in Settings. Please anchor this escalation to true integration/platform connection failures (or preserve errors that already name a recovery tool).

Useful? React with 👍 / 👎.

|| 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 {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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.
Expand Down
Loading