fix(flows): account for Composio's data-wrapper in tool output paths (B1)#4645
Conversation
…er in tool output paths (B1)
The builder wrote split_out.path='json.messages' for a Gmail source, but the emails
are at json.data.messages → resolved null at runtime → hollow run. Root cause: a
Composio tool_call's runtime output is ComposioExecuteResponse {data:<payload>,
successful,costUsd} (serialized verbatim by OpenHumanTools::invoke, then enveloped
as {json:{data,…},text,raw}), so the payload lives at item.json.data.<…>. But
Composio's output_parameters schema describes only the PAYLOAD (data's content), so
compute_primary_array_path returned 'messages' and the builder dropped the
deterministic 'data.' segment. (Native oh: tool_calls have no wrapper; agent/http
outputs have no wrapper.)
- caps.rs: compute_composio_array_path wraps the pure schema walker, prefixing
'data.' (unless the schema already declares a top-level 'data' property —
schema_has_top_level_data_property guards against double-prefixing).
fetch_live_toolkit_catalog uses it for ToolContract.primary_array_path →
'data.messages' → builder writes 'json.data.messages'. +3 tests.
- ops.rs: parse_node_binding captures the full dotted remainder; graph_output_field_
warnings now flags a tool_call binding MISSING the 'data.' segment (the silent-null
bug), else strips 'data.' and checks the real field; validate_binding_resolvability's
agent-schema check uses the first segment (agents have no wrapper).
- builder_tools.rs + prompt.md: teach that a Composio tool_call output binds via
=nodes.<id>.item.json.data.<field> (agent/http/native oh: unchanged).
cargo check/fmt clean; flows:: 363 pass (+ a regression test that a real-but-missing-
data. binding now warns). 1 pre-existing unrelated failure (on_error=route, repro'd on
pre-fix code).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdates Composio ChangesComposio data envelope fix
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
coderabbitai
Bot
added
bug
rust-core
![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: 52dc7cc636
ℹ️ 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".
| // under `data` (see this fn's doc) — a binding missing that | ||
| // segment is wrong regardless of whether the rest of the path | ||
| // happens to name a real field. | ||
| let Some(field) = field_path.strip_prefix("data.") else { |
There was a problem hiding this comment.
When a downstream node intentionally consumes the whole provider payload with =nodes.fetch.item.json.data (for example as an agent input_context) or checks the execute envelope via .item.json.successful/.item.json.error, this branch treats the binding as missing the data. wrapper and suggests .item.json.data.data or .item.json.data.successful. OpenHumanTools::invoke serializes ComposioExecuteResponse, so data, successful, and error are real top-level fields under item.json; following this warning rewires valid flows to null paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0381718 — good catch. .item.json.data (whole-payload access, e.g. as an agent input_context) and the other ComposioExecuteResponse envelope fields (successful, error, costUsd, markdownFormatted) are now exempted from the "missing data. segment" check before it runs — they're real top-level .item.json.<x> fields alongside data, not inside it. Added two regression tests (graph_wiring_warnings_is_silent_for_a_whole_payload_binding, graph_wiring_warnings_is_silent_for_composio_envelope_metadata_fields) covering exactly the false-positive shapes you flagged.
| if schema_has_top_level_data_property(schema) { | ||
| return Some(path); |
There was a problem hiding this comment.
If a Composio payload schema itself contains a top-level data object with the array inside it, this returns data.messages, so get_tool_contract/the split warning tells authors to use json.data.messages. The runtime value is still the serialized ComposioExecuteResponse, meaning the payload is under the wrapper's data field and the actual array is json.data.data.messages; this causes fan-out flows for such tools to split the wrong path. Avoid dropping the wrapper prefix merely because the payload schema has a data property.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0381718 — you're right, the escape hatch was backwards. output_parameters always describes the unwrapped payload (per ToolContract::output_fields's own invariant), so a payload schema that happens to declare its own top-level data key (e.g. {data: {messages: [...]}}) still needs the wrapper's data. prefix stacked on top — the real runtime path is data.data.messages, not data.messages. Removed schema_has_top_level_data_property and the double-prefix avoidance entirely; compute_composio_array_path now unconditionally prefixes data.. Updated the corresponding test to assert data.data.messages.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…esses @chatgpt-codex-connector on ops.rs:289, caps.rs:1678) graph_output_field_warnings treated `.item.json.data` (whole-payload access) and ComposioExecuteResponse's other top-level fields (successful/error/ costUsd/markdownFormatted) as bindings missing the `data.` wrapper segment, suggesting nonsense rewires like `.item.json.data.data` and `.item.json.data.successful`. Exempt whole-payload access and the envelope's own metadata fields from the check. compute_composio_array_path's escape hatch for a payload schema that itself declares a top-level `data` property was backwards: output_parameters always describes the unwrapped payload (per ToolContract::output_fields' own invariant), so a payload shaped like `{data: {messages: [...]}}` still needs the wrapper's `data.` prefix stacked on top (`data.data.messages`), not `data.messages`. Removed the escape hatch; updated the corresponding test. Added regression tests for both cases.
1 participant
The bug
The builder wrote
split_out.path = "json.messages"for a Gmail source, but the emails are really atjson.data.messages→ resolved null → hollow run ("No email content"). Proven live on flow38ea5d79.Root cause (deterministic, not schema inaccuracy)
A Composio
tool_call's runtime output isComposioExecuteResponse { data: <payload>, successful, costUsd }(serialized verbatim byOpenHumanTools::invoke, then wrapped by the engine as{json:{data,…},text,raw}). So the payload — and its arrays/fields — always live atitem.json.data.<…>. But Composio'soutput_parametersschema describes only the payload (data's content), socompute_primary_array_pathreturnedmessagesand the builder dropped the deterministicdata.segment.Fix
caps.rs—compute_composio_array_pathprefixesdata.(guarded against double-prefix if a schema already nests underdata);ToolContract.primary_array_path→data.messages→ builder writesjson.data.messages.ops.rs— the enforcement gate now flags a tool_call binding missing thedata.segment (the silent-null bug), and checks the real field name underdata. Agent-node checks unchanged.builder_tools.rs+prompt.md— teach: Composio tool_call outputs bind via=nodes.<id>.item.json.data.<field>; agent / http_request / nativeoh:outputs are unchanged (no wrapper).Kills the whole class of "builder guessed the output path" for Composio source tools, deterministically — no live calls, works for unconnected apps. (Robustness follow-up for schema-less/inner-inaccurate actions: real-output probing, designed + logged in
my_docs/flows_workflow_bugs.mdas B12.)Verification
cargo check/fmtclean · flows:: 363 pass (+ a regression test that a real-but-missing-data.binding now warns). 1 pre-existing unrelated failure (on_error=route, repro'd on pre-fix code).Summary by CodeRabbit
Documentation
data.wrapper and correct array-path guidance.Bug Fixes
tool_calloutputs, supporting full dotted field paths and reducing false positives.Tests
data.prefix.data.bindings, plus additional “must be silent” cases.