mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(flows): tighten builder input grounding — arg-name validation + input_context dry-run + boolean typing (B13, B7, B8) (#4665)
This commit is contained in:
@@ -139,10 +139,13 @@ connected, `list_flow_connections` → build the `tool_call` node with the real
|
||||
left as a plain instruction — never a `.item`/`nodes.` reference woven
|
||||
into prose. `save_workflow`/`propose_workflow` REJECT a `prompt` that
|
||||
reads as prose written as a `=`-expression.
|
||||
- If `dry_run_workflow` reports `"ok": false` with a `null_resolutions` or
|
||||
`agent_prompt_nulls` list, **fix every one** before proposing — add the
|
||||
missing schema, move data into `input_context`, or rewire the expression
|
||||
to a real upstream field. Don't propose/save a graph `dry_run_workflow`
|
||||
- If `dry_run_workflow` reports `"ok": false` with a `null_resolutions`,
|
||||
`agent_prompt_nulls`, or `agent_input_context_nulls` list, **fix every
|
||||
one** before proposing — add the missing schema, move data into
|
||||
`input_context`, or rewire the expression to a real upstream field.
|
||||
`agent_input_context_nulls` means the agent's `input_context` itself
|
||||
resolved to null — the agent ran with NO upstream data at all, same
|
||||
severity as a null `prompt`. Don't propose/save a graph `dry_run_workflow`
|
||||
flagged.
|
||||
5. **`propose_workflow`** (first draft) or **`revise_workflow`** (iterating on a
|
||||
prior draft — apply the change to the existing graph, don't regenerate from
|
||||
@@ -198,6 +201,16 @@ A `WorkflowGraph` is `{ name?, nodes: [...], edges: [...] }`.
|
||||
address (`=nodes.<agent_id>.item.json.<field>` — see "the envelope" below).
|
||||
Without a schema the agent emits `{text: "..."}` (no other fields) and any
|
||||
`.item.json.<field>`-style binding to it resolves to null.
|
||||
|
||||
**If an agent's output field feeds a `condition` (or is otherwise used as
|
||||
a boolean), declare that field `"type": "boolean"` in
|
||||
`config.output_parser.schema`.** Routing itself is correct once the value
|
||||
IS a real boolean — the failure mode is authoring one that isn't: an
|
||||
ungrounded/loosely-typed field lets the model emit the STRING `"false"`,
|
||||
which is truthy, so a condition meant to route on `false` silently takes
|
||||
the `true` branch instead. Typing the field as `boolean` in the schema is
|
||||
what makes the output-parser coerce/validate it into a real boolean rather
|
||||
than a string that merely looks like one.
|
||||
3. **`tool_call`** — an action. Two flavours by `config.slug`:
|
||||
- **Composio app action** — `config.slug` = a real action slug (from
|
||||
`search_tool_catalog`, e.g. `GMAIL_SEND_EMAIL`) + `config.connection_ref`
|
||||
@@ -212,6 +225,17 @@ A `WorkflowGraph` is `{ name?, nodes: [...], edges: [...] }`.
|
||||
expression misses) fails BEFORE the provider call — in
|
||||
`propose_workflow`/`revise_workflow`/`save_workflow` (hard reject),
|
||||
`dry_run_workflow`, and real runs — with an error naming the field.
|
||||
- **Every key in `config.args` must be one of `input_schema`'s real
|
||||
property names — NEVER a guessed one.** A field that "sounds right" but
|
||||
isn't declared in `input_schema.properties` (e.g. wiring
|
||||
`SLACK_SEND_MESSAGE` with `text` when the action's real schema names the
|
||||
field `markdown_text`) is REJECTED at `propose_workflow`/
|
||||
`revise_workflow`/`save_workflow` naming the bad key and, when derivable,
|
||||
the schema's valid property names — a value being present under the
|
||||
WRONG key still 400s against the real provider at runtime, so this is a
|
||||
hard gate, not just an advisory. Always read the exact property names
|
||||
off `get_tool_contract`'s `input_schema` before wiring `config.args`,
|
||||
never off memory/convention for that app.
|
||||
- **The slug itself is enforced too.** `propose_workflow` /
|
||||
`revise_workflow` / `save_workflow` HARD-REJECT a `tool_call` whose
|
||||
slug isn't a real action in the live Composio catalog for its toolkit —
|
||||
@@ -248,7 +272,11 @@ A `WorkflowGraph` is `{ name?, nodes: [...], edges: [...] }`.
|
||||
`body`; `config.connection_ref` = an `http_cred:<name>` for auth.
|
||||
5. **`code`** — `config.language` (`"javascript"` | `"python"`) + `config.source`.
|
||||
6. **`condition`** — boolean gate on `config.field`; routes to the **`true`** or
|
||||
**`false`** port. Wire both (or the `false` branch dead-ends).
|
||||
**`false`** port. Wire both (or the `false` branch dead-ends). If
|
||||
`config.field` binds to an `agent` node's output, that field's
|
||||
`output_parser.schema` property MUST be declared `"type": "boolean"` (see
|
||||
the `agent` node kind above) — an untyped/string field can carry the
|
||||
truthy string `"false"` and route to the wrong port.
|
||||
7. **`switch`** — multi-way on `config.expression` or `config.field`; routes to
|
||||
the matching **case** port, else **`default`**.
|
||||
8. **`merge`** — fan-in barrier; passes inputs through. No config.
|
||||
|
||||
@@ -944,6 +944,16 @@ impl Tool for ListAgentProfilesTool {
|
||||
/// `agent_prompt_nulls` (`{ node_id, location, expression, suggestion }`) and
|
||||
/// added to the same `ok: false` condition as `null_resolutions`.
|
||||
///
|
||||
/// **Agent-`input_context` null check:** the SAME treatment applies to a
|
||||
/// null-resolved **`input_context`** (`location == "input_context"`) — since
|
||||
/// #4590 this is the agent's primary upstream-data channel (the very field
|
||||
/// `prompt`-embedded jq expressions were supposed to stop needing), so a
|
||||
/// `null` here is just as execution-breaking as a null `prompt`: the agent
|
||||
/// runs with no upstream data at all. Collected separately into
|
||||
/// `agent_input_context_nulls` (`{ node_id, location, expression, suggestion }`,
|
||||
/// mirroring `agent_prompt_nulls` exactly) and added to the same `ok: false`
|
||||
/// condition as `null_resolutions`/`agent_prompt_nulls`.
|
||||
///
|
||||
/// **`on_error: continue`/`route` does not mask a `tool_call` failure either.**
|
||||
/// Those policies convert an executor error (e.g. the required-arg preflight
|
||||
/// rejecting a null arg) into a routed error ITEM so the *run* still completes
|
||||
@@ -1181,6 +1191,32 @@ impl Tool for DryRunWorkflowTool {
|
||||
})
|
||||
.collect();
|
||||
|
||||
// Collect every null-resolved `agent`-node `input_context` — mirrors
|
||||
// `agent_prompt_nulls` exactly (see the struct doc's "Agent-
|
||||
// `input_context` null check" section): `input_context` has been the
|
||||
// agent's primary upstream-data channel since #4590, so a null
|
||||
// resolution here is just as execution-breaking as a null `prompt` —
|
||||
// the agent runs with no upstream data at all.
|
||||
let agent_input_context_nulls: Vec<Value> = observer
|
||||
.steps()
|
||||
.iter()
|
||||
.filter(|step| agent_node_ids.contains(step.node_id.as_str()))
|
||||
.flat_map(|step| {
|
||||
step.diagnostics.iter().filter_map(|diag| {
|
||||
(diag.location == "input_context").then(|| {
|
||||
json!({
|
||||
"node_id": step.node_id,
|
||||
"location": diag.location,
|
||||
"expression": diag.expression,
|
||||
"suggestion": "Wire input_context from a real upstream field, e.g. \
|
||||
\"=nodes.<node_id>.item.json.<field>\" (or \"=item\" off the \
|
||||
trigger), not an expression that resolves to null.",
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
|
||||
// Collect every `tool_call` node whose EXECUTOR errored (e.g. the
|
||||
// Composio required-arg preflight rejecting a missing/null arg) —
|
||||
// regardless of that node's `on_error` policy. A `"continue"`/`"route"`
|
||||
@@ -1224,34 +1260,41 @@ impl Tool for DryRunWorkflowTool {
|
||||
pending_approvals = outcome.pending_approvals.len(),
|
||||
null_resolution_count = null_resolutions.len(),
|
||||
agent_prompt_null_count = agent_prompt_nulls.len(),
|
||||
agent_input_context_null_count = agent_input_context_nulls.len(),
|
||||
node_error_count = node_errors.len(),
|
||||
"[flows] dry_run_workflow: sandbox run finished"
|
||||
);
|
||||
|
||||
if !null_resolutions.is_empty() || !agent_prompt_nulls.is_empty() || !node_errors.is_empty()
|
||||
if !null_resolutions.is_empty()
|
||||
|| !agent_prompt_nulls.is_empty()
|
||||
|| !agent_input_context_nulls.is_empty()
|
||||
|| !node_errors.is_empty()
|
||||
{
|
||||
tracing::debug!(
|
||||
target: "flows",
|
||||
?null_resolutions,
|
||||
?agent_prompt_nulls,
|
||||
?agent_input_context_nulls,
|
||||
?node_errors,
|
||||
"[flows] dry_run_workflow: tool_call/agent-prompt issue(s) found — failing the \
|
||||
dry run"
|
||||
"[flows] dry_run_workflow: tool_call/agent-prompt/agent-input_context issue(s) \
|
||||
found — failing the dry run"
|
||||
);
|
||||
return Ok(ToolResult::success(serde_json::to_string_pretty(&json!({
|
||||
"sandbox": true,
|
||||
"ok": false,
|
||||
"null_resolutions": null_resolutions,
|
||||
"agent_prompt_nulls": agent_prompt_nulls,
|
||||
"agent_input_context_nulls": agent_input_context_nulls,
|
||||
"node_errors": node_errors,
|
||||
"message": "These tool_call args resolved to null, an agent node's prompt \
|
||||
resolved to null (an EMPTY prompt — see agent_prompt_nulls), or a tool_call \
|
||||
"message": "These tool_call args resolved to null, an agent node's prompt or \
|
||||
input_context resolved to null (an EMPTY prompt — see agent_prompt_nulls — \
|
||||
or no upstream data at all — see agent_input_context_nulls), or a tool_call \
|
||||
node failed during the sandbox run (even one recovered via on_error: \
|
||||
continue/route) — wire null-resolved args from an upstream node's real \
|
||||
output (give any agent node an output_parser.schema so its fields are \
|
||||
addressable), feed upstream data into a null-resolved agent prompt via \
|
||||
input_context instead of a jq expression inside the prompt text, and fix or \
|
||||
rewire whatever tool_call node_errors names.",
|
||||
addressable), feed upstream data into a null-resolved agent prompt/ \
|
||||
input_context from a real upstream field instead of a jq expression inside \
|
||||
the prompt text, and fix or rewire whatever tool_call node_errors names.",
|
||||
}))?));
|
||||
}
|
||||
|
||||
@@ -1262,6 +1305,7 @@ impl Tool for DryRunWorkflowTool {
|
||||
"pending_approvals": outcome.pending_approvals,
|
||||
"null_resolutions": null_resolutions,
|
||||
"agent_prompt_nulls": agent_prompt_nulls,
|
||||
"agent_input_context_nulls": agent_input_context_nulls,
|
||||
"node_errors": node_errors,
|
||||
"note": "SANDBOX (mock) output — LLM/tool/HTTP/code nodes returned deterministic echoes; NO real side effects occurred. This checks wiring/routing only, not whether real integrations work.",
|
||||
}))?))
|
||||
|
||||
@@ -872,6 +872,49 @@ async fn dry_run_flags_null_resolved_agent_prompt() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn dry_run_flags_null_resolved_agent_input_context() {
|
||||
// The B7 counterpart to `dry_run_flags_null_resolved_agent_prompt`:
|
||||
// `input_context` has been the agent's primary upstream-data channel
|
||||
// since #4590, so a null-resolved `input_context` is just as
|
||||
// execution-breaking as a null `prompt` — the agent runs with no
|
||||
// upstream data at all. Must fail the dry run the same way.
|
||||
let tool = DryRunWorkflowTool::new(
|
||||
policy(AutonomyLevel::Supervised),
|
||||
test_config(&TempDir::new().unwrap()),
|
||||
);
|
||||
let graph = json!({
|
||||
"nodes": [
|
||||
{ "id": "t", "kind": "trigger", "name": "Manual" },
|
||||
{ "id": "classify", "kind": "agent", "name": "Classify",
|
||||
"config": { "prompt": "Classify the email as urgent, normal, or low priority.",
|
||||
"input_context": "=nodes.missing.item.json.body" } }
|
||||
],
|
||||
"edges": [ { "from_node": "t", "to_node": "classify" } ]
|
||||
});
|
||||
|
||||
let result = tool.execute(json!({ "graph": graph })).await.unwrap();
|
||||
assert!(!result.is_error, "{}", result.output());
|
||||
let parsed: Value = serde_json::from_str(&result.output()).unwrap();
|
||||
assert_eq!(
|
||||
parsed["ok"], false,
|
||||
"a null-resolved agent input_context must fail the dry run: {parsed}"
|
||||
);
|
||||
let agent_input_context_nulls = parsed["agent_input_context_nulls"]
|
||||
.as_array()
|
||||
.expect("agent_input_context_nulls array");
|
||||
assert_eq!(agent_input_context_nulls.len(), 1, "{parsed}");
|
||||
assert_eq!(agent_input_context_nulls[0]["node_id"], "classify");
|
||||
assert_eq!(agent_input_context_nulls[0]["location"], "input_context");
|
||||
assert!(
|
||||
agent_input_context_nulls[0]["suggestion"]
|
||||
.as_str()
|
||||
.unwrap()
|
||||
.contains("upstream"),
|
||||
"{parsed}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn dry_run_passes_when_agent_uses_input_context_instead_of_prompt_expression() {
|
||||
// The FALSE-POSITIVE-PREVENTION case: the same data need, wired the
|
||||
@@ -899,6 +942,13 @@ async fn dry_run_passes_when_agent_uses_input_context_instead_of_prompt_expressi
|
||||
parsed["agent_prompt_nulls"].as_array().unwrap().is_empty(),
|
||||
"{parsed}"
|
||||
);
|
||||
assert!(
|
||||
parsed["agent_input_context_nulls"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.is_empty(),
|
||||
"{parsed}"
|
||||
);
|
||||
}
|
||||
|
||||
/// (systemic tool-contract fix, Part 2b) A missing required Composio arg is
|
||||
|
||||
@@ -811,7 +811,9 @@ pub(crate) async fn validate_tool_contracts(config: &Config, graph: &WorkflowGra
|
||||
use crate::openhuman::memory_sync::composio::providers::{
|
||||
catalog_for_toolkit, get_provider, toolkit_from_slug,
|
||||
};
|
||||
use crate::openhuman::tinyflows::caps::{fetch_live_toolkit_catalog, missing_required_args};
|
||||
use crate::openhuman::tinyflows::caps::{
|
||||
fetch_live_toolkit_catalog, missing_required_args, unsupported_arg_names,
|
||||
};
|
||||
|
||||
let mut errors = Vec::new();
|
||||
for node in &graph.nodes {
|
||||
@@ -915,6 +917,63 @@ pub(crate) async fn validate_tool_contracts(config: &Config, graph: &WorkflowGra
|
||||
node.id, missing[0]
|
||||
));
|
||||
}
|
||||
|
||||
// [B13] Arg-NAME validity: `missing_required_args` only proves a
|
||||
// required arg is PRESENT — it says nothing about whether every arg
|
||||
// the builder wired is actually a property this action's schema
|
||||
// recognizes. A misnamed/unsupported field (the live bug: wiring
|
||||
// `SLACK_SEND_MESSAGE` with `text` when the action wants
|
||||
// `markdown_text`) sails through the check above unrejected — a
|
||||
// value IS present, just under the wrong key — and only surfaces as
|
||||
// a runtime 400 from the real provider. `unsupported_arg_names`
|
||||
// returns `None` when the schema can't be used to validate names
|
||||
// (unknown schema, or `additionalProperties: true`) — that case is
|
||||
// deliberately never rejected here (best-effort, same posture as the
|
||||
// rest of this gate).
|
||||
if let Some(unsupported) = unsupported_arg_names(contract.input_schema.as_ref(), &args) {
|
||||
if !unsupported.is_empty() {
|
||||
let valid_names: Vec<String> = contract
|
||||
.input_schema
|
||||
.as_ref()
|
||||
.and_then(|s| s.get("properties"))
|
||||
.and_then(Value::as_object)
|
||||
.map(|props| {
|
||||
let mut names: Vec<String> = props.keys().cloned().collect();
|
||||
names.sort();
|
||||
names
|
||||
})
|
||||
.unwrap_or_default();
|
||||
tracing::warn!(
|
||||
target: "flows",
|
||||
node = %node.id,
|
||||
%slug,
|
||||
?unsupported,
|
||||
?valid_names,
|
||||
"[flows] tool-contract check: arg name(s) not declared by the action's \
|
||||
input schema — rejecting"
|
||||
);
|
||||
let bad_list = unsupported
|
||||
.iter()
|
||||
.map(|m| format!("`{m}`"))
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ");
|
||||
let valid_suffix = if valid_names.is_empty() {
|
||||
String::new()
|
||||
} else {
|
||||
format!(
|
||||
" — valid arg names for `{slug}` are: {}",
|
||||
valid_names.join(", ")
|
||||
)
|
||||
};
|
||||
errors.push(format!(
|
||||
"Node '{}': tool_call `{slug}` has unsupported arg name(s) {bad_list} — not \
|
||||
a property of this action's input schema{valid_suffix}. Call \
|
||||
get_tool_contract {{ slug: \"{slug}\" }} and use the exact property names \
|
||||
from `input_schema` (never guess an arg name).",
|
||||
node.id
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
errors
|
||||
}
|
||||
|
||||
@@ -2000,6 +2000,156 @@ async fn validate_tool_contracts_skips_rather_than_rejects_when_the_catalog_is_u
|
||||
);
|
||||
}
|
||||
|
||||
// ── validate_tool_contracts: arg-NAME validation against the input schema
|
||||
// (B13 — a misnamed/unsupported field, e.g. `text` instead of
|
||||
// `markdown_text` for `SLACK_SEND_MESSAGE`, used to sail through
|
||||
// `missing_required_args` because SOME value was present, just under the
|
||||
// wrong key) ────────────────────────────────────────────────────────────
|
||||
|
||||
/// `SLACK_SEND_MESSAGE` with a real `input_schema` naming `channel` and
|
||||
/// `markdown_text` — models the live bug this fixes: `markdown_text` is the
|
||||
/// real field, `text` is not.
|
||||
fn seeded_slack_send_message_contract_with_schema() -> ToolContract {
|
||||
ToolContract {
|
||||
slug: "SLACK_SEND_MESSAGE".to_string(),
|
||||
toolkit: "slack".to_string(),
|
||||
description: None,
|
||||
required_args: vec![],
|
||||
input_schema: Some(json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"channel": { "type": "string" },
|
||||
"markdown_text": { "type": "string" }
|
||||
}
|
||||
})),
|
||||
output_fields: vec![],
|
||||
output_schema: None,
|
||||
primary_array_path: None,
|
||||
is_curated: true,
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn validate_tool_contracts_rejects_an_arg_name_not_in_the_input_schema() {
|
||||
seed_live_catalog_cache(
|
||||
"slack",
|
||||
vec![seeded_slack_send_message_contract_with_schema()],
|
||||
);
|
||||
let config = Config::default();
|
||||
let g = graph(json!({
|
||||
"nodes": [
|
||||
{ "id": "t", "kind": "trigger", "name": "Manual" },
|
||||
{ "id": "post", "kind": "tool_call", "name": "Post",
|
||||
"config": { "slug": "SLACK_SEND_MESSAGE",
|
||||
"args": { "channel": "#general", "text": "hi" } } }
|
||||
],
|
||||
"edges": [ { "from_node": "t", "to_node": "post" } ]
|
||||
}));
|
||||
let errors = validate_tool_contracts(&config, &g).await;
|
||||
assert_eq!(errors.len(), 1, "{errors:?}");
|
||||
assert!(errors[0].contains("post"), "{}", errors[0]);
|
||||
assert!(errors[0].contains("`text`"), "{}", errors[0]);
|
||||
assert!(errors[0].contains("markdown_text"), "{}", errors[0]);
|
||||
assert!(errors[0].contains("get_tool_contract"), "{}", errors[0]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn validate_tool_contracts_passes_the_real_arg_name_from_the_input_schema() {
|
||||
seed_live_catalog_cache(
|
||||
"slack",
|
||||
vec![seeded_slack_send_message_contract_with_schema()],
|
||||
);
|
||||
let config = Config::default();
|
||||
let g = graph(json!({
|
||||
"nodes": [
|
||||
{ "id": "t", "kind": "trigger", "name": "Manual" },
|
||||
{ "id": "post", "kind": "tool_call", "name": "Post",
|
||||
"config": { "slug": "SLACK_SEND_MESSAGE",
|
||||
"args": { "channel": "#general", "markdown_text": "hi" } } }
|
||||
],
|
||||
"edges": [ { "from_node": "t", "to_node": "post" } ]
|
||||
}));
|
||||
let errors = validate_tool_contracts(&config, &g).await;
|
||||
assert!(errors.is_empty(), "{errors:?}");
|
||||
}
|
||||
|
||||
/// Uses its own cache key/toolkit (never `"slack"`/`"gmail"`) since the
|
||||
/// arg-name check must behave identically no matter which slug it's
|
||||
/// exercised against, and a dedicated, unregistered toolkit sidesteps both
|
||||
/// the process-global `LIVE_CATALOG_CACHE` sharing risk the other
|
||||
/// `validate_tool_contracts` tests accept AND the static curated-catalog
|
||||
/// gate (this toolkit has none, so `is_curated` is irrelevant here).
|
||||
#[tokio::test]
|
||||
async fn validate_tool_contracts_skips_arg_name_check_when_input_schema_is_unknown() {
|
||||
seed_live_catalog_cache(
|
||||
"argschemaunknown",
|
||||
vec![ToolContract {
|
||||
slug: "ARGSCHEMAUNKNOWN_DO_THING".to_string(),
|
||||
toolkit: "argschemaunknown".to_string(),
|
||||
description: None,
|
||||
required_args: vec![],
|
||||
input_schema: None,
|
||||
output_fields: vec![],
|
||||
output_schema: None,
|
||||
primary_array_path: None,
|
||||
is_curated: false,
|
||||
}],
|
||||
);
|
||||
let config = Config::default();
|
||||
let g = graph(json!({
|
||||
"nodes": [
|
||||
{ "id": "t", "kind": "trigger", "name": "Manual" },
|
||||
{ "id": "post", "kind": "tool_call", "name": "Post",
|
||||
"config": { "slug": "ARGSCHEMAUNKNOWN_DO_THING",
|
||||
"args": { "totally_made_up_field": "hi" } } }
|
||||
],
|
||||
"edges": [ { "from_node": "t", "to_node": "post" } ]
|
||||
}));
|
||||
let errors = validate_tool_contracts(&config, &g).await;
|
||||
assert!(
|
||||
errors.is_empty(),
|
||||
"an unknown input_schema must skip the arg-name check, never reject: {errors:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn validate_tool_contracts_allows_arbitrary_arg_names_when_schema_permits_additional_properties(
|
||||
) {
|
||||
seed_live_catalog_cache(
|
||||
"argschemaadditional",
|
||||
vec![ToolContract {
|
||||
slug: "ARGSCHEMAADDITIONAL_DO_THING".to_string(),
|
||||
toolkit: "argschemaadditional".to_string(),
|
||||
description: None,
|
||||
required_args: vec![],
|
||||
input_schema: Some(json!({
|
||||
"type": "object",
|
||||
"properties": { "channel": { "type": "string" } },
|
||||
"additionalProperties": true
|
||||
})),
|
||||
output_fields: vec![],
|
||||
output_schema: None,
|
||||
primary_array_path: None,
|
||||
is_curated: false,
|
||||
}],
|
||||
);
|
||||
let config = Config::default();
|
||||
let g = graph(json!({
|
||||
"nodes": [
|
||||
{ "id": "t", "kind": "trigger", "name": "Manual" },
|
||||
{ "id": "post", "kind": "tool_call", "name": "Post",
|
||||
"config": { "slug": "ARGSCHEMAADDITIONAL_DO_THING",
|
||||
"args": { "channel": "#general", "any_extra_field": "hi" } } }
|
||||
],
|
||||
"edges": [ { "from_node": "t", "to_node": "post" } ]
|
||||
}));
|
||||
let errors = validate_tool_contracts(&config, &g).await;
|
||||
assert!(
|
||||
errors.is_empty(),
|
||||
"additionalProperties: true must allow arbitrary arg names: {errors:?}"
|
||||
);
|
||||
}
|
||||
|
||||
// ── graph_wiring_warnings: required-arg advisory + output-field/split_out.path
|
||||
// advisories (Part 2c/2d) ────────────────────────────────────────────────
|
||||
|
||||
|
||||
@@ -1751,6 +1751,51 @@ pub(crate) fn missing_required_args(required: &[String], args: &Value) -> Vec<St
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// [B13] Returns argument names in `args` that are NOT declared `properties`
|
||||
/// of `schema` — the NAME-VALIDITY counterpart to [`missing_required_args`]'s
|
||||
/// PRESENCE check. Catches the class of bug `missing_required_args` alone
|
||||
/// cannot: a builder wires a real, well-typed value under an arg name the
|
||||
/// action's schema doesn't recognize at all (e.g. `SLACK_SEND_MESSAGE`'s
|
||||
/// `text` when the live action actually wants `markdown_text`) — the
|
||||
/// required arg still LOOKS satisfied from `missing_required_args`'
|
||||
/// perspective (a value is present under *some* key), so the mistake sails
|
||||
/// through authoring/save and only 400s from the real provider at runtime.
|
||||
///
|
||||
/// `None` means "cannot validate this schema — skip, never reject", so a
|
||||
/// caller must never turn a `None` into a rejection:
|
||||
/// - `schema` is `None` (`ToolContract::input_schema` unknown for this slug), or
|
||||
/// - `schema` is not a JSON object, or names no object `properties` map (an
|
||||
/// unrecognized/legacy shape — nothing to check names against), or
|
||||
/// - `schema` declares `additionalProperties: true` (Composio explicitly
|
||||
/// telling us to accept arbitrary keys beyond the declared ones).
|
||||
///
|
||||
/// `Some(vec![])` means the schema WAS usable and every arg name in `args`
|
||||
/// is a real declared property. `args` must be a JSON object to check
|
||||
/// against; any other shape (including `Value::Null` — no args wired at
|
||||
/// all) yields `Some(vec![])`, mirroring `missing_required_args`' treatment
|
||||
/// of an absent/non-object `args`.
|
||||
pub(crate) fn unsupported_arg_names(schema: Option<&Value>, args: &Value) -> Option<Vec<String>> {
|
||||
let schema_obj = schema?.as_object()?;
|
||||
if schema_obj
|
||||
.get("additionalProperties")
|
||||
.and_then(Value::as_bool)
|
||||
== Some(true)
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let properties = schema_obj.get("properties")?.as_object()?;
|
||||
let Some(args_obj) = args.as_object() else {
|
||||
return Some(Vec::new());
|
||||
};
|
||||
let mut unsupported: Vec<String> = args_obj
|
||||
.keys()
|
||||
.filter(|k| !properties.contains_key(k.as_str()))
|
||||
.cloned()
|
||||
.collect();
|
||||
unsupported.sort();
|
||||
Some(unsupported)
|
||||
}
|
||||
|
||||
/// Required-arg preflight for a Composio `tool_call`: fails **before** the
|
||||
/// Composio dispatch when a required arg is missing or resolved to `null`,
|
||||
/// with a message that names the field and the likely fix — instead of letting
|
||||
@@ -3616,6 +3661,76 @@ mod tests {
|
||||
assert!(response_fields_from_schema(Some(&json!({}))).is_empty());
|
||||
}
|
||||
|
||||
// ── unsupported_arg_names (B13) ──────────────────────────────────────────
|
||||
// Direct unit tests for the pure name-validity check — see
|
||||
// `openhuman::flows::ops_tests` for the end-to-end
|
||||
// `validate_tool_contracts` coverage of the same behavior.
|
||||
|
||||
#[test]
|
||||
fn unsupported_arg_names_flags_a_name_not_in_properties() {
|
||||
let schema = json!({
|
||||
"type": "object",
|
||||
"properties": { "channel": {"type": "string"}, "markdown_text": {"type": "string"} }
|
||||
});
|
||||
let args = json!({ "channel": "#general", "text": "hi" });
|
||||
assert_eq!(
|
||||
unsupported_arg_names(Some(&schema), &args),
|
||||
Some(vec!["text".to_string()])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsupported_arg_names_empty_when_every_name_is_a_real_property() {
|
||||
let schema = json!({
|
||||
"type": "object",
|
||||
"properties": { "channel": {"type": "string"}, "markdown_text": {"type": "string"} }
|
||||
});
|
||||
let args = json!({ "channel": "#general", "markdown_text": "hi" });
|
||||
assert_eq!(unsupported_arg_names(Some(&schema), &args), Some(vec![]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsupported_arg_names_skips_when_schema_is_none() {
|
||||
let args = json!({ "anything": "goes" });
|
||||
assert_eq!(unsupported_arg_names(None, &args), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsupported_arg_names_skips_when_schema_has_no_properties_object() {
|
||||
// Legacy/loose schema shape (no `properties` map at all) — nothing to
|
||||
// validate names against, so this must skip, not reject.
|
||||
let schema = json!({ "type": "object", "description": "legacy shape" });
|
||||
let args = json!({ "anything": "goes" });
|
||||
assert_eq!(unsupported_arg_names(Some(&schema), &args), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsupported_arg_names_skips_when_additional_properties_is_true() {
|
||||
let schema = json!({
|
||||
"type": "object",
|
||||
"properties": { "channel": {"type": "string"} },
|
||||
"additionalProperties": true
|
||||
});
|
||||
let args = json!({ "channel": "#general", "any_extra_field": "hi" });
|
||||
assert_eq!(unsupported_arg_names(Some(&schema), &args), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsupported_arg_names_empty_for_null_or_non_object_args() {
|
||||
let schema = json!({
|
||||
"type": "object",
|
||||
"properties": { "channel": {"type": "string"} }
|
||||
});
|
||||
assert_eq!(
|
||||
unsupported_arg_names(Some(&schema), &Value::Null),
|
||||
Some(vec![])
|
||||
);
|
||||
assert_eq!(
|
||||
unsupported_arg_names(Some(&schema), &json!("not an object")),
|
||||
Some(vec![])
|
||||
);
|
||||
}
|
||||
|
||||
// ── compute_primary_array_path ──────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user