From 7f74db4f219e0fe823cf84e5608eb054db7787bd Mon Sep 17 00:00:00 2001 From: Cyrus Gray <144336577+graycyrus@users.noreply.github.com> Date: Wed, 8 Jul 2026 00:36:47 +0530 Subject: [PATCH] =?UTF-8?q?fix(flows):=20tighten=20builder=20input=20groun?= =?UTF-8?q?ding=20=E2=80=94=20arg-name=20validation=20+=20input=5Fcontext?= =?UTF-8?q?=20dry-run=20+=20boolean=20typing=20(B13,=20B7,=20B8)=20(#4665)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../flows/agents/workflow_builder/prompt.md | 38 ++++- src/openhuman/flows/builder_tools.rs | 60 ++++++- src/openhuman/flows/builder_tools_tests.rs | 50 ++++++ src/openhuman/flows/ops.rs | 61 ++++++- src/openhuman/flows/ops_tests.rs | 150 ++++++++++++++++++ src/openhuman/tinyflows/caps.rs | 115 ++++++++++++++ 6 files changed, 460 insertions(+), 14 deletions(-) diff --git a/src/openhuman/flows/agents/workflow_builder/prompt.md b/src/openhuman/flows/agents/workflow_builder/prompt.md index dafa23544..169835dbe 100644 --- a/src/openhuman/flows/agents/workflow_builder/prompt.md +++ b/src/openhuman/flows/agents/workflow_builder/prompt.md @@ -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..item.json.` — see "the envelope" below). Without a schema the agent emits `{text: "..."}` (no other fields) and any `.item.json.`-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:` 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. diff --git a/src/openhuman/flows/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index f5d93d16d..51e864e30 100644 --- a/src/openhuman/flows/builder_tools.rs +++ b/src/openhuman/flows/builder_tools.rs @@ -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 = 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..item.json.\" (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.", }))?)) diff --git a/src/openhuman/flows/builder_tools_tests.rs b/src/openhuman/flows/builder_tools_tests.rs index c00f5857c..d3c1a5ac2 100644 --- a/src/openhuman/flows/builder_tools_tests.rs +++ b/src/openhuman/flows/builder_tools_tests.rs @@ -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 diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index 78fe323b9..626345490 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -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 = contract + .input_schema + .as_ref() + .and_then(|s| s.get("properties")) + .and_then(Value::as_object) + .map(|props| { + let mut names: Vec = 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::>() + .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 } diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index 7a946f1b3..f69fcabba 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -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) ──────────────────────────────────────────────── diff --git a/src/openhuman/tinyflows/caps.rs b/src/openhuman/tinyflows/caps.rs index 214155c4c..5c09b2ae1 100644 --- a/src/openhuman/tinyflows/caps.rs +++ b/src/openhuman/tinyflows/caps.rs @@ -1751,6 +1751,51 @@ pub(crate) fn missing_required_args(required: &[String], args: &Value) -> Vec, args: &Value) -> Option> { + 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 = 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]