diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b5cb028..be2c338 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,6 +44,8 @@ jobs: - run: cargo clippy --all-targets -- -D warnings - name: Test (types regenerate from the schema on build) run: cargo test -p wavekat-flow + - name: Older readers must keep reading current documents + run: cargo test -p compat-old-readers - name: Fail on stale crate-local schema copy (synced by build.rs) run: | if ! git diff --exit-code -- crates/wavekat-flow/schema; then diff --git a/Cargo.lock b/Cargo.lock index bac6300..dbf0893 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -117,6 +117,14 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "compat-old-readers" +version = "0.0.0" +dependencies = [ + "wavekat-flow 0.0.11", + "wavekat-flow 0.0.6", +] + [[package]] name = "deranged" version = "0.5.8" @@ -1552,6 +1560,26 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "wavekat-flow" +version = "0.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa8de375720073be7590b3e3a0cb70ef88569bc483c3938d147d7a02c12408ca" +dependencies = [ + "anyhow", + "async-trait", + "prettyplease", + "schemars", + "serde", + "serde_json", + "serde_yaml_ng", + "syn 2.0.119", + "thiserror 2.0.19", + "time", + "time-tz", + "typify", +] + [[package]] name = "wavekat-flow" version = "0.0.11" diff --git a/conformance/v2/valid/notes.expected.json b/conformance/v2/valid/notes.expected.json new file mode 100644 index 0000000..d7529e6 --- /dev/null +++ b/conformance/v2/valid/notes.expected.json @@ -0,0 +1,5 @@ +{ + "description": "The author's `note` on the flow and on every kind of step. A note is free text for people, never read by the engine, so it must not change any verdict. An older reader that predates `note` ignores it as an unknown field and must still read the document.", + "structurallyValid": true, + "semantic": { "ok": true, "errors": [] } +} diff --git a/conformance/v2/valid/notes.yaml b/conformance/v2/valid/notes.yaml new file mode 100644 index 0000000..9b0a55e --- /dev/null +++ b/conformance/v2/valid/notes.yaml @@ -0,0 +1,51 @@ +schema_version: 2 +id: flow_notes +name: Notes on the flow and on every kind of step +note: Written by the front desk; ring the owner first. +entry: welcome +nodes: + welcome: + kind: greeting + note: Keep the greeting short. + prompt: Thanks for calling. + exits: { next: check_hours } + check_hours: + kind: hours + note: Open Tuesday only. + timezone: America/New_York + schedule: + tue: [{ open: "09:00", close: "17:00" }] + exits: { open: front_desk, closed: menu } + front_desk: + kind: ring + note: Rings the shop phone. + timeout_secs: 20 + exits: { no_answer: leave_message } + menu: + kind: menu + note: Press 1 to book, 2 for the owner. + prompt: Press 1 to book, or 2 to reach the owner. + options: { "1": Book, "2": Owner } + exits: { "1": book, "2": handoff, no_input: leave_message, invalid: leave_message } + book: + kind: book + note: Thirty-minute slots. + prompt: Here are the next available times. + confirm_prompt: You're booked for + timezone: America/New_York + schedule: + tue: [{ open: "09:00", close: "17:00" }] + duration_mins: 30 + exits: { booked: bye, no_slots: leave_message, no_input: leave_message, unavailable: leave_message } + handoff: + kind: transfer + note: The owner's mobile. + target: "+15555550123" + leave_message: + kind: message + note: Goes to voicemail. + prompt: Leave your name and number. + bye: + kind: hangup + note: Always end politely. + prompt: Goodbye. diff --git a/conformance/v2/valid/unknown-field.expected.json b/conformance/v2/valid/unknown-field.expected.json new file mode 100644 index 0000000..6817228 --- /dev/null +++ b/conformance/v2/valid/unknown-field.expected.json @@ -0,0 +1,6 @@ +{ + "description": "Version 2 counterpart of v1/valid/unknown-field: unknown fields (top-level `color`, node-level `volume`) are ACCEPTED. This is what lets an older reader keep reading a newer document that carries a field it has never heard of; tightening it would break every installed daemon still running an older build.", + "structurallyValid": true, + "semantic": { "ok": true, "errors": [] }, + "tsWarnings": ["unknown_field"] +} diff --git a/conformance/v2/valid/unknown-field.yaml b/conformance/v2/valid/unknown-field.yaml new file mode 100644 index 0000000..16e9948 --- /dev/null +++ b/conformance/v2/valid/unknown-field.yaml @@ -0,0 +1,14 @@ +schema_version: 2 +id: flow_unknown2 +name: Unknown fields are accepted +entry: hello +color: purple +nodes: + hello: + kind: greeting + prompt: Hi. + exits: { next: bye } + volume: 11 + bye: + kind: hangup + prompt: Bye. diff --git a/crates/compat-old-readers/Cargo.toml b/crates/compat-old-readers/Cargo.toml new file mode 100644 index 0000000..1050ad2 --- /dev/null +++ b/crates/compat-old-readers/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "compat-old-readers" +version = "0.0.0" +edition.workspace = true +license.workspace = true +rust-version.workspace = true +repository.workspace = true +authors.workspace = true +description = "Test-only: checks that documents of the current format still parse under the oldest wavekat-flow release still in the field." +# Never published, and not in release-please-config.json. +publish = false + +[dev-dependencies] +# The oldest crate release still running in installed voice daemons. See +# OLDEST_READER in tests/old_readers.rs before changing this pin. +old-flow = { package = "wavekat-flow", version = "=0.0.6" } +# The current format, from this checkout. +wavekat-flow = { path = "../wavekat-flow" } diff --git a/crates/compat-old-readers/src/lib.rs b/crates/compat-old-readers/src/lib.rs new file mode 100644 index 0000000..4686e0b --- /dev/null +++ b/crates/compat-old-readers/src/lib.rs @@ -0,0 +1 @@ +//! Test-only crate; see tests/old_readers.rs. diff --git a/crates/compat-old-readers/tests/old_readers.rs b/crates/compat-old-readers/tests/old_readers.rs new file mode 100644 index 0000000..310cfd3 --- /dev/null +++ b/crates/compat-old-readers/tests/old_readers.rs @@ -0,0 +1,151 @@ +//! Older readers must keep reading newer documents. +//! +//! Voice daemons keep running whatever `wavekat-flow` release they were built +//! with, while the platform writes the current format. This crate pins the +//! oldest release still in the field (`old-flow` in Cargo.toml) next to the +//! current crate and checks that everything the current format produces still +//! parses, and is judged the same way, by the old one. +//! +//! OLDEST_READER tracks the oldest crate release still running in installed +//! daemons. To retire it once no daemon that old remains: bump the `=x.y.z` +//! pin in Cargo.toml to the new oldest release and update the constant below. +//! Never bump it just to make a failure go away: a failure here means a +//! document we write today would break a daemon that is already installed. +//! +//! Corpus cases are frozen, so this also covers every document the format has +//! ever accepted that the old reader's schema versions can read. + +use std::{fs, path::Path, path::PathBuf}; + +use old_flow::validate::validate as old_validate; +use wavekat_flow::validate::validate as new_validate; + +/// The oldest `wavekat-flow` release still running in installed daemons. +/// Must match the `old-flow` pin in Cargo.toml (asserted below). +const OLDEST_READER: &str = "0.0.6"; + +fn corpus() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../conformance") +} + +fn yaml_files(dir: PathBuf) -> Vec { + let Ok(entries) = fs::read_dir(&dir) else { + return Vec::new(); + }; + let mut out: Vec = entries + .flatten() + .map(|e| e.path()) + .filter(|p| p.extension().and_then(|e| e.to_str()) == Some("yaml")) + .collect(); + out.sort(); + out +} + +/// Every document the current format is expected to produce or accept: the +/// valid and outbound-valid corpus of every version, and the canonical +/// outputs of the serializer. +fn current_documents() -> Vec { + let mut out = Vec::new(); + for version in fs::read_dir(corpus()).unwrap().flatten() { + let name = version.file_name().to_string_lossy().to_string(); + if name.starts_with('v') && name[1..].chars().all(|c| c.is_ascii_digit()) { + out.extend(yaml_files(version.path().join("valid"))); + out.extend(yaml_files(version.path().join("outbound").join("valid"))); + } + } + out.extend( + yaml_files(corpus().join("canonical")) + .into_iter() + .filter(|p| p.to_string_lossy().ends_with(".canonical.yaml")), + ); + out +} + +/// Read the version straight off the text, without going through either +/// crate's model. +fn schema_version(yaml: &str) -> u32 { + yaml.lines() + .find_map(|l| l.strip_prefix("schema_version:")) + .expect("schema_version line") + .trim() + .parse() + .expect("numeric schema_version") +} + +fn rel(p: &Path) -> String { + p.strip_prefix(corpus()).unwrap().display().to_string() +} + +/// The documents the old reader claims to support, as (name, yaml). +fn readable_by_old() -> Vec<(String, String)> { + current_documents() + .iter() + .map(|p| (rel(p), fs::read_to_string(p).unwrap())) + .filter(|(_, yaml)| old_flow::SUPPORTED_SCHEMA_VERSIONS.contains(&schema_version(yaml))) + .collect() +} + +#[test] +fn pin_matches_constant() { + let manifest = + fs::read_to_string(PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("Cargo.toml")).unwrap(); + assert!( + manifest.contains(&format!("version = \"={OLDEST_READER}\"")), + "OLDEST_READER and the old-flow pin in Cargo.toml disagree" + ); +} + +#[test] +fn old_reader_parses_every_current_document() { + let docs = readable_by_old(); + assert!( + docs.len() > 10, + "only {} documents reached the old reader", + docs.len() + ); + for (case, yaml) in docs { + old_flow::Flow::from_yaml(&yaml).unwrap_or_else(|e| { + panic!("{OLDEST_READER} cannot parse {case}, which the current format writes: {e}") + }); + } +} + +#[test] +fn old_reader_judges_like_the_current_one() { + for (case, yaml) in readable_by_old() { + let old = old_flow::Flow::from_yaml(&yaml).unwrap(); + let new = wavekat_flow::Flow::from_yaml(&yaml).unwrap(); + assert_eq!( + old_validate(&old).is_ok(), + new_validate(&new).is_ok(), + "{OLDEST_READER} and the current crate disagree on {case}" + ); + } +} + +#[test] +fn unknown_keys_do_not_disturb_the_fields_the_old_reader_knows() { + // Not just "does not fail": the keys it skips must not consume or + // reorder what it does understand. + let yaml = fs::read_to_string(corpus().join("v2/valid/notes.yaml")).unwrap(); + let old = old_flow::Flow::from_yaml(&yaml).unwrap(); + let new = wavekat_flow::Flow::from_yaml(&yaml).unwrap(); + assert_eq!(old.entry, new.entry); + // Order is the author's reading order and irrelevant to execution; the + // old reader may not keep it, so compare the node sets. + let mut old_ids: Vec<_> = old.nodes.keys().map(|k| k.to_string()).collect(); + let mut new_ids: Vec<_> = new.nodes.keys().map(|k| k.to_string()).collect(); + old_ids.sort(); + new_ids.sort(); + assert_eq!(old_ids, new_ids); +} + +#[test] +fn old_reader_refuses_versions_it_does_not_know() { + // The other half of the contract: a version bump is how a change old + // readers cannot handle is announced, and they must say so rather than + // guess. + let yaml = "schema_version: 99\nid: f\nname: f\nentry: a\nnodes:\n a:\n kind: hangup\n"; + let flow = old_flow::Flow::from_yaml(yaml).expect("parses; the version gate is validate"); + assert!(old_validate(&flow).is_err()); +} diff --git a/crates/wavekat-flow/tests/tolerance.rs b/crates/wavekat-flow/tests/tolerance.rs new file mode 100644 index 0000000..5589568 --- /dev/null +++ b/crates/wavekat-flow/tests/tolerance.rs @@ -0,0 +1,49 @@ +//! Older readers must keep reading newer documents. A daemon built from an +//! older crate parses whatever the platform writes today, so the schema must +//! never close an object to unknown keys: no `additionalProperties: false` +//! and no `unevaluatedProperties: false` anywhere. The generated serde types +//! follow the schema (typify emits `deny_unknown_fields` for exactly that), +//! so this one check covers the Rust model too. The TS package has the same +//! test (packages/flow-schema/test/tolerance.test.ts). +//! +//! There is no intentional exception: the opaque `ui` block is simply +//! unconstrained, and the `additionalProperties` that do exist are +//! sub-schemas (node map, option labels), not `false`. + +use serde_json::Value; + +fn closed_objects(value: &Value, path: &str, out: &mut Vec) { + match value { + Value::Object(map) => { + for key in ["additionalProperties", "unevaluatedProperties"] { + if map.get(key) == Some(&Value::Bool(false)) { + out.push(format!("{path}/{key}")); + } + } + for (k, v) in map { + closed_objects(v, &format!("{path}/{k}"), out); + } + } + Value::Array(items) => { + for (i, v) in items.iter().enumerate() { + closed_objects(v, &format!("{path}/{i}"), out); + } + } + _ => {} + } +} + +#[test] +fn no_schema_version_closes_an_object_to_unknown_keys() { + for version in wavekat_flow::SUPPORTED_SCHEMA_VERSIONS { + let schema: Value = + serde_json::from_str(wavekat_flow::flow_schema(*version).expect("schema")).unwrap(); + let mut closed = Vec::new(); + closed_objects(&schema, "", &mut closed); + assert!( + closed.is_empty(), + "schema v{version} closes objects to unknown keys, which would break older readers \ + of newer documents: {closed:?}" + ); + } +} diff --git a/packages/flow-schema/test/tolerance.test.ts b/packages/flow-schema/test/tolerance.test.ts new file mode 100644 index 0000000..94cf80b --- /dev/null +++ b/packages/flow-schema/test/tolerance.test.ts @@ -0,0 +1,45 @@ +// Older readers must keep reading newer documents. A daemon built from an +// older release parses whatever the platform writes today, so the schema must +// never close an object to unknown keys: no `additionalProperties: false` and +// no `unevaluatedProperties: false` anywhere. The Rust crate has the same test +// (crates/wavekat-flow/tests/tolerance.rs). +// +// There is no intentional exception: the opaque `ui` block is simply +// unconstrained, and the `additionalProperties` that do exist are sub-schemas +// (node map, option labels), not `false`. + +import { readFileSync, readdirSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { describe, expect, it } from 'vitest'; + +const schemaDir = resolve(dirname(fileURLToPath(import.meta.url)), '..', '..', '..', 'schema'); + +function closedObjects(value: unknown, path: string, out: string[]): void { + if (Array.isArray(value)) { + value.forEach((v, i) => closedObjects(v, `${path}/${i}`, out)); + } else if (value !== null && typeof value === 'object') { + for (const [key, v] of Object.entries(value)) { + if ((key === 'additionalProperties' || key === 'unevaluatedProperties') && v === false) { + out.push(`${path}/${key}`); + } + closedObjects(v, `${path}/${key}`, out); + } + } +} + +const schemas = readdirSync(schemaDir).filter((f) => /^flow\.v\d+\.schema\.json$/.test(f)); + +describe('schema tolerates unknown keys', () => { + it('finds the schema files', () => { + expect(schemas.length).toBeGreaterThan(0); + }); + + it.each(schemas)('%s closes no object to unknown keys', (file) => { + const schema = JSON.parse(readFileSync(resolve(schemaDir, file), 'utf8')); + const closed: string[] = []; + closedObjects(schema, '', closed); + expect(closed, 'closing an object would break older readers of newer documents').toEqual([]); + }); +});