fix(flows): real-output probe to derive the array path when the tool schema is absent (B16)#4702
Conversation
…field tools (B16)
B1 fixed the ComposioExecuteResponse `data.` wrapper, but the builder still guessed the
array WITHIN data: for GITHUB_LIST_REPOSITORY_ISSUES it wrote split_out.path="json.data"
(the `{issues:[...]}` container) instead of "json.data.issues" (the array) → split emitted
1 mega-item → the condition/agent got the wrong shape → 0 output. The output_parameters
schema doesn't reliably name the array, so schema-walk alone can't find it.
- caps.rs: real-output probe — a bounded READ-ONLY Composio call for read-scope actions
(gated by the tool classifier + a connected toolkit), derives primary_array_path/
output_fields from the ACTUAL response (compute_primary_array_path_from_value walks the
real value, skipping envelope meta keys), cached per slug; overrides the schema hint,
falls back to B1's schema path when unconnected/not-read/failed.
- builder_tools.rs + agent.toml + prompt.md: get_tool_output_sample builder tool so the
builder gets the real array path before wiring split_out; prompt teaches it.
- ops.rs: enforcement — flag a split_out.path that resolves to a NON-array (an object/
scalar), the "wrong array path" signal, with the likely nested path.
For GITHUB_LIST_REPOSITORY_ISSUES: primary_array_path -> data.issues. cargo check clean;
flows:: green (+ probe/value-walk/enforcement tests). Keeps data.messages/data.items green.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a read-only Composio output-sampling probe, wires its results into tool-contract and warning logic, and updates workflow-builder policy, registry, prompt guidance, and tests to use the new ChangesReal-output probe carve-out
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50b0189680
ℹ️ 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".
| return curated.scope; | ||
| } | ||
| } | ||
| } | ||
| classify_unknown(slug) |
There was a problem hiding this comment.
Reject uncurated slugs before probing
For toolkits with a static curated catalog, falling back to classify_unknown when find_curated misses lets get_tool_output_sample execute any uncurated slug whose name looks read-only (for example a connected Gmail/GitHub LIST/GET action). That bypasses the stricter runtime path documented in flow_tool_allowed and mirrored by validate_tool_contracts, which reject uncurated actions for catalogued toolkits; the workflow-builder can now make real Composio read calls that a saved flow would never be allowed to run. Fail closed when a catalog exists but the slug is not curated, and only use the heuristic for toolkits without a static catalog.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0f8aa8f — good catch, confirmed the hole.
resolve_composio_action_scope now returns Option<ToolScope> instead of always falling back to classify_unknown. For a toolkit that ships a static curated catalog (e.g. github), an uncurated slug now resolves to None — refused before any client call, never guessed via the verb heuristic. The heuristic is only consulted when the toolkit has no static catalog at all, matching flow_tool_allowed's Path A / Path B split.
Verified GITHUB_LIST_REPOSITORY_ISSUES (curated Read) is still probe-eligible, and added resolve_composio_action_scope_rejects_an_uncurated_slug_on_a_cataloged_toolkit + probe_tool_output_sample_refuses_an_uncurated_slug_on_a_cataloged_toolkit_before_any_client_call, both using GITHUB_LIST_WORKFLOWS (a real GitHub action deliberately left uncurated) to prove it's rejected without reaching a client call.
cargo check/fmt/clippy -p openhuman clean, cargo test --lib flows:: 410/410 pass.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/openhuman/flows/ops_tests.rs (1)
2374-2524: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage of the non-array enforcement and probe-override suggestion paths.
Traced all three new tests against the implementation in
ops.rs(schema-known-non-array flags, schema-wholly-unknown stays silent, probe-seeded path suggestion + fix clears) — all assertions are consistent with the reviewed logic.One gap: these tests only cover the
split_out.pathenforcement; there's no equivalent test exercising the probe-override path forgraph_output_field_warnings(i.e., a probe supplyingoutput_fieldswhen the catalog schema is unknown, mirroring the split_out probe test at Line 2459-2524). Consider adding one for parity, especially given the single-sample-replace concern raised inops.rs.🤖 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/flows/ops_tests.rs` around lines 2374 - 2524, Add a parity test for graph_output_field_warnings that mirrors the split_out probe-override coverage: when the catalog schema is unknown but seed_probe_cache provides a ProbedOutputSample with output_fields, verify the warning logic uses the probed fields instead of the missing schema and that the warning clears when the configured output field matches. Reuse the same testing pattern and symbols used in graph_wiring_warnings_suggests_the_probed_split_out_path_when_schema_is_unknown, but target graph_output_field_warnings and the relevant output-field config path to ensure the probe override path is exercised.
🤖 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/tinyflows/caps.rs`:
- Around line 1941-1967: The live probe path in probe_tool_output_sample is
trusting heuristic-only results from resolve_composio_action_scope, which can
misclassify uncatalogued Composio actions as Read. Update
resolve_composio_action_scope to return both the ToolScope and whether it came
from an authoritative curated catalog entry, then change the probe gating to
allow real execution only when the scope is a catalog-backed Read; for
classify_unknown fallbacks and any non-authoritative result, fail closed and
refuse the probe.
- Around line 1867-1890: The process-wide slug cache in PROBE_CACHE is retaining
the full provider payload via ProbedOutputSample::sample, which can expose one
caller’s live data to later in-process callers. Update probe_tool_output_sample
and the ProbedOutputSample shape so the cache stores only derived metadata
needed by callers (such as primary_array_path and output_fields), or otherwise
scope/expire entries by user/connection/args instead of uppercased action slug
alone. Remove or avoid persisting sample in the cached entry and make the same
change anywhere else the probe result is cached or serialized, including the
related probe paths noted in the diff.
- Around line 1929-1933: A successful probe that returns no array should clear
any stale schema-derived primary array path instead of leaving it untouched.
Update apply_probe_override in caps.rs so that when
probed_output_sample(&contract.slug) returns a probe with primary_array_path set
to None, contract.primary_array_path is explicitly cleared; keep the existing
override behavior for Some. Use apply_probe_override and probed_output_sample as
the key symbols when making the change.
---
Nitpick comments:
In `@src/openhuman/flows/ops_tests.rs`:
- Around line 2374-2524: Add a parity test for graph_output_field_warnings that
mirrors the split_out probe-override coverage: when the catalog schema is
unknown but seed_probe_cache provides a ProbedOutputSample with output_fields,
verify the warning logic uses the probed fields instead of the missing schema
and that the warning clears when the configured output field matches. Reuse the
same testing pattern and symbols used in
graph_wiring_warnings_suggests_the_probed_split_out_path_when_schema_is_unknown,
but target graph_output_field_warnings and the relevant output-field config path
to ensure the probe override path is exercised.
🪄 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
Run ID: c4175f7d-35ba-4a9c-8f05-48e2c040fb35
📒 Files selected for processing (9)
src/openhuman/agent_registry/agents/loader.rssrc/openhuman/flows/agents/workflow_builder/agent.tomlsrc/openhuman/flows/agents/workflow_builder/prompt.mdsrc/openhuman/flows/builder_tools.rssrc/openhuman/flows/builder_tools_tests.rssrc/openhuman/flows/ops.rssrc/openhuman/flows/ops_tests.rssrc/openhuman/tinyflows/caps.rssrc/openhuman/tools/ops.rs
… probe (addresses @chatgpt-codex-connector on caps.rs:1966) resolve_composio_action_scope fell back to classify_unknown's verb heuristic whenever find_curated missed, even for a toolkit that ships a static curated catalog (e.g. github). That let get_tool_output_sample probe (execute) any uncurated slug whose name merely looks read-only, bypassing the stricter curated-action gate flow_tool_allowed/validate_tool_contracts already enforce for catalogued toolkits — an uncurated write action named like a read (unlikely but not impossible) would have been executed for real. resolve_composio_action_scope now returns Option<ToolScope>: for a toolkit with a static curated catalog, an uncurated slug resolves to None (refuse, never guess) instead of falling through to the heuristic; the heuristic is only consulted when the toolkit has no static catalog at all. Verified GITHUB_LIST_REPOSITORY_ISSUES (curated Read) stays probe-eligible, and added tests proving GITHUB_LIST_WORKFLOWS (a real, deliberately uncurated GitHub action) is rejected before any client call.
…ervation clear a stale path (addresses @coderabbitai on caps.rs:1890,1933) Two Major findings from CodeRabbit's B12 probe review: - apply_probe_override only overlaid primary_array_path when the probe found Some(path), leaving a stale schema-derived (possibly wrong) path in place when a real probe call confirmed the response has NO array at all. A probe's None is itself a confirmed observation, not "no opinion" — now overlaid unconditionally so a since-disproved path can no longer look supported. - PROBE_CACHE (process-lifetime, keyed by slug only) was retaining the full observed response verbatim via ProbedOutputSample.sample. That's a real probe call's actual data (repo issues, messages, ...), and nothing that reads the cache ever needs the raw payload — only the derived primary_array_path/output_fields. cache_probe_result now redacts sample to Value::Null before inserting; the immediate caller of probe_tool_output_sample still gets the full sample for that one call, it just never persists. Also adds the output_fields parity test CodeRabbit asked for (probed fields override an absent schema for graph_output_field_warnings, mirroring the existing split_out.path probe-override coverage). cargo check/fmt/clippy -p openhuman clean; cargo test --lib flows:: 413/413 pass.
Fixes B16 — the builder guessed the wrong array path for Composio tools whose response schema is absent. Live:
GITHUB_LIST_REPOSITORY_ISSUES→ builder wrotesplit_out.path="json.data"(the{issues:[...]}container) instead of"json.data.issues"→ split emitted 1 mega-item → condition/agent got the wrong shape → 0 output. (This is the bug B15 exposed once it stopped the fake-content spam.)Root cause (verified live)
get_tool_contract{slug:"GITHUB_LIST_REPOSITORY_ISSUES"}returnsoutput_schema: null, primary_array_path: null— for every GitHub action in Composio's backend-proxied catalog mode. It's not a walker bug (compute_primary_array_pathis correct — it just has nothing to walk); the codebase even documents that the proxied path "may not forward"output_parameters. B1'sdata.-prefix + schema-walk can't help when there's no schema.Fix — B12 real-output probe (the robust general solution)
caps.rs—compute_primary_array_path_from_value(BFS over a REAL response value, skipping envelope meta keys), a per-slugPROBE_CACHE,resolve_composio_action_scope(hard Read-only classifier, ignores user prefs), andprobe_tool_output_sample— a bounded READ-ONLYexecute_toolcall gated on Read-scope + a connected toolkit; derives the realprimary_array_path/output_fieldsand overrides the (null) schema hint. Falls back to B1's schema path when unconnected / not-read / call fails.builder_tools.rs+agent.toml+prompt.md— aget_tool_output_samplebuilder tool; the prompt teaches the builder to call it (instead of guessingjson.data) whenprimary_array_pathis null.GITHUB_LIST_REPOSITORY_ISSUESis a Read action → probe-able →primary_array_path → data.issues.ops.rsenforcement —graph_split_out_path_warningsnow flags asplit_out.paththat resolves to a non-array against a known schema/probe (the "wrong array path" signal), so this class is caught at build time.Verification
cargo check/fmtclean ·flows::420 pass (+ probe / value-walk / enforcement tests) · agent-registry pin test updated + green · keepsdata.messages/data.itemsgreen.Summary by CodeRabbit
get_tool_output_samplecapability to the workflow builder to sample real action outputs and infer the correct array path and output fields when schemas are incomplete.split_outwhen the primary array path is unknown.