diff --git a/CHANGELOG.md b/CHANGELOG.md index 6af93fdc..64d20911 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **`migrate::deserialize_graph`** — `migrate` plus deserialization into a + `WorkflowGraph` whose failures name the offending member + (`nodes[1]: missing field \`name\``) or top-level field, instead of serde's + pathless message. Moved from a host that had been carrying it. + - **`crates/tinyflows-catalog`** — the saved-workflow model *around* a graph: `Flow` and its revision history, `FlowRun` and its steps, authoring drafts and suggestions, the run and build cancellation registries, the n8n importer, and diff --git a/crates/tinyflows/src/compat_router_tests.rs b/crates/tinyflows/src/compat_router_tests.rs new file mode 100644 index 00000000..3cd797f6 --- /dev/null +++ b/crates/tinyflows/src/compat_router_tests.rs @@ -0,0 +1,289 @@ +use super::*; + +// ---- main-port labels, loop back-edges, router exhaustiveness ---- +// +// Ported from OpenHuman's host-side compatibility tests, which only ever +// exercised this crate's `errors` through a wrapper. + +/// A switch's `main` label reaching a fan-in beside a fan-out sibling. +fn main_port_conditional_fan_in_graph() -> WorkflowGraph { + graph(json!({ + "name": "main-port-conditional-fan-in", + "nodes": [ + { "id": "start", "kind": "trigger", "name": "Trigger" }, + { "id": "route", "kind": "switch", "name": "Route", "config": { "field": "kind" } }, + { "id": "a", "kind": "output_parser", "name": "A" }, + { "id": "other", "kind": "output_parser", "name": "Other" }, + { "id": "c", "kind": "output_parser", "name": "C" }, + { "id": "m", "kind": "merge", "name": "Merge" } + ], + "edges": [ + { "from_node": "start", "from_port": "main", "to_node": "route" }, + { "from_node": "start", "from_port": "main", "to_node": "c" }, + { "from_node": "route", "from_port": "main", "to_node": "a" }, + { "from_node": "route", "from_port": "other", "to_node": "other" }, + { "from_node": "a", "from_port": "main", "to_node": "m" }, + { "from_node": "c", "from_port": "main", "to_node": "m" } + ] + })) +} + +/// `outer` (condition) -> `inner` (`inner_kind`, wired on `inner_ports`) -> `a`, +/// reconverging with `c` at merge `m`. +fn nested_router_reconvergence_graph(inner_kind: &str, inner_ports: &[&str]) -> WorkflowGraph { + let mut edges = vec![ + json!({ "from_node": "start", "from_port": "main", "to_node": "outer" }), + json!({ "from_node": "start", "from_port": "main", "to_node": "c" }), + json!({ "from_node": "outer", "from_port": "true", "to_node": "inner" }), + json!({ "from_node": "outer", "from_port": "false", "to_node": "outer_else" }), + ]; + edges.extend( + inner_ports + .iter() + .map(|port| json!({ "from_node": "inner", "from_port": port, "to_node": "a" })), + ); + edges.extend([ + json!({ "from_node": "a", "from_port": "main", "to_node": "m" }), + json!({ "from_node": "c", "from_port": "main", "to_node": "m" }), + ]); + + graph(json!({ + "name": "nested-router-reconvergence", + "nodes": [ + { "id": "start", "kind": "trigger", "name": "Trigger" }, + { "id": "outer", "kind": "condition", "name": "Outer", "config": { "field": "outer" } }, + { "id": "inner", "kind": inner_kind, "name": "Inner", "config": { "field": "inner" } }, + { "id": "outer_else", "kind": "output_parser", "name": "Outer else" }, + { "id": "a", "kind": "output_parser", "name": "A" }, + { "id": "c", "kind": "output_parser", "name": "C" }, + { "id": "m", "kind": "merge", "name": "Merge" } + ], + "edges": edges + })) +} + +#[test] +fn engine_compatibility_rejects_main_label_on_conditional_fan_in_path() { + let g = main_port_conditional_fan_in_graph(); + let errs = errors(&g); + assert_eq!(errs.len(), 1); + assert_eq!(errs[0].code, UNSUPPORTED_MAIN_PORT_CONDITIONAL_FAN_IN); + assert_eq!(errs[0].node_id.as_deref(), Some("m")); + + let reconverged = graph(json!({ + "name": "main-port-reconverges-before-fan-in", + "nodes": [ + { "id": "start", "kind": "trigger", "name": "Trigger" }, + { "id": "route", "kind": "switch", "name": "Route", "config": { "field": "kind" } }, + { "id": "a", "kind": "output_parser", "name": "A" }, + { "id": "c", "kind": "output_parser", "name": "C" }, + { "id": "m", "kind": "merge", "name": "Merge" } + ], + "edges": [ + { "from_node": "start", "from_port": "main", "to_node": "route" }, + { "from_node": "start", "from_port": "main", "to_node": "c" }, + { "from_node": "route", "from_port": "main", "to_node": "a" }, + { "from_node": "route", "from_port": "default", "to_node": "a" }, + { "from_node": "a", "from_port": "main", "to_node": "m" }, + { "from_node": "c", "from_port": "main", "to_node": "m" } + ] + })); + assert!(errors(&reconverged).is_empty()); +} + +/// A loop head has two incoming edges, and this gate mirrors the engine's +/// fan-in classification — so without excluding back-edges it would report +/// every legal bounded loop as an unrelieved fan-in and refuse to save it. +#[test] +fn engine_compatibility_does_not_treat_a_loop_back_edge_as_a_fan_in() { + let looping = graph(json!({ + "name": "bounded-loop", + "nodes": [ + { "id": "start", "kind": "trigger", "name": "Trigger" }, + { "id": "l", "kind": "loop", "name": "Loop", + "config": { "max_iterations": 3, "on_exceeded": "continue" } }, + { "id": "work", "kind": "output_parser", "name": "Work" }, + { "id": "out", "kind": "output_parser", "name": "Out" } + ], + "edges": [ + { "from_node": "start", "from_port": "main", "to_node": "l" }, + { "from_node": "l", "from_port": "body", "to_node": "work" }, + { "from_node": "work", "from_port": "main", "to_node": "l" }, + { "from_node": "l", "from_port": "done", "to_node": "out" } + ] + })); + assert!( + errors(&looping).is_empty(), + "a bounded loop must save cleanly: {:?}", + errors(&looping) + ); +} + +#[test] +fn engine_compatibility_requires_exhaustive_router_choices_for_reconvergence() { + let exhaustive_condition = nested_router_reconvergence_graph("condition", &["true", "false"]); + assert!(errors(&exhaustive_condition).is_empty()); + + let missing_condition_branch = nested_router_reconvergence_graph("condition", &["true"]); + let errs = errors(&missing_condition_branch); + assert_eq!(errs.len(), 1); + assert_eq!(errs[0].code, UNSUPPORTED_NESTED_CONDITIONAL_FAN_IN); + + let exhaustive_switch = nested_router_reconvergence_graph("switch", &["known-case", "default"]); + assert!(errors(&exhaustive_switch).is_empty()); + + // Same-port fan-out is unconditional: TinyFlows schedules both `main` + // successors. A side path after an exhaustive router must not make the + // reconverging path look like another conditional choice. + let exhaustive_switch_with_main_fanout = graph(json!({ + "nodes": [ + { "id": "start", "kind": "trigger", "name": "Trigger" }, + { "id": "outer", "kind": "condition", "name": "Outer", "config": { "field": "outer" } }, + { "id": "inner", "kind": "switch", "name": "Inner", "config": { "field": "inner" } }, + { "id": "outer_else", "kind": "output_parser", "name": "Outer else" }, + { "id": "fanout", "kind": "output_parser", "name": "Fan out" }, + { "id": "a", "kind": "output_parser", "name": "A" }, + { "id": "side", "kind": "output_parser", "name": "Side" }, + { "id": "c", "kind": "output_parser", "name": "C" }, + { "id": "m", "kind": "merge", "name": "Merge" } + ], + "edges": [ + { "from_node": "start", "from_port": "main", "to_node": "outer" }, + { "from_node": "start", "from_port": "main", "to_node": "c" }, + { "from_node": "outer", "from_port": "true", "to_node": "inner" }, + { "from_node": "outer", "from_port": "false", "to_node": "outer_else" }, + { "from_node": "inner", "from_port": "known-case", "to_node": "fanout" }, + { "from_node": "inner", "from_port": "default", "to_node": "fanout" }, + { "from_node": "fanout", "from_port": "main", "to_node": "a" }, + { "from_node": "fanout", "from_port": "main", "to_node": "side" }, + { "from_node": "a", "from_port": "main", "to_node": "m" }, + { "from_node": "c", "from_port": "main", "to_node": "m" } + ] + })); + assert!(errors(&exhaustive_switch_with_main_fanout).is_empty()); + + // A switch with only `default` is exhaustive: every input takes that edge, + // so it is an unconditional step even though it has a single wired port. + let default_only_switch = nested_router_reconvergence_graph("switch", &["default"]); + assert!(errors(&default_only_switch).is_empty()); + + let missing_switch_default = + nested_router_reconvergence_graph("switch", &["known-case", "other-case"]); + let errs = errors(&missing_switch_default); + assert!(!errs.is_empty()); + assert!( + errs.iter() + .all(|error| error.code == UNSUPPORTED_NESTED_CONDITIONAL_FAN_IN) + ); + // Both the switch's own reconvergence and the downstream merge are unsafe; + // multiple switch ports may also report the same predecessor. Pin the + // affected fan-ins without coupling the test to diagnostic multiplicity. + assert!( + errs.iter() + .any(|error| error.node_id.as_deref() == Some("a")) + ); + assert!( + errs.iter() + .any(|error| error.node_id.as_deref() == Some("m")) + ); +} + +#[test] +fn engine_compatibility_rejects_reconvergence_before_nested_router() { + let g = graph(json!({ + "name": "reconverged-before-nested-router", + "nodes": [ + { "id": "start", "kind": "trigger", "name": "Trigger" }, + { "id": "outer", "kind": "condition", "name": "Outer", "config": { "field": "outer" } }, + { "id": "inner", "kind": "condition", "name": "Inner", "config": { "field": "inner" } }, + { "id": "a", "kind": "output_parser", "name": "A" }, + { "id": "inner_else", "kind": "output_parser", "name": "Inner else" }, + { "id": "c", "kind": "output_parser", "name": "C" }, + { "id": "m", "kind": "merge", "name": "Merge" } + ], + "edges": [ + { "from_node": "start", "from_port": "main", "to_node": "outer" }, + { "from_node": "start", "from_port": "main", "to_node": "c" }, + { "from_node": "outer", "from_port": "true", "to_node": "inner" }, + { "from_node": "outer", "from_port": "false", "to_node": "inner" }, + { "from_node": "inner", "from_port": "true", "to_node": "a" }, + { "from_node": "inner", "from_port": "false", "to_node": "inner_else" }, + { "from_node": "a", "from_port": "main", "to_node": "m" }, + { "from_node": "c", "from_port": "main", "to_node": "m" } + ] + })); + let errs = errors(&g); + assert_eq!(errs.len(), 1); + assert_eq!(errs[0].code, UNSUPPORTED_NESTED_CONDITIONAL_FAN_IN); +} + +#[test] +fn engine_compatibility_treats_single_wired_router_outputs_as_conditional() { + let g = graph(json!({ + "name": "single-wired-nested-router-fan-in", + "nodes": [ + { "id": "start", "kind": "trigger", "name": "Trigger" }, + { "id": "outer", "kind": "switch", "name": "Outer", "config": { "field": "outer" } }, + { "id": "inner", "kind": "condition", "name": "Inner", "config": { "field": "inner" } }, + { "id": "a", "kind": "output_parser", "name": "A" }, + { "id": "c", "kind": "output_parser", "name": "C" }, + { "id": "m", "kind": "merge", "name": "Merge" } + ], + "edges": [ + { "from_node": "start", "from_port": "main", "to_node": "outer" }, + { "from_node": "start", "from_port": "main", "to_node": "c" }, + { "from_node": "outer", "from_port": "case", "to_node": "inner" }, + { "from_node": "inner", "from_port": "true", "to_node": "a" }, + { "from_node": "a", "from_port": "main", "to_node": "m" }, + { "from_node": "c", "from_port": "main", "to_node": "m" } + ] + })); + + let errs = errors(&g); + assert_eq!(errs.len(), 1); + assert_eq!(errs[0].code, UNSUPPORTED_NESTED_CONDITIONAL_FAN_IN); + assert_eq!(errs[0].node_id.as_deref(), Some("m")); +} + +#[test] +fn engine_compatibility_detects_a_router_directly_preceding_fan_in() { + let nested = graph(json!({ + "name": "direct-nested-router-fan-in", + "nodes": [ + { "id": "start", "kind": "trigger", "name": "Trigger" }, + { "id": "outer", "kind": "switch", "name": "Outer", "config": { "field": "outer" } }, + { "id": "inner", "kind": "condition", "name": "Inner", "config": { "field": "inner" } }, + { "id": "c", "kind": "output_parser", "name": "C" }, + { "id": "m", "kind": "merge", "name": "Merge" } + ], + "edges": [ + { "from_node": "start", "from_port": "main", "to_node": "outer" }, + { "from_node": "start", "from_port": "main", "to_node": "c" }, + { "from_node": "outer", "from_port": "case", "to_node": "inner" }, + { "from_node": "inner", "from_port": "true", "to_node": "m" }, + { "from_node": "c", "from_port": "main", "to_node": "m" } + ] + })); + let errs = errors(&nested); + assert_eq!(errs.len(), 1); + assert_eq!(errs[0].code, UNSUPPORTED_NESTED_CONDITIONAL_FAN_IN); + + let main_port = graph(json!({ + "name": "direct-main-port-router-fan-in", + "nodes": [ + { "id": "start", "kind": "trigger", "name": "Trigger" }, + { "id": "route", "kind": "switch", "name": "Route", "config": { "field": "kind" } }, + { "id": "c", "kind": "output_parser", "name": "C" }, + { "id": "m", "kind": "merge", "name": "Merge" } + ], + "edges": [ + { "from_node": "start", "from_port": "main", "to_node": "route" }, + { "from_node": "start", "from_port": "main", "to_node": "c" }, + { "from_node": "route", "from_port": "main", "to_node": "m" }, + { "from_node": "c", "from_port": "main", "to_node": "m" } + ] + })); + let errs = errors(&main_port); + assert_eq!(errs.len(), 1); + assert_eq!(errs[0].code, UNSUPPORTED_MAIN_PORT_CONDITIONAL_FAN_IN); +} diff --git a/crates/tinyflows/src/compat_tests.rs b/crates/tinyflows/src/compat_tests.rs index ed1a72ba..d1930d2b 100644 --- a/crates/tinyflows/src/compat_tests.rs +++ b/crates/tinyflows/src/compat_tests.rs @@ -239,3 +239,6 @@ fn the_depth_budget_is_read_off_the_trigger() { crate::engine::MAX_SUB_WORKFLOW_DEPTH ); } + +#[path = "compat_router_tests.rs"] +mod router_tests; diff --git a/crates/tinyflows/src/gates/gates_prompt_tests.rs b/crates/tinyflows/src/gates/gates_prompt_tests.rs new file mode 100644 index 00000000..d6e83a92 --- /dev/null +++ b/crates/tinyflows/src/gates/gates_prompt_tests.rs @@ -0,0 +1,76 @@ +use super::*; + +// ---- prompts that are real jq, and literal args ---- +// +// Ported from OpenHuman's host-side gate tests, which pinned these scenarios +// through a wrapper around this module. + +#[test] +fn a_jq_string_concatenation_prompt_is_accepted() { + let graph = graph(json!([ + { "id": "greet", "kind": "agent", "name": "Greet", + "config": { "prompt": "=\"Hi \" + .item.name" } }, + ])); + + assert!(failures(&graph).is_empty(), "{:?}", failures(&graph)); +} + +/// Regression for the quote-toggle desync: an escaped quote inside a jq string +/// literal must not flip the string-stripping pass's in-string state, or the +/// text between the escaped and the closing quote leaks out as bare code and +/// trips the prose heuristic. +#[test] +fn an_escaped_quote_inside_a_jq_string_is_not_mistaken_for_prose() { + let graph = graph(json!([ + { "id": "greet", "kind": "agent", "name": "Greet", + "config": { "prompt": "=\"Say \\\"hello world\\\" nicely\" + .item.name" } }, + ])); + + assert!(failures(&graph).is_empty(), "{:?}", failures(&graph)); +} + +#[test] +fn literal_tool_args_are_not_inspected() { + let graph = graph(json!([ + { "id": "post", "kind": "tool_call", "name": "Post", + "config": { "slug": "SLACK_SEND_MESSAGE", + "args": { "channel": "general", "count": 3, "cc": ["a@b.com"] } } }, + ])); + + assert!(failures(&graph).is_empty(), "{:?}", failures(&graph)); +} + +#[test] +fn a_binding_to_a_schema_less_agent_is_unverifiable_not_refused() { + let graph = graph(json!([ + { "id": "summarize", "kind": "agent", "name": "Summarize", + "config": { "agent_ref": "researcher", "prompt": "summarize" } }, + { "id": "post", "kind": "tool_call", "name": "Post", + "config": { "slug": "SLACK_SEND_MESSAGE", + "args": { "channel": "=nodes.summarize.item.json.channel" } } }, + ])); + + assert!(failures(&graph).is_empty(), "{:?}", failures(&graph)); +} + +/// Skipping `.json` is refused even when the agent declares a matching schema: +/// the fault is the envelope, not the field inside it. +#[test] +fn skipping_the_envelope_is_refused_even_when_the_schema_matches() { + let graph = graph(json!([ + { "id": "summarize", "kind": "agent", "name": "Summarize", + "config": { "prompt": "summarize", + "output_parser": { "schema": { "type": "object", + "properties": { "channel": { "type": "string" } } } } } }, + { "id": "post", "kind": "tool_call", "name": "Post", + "config": { "slug": "SLACK_SEND_MESSAGE", + "args": { "channel": "=nodes.summarize.item.channel" } } }, + ])); + + let failures = failures(&graph); + assert_eq!(failures.len(), 1, "{failures:?}"); + assert!( + failures[0].contains("Fix: `=nodes.summarize.item.json.channel`."), + "{failures:?}" + ); +} diff --git a/crates/tinyflows/src/gates/gates_tests.rs b/crates/tinyflows/src/gates/gates_tests.rs index 82d74550..2d10126c 100644 --- a/crates/tinyflows/src/gates/gates_tests.rs +++ b/crates/tinyflows/src/gates/gates_tests.rs @@ -481,3 +481,6 @@ fn a_nested_path_under_an_envelope_accessor_is_accepted() { assert!(failures(&graph).is_empty(), "{:?}", failures(&graph)); } + +#[path = "gates_prompt_tests.rs"] +mod prompt_tests; diff --git a/crates/tinyflows/src/migrate.rs b/crates/tinyflows/src/migrate.rs index 29f9ebcd..4c5f3fc7 100644 --- a/crates/tinyflows/src/migrate.rs +++ b/crates/tinyflows/src/migrate.rs @@ -14,7 +14,7 @@ //! [`WorkflowGraph`]: crate::model::WorkflowGraph use crate::error::{Result, ValidationError}; -use crate::model::CURRENT_SCHEMA_VERSION; +use crate::model::{CURRENT_SCHEMA_VERSION, WorkflowGraph}; use serde_json::Value; /// Upgrades a persisted [`WorkflowGraph`] JSON value to the current schema. @@ -102,6 +102,139 @@ pub fn migrate(mut value: Value) -> Result { Ok(value) } +/// Migrates raw graph JSON and deserializes it into a [`WorkflowGraph`], +/// attributing any failure to the member that caused it. +/// +/// This is [`migrate`] followed by `serde_json::from_value`, **without** the +/// structural [`validate`](crate::validate) step, so a caller that wants every +/// structural error (via `validate::validate_all`) can run validation itself. +/// A failure here (an unmigratable schema, JSON that does not fit the model) is +/// genuinely a single error, whereas structural validation can surface many. +/// +/// `serde_json` errors carry no path, and `missing field `name`` on its own is +/// unactionable: every field of `WorkflowGraph` is `#[serde(default)]`, so the +/// fault is always in a nested object, and a reader who takes it for the +/// top-level `name` they already set retries unchanged. On failure the error +/// therefore names the offending element (`nodes[1]: missing field `name``) or +/// top-level field (`name: invalid type: integer `123`, expected a string`). +/// +/// # Errors +/// +/// The [`migrate`] error, rendered, when migration refuses the document; +/// otherwise the located serde message described above. When the fault is not +/// in a single element or field (a non-array `nodes`, a non-object graph) the +/// bare serde message is returned rather than a guessed location. +/// +/// # Examples +/// +/// ``` +/// use serde_json::json; +/// use tinyflows::migrate::deserialize_graph; +/// +/// let err = deserialize_graph(json!({ +/// "nodes": [ +/// { "id": "start", "kind": "trigger", "name": "Trigger" }, +/// { "id": "nameless", "kind": "trigger" } +/// ] +/// })) +/// .unwrap_err(); +/// assert!(err.starts_with("nodes[1]: "), "{err}"); +/// ``` +pub fn deserialize_graph(value: Value) -> std::result::Result { + let migrated = migrate(value).map_err(|e| e.to_string())?; + serde_json::from_value::(migrated.clone()) + .map_err(|e| locate_graph_error(&migrated, &e)) +} + +/// The `WorkflowGraph` fields whose elements carry their own required fields. +const ELEMENT_ARRAYS: &[&str] = &["nodes", "inputs", "agents", "edges"]; + +/// Names the element a graph-level deserialization error came from. +/// +/// Re-deserializes each member of the arrays that carry required fields and +/// reports the first that fails on its own, as `nodes[1]: `. None +/// of these types use `deny_unknown_fields`, so an element that parses in +/// isolation is one the graph-level parse accepted too, and a failure found +/// here is the real fault rather than an artefact of checking it alone. +/// +/// Runs only on the error path, and falls back to the bare message when the +/// fault is not in a single element -- a wrong type for `nodes` itself, say. +fn locate_graph_error(migrated: &Value, err: &serde_json::Error) -> String { + // Re-parse with the element arrays emptied. If that still fails, the fault + // is in the graph's own fields -- a non-string `name`, say -- and scanning + // members would pin it on the first member that happens to be invalid too, + // which is a confident wrong answer rather than a vague right one. + let mut skeleton = migrated.clone(); + if let Some(fields) = skeleton.as_object_mut() { + for field in ELEMENT_ARRAYS { + if let Some(slot) = fields.get_mut(*field) { + if slot.is_array() { + *slot = Value::Array(Vec::new()); + } + } + } + } + if serde_json::from_value::(skeleton).is_err() { + return locate_top_level_error(migrated, err); + } + + macro_rules! locate { + ($field:literal, $ty:ty) => { + if let Some(items) = migrated.get($field).and_then(Value::as_array) { + for (index, item) in items.iter().enumerate() { + if let Err(inner) = serde_json::from_value::<$ty>(item.clone()) { + return format!("{}[{}]: {}", $field, index, inner); + } + } + } + }; + } + + locate!("nodes", crate::model::Node); + locate!("inputs", crate::model::WorkflowInput); + locate!("agents", crate::model::AgentDefinition); + locate!("edges", crate::model::Edge); + + err.to_string() +} + +/// Names the graph's own field when the fault is at the top level. +/// +/// `serde_json` reports a type mismatch as `invalid type: integer \`123\`, +/// expected a string` with **no field name** -- the same unactionable shape as +/// the missing-field case this helper exists to fix, so it gets the same +/// treatment. +/// +/// Every `WorkflowGraph` field is `#[serde(default)]`, so an object carrying a +/// single field parses if and only if that field is valid. Probing one key at a +/// time therefore names the offender without a hardcoded field list. Unknown +/// keys parse (no `deny_unknown_fields`) and are skipped. +fn locate_top_level_error(migrated: &Value, err: &serde_json::Error) -> String { + if let Some(fields) = migrated.as_object() { + // A malformed collection can coexist with malformed members in a + // different collection. Report the collection shape before probing + // individual member collections, independent of JSON map key order. + for key in ELEMENT_ARRAYS { + if let Some(value) = fields.get(*key).filter(|value| !value.is_array()) { + let probe = + Value::Object([((*key).to_string(), value.clone())].into_iter().collect()); + if let Err(inner) = serde_json::from_value::(probe) { + return format!("{key}: {inner}"); + } + } + } + + for (key, value) in fields { + let probe = Value::Object([(key.clone(), value.clone())].into_iter().collect()); + if let Err(inner) = serde_json::from_value::(probe) { + return format!("{key}: {inner}"); + } + } + } + + err.to_string() +} + #[cfg(test)] #[path = "migrate_tests.rs"] mod tests; diff --git a/crates/tinyflows/src/migrate_tests.rs b/crates/tinyflows/src/migrate_tests.rs index bfcf5456..a0f2f524 100644 --- a/crates/tinyflows/src/migrate_tests.rs +++ b/crates/tinyflows/src/migrate_tests.rs @@ -273,3 +273,104 @@ proptest! { } } } + +// ---- deserialize_graph: member-located errors ---- +// +// Ported from OpenHuman, where this lived beside the flow ops. + +#[test] +fn deserialize_graph_names_the_member_in_inputs_agents_and_edges() { + let err = deserialize_graph(json!({ "inputs": [{ "type": "string" }] })) + .expect_err("input without `name`"); + assert!(err.starts_with("inputs[0]: "), "got: {err}"); + + let err = deserialize_graph(json!({ "agents": [{ "name": "no id" }] })) + .expect_err("agent without `id`"); + assert!(err.starts_with("agents[0]: "), "got: {err}"); + + let err = deserialize_graph(json!({ "edges": [{ "from_port": "main" }] })) + .expect_err("edge without endpoints"); + assert!(err.starts_with("edges[0]: "), "got: {err}"); +} + +#[test] +fn deserialize_graph_names_the_node_that_is_missing_a_field() { + let err = deserialize_graph(json!({ + "name": "top-level name is present and is NOT the problem", + "nodes": [ + { "id": "start", "kind": "trigger", "name": "Trigger" }, + { "id": "nameless", "kind": "trigger" } + ] + })) + .expect_err("a node without `name` must not deserialize"); + + assert!(err.starts_with("nodes[1]: "), "got: {err}"); + // The serde detail survives the wrapping. + assert!( + err.contains("missing field") && err.contains("name"), + "got: {err}" + ); +} + +#[test] +fn deserialize_graph_falls_back_when_no_member_is_at_fault() { + let err = deserialize_graph(json!({ "name": "valid", "nodes": "not an array" })) + .expect_err("a non-array `nodes` must not deserialize"); + + assert!(!err.contains("nodes["), "got: {err}"); + assert!(err.contains("invalid type"), "got: {err}"); +} + +#[test] +fn deserialize_graph_prefers_a_malformed_collection_over_an_invalid_member_elsewhere() { + let err = deserialize_graph(json!({ + "nodes": "not an array", + "edges": [{ "from_port": "main" }] + })) + .expect_err("both the nodes collection and an edge member are malformed"); + + assert!(err.starts_with("nodes: "), "got: {err}"); + assert!(!err.contains("edges[0]"), "got: {err}"); +} + +#[test] +fn deserialize_graph_reports_a_non_object_graph_without_inventing_a_location() { + let err = deserialize_graph(json!("this is a string, not a workflow graph")) + .expect_err("a non-object graph must not deserialize"); + + assert!(!err.contains('['), "got: {err}"); + assert!(err.contains("invalid type"), "got: {err}"); +} + +/// The premise of the member-naming tests: a graph with no top-level `name` +/// deserializes, so `missing field `name`` is ambiguous without a path. +#[test] +fn deserialize_graph_accepts_a_graph_with_no_top_level_name() { + let graph = deserialize_graph(json!({ + "nodes": [ { "id": "start", "kind": "trigger", "name": "Trigger" } ] + })) + .expect("top-level `name` is #[serde(default)] and must not be required"); + assert_eq!(graph.name, ""); + assert_eq!(graph.schema_version, CURRENT_SCHEMA_VERSION); +} + +/// When the graph's own fields are at fault *and* a node is independently +/// invalid, the error must not be pinned on the node. +#[test] +fn deserialize_graph_does_not_blame_a_node_for_a_top_level_fault() { + let err = deserialize_graph(json!({ + "name": 123, + "nodes": [ { "id": "nameless", "kind": "trigger" } ] + })) + .expect_err("a non-string top-level `name` must not deserialize"); + + assert!(!err.contains("nodes["), "got: {err}"); + assert!(err.starts_with("name: "), "got: {err}"); +} + +#[test] +fn deserialize_graph_surfaces_a_migration_refusal() { + let err = deserialize_graph(json!({ "schema_version": CURRENT_SCHEMA_VERSION + 1 })) + .expect_err("a future schema must be refused"); + assert!(!err.is_empty()); +}