Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions src/openhuman/agent/agents/integrations_agent/prompt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ mod tests {
tools: Vec::new(),
gated_tools: Vec::new(),
connected: true,
non_active_status: None,
}];
let body = build(&ctx_with(&integrations, &[])).unwrap();
assert!(body.contains("## Connected Integrations"));
Expand All @@ -265,6 +266,7 @@ mod tests {
tools: Vec::new(),
gated_tools: Vec::new(),
connected: false,
non_active_status: None,
}];
let body = build(&ctx_with(&integrations, &[])).unwrap();
assert!(!body.contains("## Connected Integrations"));
Expand Down
5 changes: 5 additions & 0 deletions src/openhuman/agent/agents/orchestrator/prompt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ mod tests {
tools: Vec::new(),
gated_tools: Vec::new(),
connected: true,
non_active_status: None,
}];
let body = build(&ctx_with(&integrations)).unwrap();
assert!(body.contains("## Connected Integrations"));
Expand All @@ -245,6 +246,7 @@ mod tests {
tools: Vec::new(),
gated_tools: Vec::new(),
connected: true,
non_active_status: None,
}];
let body = build(&ctx_with(&integrations)).unwrap();
assert!(body.contains("## Connected Integrations"));
Expand All @@ -267,13 +269,15 @@ mod tests {
tools: Vec::new(),
gated_tools: Vec::new(),
connected: true,
non_active_status: None,
},
ConnectedIntegration {
toolkit: "linear".into(),
description: "Tracker.".into(),
tools: Vec::new(),
gated_tools: Vec::new(),
connected: false,
non_active_status: None,
},
];
let body = build(&ctx_with(&integrations)).unwrap();
Expand All @@ -289,6 +293,7 @@ mod tests {
tools: Vec::new(),
gated_tools: Vec::new(),
connected: false,
non_active_status: None,
}];
let body = build(&ctx_with(&integrations)).unwrap();
assert!(!body.contains("## Connected Integrations"));
Expand Down
2 changes: 2 additions & 0 deletions src/openhuman/agent/agents/welcome/prompt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,15 @@ mod tests {
tools: Vec::new(),
gated_tools: Vec::new(),
connected: true,
non_active_status: None,
},
ConnectedIntegration {
toolkit: "notion".into(),
description: "Pitch during onboarding.".into(),
tools: Vec::new(),
gated_tools: Vec::new(),
connected: false,
non_active_status: None,
},
];
let body = build(&ctx_with(&integrations)).unwrap();
Expand Down
4 changes: 4 additions & 0 deletions src/openhuman/agent/harness/subagent_runner/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,10 @@ async fn run_typed_mode(
// the user pref and doesn't change per-spawn.
gated_tools: cached_integration.gated_tools.clone(),
connected: cached_integration.connected,
// Inherit the cached non-active status — this spawn
// path only fires on connected toolkits, but keep the
// field consistent with the source row for #2365.
non_active_status: cached_integration.non_active_status.clone(),
};
let integration = &integration;
// Fuzzy-filter the toolkit's actions against the task prompt
Expand Down
1 change: 1 addition & 0 deletions src/openhuman/agent/harness/test_support_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1564,6 +1564,7 @@ async fn orchestrator_prompt_drives_composio_call_via_delegation_chain() {
tools: Vec::new(),
gated_tools: Vec::new(),
connected: true,
non_active_status: None,
}];
let ctx = {
use crate::openhuman::context::prompt::{LearnedContextData, ToolCallFormat};
Expand Down
13 changes: 13 additions & 0 deletions src/openhuman/agent/prompts/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,19 @@ pub struct ConnectedIntegration {
/// and the orchestrator must point the user at Settings instead of
/// attempting to delegate.
pub connected: bool,
/// Raw upstream connection status when a connection row exists but
/// is not `ACTIVE` — e.g. `"INITIATED"`, `"INITIALIZING"`,
/// `"FAILED"`, `"EXPIRED"`. `None` means either the user is
/// `ACTIVE` (use `connected = true`) OR there is no connection
/// row at all (truly disconnected).
///
/// Used by the `integrations_agent` spawn-gate to surface the
/// real reason a delegation can't proceed — see issue #2365
/// ("Agent says Gmail is disconnected when sending email"). The
/// gate previously emitted the same "not authorized yet" message
/// regardless of whether OAuth was mid-flight, the token had
/// expired, or the user had simply never started the flow.
pub non_active_status: Option<String>,
}

/// A toolkit action that exists in the catalog but is currently hidden from
Expand Down
50 changes: 50 additions & 0 deletions src/openhuman/composio/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1668,6 +1668,51 @@ async fn fetch_connected_integrations_uncached(
.filter(|toolkit| !toolkit.is_empty())
.collect();

// Most-informative *non-active* status per toolkit slug. Lets the
// integrations_agent spawn-gate (#2365) emit a precise message
// when a connection row exists but isn't usable yet (`INITIATED`
// — OAuth still in progress) or any longer (`EXPIRED` / `FAILED`)
// — instead of the legacy generic "available but not authorized".
//
// Status priority (UI-actionability):
// 1. EXPIRED — reconnect path
// 2. FAILED / ERROR — reconnect path
// 3. INITIATED / INITIALIZING / PENDING — finish OAuth in browser
// 4. anything else — passes through verbatim
let non_active_status_by_slug: std::collections::HashMap<String, String> = {
fn priority(status: &str) -> u8 {
let s = status.trim().to_ascii_uppercase();
match s.as_str() {
"EXPIRED" => 4,
"FAILED" | "ERROR" => 3,
"INITIATED" | "INITIALIZING" | "PENDING" => 2,
_ => 1,
}
}
let mut map: std::collections::HashMap<String, (u8, String)> =
std::collections::HashMap::new();
for conn in connections.iter().filter(|c| !c.is_active()) {
let slug = conn.normalized_toolkit();
if slug.is_empty() {
continue;
}
// Don't override an ACTIVE-slug — those carry no non-active
// status from this map's perspective.
if connected_slugs.contains(&slug) {
continue;
}
let p = priority(&conn.status);
map.entry(slug)
.and_modify(|cur| {
if p > cur.0 {
*cur = (p, conn.status.clone());
}
})
.or_insert_with(|| (p, conn.status.clone()));
}
map.into_iter().map(|(k, (_, v))| (k, v)).collect()
};

// Deduplicate the allowlist so a backend that returns duplicates
// doesn't produce dual entries downstream.
let mut unique_toolkits: Vec<String> = allowlisted_toolkits.clone();
Expand Down Expand Up @@ -1764,6 +1809,11 @@ async fn fetch_connected_integrations_uncached(
tools,
gated_tools,
connected,
non_active_status: if connected {
None
} else {
non_active_status_by_slug.get(slug).cloned()
},
});
}

