From e49bdeb55a636ad5ada63b0db4a959e00934bb30 Mon Sep 17 00:00:00 2001 From: Eason Guo Date: Sat, 17 Oct 2026 17:45:01 +1300 Subject: [PATCH 1/3] feat: add outbound flow validation Add an outbound profile for flows run on a call the app places, with no person standing by: `validate::validate_outbound` (Rust) and `validateOutbound` (TS). Each runs the base validation first and adds: - outbound_node_not_allowed: only greeting, menu, message, hangup and hours steps are allowed (no ring, book or transfer). - outbound_unbounded_loop: no loop reachable from entry made only of steps that move on without the callee (greeting, hours). Loops through a menu are fine. Shared cases live in conformance/vN/outbound/{valid,invalid}, run by both languages. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QGHE3tcTFWUtjDUxNxTWNG --- conformance/README.md | 9 + .../v1/outbound/valid/reminder.expected.json | 5 + conformance/v1/outbound/valid/reminder.yaml | 20 ++ .../v2/outbound/invalid/book.expected.json | 5 + conformance/v2/outbound/invalid/book.yaml | 16 + .../invalid/dangling-exit.expected.json | 5 + .../v2/outbound/invalid/dangling-exit.yaml | 11 + .../invalid/greeting-loop.expected.json | 5 + .../v2/outbound/invalid/greeting-loop.yaml | 13 + .../outbound/invalid/hours-loop.expected.json | 5 + .../v2/outbound/invalid/hours-loop.yaml | 17 + .../v2/outbound/invalid/ring.expected.json | 5 + conformance/v2/outbound/invalid/ring.yaml | 15 + .../outbound/invalid/transfer.expected.json | 5 + conformance/v2/outbound/invalid/transfer.yaml | 12 + .../outbound/valid/after-hours.expected.json | 5 + .../v2/outbound/valid/after-hours.yaml | 29 ++ crates/wavekat-flow/README.md | 10 + crates/wavekat-flow/src/validate.rs | 307 +++++++++++++++++- crates/wavekat-flow/tests/conformance.rs | 73 ++++- packages/flow-schema/README.md | 1 + packages/flow-schema/src/index.ts | 2 +- packages/flow-schema/src/validate.ts | 141 +++++++- packages/flow-schema/test/conformance.test.ts | 49 ++- packages/flow-schema/test/outbound.test.ts | 147 +++++++++ 25 files changed, 874 insertions(+), 38 deletions(-) create mode 100644 conformance/v1/outbound/valid/reminder.expected.json create mode 100644 conformance/v1/outbound/valid/reminder.yaml create mode 100644 conformance/v2/outbound/invalid/book.expected.json create mode 100644 conformance/v2/outbound/invalid/book.yaml create mode 100644 conformance/v2/outbound/invalid/dangling-exit.expected.json create mode 100644 conformance/v2/outbound/invalid/dangling-exit.yaml create mode 100644 conformance/v2/outbound/invalid/greeting-loop.expected.json create mode 100644 conformance/v2/outbound/invalid/greeting-loop.yaml create mode 100644 conformance/v2/outbound/invalid/hours-loop.expected.json create mode 100644 conformance/v2/outbound/invalid/hours-loop.yaml create mode 100644 conformance/v2/outbound/invalid/ring.expected.json create mode 100644 conformance/v2/outbound/invalid/ring.yaml create mode 100644 conformance/v2/outbound/invalid/transfer.expected.json create mode 100644 conformance/v2/outbound/invalid/transfer.yaml create mode 100644 conformance/v2/outbound/valid/after-hours.expected.json create mode 100644 conformance/v2/outbound/valid/after-hours.yaml create mode 100644 packages/flow-schema/test/outbound.test.ts diff --git a/conformance/README.md b/conformance/README.md index a3a3237..a43e001 100644 --- a/conformance/README.md +++ b/conformance/README.md @@ -17,8 +17,17 @@ conformance/ .yaml .expected.json v2/ # `book` — same layout, its own frozen cases + vN/outbound/ # optional: the outbound profile + valid/ invalid/ ``` +`vN/outbound/` holds cases for the **outbound** gate (`validate_outbound` / +`validateOutbound`): a flow run on a call the app places, with no person +standing by. Same file pair and expectation shape; `semantic` is the verdict +of the outbound gate, which includes the base validation. These cases stay +out of the top-level `valid/`/`invalid/` buckets because a flow refused for +outbound — one with a `ring` step, say — is a perfectly good inbound flow. + Every `.yaml` has a sibling `.expected.json`: ```json diff --git a/conformance/v1/outbound/valid/reminder.expected.json b/conformance/v1/outbound/valid/reminder.expected.json new file mode 100644 index 0000000..d65f071 --- /dev/null +++ b/conformance/v1/outbound/valid/reminder.expected.json @@ -0,0 +1,5 @@ +{ + "description": "A typical outbound flow: say something, ask the callee to confirm, and end. It loops (the menu's `2` replays the greeting), which is fine: the loop runs through a menu, which waits for the callee, and the menu's `no_input` exit ends the call when nobody answers.", + "structurallyValid": true, + "semantic": { "ok": true, "errors": [] } +} diff --git a/conformance/v1/outbound/valid/reminder.yaml b/conformance/v1/outbound/valid/reminder.yaml new file mode 100644 index 0000000..e0f36c4 --- /dev/null +++ b/conformance/v1/outbound/valid/reminder.yaml @@ -0,0 +1,20 @@ +schema_version: 1 +id: flow_reminder +name: Appointment reminder +entry: hello +nodes: + hello: + kind: greeting + prompt: Hello, this is a reminder about your appointment tomorrow at ten. + exits: { next: ask } + ask: + kind: menu + prompt: Press 1 to confirm, or 2 to hear this again. + options: { "1": Confirm, "2": Repeat } + exits: { "1": thanks, "2": hello, no_input: bye, invalid: ask } + thanks: + kind: hangup + prompt: Thanks, see you then. + bye: + kind: hangup + prompt: Sorry we missed you. Goodbye. diff --git a/conformance/v2/outbound/invalid/book.expected.json b/conformance/v2/outbound/invalid/book.expected.json new file mode 100644 index 0000000..8123a6f --- /dev/null +++ b/conformance/v2/outbound/invalid/book.expected.json @@ -0,0 +1,5 @@ +{ + "description": "A `book` step books against the number that called in, which a call the app places does not have, so outbound refuses it. The same document is valid as an inbound flow.", + "structurallyValid": true, + "semantic": { "ok": false, "errors": ["outbound_node_not_allowed"] } +} diff --git a/conformance/v2/outbound/invalid/book.yaml b/conformance/v2/outbound/invalid/book.yaml new file mode 100644 index 0000000..495a31b --- /dev/null +++ b/conformance/v2/outbound/invalid/book.yaml @@ -0,0 +1,16 @@ +schema_version: 2 +id: flow_outbound_book +name: Outbound with a booking step +entry: take_booking +nodes: + take_booking: + kind: book + prompt: I can book you in. + confirm_prompt: You're booked for + timezone: UTC + schedule: + tue: [{ open: "09:00", close: "17:00" }] + duration_mins: 30 + exits: { booked: bye, no_slots: bye, no_input: bye, unavailable: bye } + bye: + kind: hangup diff --git a/conformance/v2/outbound/invalid/dangling-exit.expected.json b/conformance/v2/outbound/invalid/dangling-exit.expected.json new file mode 100644 index 0000000..a04ef70 --- /dev/null +++ b/conformance/v2/outbound/invalid/dangling-exit.expected.json @@ -0,0 +1,5 @@ +{ + "description": "The outbound gate includes the base validation: a document the base validator rejects is rejected on outbound too, with the same codes.", + "structurallyValid": true, + "semantic": { "ok": false, "errors": ["unknown_target"] } +} diff --git a/conformance/v2/outbound/invalid/dangling-exit.yaml b/conformance/v2/outbound/invalid/dangling-exit.yaml new file mode 100644 index 0000000..5e13770 --- /dev/null +++ b/conformance/v2/outbound/invalid/dangling-exit.yaml @@ -0,0 +1,11 @@ +schema_version: 2 +id: flow_outbound_dangling +name: Outbound with a broken exit +entry: hello +nodes: + hello: + kind: greeting + prompt: Hello. + exits: { next: nowhere } + bye: + kind: hangup diff --git a/conformance/v2/outbound/invalid/greeting-loop.expected.json b/conformance/v2/outbound/invalid/greeting-loop.expected.json new file mode 100644 index 0000000..96fc239 --- /dev/null +++ b/conformance/v2/outbound/invalid/greeting-loop.expected.json @@ -0,0 +1,5 @@ +{ + "description": "Two greetings that hand off to each other: the loop never waits for the callee. Reported as an unbounded loop, alongside the base validator's `trapped` (nothing in it can end the call).", + "structurallyValid": true, + "semantic": { "ok": false, "errors": ["outbound_unbounded_loop", "trapped"] } +} diff --git a/conformance/v2/outbound/invalid/greeting-loop.yaml b/conformance/v2/outbound/invalid/greeting-loop.yaml new file mode 100644 index 0000000..2fedc64 --- /dev/null +++ b/conformance/v2/outbound/invalid/greeting-loop.yaml @@ -0,0 +1,13 @@ +schema_version: 2 +id: flow_outbound_greeting_loop +name: Says the same thing forever +entry: one +nodes: + one: + kind: greeting + prompt: Hello. + exits: { next: two } + two: + kind: greeting + prompt: Hello again. + exits: { next: one } diff --git a/conformance/v2/outbound/invalid/hours-loop.expected.json b/conformance/v2/outbound/invalid/hours-loop.expected.json new file mode 100644 index 0000000..f924050 --- /dev/null +++ b/conformance/v2/outbound/invalid/hours-loop.expected.json @@ -0,0 +1,5 @@ +{ + "description": "During opening hours, hours -> greeting -> hours repeats with nothing waiting on the callee; played into voicemail it would never end. The `closed` exit keeps it from being trapped, so the base validator accepts it — this is the case only the outbound rule catches.", + "structurallyValid": true, + "semantic": { "ok": false, "errors": ["outbound_unbounded_loop"] } +} diff --git a/conformance/v2/outbound/invalid/hours-loop.yaml b/conformance/v2/outbound/invalid/hours-loop.yaml new file mode 100644 index 0000000..ba758c3 --- /dev/null +++ b/conformance/v2/outbound/invalid/hours-loop.yaml @@ -0,0 +1,17 @@ +schema_version: 2 +id: flow_outbound_hours_loop +name: Repeats while open +entry: check +nodes: + check: + kind: hours + timezone: UTC + schedule: + mon: [{ open: "09:00", close: "17:00" }] + exits: { open: say, closed: bye } + say: + kind: greeting + prompt: We are open. + exits: { next: check } + bye: + kind: hangup diff --git a/conformance/v2/outbound/invalid/ring.expected.json b/conformance/v2/outbound/invalid/ring.expected.json new file mode 100644 index 0000000..1d35f48 --- /dev/null +++ b/conformance/v2/outbound/invalid/ring.expected.json @@ -0,0 +1,5 @@ +{ + "description": "A `ring` step hands the call to a person at the device. On a call the app places there is nobody standing by to take it, so outbound refuses the step. The same document is valid as an inbound flow.", + "structurallyValid": true, + "semantic": { "ok": false, "errors": ["outbound_node_not_allowed"] } +} diff --git a/conformance/v2/outbound/invalid/ring.yaml b/conformance/v2/outbound/invalid/ring.yaml new file mode 100644 index 0000000..1ef41dd --- /dev/null +++ b/conformance/v2/outbound/invalid/ring.yaml @@ -0,0 +1,15 @@ +schema_version: 2 +id: flow_outbound_ring +name: Outbound with a ring step +entry: hello +nodes: + hello: + kind: greeting + prompt: Please hold while we connect you. + exits: { next: desk } + desk: + kind: ring + timeout_secs: 20 + exits: { no_answer: bye } + bye: + kind: hangup diff --git a/conformance/v2/outbound/invalid/transfer.expected.json b/conformance/v2/outbound/invalid/transfer.expected.json new file mode 100644 index 0000000..e85a34b --- /dev/null +++ b/conformance/v2/outbound/invalid/transfer.expected.json @@ -0,0 +1,5 @@ +{ + "description": "A `transfer` step is not offered on outbound calls yet, so outbound refuses it. The same document is valid as an inbound flow.", + "structurallyValid": true, + "semantic": { "ok": false, "errors": ["outbound_node_not_allowed"] } +} diff --git a/conformance/v2/outbound/invalid/transfer.yaml b/conformance/v2/outbound/invalid/transfer.yaml new file mode 100644 index 0000000..e2f9ca3 --- /dev/null +++ b/conformance/v2/outbound/invalid/transfer.yaml @@ -0,0 +1,12 @@ +schema_version: 2 +id: flow_outbound_transfer +name: Outbound with a transfer step +entry: hello +nodes: + hello: + kind: greeting + prompt: Connecting you now. + exits: { next: forward } + forward: + kind: transfer + target: "+15550100" diff --git a/conformance/v2/outbound/valid/after-hours.expected.json b/conformance/v2/outbound/valid/after-hours.expected.json new file mode 100644 index 0000000..d4100a4 --- /dev/null +++ b/conformance/v2/outbound/valid/after-hours.expected.json @@ -0,0 +1,5 @@ +{ + "description": "Every outbound-allowed kind in one document — hours, greeting, menu, message and hangup — declared at schema_version 2. The hours step is not on a loop; the only loop goes through the menu.", + "structurallyValid": true, + "semantic": { "ok": true, "errors": [] } +} diff --git a/conformance/v2/outbound/valid/after-hours.yaml b/conformance/v2/outbound/valid/after-hours.yaml new file mode 100644 index 0000000..8a5b34b --- /dev/null +++ b/conformance/v2/outbound/valid/after-hours.yaml @@ -0,0 +1,29 @@ +schema_version: 2 +id: flow_outbound_hours +name: Notice with an after-hours variant +entry: check +nodes: + check: + kind: hours + timezone: America/New_York + schedule: + mon: [{ open: "09:00", close: "17:00" }] + tue: [{ open: "09:00", close: "17:00" }] + exits: { open: notice, closed: after_hours } + notice: + kind: greeting + prompt: Your order is ready for pickup. + exits: { next: options } + options: + kind: menu + prompt: Press 1 to leave a message for the store, or 9 to hear this again. + options: { "1": Message, "9": Repeat } + exits: { "1": leave_message, "9": notice, no_input: bye, invalid: bye } + leave_message: + kind: message + prompt: Leave your message after the tone. + after_hours: + kind: hangup + prompt: Your order is ready. The store opens at nine tomorrow. + bye: + kind: hangup diff --git a/crates/wavekat-flow/README.md b/crates/wavekat-flow/README.md index a4549b2..af04477 100644 --- a/crates/wavekat-flow/README.md +++ b/crates/wavekat-flow/README.md @@ -67,6 +67,16 @@ if let Err(errors) = validate::validate(&flow) { } ``` +### Outbound calls + +A flow run on a call the app places — with no person standing by — goes +through `validate::validate_outbound` instead. It runs `validate` first and +adds the outbound rules, so call it on its own: only `greeting`, `menu`, +`message`, `hangup` and `hours` steps are allowed +(`outbound_node_not_allowed`), and no loop may be made only of steps that move +on without the callee, such as a greeting that leads back to itself +(`outbound_unbounded_loop`). A loop through a `menu` is fine. + ## Upgrading when the booking grid moves `BOOK_GRANULARITY_MINS` decides which times a `book` node can ever say, so diff --git a/crates/wavekat-flow/src/validate.rs b/crates/wavekat-flow/src/validate.rs index a3baeda..f6ddf01 100644 --- a/crates/wavekat-flow/src/validate.rs +++ b/crates/wavekat-flow/src/validate.rs @@ -108,6 +108,14 @@ pub enum ValidationError { required: i64, declared: i64, }, + + #[error("node {node:?} is a {kind} step, which an outbound call cannot run")] + OutboundNodeNotAllowed { node: NodeId, kind: &'static str }, + + #[error( + "node {node:?} is on a loop that never waits for the callee, so an outbound call could repeat it forever" + )] + OutboundUnboundedLoop { node: NodeId }, } impl ValidationError { @@ -136,6 +144,8 @@ impl ValidationError { ValidationError::BookOutOfRange { .. } => "book_out_of_range", ValidationError::BookNeverOpen { .. } => "book_never_open", ValidationError::KindRequiresNewerSchema { .. } => "kind_requires_newer_schema", + ValidationError::OutboundNodeNotAllowed { .. } => "outbound_node_not_allowed", + ValidationError::OutboundUnboundedLoop { .. } => "outbound_unbounded_loop", } } } @@ -227,6 +237,128 @@ pub fn validate(flow: &Flow) -> Result<(), Vec> { } } +/// The kinds a flow may use when it runs on a call the app *places*, with +/// no person standing by. Twin: `validate.ts` `OUTBOUND_KINDS`. +/// +/// Left out: `ring` (there is nobody to hand the call to), `book` (it +/// books against the number that called in, which an outbound call does +/// not have), and `transfer` (not offered on outbound calls yet). +pub const OUTBOUND_KINDS: &[&str] = &["greeting", "menu", "message", "hangup", "hours"]; + +/// Validate a flow for an **outbound** call: everything [`validate`] checks, +/// plus the rules for a call the app places itself. +/// +/// This runs [`validate`] first and adds to its errors, so it is the one +/// gate to call for an outbound flow — there is no need to call both. All +/// errors are collected, as with [`validate`]. +/// +/// The outbound rules: +/// +/// - every node is one of [`OUTBOUND_KINDS`] (`outbound_node_not_allowed`); +/// - no loop reachable from `entry` is made only of steps that move on +/// without the callee (`greeting`, `hours`) — such a loop, played into +/// voicemail or to a callee who stays silent, would never end +/// (`outbound_unbounded_loop`). A loop through a `menu` is fine: the +/// menu waits for the callee and its `no_input` exit is the way out. +/// +/// Twin: `validate.ts` `validateOutbound`. +pub fn validate_outbound(flow: &Flow) -> Result<(), Vec> { + let mut errs = validate(flow).err().unwrap_or_default(); + + // In id order, so the errors come out the same every time. + let mut nodes: Vec<(&NodeId, &Node)> = flow.nodes.iter().collect(); + nodes.sort_by_key(|(id, _)| *id); + for (id, node) in nodes { + if !OUTBOUND_KINDS.contains(&node.kind()) { + errs.push(ValidationError::OutboundNodeNotAllowed { + node: id.clone(), + kind: node.kind(), + }); + } + } + + for node in unbounded_loop_nodes(flow) { + errs.push(ValidationError::OutboundUnboundedLoop { + node: node.to_string(), + }); + } + + if errs.is_empty() { + Ok(()) + } else { + Err(errs) + } +} + +/// Whether a node moves on without waiting for anything the callee does. +/// `greeting` always takes `next`; `hours` picks by the clock. +fn moves_on_by_itself(node: &Node) -> bool { + matches!(node, Node::Greeting { .. } | Node::Hours { .. }) +} + +/// Every node reachable from `entry` that sits on a cycle made only of +/// nodes that move on by themselves, in id order. +fn unbounded_loop_nodes(flow: &Flow) -> Vec<&str> { + let reachable = reachable_from_entry(flow); + // The subgraph the rule is about: reachable, self-advancing nodes and + // the edges between them. + let successors = |id: &str| -> Vec<&str> { + flow.nodes + .get(id) + .and_then(Node::exits) + .into_iter() + .flatten() + .map(|(_, t)| t.as_str()) + .filter(|t| reachable.contains(t) && flow.nodes.get(*t).is_some_and(moves_on_by_itself)) + .collect() + }; + + let mut on_loop = Vec::new(); + for id in &reachable { + if !flow.nodes.get(*id).is_some_and(moves_on_by_itself) { + continue; + } + // Does `id` lead back to itself within the subgraph? + let mut seen: BTreeSet<&str> = BTreeSet::new(); + let mut stack = successors(id); + while let Some(next) = stack.pop() { + if next == *id { + on_loop.push(*id); + break; + } + if seen.insert(next) { + stack.extend(successors(next)); + } + } + } + on_loop +} + +/// Node ids reachable from `entry` over exit edges (`entry` included when +/// it exists). +fn reachable_from_entry(flow: &Flow) -> BTreeSet<&str> { + let mut reachable: BTreeSet<&str> = BTreeSet::new(); + let Some((entry, _)) = flow.nodes.get_key_value(&flow.entry) else { + return reachable; + }; + let mut queue: VecDeque<&str> = VecDeque::new(); + queue.push_back(entry.as_str()); + reachable.insert(entry.as_str()); + while let Some(id) = queue.pop_front() { + let Some(node) = flow.nodes.get(id) else { + continue; + }; + for (_, target) in node.exits().into_iter().flatten() { + if let Some((target, _)) = flow.nodes.get_key_value(target) { + if reachable.insert(target.as_str()) { + queue.push_back(target.as_str()); + } + } + } + } + reachable +} + /// Exit keys must be exactly the set the node's kind defines, and every /// target must exist. fn check_exits(id: &NodeId, node: &Node, flow: &Flow, errs: &mut Vec) { @@ -407,21 +539,7 @@ fn check_prompt(id: &NodeId, prompt: &Prompt, errs: &mut Vec) { /// Reachability from `entry`, and the "no caller is trapped" guarantee. fn check_graph(flow: &Flow, errs: &mut Vec) { - // BFS from entry over exit edges. - let mut reachable: BTreeSet<&str> = BTreeSet::new(); - let mut queue: VecDeque<&str> = VecDeque::new(); - queue.push_back(flow.entry.as_str()); - reachable.insert(flow.entry.as_str()); - while let Some(id) = queue.pop_front() { - let Some(node) = flow.nodes.get(id) else { - continue; - }; - for (_, target) in node.exits().into_iter().flatten() { - if flow.nodes.contains_key(target) && reachable.insert(target.as_str()) { - queue.push_back(target.as_str()); - } - } - } + let reachable = reachable_from_entry(flow); for id in flow.nodes.keys() { if !reachable.contains(id.as_str()) { @@ -809,4 +927,163 @@ nodes: {} "#; assert_eq!(required_schema_version(&parse(src)), 1); } + // ── `validate_outbound` ────────────────────────────────────────────── + + fn outbound_codes(src: &str) -> Vec<&'static str> { + validate_outbound(&parse(src)) + .err() + .unwrap_or_default() + .iter() + .map(ValidationError::code) + .collect() + } + + const REMINDER: &str = r#" +schema_version: 1 +id: f +name: n +entry: hello +nodes: + hello: + kind: greeting + prompt: This is a reminder about your appointment tomorrow. + exits: { next: ask } + ask: + kind: menu + prompt: Press 1 to confirm, or 2 to hear this again. + options: { "1": Confirm, "2": Repeat } + exits: { "1": thanks, "2": hello, no_input: bye, invalid: ask } + thanks: + kind: hangup + prompt: Thanks, see you then. + bye: + kind: hangup +"#; + + #[test] + fn an_outbound_flow_with_a_menu_loop_validates() { + assert_eq!(validate_outbound(&parse(REMINDER)), Ok(())); + } + + #[test] + fn outbound_refuses_ring_book_and_transfer() { + let ring = r#" +schema_version: 1 +id: f +name: n +entry: r +nodes: + r: + kind: ring + timeout_secs: 20 + exits: { no_answer: bye } + bye: + kind: hangup +"#; + let errs = validate_outbound(&parse(ring)).unwrap_err(); + assert!(errs.contains(&ValidationError::OutboundNodeNotAllowed { + node: "r".into(), + kind: "ring", + })); + + let transfer = r#" +schema_version: 1 +id: f +name: n +entry: t +nodes: + t: + kind: transfer + target: "100" +"#; + assert_eq!(outbound_codes(transfer), vec!["outbound_node_not_allowed"]); + + let errs = validate_outbound(&parse(BOOKING)).unwrap_err(); + assert!(errs.contains(&ValidationError::OutboundNodeNotAllowed { + node: "appointment".into(), + kind: "book", + })); + } + + #[test] + fn outbound_runs_the_base_validation_too() { + let codes = outbound_codes(&LUIGIS.replace("next: check_hours", "next: ghost")); + assert!(codes.contains(&"unknown_target"), "{codes:?}"); + // LUIGIS also has a ring step. + assert!(codes.contains(&"outbound_node_not_allowed"), "{codes:?}"); + } + + #[test] + fn a_base_valid_flow_can_still_loop_forever_outbound() { + // While open, hours -> greeting -> hours never stops; the closed + // exit makes it base-valid (not trapped). + let src = r#" +schema_version: 1 +id: f +name: n +entry: check +nodes: + check: + kind: hours + timezone: UTC + schedule: + mon: [{ open: "09:00", close: "17:00" }] + exits: { open: say, closed: bye } + say: + kind: greeting + prompt: Still here. + exits: { next: check } + bye: + kind: hangup +"#; + assert_eq!(validate(&parse(src)), Ok(())); + let errs = validate_outbound(&parse(src)).unwrap_err(); + assert_eq!( + errs, + vec![ + ValidationError::OutboundUnboundedLoop { + node: "check".into() + }, + ValidationError::OutboundUnboundedLoop { node: "say".into() }, + ] + ); + } + + #[test] + fn a_greeting_looping_on_itself_is_unbounded() { + let src = r#" +schema_version: 1 +id: f +name: n +entry: g +nodes: + g: + kind: greeting + prompt: again + exits: { next: g } +"#; + let codes = outbound_codes(src); + assert!(codes.contains(&"outbound_unbounded_loop"), "{codes:?}"); + assert!(codes.contains(&"trapped"), "{codes:?}"); + } + + #[test] + fn an_unreachable_loop_is_not_reported_as_unbounded() { + let src = r#" +schema_version: 1 +id: f +name: n +entry: bye +nodes: + bye: + kind: hangup + g: + kind: greeting + prompt: again + exits: { next: g } +"#; + let codes = outbound_codes(src); + assert!(codes.contains(&"unreachable"), "{codes:?}"); + assert!(!codes.contains(&"outbound_unbounded_loop"), "{codes:?}"); + } } diff --git a/crates/wavekat-flow/tests/conformance.rs b/crates/wavekat-flow/tests/conformance.rs index b17610c..f87bacf 100644 --- a/crates/wavekat-flow/tests/conformance.rs +++ b/crates/wavekat-flow/tests/conformance.rs @@ -15,7 +15,7 @@ use std::{fs, path::PathBuf}; use wavekat_flow::model::Flow; -use wavekat_flow::validate::validate; +use wavekat_flow::validate::{validate, validate_outbound}; #[derive(serde::Deserialize)] struct Expectation { @@ -59,9 +59,12 @@ fn corpus_dir(version: &str, bucket: &str) -> PathBuf { } fn yaml_cases(version: &str, bucket: &str) -> Vec<(String, PathBuf)> { - let dir = corpus_dir(version, bucket); + yaml_cases_in(&corpus_dir(version, bucket)) +} + +fn yaml_cases_in(dir: &std::path::Path) -> Vec<(String, PathBuf)> { let mut out = Vec::new(); - for entry in fs::read_dir(&dir).expect("read corpus dir") { + for entry in fs::read_dir(dir).expect("read corpus dir") { let path = entry.unwrap().path(); if path.extension().and_then(|e| e.to_str()) == Some("yaml") { let stem = path.file_stem().unwrap().to_str().unwrap().to_string(); @@ -129,9 +132,18 @@ fn parse_error_code(err: &serde_yaml_ng::Error) -> &'static str { /// The full semantic gate over the raw YAML: parse (serde) + validate. Returns /// `(accepted, reported_codes)`. fn semantic_result(yaml: &str) -> (bool, Vec) { + semantic_result_with(yaml, validate) +} + +/// [`semantic_result`] with the validator chosen — the outbound corpus runs +/// the same parse step through [`validate_outbound`]. +fn semantic_result_with( + yaml: &str, + gate: fn(&Flow) -> Result<(), Vec>, +) -> (bool, Vec) { match Flow::from_yaml(yaml) { Err(e) => (false, vec![parse_error_code(&e).to_string()]), - Ok(flow) => match validate(&flow) { + Ok(flow) => match gate(&flow) { Ok(()) => (true, Vec::new()), Err(errs) => (false, errs.iter().map(|e| e.code().to_string()).collect()), }, @@ -215,3 +227,56 @@ fn corpus_required_assets_match_expectations() { // doing nothing, which is the one way it could fail silently. assert!(pinned > 0, "no corpus case pins a required-asset set"); } + +// The outbound profile: `conformance/vN/outbound/{valid,invalid}`, run +// through `validate_outbound` (which includes the base validation) with the +// same subset contract as the semantic corpus above. Kept out of the +// top-level `valid/`/`invalid/` buckets on purpose: a flow refused for +// outbound — a `ring` step, say — is a perfectly good inbound flow. The TS +// package runs the same cases through `validateOutbound`. +#[test] +fn outbound_corpus_matches_expectations() { + let mut ran = 0usize; + for version in corpus_versions() { + let number: u32 = version[1..].parse().expect("vN directory name"); + let schema: serde_json::Value = + serde_json::from_str(wavekat_flow::flow_schema(number).expect("schema")).unwrap(); + let validator = jsonschema::validator_for(&schema).expect("compile schema"); + + for bucket in ["valid", "invalid"] { + let dir = corpus_root().join(&version).join("outbound").join(bucket); + if !dir.is_dir() { + continue; + } + for (stem, path) in yaml_cases_in(&dir) { + let case = format!("{version}/outbound/{bucket}/{stem}"); + let yaml = fs::read_to_string(&path).unwrap(); + let exp: Expectation = serde_json::from_str( + &fs::read_to_string(dir.join(format!("{stem}.expected.json"))).unwrap(), + ) + .unwrap(); + + let doc: serde_json::Value = serde_yaml_ng::from_str(&yaml).unwrap(); + assert_eq!( + validator.is_valid(&doc), + exp.structurally_valid, + "structural verdict mismatch for {case}" + ); + + let (accepted, codes) = semantic_result_with(&yaml, validate_outbound); + assert_eq!( + accepted, exp.semantic.ok, + "outbound acceptance mismatch for {case} (codes: {codes:?})" + ); + for code in &exp.semantic.errors { + assert!( + codes.contains(code), + "expected outbound error {code:?} for {case}, got {codes:?}" + ); + } + ran += 1; + } + } + } + assert!(ran > 0, "the outbound corpus has no cases"); +} diff --git a/packages/flow-schema/README.md b/packages/flow-schema/README.md index 0c8b8ba..b9d3244 100644 --- a/packages/flow-schema/README.md +++ b/packages/flow-schema/README.md @@ -29,6 +29,7 @@ that executes it. | `Flow`, `Node`, `Prompt`, … | Generated model types, plus hand-written helpers (`requiredExits`, `isTerminal`, `requiredAssets`, …) | | `checkFlow` | The one-call gate: safe-subset YAML parse + semantic validation | | `parseFlow` / `validateFlow` | The two halves of `checkFlow`, usable separately | +| `validateOutbound` | `validateFlow` plus the rules for a call the app places itself (no `ring`, `book` or `transfer`; no loop that never waits for the callee) — call it instead of `validateFlow` for an outbound flow | | `validateStructure` / `flowV1Schema` | Structural (JSON Schema) validation via `ajv`, and the bundled schema itself | | `validateHours`, `isValidTimezone`, … | Business-hours / timezone checks | | `addNode`, `setExit`, `renameNode`, … | Comment-preserving YAML source edits (TS-only — the Rust side never edits) | diff --git a/packages/flow-schema/src/index.ts b/packages/flow-schema/src/index.ts index c5b847c..9d7c522 100644 --- a/packages/flow-schema/src/index.ts +++ b/packages/flow-schema/src/index.ts @@ -38,7 +38,7 @@ export type { Issue } from './issues.js'; export { MISSING_ASSET, hasErrors } from './issues.js'; export type { ParseResult } from './parse.js'; export { parseFlow } from './parse.js'; -export { validateFlow } from './validate.js'; +export { OUTBOUND_KINDS, validateFlow, validateOutbound } from './validate.js'; export type { FlowCheck } from './check.js'; export { checkFlow } from './check.js'; export { isValidDate, isValidTimezone, parseTimeMinutes, validateHours } from './hours.js'; diff --git a/packages/flow-schema/src/validate.ts b/packages/flow-schema/src/validate.ts index 1818d82..9458fd3 100644 --- a/packages/flow-schema/src/validate.ts +++ b/packages/flow-schema/src/validate.ts @@ -275,21 +275,7 @@ function checkGraph( issues: Issue[], at: (node: string) => Pick, ): void { - // BFS from entry over exit edges. - const reachable = new Set([flow.entry]); - const queue = [flow.entry]; - while (queue.length > 0) { - const id = queue.shift(); - if (id === undefined) break; - const node = flow.nodes[id]; - if (!node) continue; - for (const target of Object.values(node.exits ?? {})) { - if (target in flow.nodes && !reachable.has(target)) { - reachable.add(target); - queue.push(target); - } - } - } + const reachable = reachableFromEntry(flow); for (const id of Object.keys(flow.nodes)) { if (!reachable.has(id)) { @@ -333,3 +319,128 @@ function checkGraph( } } } + +/** Node ids reachable from `entry` over exit edges (`entry` included when + * it exists). */ +function reachableFromEntry(flow: Flow): Set { + const reachable = new Set(); + if (!(flow.entry in flow.nodes)) return reachable; + reachable.add(flow.entry); + const queue = [flow.entry]; + while (queue.length > 0) { + const id = queue.shift(); + if (id === undefined) break; + const node = flow.nodes[id]; + if (!node) continue; + for (const target of Object.values(node.exits ?? {})) { + if (target in flow.nodes && !reachable.has(target)) { + reachable.add(target); + queue.push(target); + } + } + } + return reachable; +} + +// ── Outbound profile ───────────────────────────────────────────────────── + +/** + * The kinds a flow may use when it runs on a call the app *places*, with + * no person standing by. Twin: `validate.rs` `OUTBOUND_KINDS`. + * + * Left out: `ring` (there is nobody to hand the call to), `book` (it + * books against the number that called in, which an outbound call does + * not have), and `transfer` (not offered on outbound calls yet). + */ +export const OUTBOUND_KINDS: readonly ComponentKind[] = [ + 'greeting', + 'menu', + 'message', + 'hangup', + 'hours', +]; + +/** + * Validate a flow for an **outbound** call: everything `validateFlow` + * checks, plus the rules for a call the app places itself. + * + * This runs `validateFlow` first and adds to its issues, so it is the one + * gate to call for an outbound flow — there is no need to call both. + * + * The outbound rules: + * + * - every node is one of `OUTBOUND_KINDS` (`outbound_node_not_allowed`); + * - no loop reachable from `entry` is made only of steps that move on + * without the callee (`greeting`, `hours`) — such a loop, played into + * voicemail or to a callee who stays silent, would never end + * (`outbound_unbounded_loop`). A loop through a `menu` is fine: the + * menu waits for the callee and its `no_input` exit is the way out. + * + * Twin: `validate.rs` `validate_outbound`. + */ +export function validateOutbound(flow: Flow, nodeRanges: NodeRanges = {}): Issue[] { + const issues = validateFlow(flow, nodeRanges); + const at = (node: string): Pick => ({ + severity: 'error', + node, + range: nodeRanges[node], + }); + + for (const id of Object.keys(flow.nodes).sort()) { + const node = flow.nodes[id]; + if (node && !OUTBOUND_KINDS.includes(node.kind as ComponentKind)) { + issues.push({ + ...at(id), + code: 'outbound_node_not_allowed', + message: `node "${id}" is a ${node.kind} step, which an outbound call cannot run`, + params: { kind: node.kind }, + }); + } + } + + for (const id of unboundedLoopNodes(flow)) { + issues.push({ + ...at(id), + code: 'outbound_unbounded_loop', + message: `node "${id}" is on a loop that never waits for the callee, so an outbound call could repeat it forever`, + }); + } + + return issues; +} + +/** Whether a node moves on without waiting for anything the callee does: + * `greeting` always takes `next`; `hours` picks by the clock. */ +function movesOnByItself(kind: string | undefined): boolean { + return kind === 'greeting' || kind === 'hours'; +} + +/** Every node reachable from `entry` that sits on a cycle made only of + * nodes that move on by themselves, in id order. */ +function unboundedLoopNodes(flow: Flow): string[] { + const reachable = reachableFromEntry(flow); + const inScope = (id: string): boolean => + reachable.has(id) && movesOnByItself(flow.nodes[id]?.kind); + const successors = (id: string): string[] => + Object.values(flow.nodes[id]?.exits ?? {}).filter(inScope); + + const onLoop: string[] = []; + for (const id of [...reachable].sort()) { + if (!inScope(id)) continue; + // Does `id` lead back to itself within the subgraph? + const seen = new Set(); + const stack = successors(id); + while (stack.length > 0) { + const next = stack.pop() as string; + if (next === id) { + onLoop.push(id); + break; + } + if (!seen.has(next)) { + seen.add(next); + stack.push(...successors(next)); + } + } + } + return onLoop; +} diff --git a/packages/flow-schema/test/conformance.test.ts b/packages/flow-schema/test/conformance.test.ts index a6cbb0b..7a1eea6 100644 --- a/packages/flow-schema/test/conformance.test.ts +++ b/packages/flow-schema/test/conformance.test.ts @@ -11,14 +11,14 @@ // that uses a `book` node belongs to v1, because what it pins is how a // version-1 document is treated. -import { readFileSync, readdirSync } from 'node:fs'; +import { existsSync, readFileSync, readdirSync } from 'node:fs'; import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; import { parse } from 'yaml'; -import { checkFlow, requiredAssets } from '../src/index.js'; +import { checkFlow, hasErrors, parseFlow, requiredAssets, validateOutbound } from '../src/index.js'; import { validateStructure } from '../src/structure.js'; const here = dirname(fileURLToPath(import.meta.url)); @@ -44,7 +44,7 @@ type Expectation = { requiredAssets?: string[]; }; -function casesIn(version: string, bucket: 'valid' | 'invalid'): string[] { +function casesIn(version: string, bucket: string): string[] { const dir = resolve(corpusRoot, version, bucket); return readdirSync(dir) .filter((f) => f.endsWith('.yaml')) @@ -144,3 +144,46 @@ describe.each(ASSET_CASES)('conformance corpus $version — required assets', ({ }); } }); + +// The outbound profile: `conformance/vN/outbound/{valid,invalid}`, run +// through `validateOutbound` (which includes the base validation) with the +// same subset contract as the semantic corpus above. Kept out of the +// top-level `valid/`/`invalid/` buckets on purpose: a flow refused for +// outbound — a `ring` step, say — is a perfectly good inbound flow. The +// Rust crate runs the same cases through `validate_outbound`. +const OUTBOUND_CASES = CORPUS_VERSIONS.flatMap((version) => + (['valid', 'invalid'] as const).flatMap((bucket) => { + const dir = resolve(corpusRoot, version, 'outbound', bucket); + if (!existsSync(dir)) return []; + return casesIn(version, `outbound/${bucket}`).map((name) => ({ + version, + bucket: `outbound/${bucket}`, + name, + })); + }), +); + +describe('conformance corpus — outbound (validateOutbound)', () => { + it('has cases', () => { + expect(OUTBOUND_CASES.length).toBeGreaterThan(0); + }); + + for (const { version, bucket, name } of OUTBOUND_CASES) { + it(`${version}/${bucket}/${name}`, () => { + const src = source(version, bucket, name); + const expected = expectation(version, bucket, name); + expect(validateStructure(parse(src)).valid).toBe(expected.structurallyValid); + + const parsed = parseFlow(src); + const issues = [...parsed.issues]; + if (parsed.flow) issues.push(...validateOutbound(parsed.flow, parsed.nodeRanges)); + expect(!hasErrors(issues), JSON.stringify(issues.map((i) => i.code))).toBe( + expected.semantic.ok, + ); + const reported = new Set(issues.map((issue) => issue.code)); + for (const code of expected.semantic.errors) { + expect(reported, `expected outbound error "${code}" for ${bucket}/${name}`).toContain(code); + } + }); + } +}); diff --git a/packages/flow-schema/test/outbound.test.ts b/packages/flow-schema/test/outbound.test.ts new file mode 100644 index 0000000..949c55d --- /dev/null +++ b/packages/flow-schema/test/outbound.test.ts @@ -0,0 +1,147 @@ +// Twin of the `validate_outbound` tests in `crates/wavekat-flow/src/validate.rs`. +// The shared cases live in `conformance/vN/outbound/`; these pin the details +// the corpus's subset contract does not (which node, exact sets, ordering). + +import { describe, expect, it } from 'vitest'; + +import { parseFlow } from '../src/parse.js'; +import { validateFlow, validateOutbound } from '../src/validate.js'; + +function flowOf(source: string) { + const parsed = parseFlow(source); + if (!parsed.flow) throw new Error(`test flow should parse: ${JSON.stringify(parsed.issues)}`); + return parsed.flow; +} + +function outboundCodes(source: string): string[] { + return validateOutbound(flowOf(source)).map((issue) => issue.code); +} + +const REMINDER = ` +schema_version: 1 +id: f +name: n +entry: hello +nodes: + hello: + kind: greeting + prompt: This is a reminder about your appointment tomorrow. + exits: { next: ask } + ask: + kind: menu + prompt: Press 1 to confirm, or 2 to hear this again. + options: { "1": Confirm, "2": Repeat } + exits: { "1": thanks, "2": hello, no_input: bye, invalid: ask } + thanks: + kind: hangup + prompt: Thanks, see you then. + bye: + kind: hangup +`; + +describe('validateOutbound', () => { + it('accepts an outbound flow with a menu loop', () => { + expect(validateOutbound(flowOf(REMINDER))).toEqual([]); + }); + + it('refuses ring, book and transfer, naming the node and kind', () => { + const issues = validateOutbound( + flowOf(` +schema_version: 2 +id: f +name: n +entry: r +nodes: + r: + kind: ring + timeout_secs: 20 + exits: { no_answer: t } + t: + kind: transfer + target: "100" + b: + kind: book + prompt: When suits you? + confirm_prompt: You're booked for + timezone: UTC + schedule: + tue: [{ open: "09:00", close: "17:00" }] + duration_mins: 30 + exits: { booked: t, no_slots: t, no_input: t, unavailable: t } +`), + ).filter((issue) => issue.code === 'outbound_node_not_allowed'); + expect(issues.map((issue) => [issue.node, issue.params?.kind])).toEqual([ + ['b', 'book'], + ['r', 'ring'], + ['t', 'transfer'], + ]); + }); + + it('runs the base validation too', () => { + const codes = outboundCodes(REMINDER.replace('next: ask', 'next: ghost')); + expect(codes).toContain('unknown_target'); + }); + + it('catches a loop the base validator accepts', () => { + const src = ` +schema_version: 1 +id: f +name: n +entry: check +nodes: + check: + kind: hours + timezone: UTC + schedule: + mon: [{ open: "09:00", close: "17:00" }] + exits: { open: say, closed: bye } + say: + kind: greeting + prompt: Still here. + exits: { next: check } + bye: + kind: hangup +`; + expect(validateFlow(flowOf(src))).toEqual([]); + expect( + validateOutbound(flowOf(src)).map((issue) => [issue.code, issue.node]), + ).toEqual([ + ['outbound_unbounded_loop', 'check'], + ['outbound_unbounded_loop', 'say'], + ]); + }); + + it('flags a greeting looping on itself', () => { + const codes = outboundCodes(` +schema_version: 1 +id: f +name: n +entry: g +nodes: + g: + kind: greeting + prompt: again + exits: { next: g } +`); + expect(codes).toContain('outbound_unbounded_loop'); + expect(codes).toContain('trapped'); + }); + + it('does not report an unreachable loop as unbounded', () => { + const codes = outboundCodes(` +schema_version: 1 +id: f +name: n +entry: bye +nodes: + bye: + kind: hangup + g: + kind: greeting + prompt: again + exits: { next: g } +`); + expect(codes).toContain('unreachable'); + expect(codes).not.toContain('outbound_unbounded_loop'); + }); +}); From f72b5f394e8474ec590159d33baa20039b4d6fce Mon Sep 17 00:00:00 2001 From: Eason Guo Date: Sat, 17 Oct 2026 19:27:17 +1300 Subject: [PATCH 2/3] chore: bump async-trait for clippy 1.99 Rust 1.99's new clippy::double_must_use lint fires on code generated by async-trait 0.1.91, failing `clippy -D warnings`. 0.1.92 fixes it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GNFHhzw4ZsqCFnv6ERigjd --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 158be73..9edbbb2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -39,9 +39,9 @@ checksum = "330a5ed07fa54e4702c9d6c4174f74427fc0ef6e214bbd677ae50a5099946470" [[package]] name = "async-trait" -version = "0.1.91" +version = "0.1.92" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae36dc4177970ef04fde5178d3e2429882def40e57a451f919c098f72baa6cec" +checksum = "82f6aeea286b8eb4dd3431a1be1b59d290ace00f5bfd8e2a159bc2a05e2c1667" dependencies = [ "proc-macro2", "quote", From 506aef813bcb1958393c33f4e6ddcf618a8742d1 Mon Sep 17 00:00:00 2001 From: Eason Guo Date: Sat, 17 Oct 2026 19:48:50 +1300 Subject: [PATCH 3/3] fix: catch menu no_input loops outbound A loop through a menu was always accepted, but with nobody pressing keys (voicemail, a silent callee) the menu takes no_input every time. If that exit leads back into the loop, the call never ends. Count a menu's no_input exit as a silent move, alongside greeting and hours exits. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GNFHhzw4ZsqCFnv6ERigjd --- .../invalid/menu-no-input-loop.expected.json | 5 ++ .../outbound/invalid/menu-no-input-loop.yaml | 19 +++++ crates/wavekat-flow/README.md | 7 +- crates/wavekat-flow/src/validate.rs | 71 ++++++++++++++----- packages/flow-schema/src/validate.ts | 37 ++++++---- packages/flow-schema/test/outbound.test.ts | 18 +++++ 6 files changed, 121 insertions(+), 36 deletions(-) create mode 100644 conformance/v2/outbound/invalid/menu-no-input-loop.expected.json create mode 100644 conformance/v2/outbound/invalid/menu-no-input-loop.yaml diff --git a/conformance/v2/outbound/invalid/menu-no-input-loop.expected.json b/conformance/v2/outbound/invalid/menu-no-input-loop.expected.json new file mode 100644 index 0000000..68f3309 --- /dev/null +++ b/conformance/v2/outbound/invalid/menu-no-input-loop.expected.json @@ -0,0 +1,5 @@ +{ + "description": "The menu's no_input exit leads back to the greeting, so with nobody pressing keys (voicemail) hello -> ask -> hello never ends. Pressing 1 or an unmapped key still ends the call, so the base validator accepts it — a loop through a menu is only safe on its digit exits.", + "structurallyValid": true, + "semantic": { "ok": false, "errors": ["outbound_unbounded_loop"] } +} diff --git a/conformance/v2/outbound/invalid/menu-no-input-loop.yaml b/conformance/v2/outbound/invalid/menu-no-input-loop.yaml new file mode 100644 index 0000000..e0c32d1 --- /dev/null +++ b/conformance/v2/outbound/invalid/menu-no-input-loop.yaml @@ -0,0 +1,19 @@ +schema_version: 2 +id: flow_outbound_menu_no_input_loop +name: Repeats into voicemail +entry: hello +nodes: + hello: + kind: greeting + prompt: Hello, this is a reminder about your appointment tomorrow at ten. + exits: { next: ask } + ask: + kind: menu + prompt: Press 1 to confirm. + options: { "1": Confirm } + exits: { "1": thanks, no_input: hello, invalid: bye } + thanks: + kind: hangup + prompt: Thanks, see you then. + bye: + kind: hangup diff --git a/crates/wavekat-flow/README.md b/crates/wavekat-flow/README.md index af04477..2865012 100644 --- a/crates/wavekat-flow/README.md +++ b/crates/wavekat-flow/README.md @@ -73,9 +73,10 @@ A flow run on a call the app places — with no person standing by — goes through `validate::validate_outbound` instead. It runs `validate` first and adds the outbound rules, so call it on its own: only `greeting`, `menu`, `message`, `hangup` and `hours` steps are allowed -(`outbound_node_not_allowed`), and no loop may be made only of steps that move -on without the callee, such as a greeting that leads back to itself -(`outbound_unbounded_loop`). A loop through a `menu` is fine. +(`outbound_node_not_allowed`), and no loop may be made only of moves that +happen without the callee — a greeting that leads back to itself, or a menu +whose `no_input` exit leads back into the loop (`outbound_unbounded_loop`). A +loop through a menu's digit exits is fine. ## Upgrading when the booking grid moves diff --git a/crates/wavekat-flow/src/validate.rs b/crates/wavekat-flow/src/validate.rs index f6ddf01..d72e5e4 100644 --- a/crates/wavekat-flow/src/validate.rs +++ b/crates/wavekat-flow/src/validate.rs @@ -255,11 +255,12 @@ pub const OUTBOUND_KINDS: &[&str] = &["greeting", "menu", "message", "hangup", " /// The outbound rules: /// /// - every node is one of [`OUTBOUND_KINDS`] (`outbound_node_not_allowed`); -/// - no loop reachable from `entry` is made only of steps that move on -/// without the callee (`greeting`, `hours`) — such a loop, played into -/// voicemail or to a callee who stays silent, would never end -/// (`outbound_unbounded_loop`). A loop through a `menu` is fine: the -/// menu waits for the callee and its `no_input` exit is the way out. +/// - no loop reachable from `entry` is made only of moves that happen +/// without the callee — any exit of a `greeting` or `hours` step, or a +/// `menu`'s `no_input` exit. Such a loop, played into voicemail or to a +/// callee who stays silent, would never end (`outbound_unbounded_loop`). +/// A loop through a menu's digit or `invalid` exits is fine: only a +/// callee pressing keys can take it. /// /// Twin: `validate.ts` `validateOutbound`. pub fn validate_outbound(flow: &Flow) -> Result<(), Vec> { @@ -290,34 +291,39 @@ pub fn validate_outbound(flow: &Flow) -> Result<(), Vec> { } } -/// Whether a node moves on without waiting for anything the callee does. -/// `greeting` always takes `next`; `hours` picks by the clock. -fn moves_on_by_itself(node: &Node) -> bool { - matches!(node, Node::Greeting { .. } | Node::Hours { .. }) +/// The exits a node takes without the callee doing anything: every exit of +/// a `greeting` (always `next`) or `hours` step (picked by the clock), and a +/// `menu`'s `no_input` (taken when nobody presses a key — voicemail, say). +fn silent_exits(node: &Node) -> Vec<&str> { + let exits = node.exits().into_iter().flatten(); + match node { + Node::Greeting { .. } | Node::Hours { .. } => exits.map(|(_, t)| t.as_str()).collect(), + Node::Menu { .. } => exits + .filter(|(key, _)| key.as_str() == "no_input") + .map(|(_, t)| t.as_str()) + .collect(), + _ => Vec::new(), + } } /// Every node reachable from `entry` that sits on a cycle made only of -/// nodes that move on by themselves, in id order. +/// silent exits (see [`silent_exits`]), in id order. fn unbounded_loop_nodes(flow: &Flow) -> Vec<&str> { let reachable = reachable_from_entry(flow); - // The subgraph the rule is about: reachable, self-advancing nodes and - // the edges between them. + // The subgraph the rule is about: reachable nodes and the silent exits + // between them. let successors = |id: &str| -> Vec<&str> { flow.nodes .get(id) - .and_then(Node::exits) + .map(silent_exits) + .unwrap_or_default() .into_iter() - .flatten() - .map(|(_, t)| t.as_str()) - .filter(|t| reachable.contains(t) && flow.nodes.get(*t).is_some_and(moves_on_by_itself)) + .filter(|t| reachable.contains(t)) .collect() }; let mut on_loop = Vec::new(); for id in &reachable { - if !flow.nodes.get(*id).is_some_and(moves_on_by_itself) { - continue; - } // Does `id` lead back to itself within the subgraph? let mut seen: BTreeSet<&str> = BTreeSet::new(); let mut stack = successors(id); @@ -1049,6 +1055,33 @@ nodes: ); } + #[test] + fn a_menu_whose_no_input_loops_back_is_unbounded() { + // Into voicemail, every attempt times out: hello -> ask -> hello ... + // `thanks` and `bye` can still end it, so the base check passes. + let src = REMINDER.replace( + "no_input: bye, invalid: ask", + "no_input: hello, invalid: bye", + ); + assert_eq!(validate(&parse(&src)), Ok(())); + let errs = validate_outbound(&parse(&src)).unwrap_err(); + assert_eq!( + errs, + vec![ + ValidationError::OutboundUnboundedLoop { node: "ask".into() }, + ValidationError::OutboundUnboundedLoop { + node: "hello".into() + }, + ] + ); + } + + #[test] + fn a_menu_retrying_itself_on_no_input_is_unbounded() { + let src = REMINDER.replace("no_input: bye, invalid: ask", "no_input: ask, invalid: bye"); + assert_eq!(outbound_codes(&src), vec!["outbound_unbounded_loop"]); + } + #[test] fn a_greeting_looping_on_itself_is_unbounded() { let src = r#" diff --git a/packages/flow-schema/src/validate.ts b/packages/flow-schema/src/validate.ts index 9458fd3..2176e85 100644 --- a/packages/flow-schema/src/validate.ts +++ b/packages/flow-schema/src/validate.ts @@ -370,11 +370,12 @@ export const OUTBOUND_KINDS: readonly ComponentKind[] = [ * The outbound rules: * * - every node is one of `OUTBOUND_KINDS` (`outbound_node_not_allowed`); - * - no loop reachable from `entry` is made only of steps that move on - * without the callee (`greeting`, `hours`) — such a loop, played into - * voicemail or to a callee who stays silent, would never end - * (`outbound_unbounded_loop`). A loop through a `menu` is fine: the - * menu waits for the callee and its `no_input` exit is the way out. + * - no loop reachable from `entry` is made only of moves that happen + * without the callee — any exit of a `greeting` or `hours` step, or a + * `menu`'s `no_input` exit. Such a loop, played into voicemail or to a + * callee who stays silent, would never end (`outbound_unbounded_loop`). + * A loop through a menu's digit or `invalid` exits is fine: only a + * callee pressing keys can take it. * * Twin: `validate.rs` `validate_outbound`. */ @@ -409,24 +410,32 @@ export function validateOutbound(flow: Flow, nodeRanges: NodeRanges = {}): Issue return issues; } -/** Whether a node moves on without waiting for anything the callee does: - * `greeting` always takes `next`; `hours` picks by the clock. */ -function movesOnByItself(kind: string | undefined): boolean { - return kind === 'greeting' || kind === 'hours'; +/** The exits a node takes without the callee doing anything: every exit of + * a `greeting` (always `next`) or `hours` step (picked by the clock), and a + * `menu`'s `no_input` (taken when nobody presses a key — voicemail, say). */ +function silentExits(node: Flow['nodes'][string] | undefined): string[] { + if (!node) return []; + const exits = Object.entries(node.exits ?? {}); + switch (node.kind) { + case 'greeting': + case 'hours': + return exits.map(([, target]) => target); + case 'menu': + return exits.filter(([key]) => key === 'no_input').map(([, target]) => target); + default: + return []; + } } /** Every node reachable from `entry` that sits on a cycle made only of - * nodes that move on by themselves, in id order. */ + * silent exits (see `silentExits`), in id order. */ function unboundedLoopNodes(flow: Flow): string[] { const reachable = reachableFromEntry(flow); - const inScope = (id: string): boolean => - reachable.has(id) && movesOnByItself(flow.nodes[id]?.kind); const successors = (id: string): string[] => - Object.values(flow.nodes[id]?.exits ?? {}).filter(inScope); + silentExits(flow.nodes[id]).filter((target) => reachable.has(target)); const onLoop: string[] = []; for (const id of [...reachable].sort()) { - if (!inScope(id)) continue; // Does `id` lead back to itself within the subgraph? const seen = new Set(); const stack = successors(id); diff --git a/packages/flow-schema/test/outbound.test.ts b/packages/flow-schema/test/outbound.test.ts index 949c55d..5e265a1 100644 --- a/packages/flow-schema/test/outbound.test.ts +++ b/packages/flow-schema/test/outbound.test.ts @@ -111,6 +111,24 @@ nodes: ]); }); + it('flags a menu whose no_input leads back into the loop', () => { + // Into voicemail, every attempt times out: hello -> ask -> hello ... + // `thanks` and `bye` can still end it, so the base check passes. + const src = REMINDER.replace('no_input: bye, invalid: ask', 'no_input: hello, invalid: bye'); + expect(validateFlow(flowOf(src))).toEqual([]); + expect( + validateOutbound(flowOf(src)).map((issue) => [issue.code, issue.node]), + ).toEqual([ + ['outbound_unbounded_loop', 'ask'], + ['outbound_unbounded_loop', 'hello'], + ]); + }); + + it('flags a menu retrying itself on no_input', () => { + const src = REMINDER.replace('no_input: bye, invalid: ask', 'no_input: ask, invalid: bye'); + expect(outboundCodes(src)).toEqual(['outbound_unbounded_loop']); + }); + it('flags a greeting looping on itself', () => { const codes = outboundCodes(` schema_version: 1