fix(composio): let a provider reshape supersede the backend markdown rendering - #5323
Open
yh928 wants to merge 7 commits into
Open
fix(composio): let a provider reshape supersede the backend markdown rendering#5323yh928 wants to merge 7 commits into
yh928 wants to merge 7 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughComposio now gates mutating or unknown actions, preserves execution arguments, and applies provider-specific response reshaping. Gmail thread-list results become compact summaries, with backend Markdown replaced only when appropriate. ChangesComposio action flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ComposioActionTool
participant ComposioExecuteTool
participant ProviderRegistry
participant GmailProvider
ComposioActionTool->>ComposioExecuteTool: execute Composio action
ComposioExecuteTool->>ProviderRegistry: resolve provider for successful slug
ProviderRegistry->>GmailProvider: select Gmail provider
ComposioExecuteTool->>GmailProvider: reshape response
GmailProvider-->>ComposioExecuteTool: thread summaries and Markdown decision
ComposioExecuteTool-->>ComposioActionTool: publish model-facing result
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
|
| Filename | Overview |
|---|---|
| src/openhuman/memory/sync/composio/providers/gmail/post_process.rs | Adds reshape_list_threads / reshape_thread, fixes case-sensitive dispatch to use eq_ignore_ascii_case, and promotes is_raw_html_flag_set to pub(super) for use by provider.rs. |
| src/openhuman/memory/sync/composio/providers/gmail/provider.rs | Implements reshape_supersedes_markdown returning true only for GMAIL_LIST_THREADS without raw_html; well-tested with supersede, non-supersede, and raw_html edge cases. |
| src/openhuman/memory/sync/composio/providers/traits.rs | Adds reshape_supersedes_markdown to the ComposioProvider trait with a safe false default; doc block clearly documents the per-action vs per-toolkit design choice and the raw_html edge case. |
| src/openhuman/integrations/composio/tools.rs | Extracts resolve_action_scope_sync from the async wrapper, adds action_mutates_external_state, and implements external_effect_with_args on ComposioExecuteTool — correctly errs toward gating on absent/empty slugs via is_none_or. |
| src/openhuman/integrations/composio/action_tool.rs | Adds external_effect_with_args and the reshape+supersede block to ComposioActionTool, mirroring ComposioExecuteTool; reshape_args clone is necessary and correctly timed before the dispatch consumes args. |
| src/openhuman/integrations/composio/tools_tests.rs | Adds execute_tool_gates_writes_but_not_reads_via_external_effect covering send, delete, read, and absent-slug cases for the dispatcher's approval gate. |
Sequence Diagram
sequenceDiagram
participant Agent
participant Gate as ApprovalGate
participant Tool as ComposioExecuteTool / ComposioActionTool
participant Provider as GmailProvider
participant Backend as Composio Backend
Agent->>Tool: call(args)
Tool->>Tool: external_effect_with_args(args)
alt slug is write/admin or absent
Tool->>Gate: route through approval
Gate-->>Agent: show approval card
Agent-->>Gate: approve
end
Tool->>Backend: execute action
Backend-->>Tool: "resp { data, markdown_formatted }"
Tool->>Tool: "reshape_args = args.clone()"
Tool->>Provider: "post_process_action_result(slug, reshape_args, &mut resp.data)"
Note over Provider: reshape_list_threads() rewrites data in-place
Tool->>Provider: reshape_supersedes_markdown(slug, reshape_args)
alt GMAIL_LIST_THREADS and not raw_html
Provider-->>Tool: true
Tool->>Tool: "resp.markdown_formatted = None"
Tool-->>Agent: reshaped JSON (slim thread summaries)
else any other action or raw_html
Provider-->>Tool: false
Tool-->>Agent: resp.markdown_formatted (backend rendering)
end
Reviews (3): Last reviewed commit: "fix(composio): let the reshape and the s..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/openhuman/composio/tools.rs (1)
1530-1560: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGate provider post-processing on
resp.successfulin both dispatch paths. BothComposioExecuteTool::executeandComposioActionTool::executeinvokeprovider.post_process_action_resultandprovider.reshape_supersedes_markdownunconditionally onOk(resp), without checkingresp.successful. The trait doc intraits.rs(lines 214-227) states the hook is only meant to fire for successful responses: "Errors from upstream are not routed here; onlysuccessfulresponses." The shared root cause is one missingresp.successfulguard before the provider hook runs in each dispatch path.
src/openhuman/composio/tools.rs#L1530-L1560: wrap theprovider.post_process_action_result(...)/reshape_supersedes_markdown(...)block inif resp.successful { ... }.src/openhuman/composio/action_tool.rs#L318-L348: apply the sameresp.successfulguard around the equivalent block.🤖 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 1530 - 1560, Gate the provider post-processing blocks on resp.successful so hooks only run for successful responses. In src/openhuman/composio/tools.rs lines 1530-1560, wrap the post_process_action_result and reshape_supersedes_markdown logic in the ComposioExecuteTool::execute path; apply the same guard to the equivalent block in src/openhuman/composio/action_tool.rs lines 318-348 for ComposioActionTool::execute.
🤖 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.
Inline comments:
In `@src/openhuman/memory_sync/composio/providers/gmail/post_process.rs`:
- Around line 68-73: Update the slug dispatch in the Gmail post-processing
function to use case-insensitive matching for both GMAIL_FETCH_EMAILS and
GMAIL_LIST_THREADS, while preserving the existing fallback behavior for unknown
slugs. Ensure lowercase or mixed-case tool arguments invoke the corresponding
reshape function.
---
Nitpick comments:
In `@src/openhuman/composio/tools.rs`:
- Around line 1530-1560: Gate the provider post-processing blocks on
resp.successful so hooks only run for successful responses. In
src/openhuman/composio/tools.rs lines 1530-1560, wrap the
post_process_action_result and reshape_supersedes_markdown logic in the
ComposioExecuteTool::execute path; apply the same guard to the equivalent block
in src/openhuman/composio/action_tool.rs lines 318-348 for
ComposioActionTool::execute.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c11e9440-0745-4768-a072-4bd31fea7a2f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
src/openhuman/composio/action_tool.rssrc/openhuman/composio/tools.rssrc/openhuman/composio/tools_tests.rssrc/openhuman/memory_sync/composio/providers/gmail/post_process.rssrc/openhuman/memory_sync/composio/providers/gmail/provider.rssrc/openhuman/memory_sync/composio/providers/traits.rs
yh928
added a commit
to yh928/openhuman
that referenced
this pull request
Aug 5, 2026
…e same call Two ways the pair could disagree, and both handed the model the raw MIME tree — neither the backend summary nor the slim envelope. **Casing.** `post_process` dispatched on a case-sensitive match while `reshape_supersedes_markdown` folded case. `composio_execute` takes the action as an argument, so a lowercase `gmail_list_threads` reaches both verbatim: the dispatch fell to its no-op arm, the predicate still said "my version supersedes", and the rendering was cleared on behalf of a reshape that never ran. The action is the same action whatever the model capitalises; only one of the two may decide that, so the dispatch now folds case too. **`raw_html`.** The flag makes `post_process` return early and leave `data` untouched — that is its whole point, for `GMAIL_FETCH_EMAILS` where the caller wants the original body. A slug-only predicate could not see it, so the same clearing happened for a pass-through response. The answer is a property of the call rather than of the slug, so `reshape_supersedes_markdown` now takes the caller's arguments and answers `false` when the reshape was opted out of. Reported by greptile on tinyhumansai#5323 (P1 + P2).
…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
…patcher does `ComposioExecuteTool` reshapes then publishes; the per-action tool published then reshaped. The payload names no reshaped field today, so the order is not observable — but two surfaces describing the same action must not disagree about which snapshot the event saw, or the first field added to it diverges silently between them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
…rendering GMAIL_LIST_THREADS is rendered by the backend as a bare list of thread ids. The payload behind it carries each thread's subject, sender, date, labels, and snippet, and reshape_list_threads lifts them — but both dispatch paths build the model-facing body by preferring markdownFormatted and falling back to the JSON envelope only when absent, so the reshape was written and then never read. Live, a sub-agent searching for mail that does exist got the ids, had nothing to recognise the thread by, and reported it could not be found. ComposioProvider::reshape_supersedes_markdown(slug) lets a provider say its rewrite replaces the rendering; both call sites clear markdown_formatted when it answers true. Per-action, not per-toolkit, because within Gmail the answer differs: GMAIL_FETCH_EMAILS's reshape READS markdownFormatted for the message body, so clearing it there would throw the body away. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
yh928
added a commit
to yh928/openhuman
that referenced
this pull request
Aug 5, 2026
…e same call Two ways the pair could disagree, and both handed the model the raw MIME tree — neither the backend summary nor the slim envelope. **Casing.** `post_process` dispatched on a case-sensitive match while `reshape_supersedes_markdown` folded case. `composio_execute` takes the action as an argument, so a lowercase `gmail_list_threads` reaches both verbatim: the dispatch fell to its no-op arm, the predicate still said "my version supersedes", and the rendering was cleared on behalf of a reshape that never ran. The action is the same action whatever the model capitalises; only one of the two may decide that, so the dispatch now folds case too. **`raw_html`.** The flag makes `post_process` return early and leave `data` untouched — that is its whole point, for `GMAIL_FETCH_EMAILS` where the caller wants the original body. A slug-only predicate could not see it, so the same clearing happened for a pass-through response. The answer is a property of the call rather than of the slug, so `reshape_supersedes_markdown` now takes the caller's arguments and answers `false` when the reshape was opted out of. Reported by greptile on tinyhumansai#5323 (P1 + P2).
yh928
force-pushed
the
fix/gmail-thread-list-body
branch
from
August 5, 2026 01:54
29afd8d to
4b3b202
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/openhuman/integrations/composio/tools.rs`:
- Around line 1543-1559: Run provider post-processing only for successful
responses: in src/openhuman/integrations/composio/tools.rs#L1543-L1559, guard
both ComposioProvider::post_process_action_result and
reshape_supersedes_markdown with resp.successful; apply the same guard in
src/openhuman/integrations/composio/action_tool.rs#L324-L342 before publishing
and serializing the failure response.
In `@vendor/tinyflows`:
- Line 1: Update the vendor/tinyflows submodule gitlink to a commit that exists
in the configured upstream repository, replacing the unreachable
ff9950186ac66775a554c80aa420920b4f39e4de pin. Verify the new pin can be fetched
from https://github.com/tinyhumansai/tinyflows.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4c1037e4-ee99-46c9-8510-d428c807584d
📒 Files selected for processing (9)
src/openhuman/integrations/composio/action_tool.rssrc/openhuman/integrations/composio/tools.rssrc/openhuman/integrations/composio/tools_tests.rssrc/openhuman/memory/sync/composio/providers/gmail/post_process.rssrc/openhuman/memory/sync/composio/providers/gmail/provider.rssrc/openhuman/memory/sync/composio/providers/traits.rsvendor/tinychannelsvendor/tinyflowsvendor/tinyhumans-sdk
…e same call Two ways the pair could disagree, and both handed the model the raw MIME tree — neither the backend summary nor the slim envelope. **Casing.** `post_process` dispatched on a case-sensitive match while `reshape_supersedes_markdown` folded case. `composio_execute` takes the action as an argument, so a lowercase `gmail_list_threads` reaches both verbatim: the dispatch fell to its no-op arm, the predicate still said "my version supersedes", and the rendering was cleared on behalf of a reshape that never ran. The action is the same action whatever the model capitalises; only one of the two may decide that, so the dispatch now folds case too. **`raw_html`.** The flag makes `post_process` return early and leave `data` untouched — that is its whole point, for `GMAIL_FETCH_EMAILS` where the caller wants the original body. A slug-only predicate could not see it, so the same clearing happened for a pass-through response. The answer is a property of the call rather than of the slug, so `reshape_supersedes_markdown` now takes the caller's arguments and answers `false` when the reshape was opted out of. Reported by greptile on tinyhumansai#5323 (P1 + P2).
yh928
force-pushed
the
fix/gmail-thread-list-body
branch
from
August 5, 2026 02:10
4b3b202 to
cb5dcc3
Compare
There was a problem hiding this comment.
yh928 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Both execute paths reshaped every `Ok(resp)` before looking at `resp.successful`. A failure carries the provider's diagnostics in `data`, so a reshaper written against the success shape rewrote them into an empty or wrong-shaped record — and `reshape_supersedes_markdown` then cleared the backend's error rendering on behalf of a reshape that had found nothing. The model was left with neither the error nor the diagnostics, which is the same class of loss this PR opened to fix, on the other branch. The rule is named rather than repeated: `provider_for_reshape(slug, successful)` is the one place that says a reshape needs both a registered provider and a response worth reshaping, and both `ComposioExecuteTool` and `ComposioActionTool` now ask it. Repeating the `resp.successful` check inline would leave the third call site to remember it, which is how this one was missed. Tests: a failed `GMAIL_LIST_THREADS` selects no provider while the successful one still does (asserted, so the test cannot pass vacuously), and an unknown slug selects none either way. providers 307, composio::tools 104 pass. Reported by CodeRabbit on tinyhumansai#5323. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
There was a problem hiding this comment.
yh928 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/openhuman/integrations/composio/action_tool.rs`:
- Around line 152-160: Add stable debug logging across the new Composio domain
paths: in action_tool.rs lines 152-160, log the resolved scope and approval
decision using a [domain] or [rpc] prefix and correlation field; in
action_tool.rs lines 318-350, log reshape entry, provider selection,
supersession decisions, and exit with correlation data without logging
reshape_args or response data; in registry.rs lines 71-76, log failed-response
rejection and successful provider resolution with safe slug metadata.
In `@src/openhuman/memory/sync/composio/providers/registry.rs`:
- Around line 181-188: Update
a_failed_response_selects_no_provider_to_reshape_with so it does not register
the shared "gmail" slug with DummyProvider; register a unique test toolkit
instead and use a corresponding action slug in both provider_for_reshape calls,
preserving the success assertion and failed-response assertion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bad4328e-a1f3-41b8-a848-197bb67f97d8
📒 Files selected for processing (8)
src/openhuman/integrations/composio/action_tool.rssrc/openhuman/integrations/composio/tools.rssrc/openhuman/integrations/composio/tools_tests.rssrc/openhuman/memory/sync/composio/providers/gmail/post_process.rssrc/openhuman/memory/sync/composio/providers/gmail/provider.rssrc/openhuman/memory/sync/composio/providers/mod.rssrc/openhuman/memory/sync/composio/providers/registry.rssrc/openhuman/memory/sync/composio/providers/traits.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- src/openhuman/integrations/composio/tools_tests.rs
- src/openhuman/memory/sync/composio/providers/traits.rs
- src/openhuman/memory/sync/composio/providers/gmail/provider.rs
- src/openhuman/memory/sync/composio/providers/gmail/post_process.rs
- src/openhuman/integrations/composio/tools.rs
…tion The provider registry is process-global, so registering a `DummyProvider` under the `gmail` slug hands it to any parallel test that looks Gmail up — the RwLock stops the data race, not the semantic one. Uses a `reshapeprobe` toolkit of the test's own instead; `toolkit_from_slug` splits on the first `_`, so `RESHAPEPROBE_LIST_THINGS` resolves to it and nothing real is displaced. providers::registry 6 pass. Reported by CodeRabbit on tinyhumansai#5323. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
There was a problem hiding this comment.
yh928 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Both new domain branches decided something and said nothing. Adds a debug event for the per-action approval classification (`external_effect`, i.e. whether this slug routes through the gate) and one for reshape-provider selection (`successful`, the condition that was silently wrong before), and puts the existing supersede log on the `composio` target so all three carry the tool slug as their correlation field. integrations::composio tests pass. Reported by CodeRabbit on tinyhumansai#5323. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
There was a problem hiding this comment.
yh928 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
reshape_list_threadsrewritesGMAIL_LIST_THREADSinto a usable thread summary, and nothing read it. This makes it reach the model.ComposioProvider::reshape_supersedes_markdown(slug)lets a provider declare that its rewrite replaces the backend's rendering; both dispatch paths clearmarkdown_formattedwhen it answers true.Problem
Both Composio dispatch paths build the model-facing body the same way: prefer the backend's
markdownFormatted, fall back to the JSON envelope only when it is absent or the call failed.post_process_action_resultreceives onlydata, so a provider cannot clear that field. The reshape runs, rewritesdata, and the model is handed the backend rendering instead.For a thread list the rendering is the whole problem. Captured verbatim from a live verbose response:
Bare ids. The subjects, senders, dates, labels, and snippets are all present in the payload behind that list, and all discarded. Live, a sub-agent searching for a mail that does exist got the ids, had nothing to recognise the thread by, and reported it could not be found.
Solution
A provider answers
reshape_supersedes_markdown(slug); the two call sites clearresp.markdown_formattedwhen it does. Defaultfalse, so a provider that has not thought about it keeps today's behaviour.Per action, not per toolkit, because within Gmail the answer differs.
GMAIL_FETCH_EMAILS's reshape readsmarkdownFormattedfor the message body (extract_markdown_body) — it is already URL-shortened and footer-stripped by the backend, and clearing it there would throw the body away rather than reveal it. OnlyGMAIL_LIST_THREADSreturns true.The trait doc states the trap directly, because it is not visible from the reshape's own file: a reshape of an action the backend also renders is invisible unless the provider says otherwise.
Submission Checklist
Closes #NNNin the## RelatedsectionTesting
the_thread_list_reshape_supersedes_the_backend_rendering— including the case-insensitive form, since the model's casing varies and the action does not.the_message_fetch_reshape_does_not— the failure case that matters:GMAIL_FETCH_EMAILS,GMAIL_FETCH_MESSAGE_BY_THREAD_ID, and a write action must all stay false, or the body is thrown away.Impact
GMAIL_LIST_THREADSonly. Every other action, and every other toolkit, is byte-identical.payload), so context cost goes down, not up.Related
Closes #5319
Summary by CodeRabbit
New Features
Bug Fixes