From c4701a9373d57a1d4c747e89120236cd64500f36 Mon Sep 17 00:00:00 2001 From: Steven Enamakel <31011319+senamakel@users.noreply.github.com> Date: Fri, 3 Jul 2026 20:59:53 -0700 Subject: [PATCH] fix(agent-harness): escalate user-actionable blockers to the user with a concrete ask (#4092) (#4482) --- src/openhuman/tinyagents/middleware.rs | 122 +++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/src/openhuman/tinyagents/middleware.rs b/src/openhuman/tinyagents/middleware.rs index e80797f42..4bde1cb19 100644 --- a/src/openhuman/tinyagents/middleware.rs +++ b/src/openhuman/tinyagents/middleware.rs @@ -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 { + 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.