fix(composio): gate write actions through approval + reshape agent-path results - #5259
fix(composio): gate write actions through approval + reshape agent-path results#5259yh928 wants to merge 1 commit into
Conversation
…ults Two gaps on the agent's Composio execution surface, both diagnosed live. **Approval (P1).** The human-in-the-loop approval card is raised only for tools whose `external_effect_with_args` is true, but neither `composio_execute` nor the per-action `ComposioActionTool` declared it — so a Composio mail send (`GMAIL_SEND_EMAIL`) fired with no approval prompt at all, even when the user had "ask before sending" configured. The contract gate (schema-presence) and `permission_level = Write` (channel caps) do not raise that card. Both surfaces now report external-effect for a write/admin-scoped action and stay false for a pure read, so a write routes through the `ApprovalGate` while a fetch/list flows through unprompted. Scope is classified synchronously (`resolve_action_scope`'s body has no `await`, so it is reused via `resolve_action_scope_sync`). **Reshape (P4, tinyhumansai#2585).** When the agent calls a Composio action directly, a verbose provider envelope — Gmail's full MIME tree under `payload.parts[]` — landed in context on the raw-JSON fallback body. The provider response reshape that slims it (the same one the sync path runs) was only wired into sync; it now runs inline on the agent execute + per-action paths, so `resp.data` is slimmed before it can become the tool body. A backend-rendered `markdown_formatted` body is already clean and unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
📝 WalkthroughWalkthroughComposio tools now classify external effects for gating and apply toolkit-provider response post-processing before serializing action results. Tests cover write, admin, read, and missing-tool cases. ChangesComposio action flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ToolCaller
participant ComposioExecuteTool
participant ActionScopeResolver
participant Dispatcher
participant ToolkitProvider
ToolCaller->>ComposioExecuteTool: provide tool slug and arguments
ComposioExecuteTool->>ActionScopeResolver: resolve action scope
ActionScopeResolver-->>ComposioExecuteTool: effect classification
ComposioExecuteTool->>Dispatcher: dispatch action
Dispatcher-->>ComposioExecuteTool: response data
ComposioExecuteTool->>ToolkitProvider: post-process response data
ToolkitProvider-->>ComposioExecuteTool: reshaped data
ComposioExecuteTool-->>ToolCaller: serialized tool result
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
bug
rust-core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/composio/tools.rs (1)
1477-1549: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftAdd regression tests for response reshaping on both execution paths.
The new behavior is untested: verify shaped
resp.datain the raw-JSON fallback, preservation of backendmarkdown_formatted, and forwarding ofraw_htmlarguments.
src/openhuman/composio/tools.rs#L1477-L1549: add deterministic dispatcher-path coverage insrc/openhuman/composio/tools_tests.rs.src/openhuman/composio/action_tool.rs#L304-L341: add equivalent per-action coverage in this module’s tests.As per coding guidelines, “Untested code is incomplete; add tests for new or changed behavior.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/composio/tools.rs` around lines 1477 - 1549, Add regression tests for response reshaping in src/openhuman/composio/tools.rs lines 1477-1549 by adding deterministic dispatcher-path coverage in src/openhuman/composio/tools_tests.rs, verifying shaped resp.data for the raw-JSON fallback, preservation of backend markdown_formatted, and forwarding of raw_html arguments. Add equivalent per-action coverage for src/openhuman/composio/action_tool.rs lines 304-341 in that module’s tests, covering the same behaviors.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/openhuman/composio/tools.rs`:
- Around line 1477-1549: Add regression tests for response reshaping in
src/openhuman/composio/tools.rs lines 1477-1549 by adding deterministic
dispatcher-path coverage in src/openhuman/composio/tools_tests.rs, verifying
shaped resp.data for the raw-JSON fallback, preservation of backend
markdown_formatted, and forwarding of raw_html arguments. Add equivalent
per-action coverage for src/openhuman/composio/action_tool.rs lines 304-341 in
that module’s tests, covering the same behaviors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 82754407-63cc-4cb4-8159-8cdceb516340
📒 Files selected for processing (3)
src/openhuman/composio/action_tool.rssrc/openhuman/composio/tools.rssrc/openhuman/composio/tools_tests.rs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f13e2bdad
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 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. | ||
| super::tools::action_mutates_external_state(&self.action_name) |
There was a problem hiding this comment.
When an integrations_agent uses a per-action write slug for the first time in a turn (for example GMAIL_SEND_EMAIL), this hook makes ApprovalSecurityMiddleware prompt before execute() runs, but execute() can still hit GateDecision::Surface and return only the contract/schema error before dispatching the action. That means the user approves a no-op schema probe and then gets prompted again on the retry that actually sends; if they deny the first prompt, the model never receives the contract it needs to form the safer retry.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -313,6 +325,20 @@ impl Tool for ComposioActionTool { | |||
| elapsed_ms, | |||
| }, | |||
| ); | |||
| // 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. | |||
| if let Some(provider) = super::providers::toolkit_from_slug(&self.action_name) | |||
| .and_then(|tk| super::providers::get_provider(&tk)) | |||
| { | |||
| provider.post_process_action_result( | |||
| &self.action_name, | |||
| reshape_args.as_ref(), | |||
| &mut resp.data, | |||
| ); | |||
| } | |||
There was a problem hiding this comment.
tools.rs path
In action_tool.rs the DomainEvent::ComposioActionExecuted event is published before post_process_action_result runs, whereas in ComposioExecuteTool::execute (tools.rs) the reshape runs first and the event fires afterwards. The event payload only captures resp.successful, resp.error, resp.cost_usd, and elapsed_ms — none of which are touched by the reshape — so there is no functional difference today. But the asymmetry means a future change that adds resp.data (or a derived field) to the event would silently produce inconsistent snapshots on the two surfaces: the dispatcher event would see slimmed data, and the per-action event would see the raw envelope. Moving the publish_global call to after the reshape block (matching tools.rs) removes the latent divergence at zero cost.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
| Filename | Overview |
|---|---|
| src/openhuman/composio/tools.rs | Adds resolve_action_scope_sync, action_mutates_external_state, and external_effect_with_args on ComposioExecuteTool; inlines the envelope reshape after task-window filtering; introduces reshape_args clone before dispatch to preserve args for post-processing. |
| src/openhuman/composio/action_tool.rs | Adds external_effect_with_args delegating to action_mutates_external_state, clones args as reshape_args before dispatch, and applies the provider envelope reshape after the event-bus publish (order reversed relative to the tools.rs path — minor style inconsistency, not a functional bug). |
| src/openhuman/composio/tools_tests.rs | Adds execute_tool_gates_writes_but_not_reads_via_external_effect covering send/delete (gated), fetch (not gated), and absent slug (gated). Well-structured regression test for the reported bug. |
Sequence Diagram
sequenceDiagram
participant Engine as Agent Engine
participant Gate as ApprovalGate
participant Tool as ComposioExecuteTool / ComposioActionTool
participant Dispatch as execute_dispatch
participant Provider as ProviderReshaper
Engine->>Tool: external_effect_with_args(args)
Tool->>Tool: action_mutates_external_state(slug) via resolve_action_scope_sync
alt write / admin slug (or absent slug)
Tool-->>Engine: true
Engine->>Gate: intercept_audited(...)
Gate-->>Engine: Allow / Deny
else read slug
Tool-->>Engine: false
note over Engine: no approval prompt
end
Engine->>Tool: execute(args)
Tool->>Tool: sandbox gate (ReadOnly check)
Tool->>Dispatch: execute_composio_action_kind(...)
Dispatch-->>Tool: ComposioExecuteResponse
alt provider found for toolkit
Tool->>Provider: post_process_action_result(slug, reshape_args, resp.data)
Provider-->>Tool: data slimmed in-place
end
alt resp.markdown_formatted present and success
Tool-->>Engine: ToolResult::success(markdown)
else
Tool-->>Engine: ToolResult::success(serde_json(resp))
end
Reviews (1): Last reviewed commit: "fix(composio): gate write actions throug..." | Re-trigger Greptile
1 participant
Summary
Two gaps on the agent's Composio execution surface, both diagnosed against a live instance.
Approval gate (the reported bug)
The human-in-the-loop approval card is raised only for a tool whose
external_effect_with_argsistrue, but neithercomposio_executenor the per-actionComposioActionTooldeclared it — so a Composio mail send (GMAIL_SEND_EMAIL) via theintegrations_agentfired with no approval prompt at all, even when the user had configured "ask before sending". Neither the contract gate (which only ensures the action schema is in context) norpermission_level = Write(channel caps: allow/block, not a prompt) raises that card.Both surfaces now report external-effect for a write/admin-scoped action and stay
falsefor a pure read, so a write routes through theApprovalGate(parking for the card under a WebChat turn, which the inline sub-agent inherits) while a fetch/list flows through unprompted — matching theexternal_effectcontract. Scope is classified synchronously:resolve_action_scope's body has noawait, so it is reused via aresolve_action_scope_synccore.Agent-path envelope reshape (#2585)
When the agent calls a Composio action directly, a verbose provider envelope — Gmail's full MIME tree under
payload.parts[], dozens ofReceived:headers — landed in context on the raw-JSON fallback body. The response reshape that slims it into one clean record per message was wired only into the sync path; it now runs inline on the agentcomposio_executeand per-action paths, soresp.datais slimmed before it can become the tool body. A backend-renderedmarkdown_formattedbody is already clean and unaffected.Tests
cargo test --lib composio— new coverage:execute_tool_gates_writes_but_not_reads_via_external_effectandper_action_tool_gates_writes_but_not_readsassert the gate/no-gate split for send/delete vs fetch. (Thecomposio::opsdelete-connection tests are flaky only under the parallel chunk-DB cold-open race, fixed separately in tinycortex; they pass--test-threads=1.)Summary by CodeRabbit
New Features
Bug Fixes
Tests