Skip to content
87 changes: 86 additions & 1 deletion src/openhuman/integrations/composio/action_tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,22 @@ impl Tool for ComposioActionTool {
PermissionLevel::Write
}

fn external_effect_with_args(&self, _args: &Value) -> bool {
// The per-action surface must gate on the approval card exactly like the
// `composio_execute` dispatcher: a write/admin action (this tool's own
// slug) routes through the `ApprovalGate` before it runs; a pure read
// flows through unprompted. Without this the model could send mail /
// create records via a per-action tool with no approval prompt at all.
let mutates = super::tools::action_mutates_external_state(&self.action_name);
tracing::debug!(
target: "composio",
tool = %self.action_name,
external_effect = mutates,
"[composio] per-action approval classification"
);
mutates
}

Comment thread
yh928 marked this conversation as resolved.
fn category(&self) -> ToolCategory {
ToolCategory::Workflow
}
Expand Down Expand Up @@ -292,6 +308,9 @@ impl Tool for ComposioActionTool {
let effective_connection_id = runtime_connection_id
.as_deref()
.or(self.connection_id.as_deref());
// Kept for the post-dispatch envelope reshape (#2585): the dispatch
// consumes `args`, but the reshape reads it for the `raw_html` opt-out.
let reshape_args = args.clone();
let res = super::execute_dispatch::execute_composio_action_kind_with_connection(
kind,
&self.action_name,
Expand All @@ -303,7 +322,52 @@ impl Tool for ComposioActionTool {
let elapsed_ms = started.elapsed().as_millis() as u64;

match res {
Ok(resp) => {
Ok(mut resp) => {
// Slim the provider envelope before it can become the tool body
// (#2585) — the same reshape `ComposioExecuteTool` runs, so a
// per-action Gmail fetch doesn't drop a full MIME tree into the
// agent's context. Only the raw-JSON fallback body serializes
// `resp.data`; a backend-rendered markdown body is unaffected.
// Only a successful response is reshaped. A failure carries the
// provider's diagnostics in `data`, which a reshaper written for
// the success shape rewrites into an empty or wrong-shaped
// record, and `reshape_supersedes_markdown` would then clear the
// backend's error rendering on behalf of a reshape that found
// nothing — leaving the model neither.
tracing::debug!(
target: "composio",
tool = %self.action_name,
successful = resp.successful,
"[composio] reshape provider selection"
);
if let Some(provider) =
super::providers::provider_for_reshape(&self.action_name, resp.successful)
{
provider.post_process_action_result(
&self.action_name,
reshape_args.as_ref(),
&mut resp.data,
);
// A reshape the backend also renders is invisible unless the
// provider says its version supersedes: the body below
// prefers `markdownFormatted` and only falls back to the
// JSON envelope when it is absent.
if provider
.reshape_supersedes_markdown(&self.action_name, reshape_args.as_ref())
{
tracing::debug!(
target: "composio",
tool = %&self.action_name,
"[composio] provider reshape supersedes the backend markdown rendering"
);
resp.markdown_formatted = None;
}
}
// Published after the reshape, matching `ComposioExecuteTool`.
// The payload names no reshaped field today, so the order is not
// observable — but the two surfaces describing the same action
// must not disagree about which snapshot the event saw, or the
// first field added here diverges silently between them.
crate::core::event_bus::publish_global(
crate::core::event_bus::DomainEvent::ComposioActionExecuted {
tool: self.action_name.clone(),
Expand Down Expand Up @@ -745,4 +809,25 @@ mod tests {
"direct-mode tool must not surface backend-session artifacts: {direct_msg}"
);
}

#[test]
fn per_action_tool_gates_writes_but_not_reads() {
// The per-action surface must gate on the approval card exactly like the
// dispatcher: a write action routes through the gate, a read does not.
let send = ComposioActionTool::new(
fake_config(),
"GMAIL_SEND_EMAIL".to_string(),
"send".to_string(),
None,
);
assert!(send.external_effect_with_args(&serde_json::json!({})));

let read = ComposioActionTool::new(
fake_config(),
"GMAIL_FETCH_EMAILS".to_string(),
"fetch".to_string(),
None,
);
assert!(!read.external_effect_with_args(&serde_json::json!({})));
}
}
81 changes: 79 additions & 2 deletions src/openhuman/integrations/composio/tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use crate::openhuman::tools::traits::{
use super::client::{create_composio_client, direct_list_connections, ComposioClientKind};
use super::providers::{
catalog_for_toolkit, classify_unknown, find_curated, get_provider, load_user_scope_or_default,
toolkit_from_slug, ToolScope, UserScopePref,
provider_for_reshape, toolkit_from_slug, ToolScope, UserScopePref,
};
use super::types::ComposioToolsResponse;

Expand Down Expand Up @@ -74,6 +74,15 @@ enum ToolDecision {
/// blocking rather than letting a potentially-mutating action slip
/// through uncategorised.
pub(super) async fn resolve_action_scope(slug: &str) -> ToolScope {
resolve_action_scope_sync(slug)
}

/// Synchronous core of [`resolve_action_scope`]. Every lookup it makes —
/// `toolkit_from_slug`, the provider/curated-catalog resolution, and the
/// `classify_unknown` heuristic — is over static data with no `await`, so a
/// caller that cannot be `async` (the `Tool::external_effect_with_args`
/// gate-decision hook) can classify a slug directly.
pub(super) fn resolve_action_scope_sync(slug: &str) -> ToolScope {
let Some(toolkit) = toolkit_from_slug(slug) else {
return ToolScope::Write;
};
Expand All @@ -88,6 +97,18 @@ pub(super) async fn resolve_action_scope(slug: &str) -> ToolScope {
classify_unknown(slug)
}

/// Whether a Composio action slug mutates external state, i.e. is
/// `Write`/`Admin`-scoped. This is the predicate the approval gate keys off —
/// a write/admin Composio action must route through the human-in-the-loop
/// `ApprovalGate` before it runs, while a pure `Read` flows through unprompted
/// (matching the `external_effect` contract on the `Tool` trait).
pub(super) fn action_mutates_external_state(slug: &str) -> bool {
matches!(
resolve_action_scope_sync(slug),
ToolScope::Write | ToolScope::Admin
)
}

/// Decide whether a Composio action slug should be visible / executable
/// for the current user, given the registered provider's curated list
/// (if any) and the user's stored scope preference.
Expand Down Expand Up @@ -1313,6 +1334,18 @@ impl Tool for ComposioExecuteTool {
// as write-level to respect channel permission caps.
PermissionLevel::Write
}
fn external_effect_with_args(&self, args: &Value) -> bool {
// Route a write/admin Composio action (send mail, create issue, delete,
// …) through the approval gate; a pure read flows through unprompted.
// The action slug is the `tool` argument. An empty/absent slug errs on
// the side of gating rather than letting a possibly-mutating call slip
// past the prompt.
args.get("tool")
.and_then(|v| v.as_str())
.map(str::trim)
.filter(|slug| !slug.is_empty())
.is_none_or(action_mutates_external_state)
}
fn category(&self) -> ToolCategory {
ToolCategory::Workflow
}
Expand Down Expand Up @@ -1441,6 +1474,10 @@ impl Tool for ComposioExecuteTool {
Some(since) => super::task_window::apply_window_args(&tool, arguments, since),
None => arguments,
};
// Kept for the post-dispatch envelope reshape (#2585): the dispatch
// below consumes `arguments`, but the reshape reads it for the
// `raw_html` opt-out.
let reshape_args = arguments.clone();

// Resolve the client through the mode-aware factory on every
// call so a direct-mode toggle takes effect immediately
Expand Down Expand Up @@ -1490,10 +1527,50 @@ impl Tool for ComposioExecuteTool {
// than the window. No-op unless a window is installed AND the
// slug is a curated task-fetch action. Runs before the
// markdown/JSON body decision so the agent reads filtered data.
let resp = match task_window_since {
let mut resp = match task_window_since {
Some(since) => super::task_window::filter_response(&tool, resp, since),
None => resp,
};
// Slim the provider envelope before it can become the tool body
// (#2585). A verbose Composio payload — Gmail's full MIME tree
// under `payload.parts[]`, dozens of `Received:` headers — is
// reshaped into one clean record per message. Only the fallback
// body path serializes `resp.data`, so this shrinks exactly the
// raw-JSON case; a backend-rendered `markdown_formatted` body is
// already clean and unaffected. The sync path applies the same
// reshape via `ReshapingExecutor`; here we run it inline on the
// agent's direct call.
// Only a successful response is reshaped. A failure carries the
// provider's diagnostics in `data`, which a reshaper written for
// the success shape rewrites into an empty or wrong-shaped
// record, and `reshape_supersedes_markdown` would then clear the
// backend's error rendering on behalf of a reshape that found
// nothing — leaving the model neither.
tracing::debug!(
target: "composio",
tool = %tool,
successful = resp.successful,
"[composio] reshape provider selection"
);
if let Some(provider) = provider_for_reshape(&tool, resp.successful) {
provider.post_process_action_result(
&tool,
reshape_args.as_ref(),
&mut resp.data,
);
// A reshape the backend also renders is invisible unless the
// provider says its version supersedes: the body below
// prefers `markdownFormatted` and only falls back to the
// JSON envelope when it is absent.
if provider.reshape_supersedes_markdown(&tool, reshape_args.as_ref()) {
tracing::debug!(
target: "composio",
tool = %&tool,
"[composio] provider reshape supersedes the backend markdown rendering"
);
resp.markdown_formatted = None;
}
}
tracing::info!(
tool = %tool,
successful = resp.successful,
Expand Down
25 changes: 25 additions & 0 deletions src/openhuman/integrations/composio/tools_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1123,3 +1123,28 @@ fn parse_composio_connect_timeout_honors_override_and_zero_opt_out() {
// `0` → opt out of the composio-side bound (fall back to the gate TTL).
assert_eq!(parse_composio_connect_timeout(Some("0")), None);
}

#[test]
fn execute_tool_gates_writes_but_not_reads_via_external_effect() {
// The approval gate keys off `external_effect_with_args`. A write/admin
// Composio action (send/create/delete) must route through the gate; a pure
// read (fetch/list) must flow through unprompted. Regression: neither
// surface declared external_effect, so mail sends fired with no approval.
let t = ComposioExecuteTool::new(fake_config_arc());
assert!(
t.external_effect_with_args(&serde_json::json!({ "tool": "GMAIL_SEND_EMAIL" })),
"a send action must be gated"
);
assert!(
t.external_effect_with_args(&serde_json::json!({ "tool": "GMAIL_DELETE_MESSAGE" })),
"a delete action must be gated"
);
assert!(
!t.external_effect_with_args(&serde_json::json!({ "tool": "GMAIL_FETCH_EMAILS" })),
"a read action must not prompt"
);
assert!(
t.external_effect_with_args(&serde_json::json!({})),
"an absent slug errs on the side of gating"
);
}
Loading
Loading