Expand Down
1 change: 1 addition & 0 deletions src/openhuman/composio/ops_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,7 @@ fn integration(toolkit: &str, connected: bool) -> ConnectedIntegration {
tools: Vec::new(),
gated_tools: Vec::new(),
connected,
non_active_status: None,
}
}

Expand Down
159 changes: 152 additions & 7 deletions src/openhuman/tools/impl/agent/spawn_subagent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,13 +352,21 @@ impl Tool for SpawnSubagentTool {
// doesn't render this as a failed tool call.
// The model still reads the explanation and produces
// an appropriate user-facing response.
return Ok(ToolResult::success(format!(
"Integration '{tk}' is available but the user has not \
authorized it yet. Do NOT retry this spawn. Tell the user \
the integration is available and ask them to authorize \
'{tk}' in Connections → Integrations before retrying the \
original request."
)));
//
// Split (#2365) into 4 cases driven by the upstream
// status field on the most-informative connection
// row, instead of the legacy generic
// "not authorized yet" copy. Before this split,
// an OAuth-in-progress / expired / failed Gmail
// surfaced the same "you need to connect Gmail"
// message — which Settings UI contradicted (it
// shows the connection as initiated/expired), so
// users concluded the agent was confused.
let message = describe_unconnected_state(
&ci.toolkit,
ci.non_active_status.as_deref(),
);
return Ok(ToolResult::success(message));
}
Some(_) => {
tracing::debug!(
Expand Down Expand Up @@ -631,6 +639,63 @@ fn render_worker_thread_result(
)
}

/// Build the user-facing explanation for an allowlisted-but-not-active
/// integration during an `integrations_agent` spawn (#2365).
///
/// The single message that previously covered every cause ("available
/// but the user has not authorized it yet") looked confused to users
/// who had Gmail showing in Settings (because Settings reflects the
/// FE's optimistic post-OAuth view, while the spawn gate reads the
/// backend's authoritative status). We now pivot on the upstream
/// connection status:
///
/// - `INITIATED` / `INITIALIZING` / `PENDING` — OAuth in progress;
/// ask the user to finish the flow in their browser.
/// - `EXPIRED` — token rolled over; reconnect.
/// - `FAILED` / `ERROR` — handshake didn't land; reconnect.
/// - any other non-active status — quote the upstream verbatim.
/// - `None` — no connection row at all (truly disconnected).
///
/// Returns text the model reads literally; the orchestrator paraphrases
/// it into a user-facing reply. Keep the *intent* stable across
/// rewordings — the "Settings → Connections → {toolkit}" path is
/// load-bearing for the UI navigation tests.
pub(crate) fn describe_unconnected_state(toolkit: &str, status: Option<&str>) -> String {
match status.map(|s| s.trim().to_ascii_uppercase()).as_deref() {
Some("INITIATED") | Some("INITIALIZING") | Some("PENDING") => format!(
"Integration '{toolkit}' has an OAuth flow in progress but it hasn't reached \
ACTIVE yet. Do NOT retry this spawn. Tell the user the authorization is \
pending and ask them to finish the browser OAuth flow (Settings → \
Connections → '{toolkit}') before retrying. If they already closed the \
browser tab, they can restart the connection from the same Settings page."
),
Some("EXPIRED") => format!(
"Integration '{toolkit}' is connected but the OAuth token has expired. \
Do NOT retry this spawn. Tell the user the connection expired and ask \
them to reconnect '{toolkit}' at Settings → Connections → '{toolkit}' \
before retrying the original request."
),
Some("FAILED") | Some("ERROR") => format!(
"Integration '{toolkit}' has a previous OAuth attempt in a FAILED state. \
Do NOT retry this spawn. Tell the user the connection failed and ask them \
to reconnect '{toolkit}' at Settings → Connections → '{toolkit}' before \
retrying the original request."
),
Some(other) if !other.is_empty() => format!(
"Integration '{toolkit}' has a connection row but its status is `{other}`, \
which is not yet usable. Do NOT retry this spawn. Tell the user the \
connection is in an unusable state and ask them to reconnect '{toolkit}' \
at Settings → Connections → '{toolkit}'."
),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
_ => format!(
"Integration '{toolkit}' is available but the user has not authorized it \
yet. Do NOT retry this spawn. Tell the user the integration is available \
and ask them to authorize '{toolkit}' in Settings → Connections → \
'{toolkit}' before retrying the original request."
),
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -887,4 +952,84 @@ mod tests {
)));
assert!(out.contains("Valid toolkits"));
}

// ── #2365: describe_unconnected_state per upstream status ───────

#[test]
fn describe_unconnected_state_initiated_says_oauth_in_progress() {
let msg = describe_unconnected_state("gmail", Some("INITIATED"));
assert!(
msg.contains("OAuth flow in progress"),
"INITIATED must surface the in-progress wording: {msg}"
);
assert!(msg.contains("Settings → Connections → 'gmail'"));
// The legacy "not authorized yet" copy must NOT leak into the
// pending-OAuth branch — that was the user-perception bug
// from #2365 (Settings UI showed Gmail connected, agent said
// "not authorized").
assert!(
!msg.contains("has not authorized it yet"),
"INITIATED must not borrow the truly-disconnected copy: {msg}"
);
}

#[test]
fn describe_unconnected_state_pending_and_initializing_are_aliased() {
for status in ["PENDING", "INITIALIZING"] {
let msg = describe_unconnected_state("gmail", Some(status));
assert!(
msg.contains("OAuth flow in progress"),
"{status} must hit the in-progress branch: {msg}"
);
}
}

#[test]
fn describe_unconnected_state_expired_says_reconnect() {
let msg = describe_unconnected_state("gmail", Some("EXPIRED"));
assert!(msg.contains("OAuth token has expired"));
assert!(msg.contains("reconnect 'gmail'"));
assert!(!msg.contains("OAuth flow in progress"));
}

#[test]
fn describe_unconnected_state_failed_and_error_route_to_reconnect() {
for status in ["FAILED", "ERROR"] {
let msg = describe_unconnected_state("gmail", Some(status));
assert!(
msg.contains("FAILED state"),
"{status} must classify as failed: {msg}"
);
assert!(msg.contains("reconnect 'gmail'"));
}
}

#[test]
fn describe_unconnected_state_quotes_unknown_status_verbatim() {
let msg = describe_unconnected_state("gmail", Some("DEAUTH_REQUIRED"));
assert!(
msg.contains("`DEAUTH_REQUIRED`"),
"unknown statuses must be quoted so triage can act on them: {msg}"
);
}

#[test]
fn describe_unconnected_state_none_is_truly_disconnected() {
let msg = describe_unconnected_state("gmail", None);
assert!(
msg.contains("has not authorized it yet"),
"None must hit the legacy never-connected copy: {msg}"
);
assert!(msg.contains("Settings → Connections → 'gmail'"));
}

#[test]
fn describe_unconnected_state_status_match_is_case_insensitive() {
// The status string flows in from Composio's wire format; we
// can't assume casing. The classifier must normalise.
let initiated = describe_unconnected_state("gmail", Some("initiated"));
assert!(initiated.contains("OAuth flow in progress"));
let expired = describe_unconnected_state("gmail", Some("Expired"));
assert!(expired.contains("OAuth token has expired"));
}
}
3 changes: 3 additions & 0 deletions src/openhuman/tools/orchestrator_tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ mod tests {
tools: vec![],
gated_tools: vec![],
connected: true,
non_active_status: None,
}
}

Expand Down Expand Up @@ -465,6 +466,7 @@ mod tests {
tools: vec![],
gated_tools: vec![],
connected: false, // not connected — must not appear in the enum
non_active_status: None,
},
integration("notion", "Read and write pages."),
];
Expand Down Expand Up @@ -539,6 +541,7 @@ mod tests {
tools: vec![],
gated_tools: vec![],
connected: true,
non_active_status: None,
},
integration("gmail", "Email."),
];
Expand Down
Loading