diff --git a/docs/SURFACE.md b/docs/SURFACE.md index 1343c64b5..24577d0e5 100644 --- a/docs/SURFACE.md +++ b/docs/SURFACE.md @@ -58,7 +58,10 @@ No process runs between events: the handler wakes, executes to its next await, p ## 2. The semantic laws 1. **Three step verbs** — `run` / `llm` / `agent` — one per rung of the ladder. Four resident verbs — `on` / `human` / `dispatch` / `done`. The kernel vocabulary stops there. -2. **Gates are postfix on the step they guard.** Never a separate machinery block. +2. **Gates are postfix on the step they guard.** Never a separate machinery + block. A named data gate lowers to the guarded step's existing kernel + `verification`; an author callback remains TypeScript runtime code. The + distinction is explicit in the gate contract below. 3. **Helpers, not primitives.** Authors never see mount paths, tokens, or protocol frames. Named helpers wrap every substrate: - `f.slack` / `f.github` / `f.linear` / … — **generated from the relayfile adapters** (50 providers → 50 namespaces for free), each verb compiling to a mount write. The receipt a helper returns *is* the journaled effect record (RFC Appendix A), so exactly-once dedup rides along invisibly. - `f.memory` — relayhistory: `f.memory.recall(query)`, `f.memory.why(task)`, `f.memory.learn(finding)`. @@ -278,8 +281,71 @@ typed `run_not_found` refusal. A dropped connection, request failure, or journal may already have changed and the CLI cannot honestly claim the resume was refused before a write. -## 6. Open surface questions (for gate-1 SDK work) +## 6. The gate contract, and remaining open surface questions + +The data/code split is settled: Relayflows does not have a serializable +expression language. YAML keeps the existing `verification:` spelling and may +name only checks that lower to the closed kernel fields available today: +`exit_code`, `output_contains`, and `json_schema`. `flows check` validates that +data—including compiling JSON Schema declarations with the kernel's supported +drafts—and prints the exact kernel checks for each step. "Preflightable" means +the declaration and its parameters are inspectable before execution; it does +not mean preflight can predict an output that does not exist yet. + +`exit_code` applies only to deterministic steps; placing it on an `llm` or +`agent` step is invalid rather than an empty verification. JSON Schema +declarations accept both object and boolean schemas, matching the kernel. The +compiler snapshots and freezes authoring data before validation so accessors, +callbacks, proxies, `toJSON`, and other runtime behavior cannot change what the +journal serializes. Explicitly `undefined` object optionals are omitted, as in +JSON serialization and the v1 compiler; unsafe array values remain invalid. +The public preflight boundary performs that same compilation first and refuses +invalid raw input before running probes. +Exported unknown-input helpers follow the same rule: `validateSpec` reports a +failed validation without executing proxy traps or throwing, +`kernelToAuthoring` rejects non-inert kernel values before inspecting them, and +`canonicalize`/`specHash` snapshot before serializing — an identity computed +from a value that could change between two reads is not an identity. + +A declaration is legal only if compiling it succeeds **and** validating with it +is guaranteed to terminate. A schema whose `$ref` graph cycles through only +in-place applicators (`$ref`, `allOf`, `anyOf`, `oneOf`, `not`, `if`/`then`/ +`else`, `dependentSchemas`) re-applies to the same instance forever; it +compiles cleanly and then recurses without bound at verification time, which in +Rust aborts the process rather than raising anything catchable. Both sides +therefore refuse such a declaration up front with a named `unbounded $ref +cycle` error — before a journal exists and before the step's command runs. +Cycles that pass through a child applicator (`properties`, `items`, +`prefixItems`, ...) consume one level of the instance per step, so ordinary +recursive schemas stay legal. `kernel/relayflowd-core/src/schema.rs` and +`sdk/src/json-schema-bound.ts` implement the same rule and are pinned to the +shared corpus in `testdata/json-schema-bound-cases.json`, so the kernel and the +SDK agree on which schemas are legal by construction. `verify` compiles through +the same gate, so a journal written before the bound existed fails its gate with +a verdict instead of taking the daemon down on every resume. + +`schema: {}` and `schema: true` remain legal and remain accepted — but they +accept every possible output, so `flows check` marks the gate line +`[json_schema accepts any output]` and preflight emits a `vacuous_gate` +warning. A gate that judges nothing must not read like one that judges +something. + +The kernel evaluates those checks. `run.spawned` carries the compiled +verification data and `step.completed.verification` carries its verdict, so +resume and time travel replay the journaled result rather than re-running an +author predicate. The v1 `verification:` shape remains supported and compiles +to the same kernel fields; no kernel verb or verification field is added by +this decision. + +TypeScript may additionally accept a callback such as +`.gate(value => value.length < 200, "keep the summary short")`. That callback +is author code: `flows check` cannot prove it, YAML cannot serialize it, and +the journal cannot replay the closure. A TypeScript runtime must execute it as +runtime control flow and journal the resulting step outcome before dependents +continue. It must never stringify the function into a spec or silently label +it preflightable. Authors who need portable, inspectable gates use a named data +check; plugins may contribute named checks only by compiling them to existing +kernel primitives. -- `gate:` in YAML: tiny expression language (`length < 200`) vs named checks only. Leaning: a deliberately small expression grammar + named checks for everything else. - Are YAML helper verbs (`slack:`, `mcp:`) core spec vocabulary or compile-time expansion into `run`/effect steps? Leaning: expansion — the kernel spec stays seven words; helpers stay a surface concern. - Helper generation cadence: generated from relayfile adapter manifests at build time vs published per-adapter packages. Leaning: generated, with hand-tuned verb names for the top providers. diff --git a/kernel/DESIGN.md b/kernel/DESIGN.md index 3bdd525f8..9b2878173 100644 --- a/kernel/DESIGN.md +++ b/kernel/DESIGN.md @@ -371,7 +371,7 @@ Minimal verb set for gate 1: | verb | params → result | purpose | |---|---|---| | `hello` | `{protocol: 0, client}` → `{protocol: 0, server}` | handshake; version mismatch is a hard error | -| `run.start` | `{spec}` → `{run_id}` | validate spec (zero-agent flows are legal), create run file, append `run.spawned`, begin scheduling | +| `run.start` | `{spec}` → `{run_id}` | validate spec (zero-agent flows are legal; invalid declarations return `invalid_spec` before storage), create run file, append `run.spawned`, begin scheduling | | `run.resume` | `{run_id}` → `{run_id, state}` | §3 memoized resume | | `run.cancel` | `{run_id}` → `{run_id, status, completion_reason}` | append durable intent, close active leases, and append the terminal canceled fact; repeated calls return the existing outcome | | `run.get` | `{run_id}` → `{status, steps, budget}` | snapshot for legibility | diff --git a/kernel/relayflowd-core/src/lib.rs b/kernel/relayflowd-core/src/lib.rs index 006f54efe..f861eaba9 100644 --- a/kernel/relayflowd-core/src/lib.rs +++ b/kernel/relayflowd-core/src/lib.rs @@ -10,6 +10,7 @@ pub mod event; pub mod journal; pub mod machine; pub mod retry; +mod schema; pub mod spec; pub mod state; pub mod verify; diff --git a/kernel/relayflowd-core/src/schema.rs b/kernel/relayflowd-core/src/schema.rs new file mode 100644 index 000000000..a6b416794 --- /dev/null +++ b/kernel/relayflowd-core/src/schema.rs @@ -0,0 +1,702 @@ +//! JSON Schema declaration gate: a schema is legal only if compiling it +//! succeeds **and** validating with it is guaranteed to terminate. +//! +//! Compilation alone is not enough. `jsonschema` happily compiles a schema +//! whose `$ref` graph contains a cycle that never descends into a child of the +//! instance (`$defs.a -> $defs.b -> $defs.a`), and then recurses without bound +//! the first time it validates an output. A Rust stack overflow aborts the +//! process; it cannot be caught, so the bound has to be structural and has to +//! run before the schema is accepted — before a journal exists and before the +//! step's command runs. +//! +//! The rule: follow only the **in-place** applicators (`$ref`, `allOf`, +//! `anyOf`, `oneOf`, `not`, `if`/`then`/`else`, `dependentSchemas`), which +//! re-apply a subschema to the *same* instance. A cycle among those makes no +//! progress and cannot terminate. Cycles that pass through a **child** +//! applicator (`properties`, `items`, `prefixItems`, ...) are ordinary +//! recursive schemas: each step consumes one level of the instance, so they +//! terminate, and they stay legal. +//! +//! `sdk/src/json-schema-bound.ts` implements the same rule, and +//! `testdata/json-schema-bound-cases.json` is the corpus both sides are pinned +//! to, so kernel and SDK agree on which schemas are legal by construction +//! rather than by coincidence of two engines' overflow behaviour. + +use std::collections::{BTreeMap, HashMap, HashSet}; + +use serde_json::Value; + +/// Named refusal shared with the SDK and with the parity corpus. +pub(crate) const UNBOUNDED_REF_CYCLE: &str = "unbounded $ref cycle"; + +/// Reference keywords: apply the referenced schema to the same instance. +const REFERENCE_KEYWORDS: [&str; 3] = ["$ref", "$dynamicRef", "$recursiveRef"]; +/// In-place applicators taking a single subschema. +const IN_PLACE_SINGLE: [&str; 4] = ["not", "if", "then", "else"]; +/// In-place applicators taking an array of subschemas. +const IN_PLACE_ARRAY: [&str; 3] = ["allOf", "anyOf", "oneOf"]; +/// In-place applicators taking a map of subschemas. +const IN_PLACE_MAP: [&str; 2] = ["dependentSchemas", "dependencies"]; +/// Child applicators taking a single subschema — these consume one level of +/// the instance, so a cycle through them terminates. +const CHILD_SINGLE: [&str; 7] = [ + "additionalItems", + "additionalProperties", + "contains", + "items", + "propertyNames", + "unevaluatedItems", + "unevaluatedProperties", +]; +/// Child applicators taking a map of subschemas. +const CHILD_MAP: [&str; 2] = ["properties", "patternProperties"]; +/// Child applicators taking an array of subschemas. +const CHILD_ARRAY: [&str; 1] = ["prefixItems"]; + +pub(crate) fn validate_declaration(schema: &Value) -> Result<(), String> { + compile(schema).map(|_| ()) +} + +/// Compile a declaration through the same gate the declaration preflight uses. +/// `verify` calls this rather than `jsonschema::validator_for` directly, so a +/// journal written by an older kernel — which still holds an unbounded schema — +/// fails its gate with a verdict instead of aborting the daemon on every +/// resume. +pub(crate) fn compile(schema: &Value) -> Result { + bound_declaration(schema)?; + jsonschema::validator_for(schema).map_err(|error| error.to_string()) +} + +/// Refuse a declaration whose validation is not guaranteed to terminate. +fn bound_declaration(schema: &Value) -> Result<(), String> { + if !schema.is_object() { + // Boolean schemas carry no references. + return Ok(()); + } + let Scopes { + resources, + anchors, + base_at, + has_ids, + } = collect_scopes(schema); + let mut in_place: BTreeMap> = BTreeMap::new(); + let mut seen: HashSet = HashSet::new(); + let mut queue: Vec = vec![String::new()]; + seen.insert(String::new()); + + while let Some(pointer) = queue.pop() { + let Some(object) = schema.pointer(&pointer).and_then(Value::as_object) else { + continue; + }; + let mut here: Vec = Vec::new(); + let mut children: Vec = Vec::new(); + + for keyword in REFERENCE_KEYWORDS { + let Some(reference) = object.get(keyword).and_then(Value::as_str) else { + continue; + }; + // Walking to the nearest `$id` is the expensive part, so it is done + // only for a node that actually carries a reference, and skipped + // entirely for a document with no `$id` anywhere. + // A reference is resolved as a URI against the base URI in + // effect at this node (RFC 3986 §5), and only then as a fragment + // inside the resource it names. Keying on a leading `#` instead + // would miss the standard 2020-12 *compound schema document* form + // (spec §9.3, what every bundler emits), where a `$ref` written as + // a URI names an `$id` declared inside this same document and + // `jsonschema` resolves it from the document's own resource map. + let base_uri = if has_ids { + base_at.get(&pointer).map(String::as_str).unwrap_or("") + } else { + "" + }; + // A reference that names no resource declared in this document is + // left opaque, on a claim narrower than the one this comment used + // to make. It is either (a) remote, which `validator_for` refuses + // below because no retriever is configured, or (b) a meta-schema + // bundled with `jsonschema`, which cannot reference back into this + // document and so cannot close a cycle rooted here. Neither can + // participate in an unbounded in-place cycle. + // + // The older claim — "validator_for refuses anything the bound + // cannot resolve" — was true for remote resources and FALSE for an + // in-document `$id`, which is resolved from the resource map and + // then overflows. That gap is what this resolver closes. + if let Some(target) = resolve(base_uri, reference, &resources, &anchors) { + if schema.pointer(&target).is_some() { + here.push(target); + } + } + } + for keyword in IN_PLACE_SINGLE { + if object.contains_key(keyword) { + here.push(child_pointer(&pointer, keyword)); + } + } + for keyword in IN_PLACE_ARRAY { + collect_array(object, &pointer, keyword, &mut here); + } + for keyword in IN_PLACE_MAP { + collect_map(object, &pointer, keyword, &mut here); + } + for keyword in CHILD_SINGLE { + match object.get(keyword) { + // draft-04/07 tuple form: `items` may be an array of schemas. + Some(Value::Array(items)) => { + for index in 0..items.len() { + children.push(format!("{}/{index}", child_pointer(&pointer, keyword))); + } + } + Some(_) => children.push(child_pointer(&pointer, keyword)), + None => {} + } + } + for keyword in CHILD_MAP { + collect_map(object, &pointer, keyword, &mut children); + } + for keyword in CHILD_ARRAY { + collect_array(object, &pointer, keyword, &mut children); + } + + for next in here.iter().chain(children.iter()) { + if seen.insert(next.clone()) { + queue.push(next.clone()); + } + } + // `$defs`/`definitions` are containers, not applicators: their members + // are reachable only through a `$ref`, so an unused degenerate + // definition is never validated and stays legal. + if !here.is_empty() { + in_place.insert(pointer, here); + } + } + + match find_cycle(&in_place) { + Some(cycle) => Err(format!( + "{UNBOUNDED_REF_CYCLE}: {} — this cycle re-applies to the same instance, so validation would not terminate", + cycle + .iter() + .map(|pointer| display_pointer(pointer)) + .collect::>() + .join(" -> ") + )), + None => Ok(()), + } +} + +fn collect_array( + object: &serde_json::Map, + pointer: &str, + keyword: &str, + out: &mut Vec, +) { + if let Some(Value::Array(items)) = object.get(keyword) { + for index in 0..items.len() { + out.push(format!("{}/{index}", child_pointer(pointer, keyword))); + } + } +} + +fn collect_map( + object: &serde_json::Map, + pointer: &str, + keyword: &str, + out: &mut Vec, +) { + if let Some(Value::Object(entries)) = object.get(keyword) { + for (name, value) in entries { + // draft-07 `dependencies` values may be a property-name array. + if value.is_object() || value.is_boolean() { + out.push(format!( + "{}/{}", + child_pointer(pointer, keyword), + escape(name) + )); + } + } + } +} + +fn child_pointer(pointer: &str, key: &str) -> String { + format!("{pointer}/{}", escape(key)) +} + +fn escape(segment: &str) -> String { + segment.replace('~', "~0").replace('/', "~1") +} + +fn display_pointer(pointer: &str) -> String { + if pointer.is_empty() { + "#".to_owned() + } else { + format!("#{pointer}") + } +} + +/// Split `#` into its two halves. `None` means the reference +/// carried no `#` at all, which is distinct from an empty fragment. +fn split_fragment(reference: &str) -> (&str, Option<&str>) { + match reference.find('#') { + Some(index) => (&reference[..index], Some(&reference[index + 1..])), + None => (reference, None), + } +} + +fn strip_fragment(uri: &str) -> &str { + split_fragment(uri).0 +} + +/// RFC 3986 §3.1 scheme detection: `scheme ":"`, where scheme starts with a +/// letter. Used to tell an absolute URI from a relative reference. +fn has_scheme(reference: &str) -> bool { + let mut chars = reference.char_indices(); + match chars.next() { + Some((_, first)) if first.is_ascii_alphabetic() => {} + _ => return false, + } + for (index, character) in chars { + if character == ':' { + return index > 0; + } + if !(character.is_ascii_alphanumeric() + || character == '+' + || character == '-' + || character == '.') + { + return false; + } + } + false +} + +/// RFC 3986 §5.2.4. +fn remove_dot_segments(path: &str) -> String { + let absolute = path.starts_with('/'); + let trailing = path.ends_with('/') || path.ends_with("/.") || path.ends_with("/.."); + let mut out: Vec<&str> = Vec::new(); + for segment in path.split('/') { + match segment { + "" | "." => {} + ".." => { + out.pop(); + } + other => out.push(other), + } + } + let mut resolved = String::new(); + if absolute { + resolved.push('/'); + } + resolved.push_str(&out.join("/")); + if trailing && !resolved.ends_with('/') { + resolved.push('/'); + } + resolved +} + +/// Split a URI into the part that a rooted path replaces (scheme + authority) +/// and the path itself, so `root + path == uri` for a hierarchical URI. +fn split_authority(uri: &str) -> (&str, &str) { + match uri.find("://") { + Some(index) => { + let after = &uri[index + 3..]; + match after.find('/') { + Some(slash) => uri.split_at(index + 3 + slash), + None => (uri, ""), + } + } + // Opaque URI (`urn:...`): there is no authority to preserve. + None => match uri.rfind('/') { + Some(index) => uri.split_at(index + 1), + None => (uri, ""), + }, + } +} + +/// RFC 3986 §5.3 reference resolution, enough of it for schema identifiers. +/// +/// Exactness matters less than *consistency*: every `$id` is registered +/// through this function and every `$ref` is looked up through it, so a +/// document whose identifiers are written literally — which is what a bundler +/// emits — matches regardless of how this normalizes. `collect_scopes` also +/// registers each resource under its raw `$id` string for the same reason. +fn resolve_uri(base: &str, reference: &str) -> String { + if reference.is_empty() { + return base.to_owned(); + } + if has_scheme(reference) { + return reference.to_owned(); + } + if base.is_empty() { + return reference.to_owned(); + } + if let Some(rest) = reference.strip_prefix("//") { + let scheme = base.split(':').next().unwrap_or(""); + return format!("{scheme}://{rest}"); + } + let (root, path) = split_authority(base); + if reference.starts_with('/') { + return format!("{root}{}", remove_dot_segments(reference)); + } + let merged = match path.rfind('/') { + Some(index) => format!("{}{reference}", &path[..=index]), + None => format!("/{reference}"), + }; + format!("{root}{}", remove_dot_segments(&merged)) +} + +/// Resolve a reference to the JSON pointer of the node it names, or `None` +/// when it names nothing inside this document. +fn resolve( + base: &str, + reference: &str, + resources: &HashMap, + anchors: &HashMap<(String, String), String>, +) -> Option { + let (uri, fragment) = split_fragment(reference); + let (target_base, target_pointer) = if uri.is_empty() { + (base.to_owned(), resources.get(base)?.clone()) + } else { + let resolved = resolve_uri(base, uri); + match resources.get(&resolved) { + Some(pointer) => (resolved, pointer.clone()), + // Literal fallback: the reference as written, in case this + // resolver and the `$id` that registered the resource normalized + // differently. + None => (uri.to_owned(), resources.get(uri)?.clone()), + } + }; + match fragment { + None | Some("") => Some(target_pointer), + Some(pointer) if pointer.starts_with('/') => { + Some(format!("{target_pointer}{}", percent_decode(pointer))) + } + Some(name) => anchors + .get(&(target_base, percent_decode(name))) + .cloned(), + } +} + +fn percent_decode(input: &str) -> String { + let bytes = input.as_bytes(); + let mut out = Vec::with_capacity(bytes.len()); + let mut index = 0; + while index < bytes.len() { + if bytes[index] == b'%' && index + 2 < bytes.len() { + let hex = std::str::from_utf8(&bytes[index + 1..index + 3]).unwrap_or(""); + if let Ok(byte) = u8::from_str_radix(hex, 16) { + out.push(byte); + index += 3; + continue; + } + } + out.push(bytes[index]); + index += 1; + } + String::from_utf8(out).unwrap_or_else(|_| input.to_owned()) +} + +struct Scopes { + /// Base URI of every resource declared in this document -> its pointer. + /// Each resource is registered under both its resolved and its raw `$id`. + resources: HashMap, + /// `(base URI, anchor name)` -> pointer. Anchors are scoped to the + /// resource that declares them, so the same name may appear under two + /// different `$id`s without either shadowing the other. + anchors: HashMap<(String, String), String>, + /// Base URI in effect at each node pointer. + base_at: HashMap, + has_ids: bool, +} + +fn collect_scopes(root: &Value) -> Scopes { + let mut resources: HashMap = HashMap::new(); + let mut anchors: HashMap<(String, String), String> = HashMap::new(); + let mut base_at: HashMap = HashMap::new(); + let mut has_ids = false; + + resources.insert(String::new(), String::new()); + let mut stack: Vec<(String, String, &Value)> = vec![(String::new(), String::new(), root)]; + while let Some((pointer, inherited, node)) = stack.pop() { + match node { + Value::Object(map) => { + let mut base = inherited; + if let Some(id) = map.get("$id").or_else(|| map.get("id")).and_then(Value::as_str) + { + has_ids = true; + let resolved = resolve_uri(&base, strip_fragment(id)); + resources + .entry(resolved.clone()) + .or_insert_with(|| pointer.clone()); + resources + .entry(strip_fragment(id).to_owned()) + .or_insert_with(|| pointer.clone()); + base = resolved; + } + base_at.insert(pointer.clone(), base.clone()); + for keyword in ["$anchor", "$dynamicAnchor", "$recursiveAnchor"] { + if let Some(name) = map.get(keyword).and_then(Value::as_str) { + anchors + .entry((base.clone(), name.to_owned())) + .or_insert_with(|| pointer.clone()); + } + } + for (key, value) in map { + stack.push((child_pointer(&pointer, key), base.clone(), value)); + } + } + Value::Array(items) => { + base_at.insert(pointer.clone(), inherited.clone()); + for (index, value) in items.iter().enumerate() { + stack.push((format!("{pointer}/{index}"), inherited.clone(), value)); + } + } + _ => { + base_at.insert(pointer, inherited); + } + } + } + Scopes { + resources, + anchors, + base_at, + has_ids, + } +} + +#[derive(Clone, Copy, PartialEq)] +enum Color { + Gray, + Black, +} + +/// Iterative DFS — the checker itself must not recurse, or it would inherit +/// the very unbounded recursion it exists to refuse. +fn find_cycle(edges: &BTreeMap>) -> Option> { + let mut color: HashMap<&str, Color> = HashMap::new(); + for start in edges.keys() { + if color.contains_key(start.as_str()) { + continue; + } + let mut stack: Vec<(&str, usize)> = vec![(start.as_str(), 0)]; + let mut path: Vec<&str> = vec![start.as_str()]; + color.insert(start.as_str(), Color::Gray); + while !stack.is_empty() { + let (node, index) = { + let top = stack.last_mut().expect("stack is not empty"); + top.1 += 1; + (top.0, top.1 - 1) + }; + let successors = edges.get(node).map(Vec::as_slice).unwrap_or(&[]); + if index < successors.len() { + let next = successors[index].as_str(); + match color.get(next).copied() { + Some(Color::Gray) => { + let at = path.iter().position(|step| *step == next).unwrap_or(0); + let mut cycle: Vec = + path[at..].iter().map(|step| (*step).to_owned()).collect(); + cycle.push(next.to_owned()); + return Some(cycle); + } + Some(Color::Black) => {} + None => { + color.insert(next, Color::Gray); + path.push(next); + stack.push((next, 0)); + } + } + } else { + color.insert(node, Color::Black); + path.pop(); + stack.pop(); + } + } + } + None +} + +#[cfg(test)] +mod tests { + use serde_json::{Value, json}; + + use super::*; + + fn fixture(name: &str) -> Value { + let source = match name { + "valid" => include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../testdata/json-schema-valid.json" + )), + "invalid" => include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../testdata/json-schema-invalid.json" + )), + _ => unreachable!(), + }; + serde_json::from_str(source).unwrap() + } + + fn corpus() -> Value { + serde_json::from_str(include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../testdata/json-schema-bound-cases.json" + ))) + .unwrap() + } + + #[test] + fn shared_declarations_and_boolean_schemas_are_validated() { + assert!(validate_declaration(&fixture("valid")).is_ok()); + assert!(validate_declaration(&json!(true)).is_ok()); + assert!(validate_declaration(&json!(false)).is_ok()); + assert!(validate_declaration(&fixture("invalid")).is_err()); + } + + /// The bound, not the mechanism. Each refused schema **compiles cleanly** + /// in `jsonschema`, so this test fails the moment `bound_declaration` stops + /// running — compilation alone would accept every one of them. + #[test] + fn every_refused_corpus_schema_compiles_but_is_refused_by_the_bound() { + let corpus = corpus(); + let marker = corpus["marker"].as_str().unwrap(); + let cases = corpus["refused"].as_array().unwrap(); + assert!(cases.len() >= 20, "corpus lost refusal cases"); + for case in cases { + let name = case["name"].as_str().unwrap(); + let schema = &case["schema"]; + assert!( + jsonschema::validator_for(schema).is_ok(), + "{name}: this case only proves the bound if the schema compiles" + ); + let error = validate_declaration(schema) + .expect_err(&format!("{name}: unbounded schema must be refused")); + assert!( + error.contains(marker), + "{name}: refusal must be named {marker:?}, got {error}" + ); + } + } + + /// The other half of the bound: legitimate recursion stays legal. + #[test] + fn every_accepted_corpus_schema_is_accepted() { + let corpus = corpus(); + for case in corpus["accepted"].as_array().unwrap() { + let name = case["name"].as_str().unwrap(); + assert!( + validate_declaration(&case["schema"]).is_ok(), + "{name}: legitimate schema must stay legal, got {:?}", + validate_declaration(&case["schema"]) + ); + } + } + + /// The narrowed premise, tested rather than asserted in a comment. + /// + /// `resolve` leaves a reference naming no in-document resource opaque. + /// That is only safe because the ENGINE refuses such a reference. The + /// previous, wider form of this claim — "validator_for refuses anything the + /// bound cannot resolve" — was false for an in-document `$id`, which is + /// signoff-4's P0 in one sentence. Both halves are pinned: the bound must + /// not claim these, and `validator_for` must refuse them. If a future + /// `jsonschema` accepts an unresolvable reference, this fails and the + /// opaque default has to be revisited rather than silently becoming a hole. + #[test] + fn references_the_bound_leaves_opaque_are_refused_by_the_engine() { + let corpus = corpus(); + let cases = corpus["engineRefused"].as_array().unwrap(); + assert!(!cases.is_empty(), "corpus lost the engine-refused cases"); + for case in cases { + let name = case["name"].as_str().unwrap(); + let schema = &case["schema"]; + assert!( + bound_declaration(schema).is_ok(), + "{name}: the bound must not claim a reference it cannot resolve" + ); + let error = jsonschema::validator_for(schema) + .expect_err(&format!("{name}: the engine must refuse this reference")); + let error = error.to_string(); + assert!( + !error.contains(UNBOUNDED_REF_CYCLE), + "{name}: this must be the engine's refusal, not the bound's, got {error}" + ); + assert!( + compile(schema).is_err(), + "{name}: the gate as a whole must refuse it" + ); + } + } + + /// Every reference FORM in the 2020-12 vocabulary that can name a target + /// inside this document must be resolvable by the bound, not just the + /// `#`-prefixed ones. Keying on `#` is what let the compound-schema-document + /// (bundling) form through. + #[test] + fn in_document_uri_references_resolve_to_the_node_they_name() { + let schema = json!({ + "$id": "https://ex.test/root", + "$defs": { + "a": {"$id": "https://ex.test/a", "$anchor": "nm", "type": "string"} + } + }); + let Scopes { + resources, anchors, .. + } = collect_scopes(&schema); + for (base, reference, expected) in [ + ("", "https://ex.test/a", Some("/$defs/a")), + ("https://ex.test/root", "a", Some("/$defs/a")), + ("https://ex.test/root", "/a", Some("/$defs/a")), + ("https://ex.test/root", "https://ex.test/a#nm", Some("/$defs/a")), + ("https://ex.test/a", "#nm", Some("/$defs/a")), + ("https://ex.test/a", "#", Some("/$defs/a")), + ("", "https://ex.test/root", Some("")), + ("", "https://elsewhere.test/a", None), + ("https://ex.test/root", "#unknown-anchor", None), + ] { + assert_eq!( + resolve(base, reference, &resources, &anchors).as_deref(), + expected, + "base {base:?} reference {reference:?}" + ); + } + } + + #[test] + fn refusal_names_the_cycle_it_found() { + let schema = json!({ + "$defs": {"a": {"$ref": "#/$defs/b"}, "b": {"$ref": "#/$defs/a"}}, + "$ref": "#/$defs/a" + }); + let error = validate_declaration(&schema).unwrap_err(); + assert!(error.contains("#/$defs/a"), "{error}"); + assert!(error.contains("#/$defs/b"), "{error}"); + } + + /// A property literally named `$ref` is data, not a reference. + #[test] + fn a_property_named_ref_is_not_a_reference() { + let schema = json!({ + "type": "object", + "properties": {"$ref": {"type": "string"}, "allOf": {"type": "string"}} + }); + assert!(validate_declaration(&schema).is_ok()); + } + + /// The checker must not recurse: a deep schema is bounded work, not a + /// second stack overflow inside the guard. 1000 levels is an order of + /// magnitude past what can even reach the daemon — `serde_json` refuses to + /// parse past its own 128-deep nesting limit — and the checker walks it + /// with an explicit stack. + #[test] + fn deeply_nested_schemas_do_not_overflow_the_checker() { + let mut schema = json!({"type": "string"}); + for _ in 0..1_000 { + schema = json!({"type": "array", "items": schema}); + } + assert!(bound_declaration(&schema).is_ok()); + assert!( + serde_json::from_str::(&schema.to_string()).is_err(), + "serde_json is expected to refuse this depth on the wire" + ); + } +} diff --git a/kernel/relayflowd-core/src/spec.rs b/kernel/relayflowd-core/src/spec.rs index 8338848ac..4180ffccc 100644 --- a/kernel/relayflowd-core/src/spec.rs +++ b/kernel/relayflowd-core/src/spec.rs @@ -156,6 +156,14 @@ impl RunSpec { } } } + if let Some(schema) = &step.verification.json_schema { + crate::schema::validate_declaration(schema).map_err(|detail| { + SpecError::InvalidJsonSchema { + step: step.id.clone(), + detail, + } + })?; + } step.retry.validate(&step.id)?; } @@ -583,6 +591,8 @@ pub enum SpecError { DuplicateStep(String), #[error("step {0} must allow at least one iteration")] ZeroIterations(String), + #[error("step {step} declares an invalid JSON Schema: {detail}")] + InvalidJsonSchema { step: String, detail: String }, #[error("step {step} depends on unknown step {dependency}")] UnknownDependency { step: String, dependency: String }, #[error("dependency cycle includes step {0}")] diff --git a/kernel/relayflowd-core/src/state/tests.rs b/kernel/relayflowd-core/src/state/tests.rs index 8ce8455e1..05c17df12 100644 --- a/kernel/relayflowd-core/src/state/tests.rs +++ b/kernel/relayflowd-core/src/state/tests.rs @@ -14,7 +14,7 @@ fn spec() -> RunSpec { } #[test] -fn completed_output_is_memoized_and_unlocks_dependents() { +fn journal_replays_data_gate_verdict_without_rerunning_completed_code() { let completion = JournalEntry::new( EntryType::StepCompleted, "run", @@ -44,6 +44,13 @@ fn completed_output_is_memoized_and_unlocks_dependents() { "once" ); assert_eq!(state.steps["two"].state, StepState::Runnable); + let actions = crate::next_actions(&state, 6); + assert!(actions.iter().all(|action| match action { + crate::Action::Append(entry) => entry.step_id.as_deref() != Some("one"), + crate::Action::ExecDeterministic { step, .. } => step.id != "one", + crate::Action::Dispatch { step, .. } => step.id != "one", + _ => true, + })); } #[test] diff --git a/kernel/relayflowd-core/src/verify.rs b/kernel/relayflowd-core/src/verify.rs index 7bfca6730..7f6700af6 100644 --- a/kernel/relayflowd-core/src/verify.rs +++ b/kernel/relayflowd-core/src/verify.rs @@ -32,7 +32,7 @@ pub fn verify(step: &StepSpec, output: &Value) -> VerificationRecord { if let Some(schema) = &step.verification.json_schema { gates.push("json_schema"); - match jsonschema::validator_for(schema) { + match crate::schema::compile(schema) { Ok(validator) => { if let Err(error) = validator.validate(output) { failures.push(format!("JSON schema rejected output: {error}")); @@ -94,6 +94,34 @@ mod tests { ); } + /// A journal written by an older kernel can still hold an unbounded + /// schema; `run.resume` does not re-validate the stored spec. Routing + /// `verify` through the same declaration gate turns that into a gate + /// failure with a verdict instead of a SIGABRT. Remove the bound and this + /// test does not fail — it aborts the whole test binary. + #[test] + fn an_unbounded_schema_in_a_journal_fails_its_gate_instead_of_aborting() { + let step: StepSpec = serde_json::from_value(json!({ + "id": "poisoned", + "type": "deterministic", + "command": "true", + "verification": { + "json_schema": { + "$defs": {"a": {"$ref": "#/$defs/b"}, "b": {"$ref": "#/$defs/a"}}, + "$ref": "#/$defs/a" + } + } + })) + .unwrap(); + let record = verify(&step, &json!({"exit_code": 0, "stdout_tail": ""})); + assert_eq!(record.verdict, VerificationVerdict::Fail); + assert!( + record.detail.contains("unbounded $ref cycle"), + "{}", + record.detail + ); + } + #[test] fn json_schema_is_a_control_gate() { let step: StepSpec = serde_json::from_value(json!({ diff --git a/kernel/relayflowd/src/server.rs b/kernel/relayflowd/src/server.rs index 682861c7e..4a4a08a9a 100644 --- a/kernel/relayflowd/src/server.rs +++ b/kernel/relayflowd/src/server.rs @@ -147,7 +147,7 @@ fn handle_request( to_value( engine .start(spec, "protocol-v0", None) - .map_err(internal_error)?, + .map_err(run_start_error)?, ) } "run.resume" => { diff --git a/kernel/relayflowd/src/server/protocol.rs b/kernel/relayflowd/src/server/protocol.rs index 883b41625..734280782 100644 --- a/kernel/relayflowd/src/server/protocol.rs +++ b/kernel/relayflowd/src/server/protocol.rs @@ -2,6 +2,7 @@ use std::{path::Path, sync::Arc}; +use relayflowd_core::SpecError; use serde::{Serialize, de::DeserializeOwned}; use serde_json::Value; @@ -61,6 +62,14 @@ fn mutation_error(error: anyhow::Error) -> (&'static str, String) { } } +pub(super) fn run_start_error(error: anyhow::Error) -> (&'static str, String) { + if error.downcast_ref::().is_some() { + ("invalid_spec", format!("{error:#}")) + } else { + internal_error(error) + } +} + pub(super) fn internal_error(error: anyhow::Error) -> (&'static str, String) { let journal_failure = error.chain().any(|cause| { cause.is::() diff --git a/kernel/relayflowd/tests/invalid_schema_preflight.rs b/kernel/relayflowd/tests/invalid_schema_preflight.rs new file mode 100644 index 000000000..d25277502 --- /dev/null +++ b/kernel/relayflowd/tests/invalid_schema_preflight.rs @@ -0,0 +1,167 @@ +#![cfg(unix)] + +use std::{ + io::{BufRead, BufReader, Write}, + os::unix::net::UnixStream, + process::{Child, Command, Stdio}, + thread, + time::{Duration, Instant}, +}; + +use serde_json::{Value, json}; + +struct ChildGuard(Child); + +impl Drop for ChildGuard { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } +} + +struct Refusal { + response: Value, + marker_exists: bool, + registry_exists: bool, + runs_exists: bool, + daemon_alive: bool, +} + +/// Submit a spec whose only step declares `schema` as its gate, and report what +/// the daemon did with it — including whether the daemon is still running, +/// which is the assertion an unbounded schema used to fail with SIGABRT. +fn submit_gate(schema: Value) -> Refusal { + let directory = tempfile::tempdir().unwrap(); + let marker = directory.path().join("command-ran"); + let socket = directory.path().join("relayflowd.sock"); + let spec = json!({ + "steps": [{ + "id": "schema", + "type": "deterministic", + "command": format!("touch {}", marker.display()), + "verification": {"json_schema": schema} + }] + }); + + let child = Command::new(env!("CARGO_BIN_EXE_relayflowd")) + .args(["--data-dir", directory.path().to_str().unwrap(), "serve"]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .unwrap(); + let mut guard = ChildGuard(child); + let deadline = Instant::now() + Duration::from_secs(5); + while !socket.exists() { + assert!( + Instant::now() < deadline, + "relayflowd socket was not created" + ); + thread::sleep(Duration::from_millis(10)); + } + + let mut stream = UnixStream::connect(&socket).unwrap(); + let mut reader = BufReader::new(stream.try_clone().unwrap()); + writeln!( + stream, + "{}", + json!({"id": "hello", "verb": "hello", "params": {"protocol": 0, "client": "invalid-schema-test"}}) + ) + .unwrap(); + let mut hello = String::new(); + reader.read_line(&mut hello).unwrap(); + assert_eq!(serde_json::from_str::(&hello).unwrap()["ok"], true); + + let request = json!({"id": "start", "verb": "run.start", "params": {"spec": spec}}); + writeln!(stream, "{request}").unwrap(); + let mut response = String::new(); + let read = reader.read_line(&mut response).unwrap(); + assert!( + read > 0, + "the daemon closed the connection without answering run.start — \ + this is the SIGABRT signature the declaration bound exists to prevent" + ); + let response: Value = serde_json::from_str(&response).unwrap(); + + thread::sleep(Duration::from_millis(100)); + let daemon_alive = guard.0.try_wait().unwrap().is_none(); + + Refusal { + response, + marker_exists: marker.exists(), + registry_exists: directory.path().join("relayflowd.sqlite3").exists(), + runs_exists: directory.path().join("runs").exists(), + daemon_alive, + } +} + +fn assert_refused_before_any_effect(refusal: &Refusal) { + assert_eq!(refusal.response["ok"], false); + assert_eq!(refusal.response["error"]["code"], "invalid_spec"); + assert!( + !refusal.marker_exists, + "invalid spec must not execute its command" + ); + assert!(!refusal.registry_exists); + assert!(!refusal.runs_exists, "invalid spec must not create a journal"); + assert!( + refusal.daemon_alive, + "the daemon must survive an invalid declaration" + ); +} + +#[test] +fn invalid_json_schema_is_refused_before_journal_or_command() { + let schema: Value = serde_json::from_str(include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../testdata/json-schema-invalid.json" + ))) + .unwrap(); + assert_refused_before_any_effect(&submit_gate(schema)); +} + +/// The declaration bound, at the protocol boundary. Before it existed this +/// schema compiled, the journal was created, the step's command ran, and then +/// `verify` recursed until the daemon aborted with SIGABRT — leaving a run +/// stuck `running` that re-executed its effect on every resume. +#[test] +fn unbounded_json_schema_is_refused_before_journal_or_command() { + let corpus: Value = serde_json::from_str(include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../testdata/json-schema-bound-cases.json" + ))) + .unwrap(); + let marker = corpus["marker"].as_str().unwrap(); + for case in corpus["refused"].as_array().unwrap() { + let name = case["name"].as_str().unwrap(); + let refusal = submit_gate(case["schema"].clone()); + assert_refused_before_any_effect(&refusal); + let message = refusal.response["error"]["message"] + .as_str() + .unwrap_or_default(); + assert!( + message.contains(marker), + "{name}: expected a named {marker:?} refusal, got {message}" + ); + } +} + +/// Legitimate recursion must still run. Without this the bound could pass its +/// own refusal test by refusing everything. +#[test] +fn legitimately_recursive_json_schema_still_starts() { + let corpus: Value = serde_json::from_str(include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../testdata/json-schema-bound-cases.json" + ))) + .unwrap(); + for case in corpus["accepted"].as_array().unwrap() { + let name = case["name"].as_str().unwrap(); + let refusal = submit_gate(case["schema"].clone()); + assert_eq!( + refusal.response["ok"], true, + "{name}: must be accepted, got {}", + refusal.response + ); + assert!(refusal.daemon_alive, "{name}: daemon must survive"); + } +} diff --git a/ops/reviews/20260903-pr139-repair-0903.md b/ops/reviews/20260903-pr139-repair-0903.md new file mode 100644 index 000000000..98fc22546 --- /dev/null +++ b/ops/reviews/20260903-pr139-repair-0903.md @@ -0,0 +1,1412 @@ +# PR #139 repair — JSON Schema termination bound, and the rebase + +Repair owner report for AgentWorkforce/flows #139, "feat(sdk): settle data and +code gate contract". Answers the P0/P2/P3 in +`ops/reviews/20260903-pr139-signoff3-adversarial.md` (VERDICT: REVIEW_FAILED at +`e6210a2fc666df6dc8c777c009712ddf99efa877`). + +Worktree: `/Users/khaliqgant/AgentWorkforce/flows-pr139-repair-0903-wt` +Branch: `repair/pr139-0903` +Probe scripts: `/tmp/pr139repair/` (never in the worktree). + +--- + +## 0. The base, and how it moved twice + +**Final base: `512723c` — `origin/main` as of this rebase, pinned by SHA.** + +Main moved twice during this repair. The full sequence, because it explains the +shape of the work: + +| | main | what landed | what I did | +|---|---|---|---| +| brief written | `3da71e2` | — | told to rebase onto `3da71e2` | +| first rebase | `990093b` | #136 (`step-fields.ts`) | rebased onto `origin/main` — **the ref, not the SHA** — which had advanced under me. Reported it rather than quietly benefiting. Lead confirmed `990093b` was the correct target. | +| after push | `512723c` | #138 (`timeoutMs` per-verb) | PR went `CONFLICTING`. Stopped, reported, got the call, rebased onto **`512723c` by SHA**. | + +The process fault from the first rebase is worth keeping: every flows worktree +on this host shares one `.git`, so `refs/remotes/origin/main` can advance +between a `fetch` and a `rebase` with no action of yours. **Rebase onto the +SHA.** This rebase did. + +## 1. RED — the four-execution ladder, captured before anything changed + +`/tmp/pr139repair/repro-ladder.mjs`, run against `relayflowd` built from +`e6210a2` (PR #139's exact remote head), submitting a `json_schema` gate whose +schema is a mutually recursive `$ref` cycle with no body: + +```json +{ "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { "a": {"$ref": "#/$defs/b"}, "b": {"$ref": "#/$defs/a"} }, + "$ref": "#/$defs/a" } +``` + +The step's command is `echo ran >> `, so the tally file counts effect +executions. + +``` +$ node /tmp/pr139repair/repro-ladder.mjs +data-dir: /var/folders/.../T/pr139red-RiKgKk +after daemon start: runs/ exists=false entries=[] +hello -> {"id":"h","ok":true,"result":{"protocol":0,"server":"relayflowd"}} +after handshake : runs/ exists=false entries=[] +run.start -> CONN CLOSED, NO REPLY +after abort : runs/ exists=true entries=["01M1KFVFGBN4BR3XGWJ9BD2YCX.sqlite3","01M1KFVFGBN4BR3XGWJ9BD2YCX.sqlite3-shm","01M1KFVFGBN4BR3XGWJ9BD2YCX.sqlite3-wal"] +daemon: code=null signal=SIGABRT +stderr: thread '' (6287564) has overflowed its stack | fatal runtime error: stack overflow, aborting +*** did the step COMMAND execute before the crash? marker exists = true *** +start: reply=CONN CLOSED daemon=SIGABRT cumulative_effect_executions=1 +run.get -> {"id":"g","ok":true,"result":{"budget":{"dollars":"0","tokens_in":0,"tokens_out":0},"run_id":"01M1KFVFGBN4BR3XGWJ9BD2YCX","status":"running","steps":{"s":{"lease_deadline_ms":1788434330431,"state":"running","type":"deterministic"}}}} +resume #1: reply=CONN CLOSED (crash) daemon=SIGABRT cumulative_effect_executions=2 + daemon stderr: thread '' (6287595) has overflowed its stack | fatal runtime error: stack overflow, aborting +resume #2: reply=CONN CLOSED (crash) daemon=SIGABRT cumulative_effect_executions=3 +resume #3: reply=CONN CLOSED (crash) daemon=SIGABRT cumulative_effect_executions=4 + +FINAL: the command executed 4 time(s) for one logical step. +``` + +Reproduced exactly as reported: journal created, command executed, SIGABRT, +run stuck `running`, one effect executed four times, no `completionReason`, +no `step.completed`. + +### 1a. The crash family is wider than the report found + +Before designing the bound I calibrated it against the real daemon rather than +against the one reported schema (`/tmp/pr139repair/calibrate.mjs`, +`calibrate2.mjs`, still on the pre-fix binary): + +``` +mutual $defs, no body 413ms CRASH daemon=SIGABRT ... overflowed its stack +self $defs, no body 49ms ACCEPTED run status=completed +root self $ref "#" 22ms ACCEPTED run status=completed +allOf 1-cycle 336ms CRASH daemon=SIGABRT ... overflowed its stack +anyOf mutual cycle 389ms CRASH daemon=SIGABRT ... overflowed its stack +not cycle 392ms CRASH daemon=SIGABRT ... overflowed its stack +anchor mutual cycle 352ms CRASH daemon=SIGABRT ... overflowed its stack +oneOf cycle 424ms CRASH daemon=SIGABRT ... overflowed its stack +$id-scoped cycle 332ms CRASH daemon=SIGABRT ... overflowed its stack +if/then cycle 56ms ACCEPTED run status=completed +dependentSchemas cyc 46ms ACCEPTED run status=completed +cycle under properties 61ms ACCEPTED run status=completed +UNUSED degenerate def 38ms ACCEPTED run status=completed +LEGIT items:{$ref:"#"} 29ms ACCEPTED run status=failed +LEGIT recursive tree 39ms ACCEPTED run status=completed +LEGIT $defs tree 42ms ACCEPTED run status=completed +LEGIT mutual w/ bodies 35ms ACCEPTED run status=completed +LEGIT anyOf terminating 44ms ACCEPTED run status=failed +LEGIT prefixItems rec 34ms ACCEPTED run status=failed +LEGIT draft07 rec 17ms ACCEPTED run status=completed +LEGIT draft07 deps arr 21ms ACCEPTED run status=completed +LEGIT anchor no cycle 51ms ACCEPTED run status=completed +LEGIT plain object 19ms ACCEPTED run status=completed +vacuous {} / boolean true/false ~20ms ACCEPTED +external http $ref 0ms REFUSED [invalid_spec] ... Resource 'http://...' is not present +``` + +Six distinct schema shapes take the daemon down, not one — `allOf`, `anyOf`, +`oneOf`, `not`, plain-name anchors, and `$id`-scoped pointers all do it. The +three `ACCEPTED run status=completed` rows marked "cycle" above are **latent**: +they survived only because the probe's output never reached the cyclic +position (`if` never fired, the `dependentSchemas` trigger key was absent, the +`properties.p` key was absent). A different output crashes them. Any fix +keyed to the single reported schema would have left five live crashes and +three latent ones. + +--- + +## 2. The fix, and why it is structural rather than a caught exception + +A Rust stack overflow calls `abort()`. It is not a panic: `catch_unwind` never +sees it, no `Result` is produced, and the whole process — every concurrent run +in it — dies. There is no runtime recovery available at all, so the only place +this can be answered is *before* the schema is accepted. + +**The rule.** A JSON Schema keyword is either an *in-place* applicator, which +re-applies a subschema to the **same** instance (`$ref`, `$dynamicRef`, +`$recursiveRef`, `allOf`, `anyOf`, `oneOf`, `not`, `if`/`then`/`else`, +`dependentSchemas`, draft-07 `dependencies`), or a *child* applicator, which +descends into a member of the instance (`properties`, `patternProperties`, +`items`, `prefixItems`, `additionalItems`, `additionalProperties`, `contains`, +`propertyNames`, `unevaluated*`). A cycle through only in-place applicators +makes no progress: it re-applies to the same value forever, so validation +cannot terminate. A cycle that passes through a child applicator consumes one +level of the instance per turn, so it terminates — that is ordinary recursive +schema authoring and it stays legal. + +`kernel/relayflowd-core/src/schema.rs::bound_declaration` builds the reachable +schema graph, keeps only the in-place edges, and refuses if that subgraph +contains a cycle. It reports the cycle it found: + +``` +invalid run spec: step s declares an invalid JSON Schema: unbounded $ref cycle: +#/$defs/a -> #/$defs/b -> #/$defs/a — this cycle re-applies to the same +instance, so validation would not terminate +``` + +Three properties worth naming: + +- **The checker itself does not recurse.** Both the reachability walk and the + cycle detection use explicit stacks. A guard against unbounded recursion that + is itself recursive is not a guard. Pinned by + `deeply_nested_schemas_do_not_overflow_the_checker` (1000 levels — an order + of magnitude past what can reach the daemon, since `serde_json` refuses to + parse past its own 128-deep nesting limit; the test asserts that too). +- **`$id` re-basing is handled.** A JSON-pointer fragment resolves against the + nearest enclosing `$id`, not the document root. This is load-bearing, not + theoretical: `$id-scoped cycle` in the calibration above is a live SIGABRT + that root-relative resolution would miss entirely. Plain-name `$anchor` / + `$dynamicAnchor` are resolved too, for the same reason. +- **References are resolved as URIs, not by their `#` prefix.** A reference + resolves against the base URI in effect at its node (RFC 3986 §5), and only + then as a fragment inside the resource it names — so the 2020-12 *compound + schema document* form, where a `$ref` is a URI naming an `$id` declared + inside the same document, is followed like any other. Anchors are scoped to + the resource that declares them. + + **This paragraph used to say something false, and it is worth leaving the + correction visible.** It read: "Unresolvable references are left opaque, and + that is safe because a different gate closes them… The bound + under-approximates only where refusal is already guaranteed." That is true + for *remote* resources — §11 rows 1–4 demonstrate it — and false for an + in-document `$id`, which `jsonschema` resolves from the document's own + resource map and then overflows on. I demonstrated the true half and + generalised it to the false half. Signoff 4 found seven live crashes there; + §12b is the repair. The claim now made is the narrow one: a reference naming + no in-document resource is either remote, which the engine refuses, or a + bundled meta-schema, which cannot reference back into this document and so + cannot close a cycle rooted here — and the `engineRefused` corpus bucket + tests that rather than asserting it in a comment. +- **`$defs`/`definitions` are containers, not applicators.** Their members are + reached only through a `$ref`, so an unused degenerate definition — which is + never validated and cannot crash anything — stays legal. `unused degenerate + definition` is in the accepted half of the corpus for exactly this reason. + +`verify` now compiles through the same gate (`crate::schema::compile` instead +of `jsonschema::validator_for` directly). That is a *simplification* — one +schema gate instead of two call sites with different behaviour — and it means a +journal written by an older kernel, which still holds an unbounded schema, +fails its gate with a verdict rather than aborting. See §4 for what that path +actually does today. + +--- + +## 3. GREEN — the same ladder, same script, fixed daemon + +``` +$ node /tmp/pr139repair/ladder.mjs +data-dir: /var/folders/.../T/pr139green-sSxPO7 +before run.start: runs/ exists=false entries=[] +run.start -> {"id":"start","ok":false,"error":{"code":"invalid_spec","message":"invalid run spec: step s declares an invalid JSON Schema: unbounded $ref cycle: #/$defs/a -> #/$defs/b -> #/$defs/a — this cycle re-applies to the same instance, so validation would not terminate"}} +after run.start : runs/ exists=false entries=[] +daemon alive : true (exit=null) +*** did the step COMMAND execute? marker exists = false *** +start: cumulative_effect_executions=0 +resume #1: {"id":"r1","ok":false,"error":{"code":"run_not_found","message":"run 01ZZZ... does not exist"}} daemon_alive=true cumulative_effect_executions=0 +resume #2: {"id":"r2","ok":false,"error":{"code":"run_not_found","message":"run 01ZZZ... does not exist"}} daemon_alive=true cumulative_effect_executions=0 +resume #3: {"id":"r3","ok":false,"error":{"code":"run_not_found","message":"run 01ZZZ... does not exist"}} daemon_alive=true cumulative_effect_executions=0 + +FINAL: the command executed 0 time(s) for one logical step. +``` + +4 executions → 0. No journal, no command, daemon alive, named refusal. + +`invalid_json_schema_is_refused_before_journal_or_command` now holds for the +recursive case. `kernel/relayflowd/tests/invalid_schema_preflight.rs` was +rewritten around a shared harness and asserts, for **all twelve** refused +corpus schemas: `ok == false`, `error.code == "invalid_spec"`, the message +names `unbounded $ref cycle`, `!marker.exists()`, no `relayflowd.sqlite3`, no +`runs/`, **and `daemon_alive`** — that last one is the assertion that fails +loudly (the read of the `run.start` reply returns 0 bytes) if the abort ever +comes back. + +**On `completionReason`.** The brief asked for one. There is none to record on +this path and I want to be exact about why rather than fake it: the refusal +happens at declaration time, before `SqliteJournal::create`, so no run exists +and there is nothing to complete. A `completionReason` here would require +journaling a run in order to immediately fail it, which is strictly worse. The +equivalent guarantee — the failure is a closed, named verdict rather than an +abort — is what §3 and §4 demonstrate. + +--- + +## 4. A journal poisoned by the pre-fix daemon + +`run.resume` does not re-validate the stored spec (`server.rs:164` reads +`journal.run_spec()` and folds), so a run created *before* the bound existed is +a live question. I resumed the actual RED data-dir with the fixed binary: + +``` +$ node /tmp/pr139repair/ladder.mjs /var/folders/.../T/pr139red-RiKgKk +data-dir: .../pr139red-RiKgKk (REUSED: journal poisoned by the pre-fix daemon) +poisoned run: 01M1KFVFGBN4BR3XGWJ9BD2YCX, effect executions so far = 4 +run.get -> {"id":"g","ok":false,"error":{"code":"internal","message":"fold run journal: step s declares an invalid JSON Schema: unbounded $ref cycle: #/$defs/a -> #/$defs/b -> #/$defs/a — ..."}} +resume #1 -> {"id":"r1","ok":false,"error":{"code":"internal","message":"fold run journal: step s declares an invalid JSON Schema: unbounded $ref cycle: ..."}} + daemon_alive=true cumulative_effect_executions=4 +resume #2 -> {"id":"r2","ok":false,"error":{"code":"internal","message":"fold run journal: step s declares an invalid JSON Schema: unbounded $ref cycle: ..."}} + daemon_alive=true cumulative_effect_executions=4 +``` + +`RunState::fold` already calls `spec.validate()` (`state.rs:80`, pre-existing), +so the bound catches a poisoned journal there. **The exactly-once violation +stops**: executions stay frozen at 4 across both resumes instead of climbing to +5 and 6, and the daemon survives both. + +Two honest caveats: + +- The error code is `internal`, not a named kind. That is `fold`'s pre-existing + behaviour for any spec that fails validation, not something this change + introduces, and I did not widen the protocol taxonomy to fix it. +- `run.get` on such a legacy run now errors where it previously returned + `status: running`. That is a deliberate consequence of tightening validation + that `fold` already enforces; the run was unusable either way, and it can no + longer re-run its effect or kill the daemon. + +Because `fold` refuses first, the `verify.rs` half of the gate is defence in +depth rather than the live path. It is pinned by a unit test that would *abort +the whole test binary* rather than merely fail if the bound were removed: +`verify::tests::an_unbounded_schema_in_a_journal_fails_its_gate_instead_of_aborting`. + +--- + +## 5. SDK/kernel agreement on which schemas are legal + +The report's P2 divergence (SDK refuses a self-recursive `$defs`, kernel accepts +and completes) is a symptom. The cause is that "legal" was defined by two +different engines' accidental overflow behaviour: Ajv's overflow surfaces as a +catchable `RangeError`, Rust's aborts. Anywhere the two engines' recursion +limits differ, the answers differ. + +So the fix defines legality **explicitly, once, and pins both sides to one +corpus**: + +| | | +|---|---| +| Rule (kernel) | `kernel/relayflowd-core/src/schema.rs` | +| Rule (SDK) | `sdk/src/json-schema-bound.ts` — a line-for-line mirror | +| Shared corpus | `testdata/json-schema-bound-cases.json` — 12 refused, 14 accepted | +| Kernel enforcement | `schema::tests::every_refused_corpus_schema_compiles_but_is_refused_by_the_bound`, `every_accepted_corpus_schema_is_accepted`, plus the protocol-level `invalid_schema_preflight.rs` (all three tests) | +| SDK enforcement | `sdk/tests/json-schema-bound.test.ts` (43 tests) | +| Shared marker | `unbounded $ref cycle`, asserted equal to `corpus.marker` on the SDK side | + +A schema added to the corpus is enforced on both sides or one of the two suites +goes red. The self-recursive `$defs` case (`self $defs cycle with no body`) is +in the refused half, so the reported divergence is closed by construction +rather than by coincidence. + +### Does the test test the bound, or only that the mechanism fires? + +The brief's question, applied to each new test: + +- **`every_refused_corpus_schema_compiles_but_is_refused_by_the_bound`** first + asserts `jsonschema::validator_for(schema).is_ok()` for every refused case — + i.e. **compilation accepts all twelve**. The only thing rejecting them is the + bound. Delete `bound_declaration` and every one of the twelve flips to + accepted; the test cannot pass with the mechanism running but the bound gone. +- **SDK `refuses %s`** asserts `jsonSchemaBoundError(schema)` — the bound + function in isolation — before it asserts the wrapper and `compileSpec`. + Neutering the bound while leaving Ajv compilation in place fails it. +- **`every_accepted_corpus_schema_is_accepted`** and + `legitimately_recursive_json_schema_still_starts` are the other half: without + them a bound that refused everything would pass the refusal tests. +- **`deeply_nested_schemas_do_not_overflow_the_checker`** would abort the test + binary, not merely fail, if the checker were made recursive. +- **`an_unbounded_schema_in_a_journal_fails_its_gate_instead_of_aborting`** + likewise aborts the binary if the `verify` gate is reverted. +- **`invalid_schema_preflight.rs`'s `daemon_alive` assertion** is the + protocol-level version of the same idea. + +--- + +## 6. P2 — `canonicalize` and `specHash` + +Both are exported from `sdk/src/index.ts` and both took raw runtime input +unguarded; `canonicalize` additionally read every property **twice** (once in +`.filter(k => obj[k] !== undefined)`, once in `.map(...)`), which on a live +getter means two calls can disagree and `specHash` can stamp a spec nobody +declared. + +Fixed in `sdk/src/canonical.ts`: the exported entry points snapshot through +`snapshotJsonValue` — the same guard `validateSpec` and `kernelToAuthoring` +already carry — and the recursive serializer reads each key exactly once. + +Before (from the signoff): 10 proxy traps fired; the getter returned `1` to the +filter and `2` to the serializer. After (§11, row 46): refused, **0 traps, 0 +getter reads**. Pinned by three new tests in `sdk/tests/spec-parity.test.ts`, +which is the published-export boundary. + +`docs/SURFACE.md`'s enumeration of guarded unknown-input helpers now names them. + +## 7. P3 — vacuous gates + +Judgement: **surface, do not refuse.** `{}` and `true` are legal JSON Schema, +the kernel accepts both, and the corpus's accepted half includes them +deliberately — refusing them would break kernel/SDK agreement in the other +direction and is a breaking change beyond this PR's remit. But a gate that +judges nothing must not read like one that judges something. + +- `sdk/src/gate-contract.ts` exports `acceptsAnyOutput()` (true for `true`, + `{}`, or a schema carrying only annotation keywords) and sets + `acceptsAnyOutput: true` on the inspection — an optional property, so exact + `toEqual` assertions on real gates are unaffected. +- `flows check` appends ` [json_schema accepts any output]` to the GATE line. +- Preflight emits a new `vacuous_gate` warning kind. + +`vacuous_gate` had to be added to `PREFLIGHT_WARNING_KINDS`, and +`preflight.test.ts` has a standing gate asserting every declared warning kind is +reachable ("reaches every declared warning kind and leaks no probe exception +text"). It went red immediately, which is the gate working; I added the missing +scenario rather than exempting the kind. + +## 8. A divergence #133 opened on the same surface, found and closed + +Reading #133's resolutions "with particular care", as instructed, turned up a +live SDK/kernel divergence that neither the signoff nor the brief names. + +`output:` is authoring sugar that lowers to a `json_schema` gate at compile +time (`compile.ts::typedOutputVerification`). But `validateOutputDeclaration` +only checked `isObject` — it never ran the schema through `jsonSchemaError`. So +an `output` schema that the kernel refuses passed `flows check` cleanly: + +``` +$ node sdk/dist/cli.js check /tmp/pr139repair/out-bad.flow.yaml # unbounded $ref cycle in output: +GATE step "analyze" json_schema from data (kernel, journal-replayable) <-- reported as a gate +$ node sdk/dist/cli.js check /tmp/pr139repair/out-bad2.flow.yaml # type: definitely-not-a-json-schema-type +GATE step "analyze" json_schema from data (kernel, journal-replayable) <-- reported as a gate +``` + +Both are refused by the kernel at `run.start`. That is exactly the class of +divergence this PR exists to close, on a path this PR's own bound would +otherwise not reach. Fixed by routing `output` through the same gate: + +``` +$ node sdk/dist/cli.js check /tmp/pr139repair/out-bad.flow.yaml +REFUSED [invalid_spec] Relayflow spec is invalid: spec.steps[0].output: invalid JSON Schema: unbounded $ref cycle: #/$defs/a -> #/$defs/b -> #/$defs/a — this cycle re-applies to the same instance, so validation would not terminate +exit=2 +$ node sdk/dist/cli.js check /tmp/pr139repair/out-bad2.flow.yaml +REFUSED [invalid_spec] Relayflow spec is invalid: spec.steps[0].output: invalid JSON Schema: schema is invalid: data/type must be equal to one of the allowed values, data/type must be array, data/type must match a schema in anyOf +exit=2 +$ node sdk/dist/cli.js check testdata/hn-monitor.flow.yaml # the shipped fixture that uses output: +GATE step "analyze-story" json_schema from data (kernel, journal-replayable) +RESOLVED step "analyze-story" cli "preflight/analyze-story-claude-cli" model "claude-haiku-4-5-20251001" from step +CHECK PASSED testdata/hn-monitor.flow.yaml +exit=0 +``` + +Pinned by 13 new tests: every refused corpus schema is refused through the +`output` path too, plus the uncompilable case. + +--- + +## 9. The rebase — every conflict and how it was resolved + +`git rebase origin/main` over 7 commits. Three commits conflicted. + +**No conflict was resolved by taking one side wholesale.** The full list: + +| # | File | HEAD (main) | Branch (#139) | Resolution | +|---|---|---|---|---| +| 1 | `sdk/src/validate.ts` imports | main's 5 new module imports (`output-schema`, `model-name`, `unknown-keys`, `step-dependencies`, `step-fields`) | `jsonSchemaError`, later `+ snapshotJsonSchema` | Union — all six | +| 2 | `sdk/src/compile.ts` `toKernelSpec` preamble | `validateSpec(flow)` | same + explanatory comment and explicit `ValidationResult` type | Union | +| 3 | `sdk/src/compile.ts:80` | `const input = spec` + main's named-declaration comment | `const input = snapshot` | `snapshot` **and** main's comment — dropping either loses the proxy guard or the provenance note | +| 4 | `sdk/src/compile.ts` `compileStep` base | `...(verification !== undefined ? { verification } : {})` where `verification = typedOutputVerification(step)` | removed from base, re-added per verb as `s.verification` | **This is the sharp one.** Taking the branch side would have replaced the `output`-lowered gate with the raw authored one and silently dropped #133's lowering. Resolution: drive the per-verb branches from main's shared `verification`, not from `s.verification`, and narrow it with a new fail-closed `outputGate()` that refuses `exit_code` off a deterministic step. Both intents preserved. A stray auto-merged (non-conflicting) `...(s.verification …)` line in the `llm` branch was removed for the same reason. | +| 5 | `sdk/src/compile.ts` agent branch | `...(s.agent …)` | `...(s.verification …)` | main's line only — #4 already carries verification | +| 6 | `sdk/src/compile.ts` `toKernelSpec` body | `flow.*` + `resolveNamedAgent(step, flow.agents)` | `compiled.*` | `compiled.*` **and** `resolveNamedAgent(step, compiled.agents)` | +| 7 | `sdk/src/preflight.ts` entry | returns a named `invalid_spec` refusal | `compileSpec(flow)` (throws) | **Behaviour conflict, resolved in main's favour with the branch's hardening kept.** `compileSpec` inside a `try`, its `CompileError` converted to main's refusal shape. Main's structured refusal is the better contract (RFC covenant 2); the branch's compile-before-probe guarantee is what makes gates and proxies safe. Both hold. | +| 8 | `sdk/src/preflight.ts` loop | main's two-phase resolve-then-probe with `resolutionByStep` | branch's single loop over `compiled` | main's structure, over `compiled.steps`, with `unknownModelDiagnostics(compiled, …)`; `gates` added to main's early return | +| 9 | `sdk/src/preflight.ts` (last commit) | — | `warnOnVacuousGate` in the loop | moved into main's *warning* pass, not the resolve pass — the resolve pass early-returns `ok:false` on any diagnostic, so a warning there would have been a false refusal | +| 10 | `sdk/src/spec.ts` `AgentStepSpec` | `agent?: string` | `verification?: OutputVerificationSpec` | Union | +| 11 | `sdk/tests/preflight.test.ts` | main's two tests | branch's one | All three kept; the branch's rewritten to assert main's refusal contract instead of `toThrow()` | + +Three tests needed adapting to main's shape, all disclosed rather than deleted: + +- `sdk/tests/verb-field-lint.test.ts` — exact `toEqual` on `PreflightResult`, + which now carries `gates`. Added `gates: []` (a refused spec compiled + nothing, so it has no gate plan). +- `sdk/tests/gate-contract.test.ts` — "rejects proxy schemas without executing + traps at any public data boundary" iterates three boundaries and asserted + `toThrow`, but `preflight` now returns a refusal. Replaced with a + `captureRefusal` helper that accepts either shape and returns the + non-matching string `'NOT REFUSED: the boundary accepted the input'` if a + boundary accepts — so the gate is not weakened, it covers one more shape. + `proxyTraps === 0` and the source-object-unmutated assertions are untouched. +- `sdk/tests/preflight.test.ts` — the branch's raw-input test now asserts + `{ok:false, gates:[], resolutions:[], diagnostics:[{kind:'invalid_spec'}]}` + and `probeCount === 0`, which is the same substance under main's contract. + +### Files on only one side of the rebase — the full list I checked + +`git diff --name-only origin/main` and `... HEAD`, +merge-base `a0d42ffbdc7fb60b42c0b5bea4f58408249b08a2`. Hits are marked; the +rest are listed because they were checked and were clean. + +**Added by main, absent from the branch — the silent-merge risk surface:** + +| File | Concern | Does the branch touch the same concern under another name? | +|---|---|---| +| `sdk/src/step-fields.ts` | per-verb authoring allowlist, extracted from `validate.ts::STEP_TYPE_KEYS` | **YES — this is the named trap.** See §10. Defused. | +| `sdk/src/unknown-keys.ts` | unknown-key suggestions, extracted from `validate.ts` | No. Branch's `validate.ts` diff never touched that section; verified no duplicate implementation survives (`grep` for `nearestKey`/`levenshtein` in `validate.ts`: none) | +| `sdk/src/output-schema.ts` | `output` sugar → `json_schema` (#133) | **YES — same json_schema surface.** Found a real gap; §8 | +| `sdk/src/model-name.ts` | model-name validation, extracted from `validate.ts` | No | +| `sdk/src/step-dependencies.ts` | `dependsOn` validation, extracted from `validate.ts` | No | +| `sdk/src/cli-adapter.ts`, `worker-cli.ts`, `wrapper-runtime.ts`, `wrapper-session.ts` | worker/CLI adapter plane | No | +| `kernel/relayflowd-core/src/machine/cancel.rs`, `kernel/relayflowd/src/server/cancel.rs` | `run.cancel` (#142) | No. Branch's only `server.rs` hunk is the `invalid_spec` mapping; verified present post-rebase alongside main's `mod cancel` | +| `sdk/tsconfig.tests.json` | test typecheck gate | No — adopted and run as gate 2 | +| `sdk/tests/{cli-adapter,model-selection,real-cli-adapters,typed-output,verb-field-lint,worker-cli}.test.ts` | new suites | Only `verb-field-lint` needed the `gates` field; disclosed above | +| `examples/**` (24 files), `ops/reviews/**` (18 files), `.github/workflows/cloud-runtime-artifact.yml`, `scripts/run-workflow.sh`, `testdata/preflight/*`, `testdata/flows.json` | docs, fixtures, CI | No | +| `kernel/{entry,machine,machine/recovery,machine/tests,state}.rs`, `kernel/relayflowd/src/{engine,engine/remote,lib,main,server/client,server/session}.rs`, `kernel/relayflowd/tests/crash_resume*.rs` | run.cancel and engine work | No overlap; auto-merged; whole kernel suite green | + +**Added by the branch, absent from main:** `kernel/relayflowd-core/src/schema.rs`, +`kernel/relayflowd/tests/invalid_schema_preflight.rs`, `sdk/src/gate-contract.ts`, +`sdk/src/json-schema.ts`, `sdk/src/json-schema-bound.ts`, `sdk/src/json-value.ts`, +`sdk/tests/gate-contract.test.ts`, `sdk/tests/json-schema-bound.test.ts`, +`testdata/json-schema-{valid,invalid}.json`, +`testdata/json-schema-invalid.flow.yaml`, `testdata/json-schema-bound-cases.json`. +Checked the reverse direction too — does main address any of these concerns +under a different name? One hit: `output-schema.ts` vs `json-schema.ts`, §8. + +**Deleted on either side:** none. + +--- + +## 9b. The second rebase — onto `512723c` (#138) + +`git merge-tree` predicted it exactly: **two conflicting files**, +`sdk/src/compile.ts` and `sdk/src/spec.ts`. Everything else auto-merged +(`validate.ts`, `verb-field-lint.test.ts`, `validate.test.ts`, kernel +`spec.rs`, `spec/tests.rs`). Both conflicts landed on the traps. + +| # | File | HEAD (main/#138) | Branch (#139) | Resolution | +|---|---|---|---|---| +| 12 | `spec.ts` `DeterministicStepSpec` | `+ timeoutMs?: number` | `+ verification?: VerificationSpec` | Union. Both sides removed a field from `BaseStepSpec` and re-declared it per verb — #138 for `timeoutMs`, #139 for `verification` — and those removals auto-merged cleanly. Only the two additions collided. | +| 13 | `compile.ts` deterministic branch | `const verification = s.verification ?? exit_code` (shadowing), then `verification,` + the `timeoutMs` spread | the shadow deleted; `verification: verification ?? exit_code` reading the **outer** binding | **Trap 2 and trap 3 in one hunk.** Kept the branch's outer-binding read *and* #138's `timeoutMs` spread. | + +On conflict 13, the two sides are semantically equal *today*: +`typedOutputVerification` returns `step.verification` unchanged for a +deterministic step, because `output` is not an authorable field on that verb. +Taking either would pass every test. I kept the shared-binding form anyway, +because it is what holds the invariant — every branch of the switch reads the +lowered gate — and left a comment saying so. The equality is a coincidence of +the current `typedOutputVerification`, not a property to rely on. + +### Enumerating what main lost, including what auto-merged + +Per #138's own signoff method: re-derive the merged PR as +`git diff --name-status 990093b 512723c`, blob-compare every file, and attribute +every deleted line. That is the check that catches an auto-merged revert, which +no conflict marker will show you. + +**Blob comparison, all 15 files #138 touched:** + +``` +kernel/relayflowd-core/src/spec/dependencies.rs b410eecd b410eecd IDENTICAL +kernel/relayflowd-core/src/spec/tests.rs 6412b9c5 6412b9c5 IDENTICAL +ops/reviews/20260903-pr138-rebase-0903.md 695f9b8f 695f9b8f IDENTICAL +sdk/src/step-dependencies.ts e96e6cd0 e96e6cd0 IDENTICAL +sdk/src/step-fields.ts 60490fe9 60490fe9 IDENTICAL <-- trap 1 +sdk/tests/dependency-validation.test.ts 8dea5e92 8dea5e92 IDENTICAL +sdk/tsconfig.type-tests.json 481c3e0d 481c3e0d IDENTICAL +sdk/type-tests/step-fields.ts f3588bcf f3588bcf IDENTICAL <-- trap 1 +kernel/relayflowd-core/src/spec.rs 97abb689 086cd14e changed-by-me +sdk/package.json d905764c d9eba9a6 changed-by-me +sdk/src/compile.ts f931359e 57dc2712 changed-by-me +sdk/src/spec.ts e049822d 18ba9e01 changed-by-me +sdk/src/validate.ts ec0b75b5 7027ebca changed-by-me +sdk/tests/validate.test.ts 070093b8 e5992193 changed-by-me +sdk/tests/verb-field-lint.test.ts 470e88e9 bb2d3e15 changed-by-me +``` + +Nothing lost. **`step-fields.ts` and `type-tests/step-fields.ts` are +byte-identical to `512723c`** — trap 1 defused by construction rather than by +inspection, which is the strongest form the check can take. The five +`changed-by-me` source files were then diffed line by line against `512723c` +and every deletion attributed: + +| Deleted from main's version | Attribution | +|---|---| +| `compile.ts` `const validation = validateSpec(spec)` | present, now `validateSpec(snapshot)` — the proxy guard. Widened. | +| `compile.ts` `const input = spec as FlowSpec` | `const input = snapshot as FlowSpec`. Intentional. | +| `compile.ts` `base`'s `...(verification …)` spread | moved per-verb by #139; all three branches carry it. Intentional. | +| `compile.ts` deterministic `const verification = s.verification ?? …` | conflict 13, resolved above. | +| `compile.ts` `toKernelSpec`'s `validateSpec(flow)` guard | replaced by `compileSpec(flow)`, which validates **and** snapshots. Strictly stronger. | +| `compile.ts` `flow.*` → `compiled.*` (12 lines) | intentional, conflict 6. | +| `compile.ts` `requireNoTimeout` | **#138's own deletion, correctly preserved** — verified absent. | +| `spec.ts` `verification?: VerificationSpec` in `BaseStepSpec` | moved per-verb by #139. Intentional. | +| `spec.ts` `schema: Record` | **false alarm — a type widening.** Now `boolean \| Record`, because `true`/`false` are legal schemas (§7). | +| `spec.ts` `json_schema?: Record` | same widening. | +| `validate.ts` `validateVerification(v, at)` | **false alarm — a signature widening.** Still present as `validateVerification(v, at, stepType)`, now calling `jsonSchemaError`. | + +Two of the eleven were the false-alarm shape #138's signoff warned about — a +type widening and a signature widening, both reading as deletions in a line +diff. Same lesson, same method, caught the same way. + +### One more test adapted, same class as before + +`sdk/tests/dependency-validation.test.ts` (new in #138) failed on first run: + +``` +FAIL tests/dependency-validation.test.ts > still rejects a dependency cycle + fail-closed through every direct public boundary +AssertionError: expected { ok: false, gates: [], …(2) } to deeply equal + { ok: false, resolutions: [], …(1) } ++ "gates": Array [], +``` + +An exact `toEqual` on `PreflightResult`, which #139 widens with `gates`. Added +`gates: []` — a refused spec compiled nothing, so it has no gate plan. + +**Corrected after signoff 4's P3-2: there are FOUR such adaptations, not +three.** The full list, since the point of a disclosure is that a reviewer can +trust the count: + +| File | Change | Shape | +|---|---|---| +| `verb-field-lint.test.ts` | 2 × `gates: []` | refused spec, empty gate plan | +| `preflight.test.ts` | assertion rewritten to main's refusal contract | disclosed in §9 | +| `dependency-validation.test.ts` | 1 × `gates: []` | refused spec, empty gate plan | +| `cli.test.ts` | 2 × `gates: [{stepId, kind, checks, evaluator, preflightable, replayable}]` | **accepted** spec, so a populated gate plan | + +`cli.test.ts` is the one I missed, and it is the least like the others: its +specs compile, so the added field carries a real gate plan rather than `[]`. +No assertion was weakened in any of the four and no file's test-name set +changed — signoff 4 verified that independently by name set-diff. + +## 9c. The third rebase — onto `16860d2` (#151), and trap 4 + +`origin/main` moved a third time, to `16860d2` (#151, "lower trigger keys to +the kernel dialect, and add a scheduled trigger"), immediately after the push of +`105411c`. Rebased onto the pinned SHA. Two conflicts, and **the first one is +trap 2's shape a fourth time**. + +| # | File | HEAD (#151) | Branch (#139) | Resolution | +|---|---|---|---|---| +| 14 | `compile.ts` imports | `TriggerSpec` | `VerificationSpec` | Union | +| 15 | `compile.ts` `toKernelSpec` body | `flow.*`, and `triggers: flow.triggers.map(toKernelTrigger)` | `compiled.*`, and `triggers: compiled.triggers` | **Both halves load-bearing.** `compiled.*` is #139's snapshot guard; `.map(toKernelTrigger)` is #151's *lowering*. Resolution: `compiled.triggers.map(toKernelTrigger)`. | +| 16 | `spec-parity.test.ts` | #151's trigger-dialect suite (5 tests) | #139's nested-proxy test | Union — both are additions at the same line | + +### Trap 4, named + +Conflict 15 is the same defect class as trap 2, in the same function, one +release later. `toKernelTrigger` turns authoring keys into the kernel's +snake_case dialect — `eventType` → `event_type`, `dedupeKeyTemplate` → +`dedupe_key_template`, `staleAfterMs` → `stale_after_ms`. That is *authoring +sugar becoming a different object at the boundary*, exactly like `output:`. + +Taking my own side of that hunk — which looks like the obvious "keep my +snapshot-guard refactor" resolution — would have dropped `.map(toKernelTrigger)` +and sent camelCase to a kernel that refuses unknown fields. #151's own commit +message records what that costs: + +``` +malformed run spec: unknown field `dedupeKeyTemplate`, expected one of +`id`, `executor`, `event_type`, `pattern`, `dedupe_key_template`, `stale_after_ms` +``` + +**And the prediction from §10 held exactly.** `validateSpec` and `flows check` +are blind to it: + +``` +--- PRIMARY: compileYaml + toKernelSpec --- + authoring trigger : {"id":"every-minute","executor":"agent-worker","eventType":"flows.tick",...,"dedupeKeyTemplate":"...","staleAfterMs":180000} + kernel trigger : {"id":"every-minute","executor":"agent-worker","event_type":"flows.tick",...,"dedupe_key_template":"...","stale_after_ms":180000} + LOWERED? true + +--- the blind paths: both look correct either way --- + validateSpec ok = true + flows check exit=0 CHECK PASSED testdata/tick-heartbeat.flow.yaml +``` + +`validateSpec` and `flows check` return exactly those two lines whether the +lowering happened or not. Only reading the kernel object distinguishes them. +This is now the fourth trap of this shape (`step-fields` allowlist, `output:` +lowering, `timeoutMs` spread, trigger lowering), and the third where the +primary-assertion rule was what caught it. + +Unlike traps 1–3, this one also has a committed fixture behind it: #151 pinned +`testdata/tick-heartbeat.spec.canonical.json` and `.spec.sha256`, and +`spec-parity.test.ts` compares against both. So a reverted lowering would have +gone red in the suite as well — the first of the four traps with a standing +test rather than only a proof. + +### Blob comparison against `16860d2` + +All 14 files #151 touched, `git rev-parse :`: + +``` +ops/reviews/20260903-scheduled-trigger-design.md 3315915a 3315915a IDENTICAL +sdk/src/tick-source.ts 2f9bb5e0 2f9bb5e0 IDENTICAL +sdk/tests/live-kernel.test.ts 577752a4 577752a4 IDENTICAL +sdk/tests/tick-source.test.ts 549c5888 549c5888 IDENTICAL +testdata/preflight/tick-slot-report-cli f9ae33f3 f9ae33f3 IDENTICAL +testdata/tick-heartbeat.flow.yaml 588cce12 588cce12 IDENTICAL +testdata/tick-heartbeat.spec.canonical.json aceb55ac aceb55ac IDENTICAL +testdata/tick-heartbeat.spec.sha256 6350ee30 6350ee30 IDENTICAL +sdk/src/compile.ts 5ed22b68 4534b87d changed-by-me +sdk/src/index.ts a684f80b 27798eda changed-by-me +sdk/src/spec.ts 3e3f8df7 410d1fa4 changed-by-me +sdk/src/validate.ts fda98e7e 759c031c changed-by-me +sdk/tests/spec-parity.test.ts 662f47b8 32804f94 changed-by-me +sdk/tests/validate.test.ts 8cac842b e0e57928 changed-by-me +``` + +Nothing lost. The pinned canonical-form and hash fixtures are byte-identical, +which is the strongest available statement that the trigger lowering survived. + +Every deletion in the six changed files was attributed. Two are new this +rebase, both #139's proxy guard doing what it does everywhere else: + +| Deleted | Attribution | +|---|---| +| `compile.ts` `requireKernelObject(value, …)` | now `requireKernelObject(snapshot, …)` in `kernelToAuthoring` | +| `validate.ts` `return new Validator().run(spec)` | now `run(snapshotJsonValue(spec, 'spec'))` | +| `compile.ts` `flow.triggers.map(toKernelTrigger)` | conflict 15 — `toKernelTrigger` verified still present at `compile.ts:321` and called at `:252` | + +The rest are the same set attributed in §9b. + +### Gates after the third rebase + +``` +tsc --noEmit G1=0 +tsc -p tsconfig.type-tests.json G2=0 +tsc -p tsconfig.tests.json G3=0 +vitest run Test Files 26 passed | 1 skipped (27) + Tests 538 passed | 3 skipped (541) G4=0 +cargo test --workspace 114 tests, 13 binaries, failures 0, never-executed 0 G5=0 +``` + +The signoff-4 repair re-verified on the rebased build: URI ladder still +`FINAL: the command executed 0 time(s)`, URI family still `ATTACKS STILL LIVE: 0`. + +## 10. The traps — two of them, and the three-path `output` proof + +There were **three** silent-revert traps across the two rebases. They are the +same class — a conflict resolution that looks obviously correct in isolation +and quietly reverts freshly merged behaviour — and only one of them was named +in the brief. + +**Trap 1 (named in the brief): `step-fields.ts`.** Main moved the per-verb +authoring allowlist out of `validate.ts::STEP_TYPE_KEYS` into +`step-fields.ts::STEP_FIELDS_BY_TYPE`. A branch resolving in favour of its own +inline copy drops `output` from the allowlist. + +**Trap 2 (not named, and the sharper of the two): `compile.ts::compileStep`.** +Main computes `verification` once from `typedOutputVerification(step)` — the +`output:` sugar *lowered* into a kernel `json_schema` gate — and spreads it into +the step. #139 had removed that from the base and re-added `s.verification` +(the *raw authored* gate) per verb. Taking the branch side reads as an obvious +"keep my per-verb refactor" resolution and silently replaces every lowered gate +with the raw authored one, reverting #133. Resolution: drive the per-verb +branches from main's shared `verification`, narrowed by a new fail-closed +`outputGate()`. A stray auto-merged — **non-conflicting** — `...(s.verification +…)` line in the `llm` branch had to be removed for the same reason, which is +worth noting: git never flagged it. + +**Trap 3 (new with #138): `timeoutMs` must be deterministic-only in TWO +places.** #138 moved it out of `BaseStepSpec`/`STEP_COMMON_FIELDS` and into +`DeterministicStepSpec`/`STEP_FIELDS_BY_TYPE.deterministic`, and out of +`compileStep`'s shared `base` into the deterministic branch. Getting the +allowlist right while missing the spread — or the reverse — produces a spec that +**validates but lowers wrong**, which is trap 2's shape exactly. + +### The verification standard this changes + +The three paths are not three independent witnesses. Against trap 2 and trap 3, +two of them are blind: + +| Path | Sees trap 1 (allowlist) | Sees trap 2 (lowering) | Sees trap 3 (spread) | +|---|---|---|---| +| `validateSpec` | yes | **no** | **no** | +| `flows check` | yes | **no** | **no** | +| `preflight` | yes | **no** | **no** | +| `compileYaml` + `toKernelSpec` | yes | **yes** | **yes** | + +`validateSpec`, `preflight` and `flows check` all answer "was the key +accepted?". Only `compileYaml` + `toKernelSpec`, read against the *kernel* +object, answers "did the key become a gate / reach the kernel step?" — and that +is the question both traps turn on. **`compileYaml` + `toKernelSpec` is the +primary assertion; the others are corroborating.** A proof that runs three +paths and treats them as equals is one witness and two that cannot see. + +And the detail that makes this dangerous rather than merely subtle: on the first +rebase, one of the reverting lines — a stray `...(s.verification …)` in the +`llm` branch — **auto-merged. Git raised no conflict on it.** Reviewing only the +conflict markers would have missed it and the suite would have been green. + +### The proofs + +The allowlist that must not lose `output`, in my rebased tree, byte-identical +to main's: + +``` +$ sed -n '32,37p' sdk/src/step-fields.ts +export const STEP_FIELDS_BY_TYPE = { + deterministic: ['command', 'timeoutMs'], + llm: ['prompt', 'model', 'cli', 'output'], + agent: ['instruction', 'agent', 'cli', 'model', 'surfaces', 'recoveryMode', 'permissions', 'output'], +} as const satisfies Record; +``` + +*(Corrected after signoff 4's P3-1. This block previously quoted the first +rebase base, `990093b`, where `deterministic` was still `['command']`. The code +was right and the citation was from the wrong tree — which reads as fabrication +even when the work is real, so: this is `sed` output from the final tree, and +`deterministic` carries `timeoutMs` because #138 put it there.)* + +Proved through all three paths, because they diverge +(`/tmp/pr139repair/output-proof.mjs`): + +``` +PATH 1 — validateSpec + llm ok=true + agent ok=true + deterministic ok=false spec.steps[0]: unknown key "output" (expected one of id | type | dependsOn | verification | maxI... + +PATH 2 — compileYaml (+ toKernelSpec lowering) + llm ACCEPTED kernel verification = {"json_schema":{"type":"object","properties":{"answer":{"type":"string"}},"required":["answer"]}} + agent ACCEPTED kernel verification = {"json_schema":{"type":"object","properties":{"answer":{"type":"string"}},"required":["answer"]}} + deterministic REFUSED spec compile failed: - spec.steps[0]: unknown key "output" (expected one of id | type | dependsO... + +PATH 3 — flows check + llm GATE step "s" json_schema from data (kernel, journal-replayable) + agent GATE step "s" json_schema from data (kernel, journal-replayable) + deterministic REFUSED [invalid_spec] Relayflow spec is invalid: spec.steps[0]: unknown key "output" (expected one of id | ty... +``` + +Path 2 is the one that would have caught conflict #4: it shows the `output` +declaration actually **lowered into a kernel `json_schema` gate**, not merely +accepted as a key. Had I taken the branch side of that hunk, path 1 and path 3 +would still have looked correct and path 2 would have emitted `{}`. + +Path 3's llm/agent cases exit 2 in the synthetic fixture, but for +`cli_unsupported` — my probe declares `cli: sh`, and this host has no +conforming `relayflows-agent-cli-v1` wrapper. The clean end-to-end path-3 +proof is the shipped fixture main modified in #133, which uses `output:` on an +`llm` step: + +``` +$ node sdk/dist/cli.js check testdata/hn-monitor.flow.yaml +GATE step "analyze-story" json_schema from data (kernel, journal-replayable) +RESOLVED step "analyze-story" cli "preflight/analyze-story-claude-cli" model "claude-haiku-4-5-20251001" from step +CHECK PASSED testdata/hn-monitor.flow.yaml +exit=0 +``` + +--- + +### Trap 3 — the four-path `timeoutMs` proof + +`/tmp/pr139repair/timeout-proof.mjs`. `timeoutMs: 5000` declared on each verb, +through all four boundaries: + +``` +PATH 1 - validateSpec + deterministic ok=true + llm ok=false spec.steps[0]: unknown key "timeoutMs" (expected one of id | type | dependsOn | verification + agent ok=false spec.steps[0]: unknown key "timeoutMs" (expected one of id | type | dependsOn | verification + +PATH 2 - compileYaml + toKernelSpec (does it reach the KERNEL step?) + deterministic ACCEPTED kernel step timeout_ms = 5000 + llm REFUSED spec.steps[0]: unknown key "timeoutMs" ... + agent REFUSED spec.steps[0]: unknown key "timeoutMs" ... + (none declared) ACCEPTED kernel step timeout_ms = undefined (absent = correct) + +PATH 3 - preflight + deterministic ok=true + llm ok=false Relayflow spec is invalid: spec.steps[0]: unknown key "timeoutMs" ... + agent ok=false Relayflow spec is invalid: spec.steps[0]: unknown key "timeoutMs" ... + +PATH 4 - flows check + deterministic exit=0 CHECK PASSED /tmp/pr139repair/to-deterministic.flow.yaml + llm exit=2 REFUSED [invalid_spec] ... unknown key "timeoutMs" ... + agent exit=2 REFUSED [invalid_spec] ... unknown key "timeoutMs" ... +``` + +Path 2 is again the load-bearing one, and this time it carries a row the other +three cannot produce at all: **`timeout_ms = 5000` in the kernel step**. Paths +1, 3 and 4 prove the allowlist half of trap 3 (the field is accepted on +`deterministic`, refused on `llm`/`agent`); only path 2 proves the spread half — +that the accepted field actually reaches the kernel object rather than being +silently dropped from `compileStep`'s deterministic branch. The +`(none declared)` row is the negative control: no `timeoutMs` in, no +`timeout_ms` out. + +## 11. Signoff positives, re-run against the rebased tree + +### Proxy / accessor rejection — `/tmp/pr139repair/proxy-attacks.mjs`, against built `sdk/dist/index.js` + +``` +34 TOCTOU flip-proxy as schema spec compile failed: ... schema: ex traps=0 +34 ... same, toKernelSpec spec compile failed: ... schema: ex traps=0 +35 revoked proxy as schema spec compile failed: ... schema: ex traps=n/a +36 proxy-wrapped ARRAY at schema.required spec compile failed: ... schema.req traps=0 +37 proxy nested THREE levels deep spec compile failed: ... schema.pro traps=0 +38 proxy as whole spec -> validateSpec spec: expected JSON-compatible data; Proxy objects are not all traps=0 +39 proxy as whole spec -> preflight Relayflow spec is invalid: spec: expected JSON-compatible data traps=0 +40 plain enumerable getter inside schema spec compile failed: ... schema.typ getterReads=0 +41 plain getter as step.verification spec compile failed: ... verification.type: expe getterReads=0 +46 raw proxy -> canonicalize value: expected JSON-compatible data; Proxy objects are not al traps=0 +46 raw proxy -> specHash spec: expected JSON-compatible data; Proxy objects are not all traps=0 +46 plain getter -> canonicalize value.k: expected JSON-compatible data; accessors are not allo getterReads=0 +47 unknown gate type {type:expression} spec compile failed: ... verification.type: expe n/a +48 gate that is a function spec compile failed: ... verification: expected n/a +44 null-prototype object as schema NOT REFUSED n/a +45 symbol-keyed property in schema spec compile failed: ... schema: ex n/a +43 class instance with toJSON as schema spec compile failed: ... schema: ex n/a +42 non-enumerable data property in schema spec compile failed: ... schema.hid n/a +19 empty output_contains value "" spec compile failed: ... verification.value: exp n/a +33 exit_code on an llm step spec compile failed: ... verification: exit_code n/a +``` + +Identical to the signoff, including #44 (null-prototype accepted — correct, it +is inert), and with #46 now refused at 0 traps / 0 getter reads where the +signoff measured 10 traps and 2 getter reads. #39 changed shape (a returned +refusal rather than a throw), by design; the trap count is still 0. + +### No network from a gate + +``` +$ grep -n 'jsonschema' kernel/Cargo.toml +13:jsonschema = { version = "0.33", default-features = false } +reqwest in Cargo.lock: 0 +tokio in Cargo.lock: 0 +hyper in Cargo.lock: 0 +``` + +### Kernel gate behaviour — `/tmp/pr139repair/kernel-attacks.mjs` + +``` +$ref http:// external 17ms REFUSED [invalid_spec] ... Resource 'http://exa daemon=alive +$ref file:///etc/passwd 1ms REFUSED [invalid_spec] ... Resource 'file:///et daemon=alive +$ref ../../../../etc/passwd 1ms REFUSED [invalid_spec] ... Resource '../../../. daemon=alive +unknown $schema draft 0ms REFUSED [invalid_spec] ... Unknown specificatio daemon=alive +ReDoS ^(a+)+$ vs 40 a + b 51ms run failed daemon=alive +exit 0 15ms run completed daemon=alive +exit 1 13ms run failed daemon=alive +exit 256 (POSIX wrap) 14ms run completed daemon=alive +killed by SIGKILL 14ms run failed daemon=alive +killed by SIGSEGV 14ms run failed daemon=alive +10MB stdout 349ms run completed daemon=alive +non-UTF8 stdout 13ms run completed daemon=alive +impossible schema required:[absent] 24ms run failed daemon=alive +needle only on stderr 14ms run failed daemon=alive +needle genuinely on stdout 13ms run completed daemon=alive +needle = JSON key name exit_code 13ms run failed daemon=alive +``` + +Every row matches the signoff. ReDoS is 51ms here vs the signoff's 283ms — the +bound adds no measurable cost and there is no backtracking blow-up. + +### Zero false positives over every shipped fixture + +``` +testdata/backlog-picker.flow.yaml exit=0 WARNING [unprovable_effects] ... +testdata/dir-watcher.flow.yaml exit=0 GATE step "describe-file" json_schema from data (kernel, journal-replayable) +testdata/hello-agent.flow.yaml exit=0 WARNING [unprovable_effects] ... +testdata/hello-deterministic.flow.yaml exit=0 WARNING [unprovable_effects] ... +testdata/hello-ladder.flow.yaml exit=0 WARNING [unprovable_effects] ... +testdata/hello-llm.flow.yaml exit=0 WARNING [unprovable_effects] ... +testdata/hn-monitor.flow.yaml exit=0 GATE step "analyze-story" json_schema from data (kernel, journal-replayable) +testdata/json-schema-invalid.flow.yaml exit=2 REFUSED [invalid_spec] ... spec.steps[0].verification.schema: invalid ... +workflows/bootstrap-gate1.yaml exit=2 REFUSED [invalid_spec] ... unknown key "swarm" ... +workflows/drive-cloud.yaml exit=2 REFUSED [invalid_spec] ... unknown key "swarm" ... +workflows/drive.yaml exit=2 REFUSED [invalid_spec] ... unknown key "swarm" ... +workflows/preswarm-check.yaml exit=0 WARNING [unprovable_effects] ... +workflows/review-swarm.yaml exit=2 REFUSED [invalid_spec] ... unknown key "swarm" ... +workflows/watchdog.yaml exit=2 REFUSED [invalid_spec] ... unknown key "swarm" ... +workflows/probes/cloud-credential-probe.yaml exit=2 REFUSED [invalid_spec] ... unknown key "swarm" ... +workflows/probes/cloud-repo-probe.yaml exit=2 REFUSED [invalid_spec] ... unknown key "swarm" ... +``` + +Every `testdata/*.flow.yaml` still passes; the single `exit=2` there is the +intentional negative fixture. The `workflows/*.yaml` refusals are pre-existing +(`unknown key "swarm"`, a root key this generation does not carry) and match +the signoff row for row. No new refusal, and no gate line acquired a +`[json_schema accepts any output]` marker — no shipped fixture declares a +vacuous gate. + +### `sdk/tests/spec-parity.test.ts` is still strengthened, not loosened + +``` +$ git diff origin/main -- sdk/tests/spec-parity.test.ts +- kernelToAuthoring, + toKernelSpec, + } from '../src/compile.js'; ++import { canonicalize, kernelToAuthoring, specHash } from '../src/index.js'; +… ++ it('refuses nested proxy data before executing any trap', () => { ++ expect(() => kernelToAuthoring(kernel)).toThrow(/proxy/i); ++ expect(proxyTraps).toBe(0); +… ++ it('refuses a proxy at canonicalize and specHash before executing any trap', … ++ it('refuses a getter at canonicalize and never reads it', … ++ it('canonicalizes inert data identically on every call', … +``` + +`kernelToAuthoring` still imported from `../src/index.js` (the published +export), `toThrow(/proxy/i)` and `proxyTraps === 0` intact, +4 tests, nothing +deleted, no assertion loosened, no `.skip`/`.only`. + +--- + +## 12. Gates — literal output + +**Five gates, not four.** #138 added `tsc -p tsconfig.type-tests.json` and +folded it into `npm run typecheck`; the directory it introduced +(`sdk/type-tests/step-fields.ts`) is the compile-time half of the per-verb field +lint. It is run below and it passes. All five are against the final head with a +clean tree, rebased onto `512723c`. + +``` +$ cd /sdk && ./node_modules/.bin/tsc --noEmit +G1=0 +$ ./node_modules/.bin/tsc -p tsconfig.type-tests.json # NEW, from #138 +G2=0 +$ ./node_modules/.bin/tsc -p tsconfig.tests.json +G3=0 +``` + +``` +$ RELAYFLOWD_BIN= ./node_modules/.bin/vitest run + Test Files 25 passed | 1 skipped (26) + Tests 457 passed | 3 skipped (460) +G4_VITEST=0 +``` + +``` +$ cd /kernel && PATH="$HOME/.cargo/bin:$PATH" RUSTUP_TOOLCHAIN=stable sh ../ops/cargo.sh test --workspace +G5_KERNEL=0 +per-binary: 22 0 22 1 1 3 3 38 5 17 0 0 0 = 112 tests across 13 binaries +failures lines: 0 +never-executed: 0 +``` + +### Per-file accounting against the actual base + +Measured at `512723c` itself — a worktree at that SHA, this worktree's +`sdk/node_modules` symlinked in (`npm` is unusable on this host), built with +`tsc && node scripts/make-cli-executable.mjs`: + +``` +$ cd /tmp/pr139repair/base2/sdk && RELAYFLOWD_BIN=... vitest run + Test Files 23 passed | 1 skipped (24) + Tests 388 passed | 3 skipped (391) +BASE_VITEST=0 + +$ cd /tmp/pr139repair/base2/kernel && ... cargo.sh test --workspace +BASE_KERNEL=0 +per-binary: 22 0 22 1 1 3 31 5 17 0 0 0 = 102 tests across 12 binaries +``` + +SDK, 388 -> 457 (+69), every one accounted for: + +| File | base | now | delta | why | +|---|---|---|---|---| +| `json-schema-bound.test.ts` | — | 43 | +43 | new: 12 refused directly + the same 12 through the `output` path, 14 accepted, marker, cycle naming, `$ref`-named property, deep schema, uncompilable `output` | +| `gate-contract.test.ts` | — | 20 | +20 | new in #139 (13) + 7 vacuous-gate tests added by this repair | +| `spec-parity.test.ts` | 15 | 19 | +4 | #139's nested-proxy test (+1); this repair's canonicalize/specHash proxy, getter and determinism tests (+3) | +| `preflight.test.ts` | 24 | 25 | +1 | #139's raw-input-before-any-probe test | +| `validate.test.ts` | 36 | 37 | +1 | #139's gate validation test | +| `dependency-validation.test.ts` | 6 | 6 | 0 | #138's suite; only the `gates: []` field added, no test added or removed | +| `verb-field-lint.test.ts` | 78 | 78 | 0 | #138's suite; same, two `gates: []` fields | +| all 19 others | — | — | 0 | unchanged | + +Kernel, 102 -> 112 (+10), 12 -> 13 binaries: + +| Where | base | now | delta | why | +|---|---|---|---|---| +| `relayflowd-core` lib | 31 | 38 | +7 | `schema.rs` 6 (shared declarations, refused corpus, accepted corpus, cycle naming, `$ref`-named property, deep schema) + `verify.rs` 1 (poisoned journal) | +| `invalid_schema_preflight` | — | 3 | +3 | new binary | +| all others | — | — | 0 | unchanged | + + +### Two gate runs failed for environmental reasons — both disclosed + +The first full `vitest run` after the rebase reported 10 `live-kernel.test.ts` +failures. They were **ENOSPC**: this host's root volume was at 97% with 428 MiB +free, and the harness could not create temp directories or write vitest's own +results cache. I freed space by deleting three orphaned +`~/.relayflows-toolchain/target/` build directories whose worktree paths no +longer exist (2.7 GB; every live worktree's key was computed and preserved — no +other agent's build cache was touched), and re-ran. The result above is that +re-run. Nothing in the tree changed between the two runs. + +The kernel gate also failed once, on its first run against `8fb8f01`, with +`KERNEL_EXIT=101` — but with **zero** `failures:` lines and no failing test: + +``` + Running tests/spec_parity.rs (…/debug/deps/spec_parity-2e346aa0d81e3bc8) +error: test failed, to rerun pass `-p relayflowd-core --test spec_parity` +Caused by: + could not execute process `…/debug/deps/spec_parity-2e346aa0d81e3bc8` (never executed) +Caused by: + No such file or directory (os error 2) +``` + +The test *binary* was missing from `CARGO_TARGET_DIR` (shared per worktree +under `~/.relayflows-toolchain/target//`) — a build-artifact problem, not a +test result. The immediate re-run with no tree change is the `KERNEL_EXIT=0` +above. + +**What I can characterise, and what I cannot.** The affected binary is named: +`relayflowd-core`'s `tests/spec_parity.rs`, and only that one — every other +binary in the same invocation ran and passed, so this was not a broken build. +Disk is ruled out: `df -h /` reported 30 GiB free during both the failing run +and the re-run, well clear of the ENOSPC episode above. I attempted one clean +reproduction — a full `cargo test --workspace` with no tree change — and it +came back `exit=0`, zero `failures:` lines, zero `never executed`. I did not +get a second or third attempt (they exceeded my time budget), and I did not try +to reproduce it under concurrent load, which is the condition I would test next: +several flows worktrees on this host build simultaneously against sibling target +directories, and a shared `~/.relayflows-toolchain` toolchain. + +**It did not recur across the second rebase.** Every gate run against +`512723c` — three typechecks, vitest, and two full `cargo test --workspace` +runs including the `512723c` baseline — came back clean, with `never-executed` +count 0. + +**So: unexplained, one failed reproduction attempt, no recurrence.** I am +deliberately not calling it flaky. A non-zero exit with zero failing tests reads as a real +failure in a CI summary and as success to a human skimming the test lines, and +until it is explained it should be treated as the former. + +The two `live-kernel` failures the brief predicted did **not** occur: +`statSync is not defined` never fired because `RELAYFLOWD_BIN` was passed +explicitly (the toolchain-scan path is never reached), and `unsupported_verb: +run.cancel` never fired because the kernel was built first. **No `statSync` +import was committed** and `live-kernel.test.ts` is unmodified. + +--- + +## 12b. Signoff 4 — the URI reference form, and why the corpus missed it + +Signoff 4 (`REVIEW_FAILED` at `0ea58b57`) found the P0 still open through a +route the bound could not see. It was right, the finding is exact, and it is +closed here. + +### What was wrong + +Both resolvers keyed on a leading `#`: + +```rust +// kernel/relayflowd-core/src/schema.rs, before +fn resolve(base: &str, reference: &str, anchors: &HashMap) -> Option { + if reference == "#" { return Some(base.to_owned()); } + if let Some(rest) = reference.strip_prefix("#/") { return Some(format!("{base}/{}", percent_decode(rest))); } + if let Some(name) = reference.strip_prefix('#') { return anchors.get(&percent_decode(name)).cloned(); } + None +} +``` + +So a `$ref` written as a **URI naming an `$id` declared inside the same +document** — the 2020-12 *compound schema document* form, spec §9.3, what every +bundler emits — produced no edge, no cycle was found, and the schema was +accepted. `jsonschema` then resolved it from the document's own resource map +and overflowed. + +The load-bearing error was the comment justifying it: + +```rust +// An unresolvable reference (external URI, absolute URI, unknown +// anchor) is left opaque here: `jsonschema::validator_for` below +// refuses it outright, so nothing unresolved reaches validation. +``` + +That premise is true for **remote** resources and false for an **in-document +`$id`**. I verified the true half and generalised it to the false half, and +§2's "the bound under-approximates only where refusal is already guaranteed" +repeated the same over-claim. The comment is why the gap looked deliberate. + +### RED — the reviewer's exact schema, my script, the pushed head + +`/tmp/pr139repair/uri-ladder.mjs` against a `relayflowd` built from `0ea58b5` +in a scratch worktree (`CARGO_TARGET_DIR` key `2645464646`, nobody else's): + +``` +binary : .../target/2645464646/debug/relayflowd +SCHEMA : {"$id":"https://ex.test/root","$defs":{"a":{"$id":"https://ex.test/a","$ref":"https://ex.test/b"},"b":{"$id":"https://ex.test/b","$ref":"https://ex.test/a"}},"$ref":"https://ex.test/a"} +before run.start: runs = [] execs = 0 +run.start -> CONN CLOSED, NO REPLY + daemon alive=false stderr: thread '' (6954064) has overflowed its stack fatal runtime error: stack overflow, aborting + runs = ["01M1KN8159B1P2QGHVXH408791.sqlite3"] + *** command executions so far = 1 *** +resume #1: run.get -> {... "status":"running" ...} resume -> CONN CLOSED, NO REPLY *** cumulative executions = 2 *** +resume #2: run.get -> {... "status":"running" ...} resume -> CONN CLOSED, NO REPLY *** cumulative executions = 3 *** +resume #3: run.get -> {... "status":"running" ...} resume -> CONN CLOSED, NO REPLY *** cumulative executions = 4 *** + +FINAL: the command executed 4 time(s) for one logical step. +``` + +Reproduced independently and verbatim, including the run stuck `running` and +each resume re-executing the effect. + +### GREEN — same script, same schema, fixed binary + +``` +before run.start: runs = [] execs = 0 +run.start -> {"id":"start","ok":false,"error":{"code":"invalid_spec","message":"invalid run spec: step s declares an invalid JSON Schema: unbounded $ref cycle: #/$defs/a -> #/$defs/b -> #/$defs/a — this cycle re-applies to the same instance, so validation would not terminate"}} + daemon alive=true stderr: + runs = [] + *** command executions so far = 0 *** +resume #1: resume -> {"ok":false,"error":{"code":"run_not_found",...}} *** cumulative executions = 0 *** +resume #2: resume -> {"ok":false,"error":{"code":"run_not_found",...}} *** cumulative executions = 0 *** +resume #3: resume -> {"ok":false,"error":{"code":"run_not_found",...}} *** cumulative executions = 0 *** + +FINAL: the command executed 0 time(s) for one logical step. +``` + +4 → 0. Named refusal, no journal, no command, daemon alive across three resumes. + +### The fix + +Reference resolution is now URI-aware on both sides, mirrored function for +function (`schema.rs::resolve_uri`/`resolve`/`collect_scopes`, +`json-schema-bound.ts::resolveUri`/`resolve`/`collectScopes`): + +- **`collect_scopes` builds a resource map.** Every `$id` in the document is + resolved against its enclosing base (RFC 3986 §5.3) and registered + base-URI → pointer, *and* registered a second time under its raw `$id` + string. The raw registration is deliberate: exact RFC compliance matters less + than **consistency between registration and lookup**, and a bundled document + writes the same literal in `$id` and `$ref`, so the literal key matches + whatever either side does with normalization. +- **Anchors are scoped per resource**, keyed `(base URI, name)` rather than + document-wide first-match-wins. This is what closes `duplicate anchor name + under two $id` — and it retires one of the "suspected, not demonstrated" + items I listed in §13, because it is now demonstrated and fixed. +- **`resolve` splits `#`**, resolves the URI part against the + base in effect at that node, looks the result up in the resource map, and + only then applies the fragment as a pointer or an anchor within *that* + resource. +- **`base_at`** records the base URI at every pointer during the same walk, so + the old `nearest_id_base` re-walk per reference is gone. +- The checker is still iterative — the new resolver adds no recursion, and + `deeply_nested_schemas_do_not_overflow_the_checker` still passes. + +### The seven witnesses, red to green + +`/tmp/pr139repair/uri-probe.mjs`, each schema submitted as a real `json_schema` +gate through `run.start`, step command appending to a tally file: + +``` + RED (0ea58b5) GREEN (fixed) +absolute-URI $ref to internal $id CRASH SIGABRT alive=false execs=1 -> REFUSED [invalid_spec] unbounded $ref cycle alive=true execs=0 runs=false +relative-URI $ref to internal $id CRASH SIGABRT alive=false execs=1 -> REFUSED [invalid_spec] unbounded $ref cycle alive=true execs=0 runs=false +urn $id self cycle ACCEPTED completed execs=1 -> REFUSED [invalid_spec] unbounded $ref cycle alive=true execs=0 runs=false +absolute-URI-with-pointer-fragment ACCEPTED completed execs=1 -> REFUSED [invalid_spec] unbounded $ref cycle alive=true execs=0 runs=false +absolute-URI-with-anchor-fragment ACCEPTED completed execs=1 -> REFUSED [invalid_spec] unbounded $ref cycle alive=true execs=0 runs=false +duplicate anchor name under two $id CRASH SIGABRT alive=false execs=1 -> REFUSED [invalid_spec] unbounded $ref cycle alive=true execs=0 runs=false +$ref into $defs via nested $id base CRASH SIGABRT alive=false execs=1 -> REFUSED [invalid_spec] unbounded $ref cycle alive=true execs=0 runs=false + +ATTACKS STILL LIVE: 0 +``` + +**Honest note on my reconstructions.** Signoff 4 reports all seven as +`CRASH SIGABRT`; my rebuilt versions crash four and *latently* accept three — +the same latency I documented in §1a, where a schema survives only because the +probe's output never reaches the cyclic position. The three are unbounded +either way and are refused now; I did not have the reviewer's literal schema +text for those three, so they are reconstructions from the case names, and I am +saying so rather than presenting them as byte-identical. + +### P0-2 and P1 — `flows check`, both directions + +``` +absolute-URI $ref to internal $id exit=2 REFUSED [invalid_spec] ... invalid JSON Schema: unbounded $ref cycle ... +relative-URI $ref to internal $id exit=2 REFUSED ... +urn $id self cycle exit=2 REFUSED ... +absolute-URI-with-pointer-fragment exit=2 REFUSED ... +absolute-URI-with-anchor-fragment exit=2 REFUSED ... +duplicate anchor name under two $id exit=2 REFUSED ... +$ref into $defs via nested $id base exit=2 REFUSED ... +$id-scoped NON-cycle (P1) exit=0 CHECK PASSED +bundled compound document, terminating exit=0 CHECK PASSED +absolute-URI ref through a child applicator exit=0 CHECK PASSED +external http $ref exit=2 REFUSED (engine: "can't resolve reference ...") +two distinct $id resources, no cycle exit=0 CHECK PASSED +``` + +P0-2 closed: no spec that kills the kernel passes `flows check`. + +**P1 closed, and closed by the rule rather than by an engine.** The reviewer's +schema — legal by the rule, accepted and completed by the kernel — was refused +by `flows check` with `invalid JSON Schema: Maximum call stack size exceeded`, +a raw Ajv message leaked as a named refusal. The fix is one line with a long +comment in `sdk/src/json-schema.ts`: a `RangeError` out of Ajv's *compile* is +caught and discarded. + +The reasoning matters more than the line. **The rule decides legality; the +engine decides only well-formedness; a stack overflow is neither verdict.** By +the time Ajv runs, the bound has already proved this declaration terminates, +and the kernel — the engine that actually validates outputs — accepts it. Letting +Ajv's stack decide would put legality back in the hands of an engine's +accidental overflow behaviour, which is the exact defect §5 exists to remove. +This is narrow by construction: a schema the bound refuses never reaches Ajv, +so nothing that was refused becomes accepted. + +Kernel/SDK now agree across the whole family (`/tmp/pr139repair/sdk-uri-check.mjs`): + +``` +attack (all 7) bound=REFUSE(unbounded $ref cycle ...) jsonSchemaError=REFUSE(unbounded $ref cycle ...) +legit $id-scoped NON-cycle (P1) bound=ACCEPT jsonSchemaError=ACCEPT +legit bundled compound document bound=ACCEPT jsonSchemaError=ACCEPT +legit child-applicator URI ref bound=ACCEPT jsonSchemaError=ACCEPT +legit two distinct $id resources bound=ACCEPT jsonSchemaError=ACCEPT +legit external http $ref bound=ACCEPT jsonSchemaError=REFUSE(can't resolve reference ...) +``` + +### The corpus, extended by derivation — and how to check the enumeration + +Signoff 4's diagnosis of *why* five green gates missed a live P0 is the part +worth keeping: the wiring was right and the corpus was the hole. A shared +corpus makes both sides agree; it cannot make them complete. + +So the new cases are enumerated from the **specification's reference forms**, +not from the file. The axis is *how a reference can name its target*: + +| | Form | refused case | accepted case | +|---|---|---|---| +| F1 | same-resource root `#` | already present | already present | +| F2 | same-resource pointer `#/$defs/a` | already present | already present | +| F3 | same-resource anchor `#loop` | already present | already present | +| F4 | absolute URI naming an in-document `$id` | **new** | **new** ×3 | +| F5 | absolute URI + pointer fragment | **new** | **new** | +| F6 | absolute URI + anchor fragment | **new** | **new** | +| F7 | relative URI reference | **new** | **new** | +| F8 | rooted relative `/name` | **new** | — | +| F9 | opaque absolute URI (`urn:`) | **new** | **new** | +| F10 | resolved against a nested `$id` base | **new** | — | +| F11 | same anchor name under two `$id` scopes | **new** | **new** | +| F12 | external URI, no in-document `$id` | — (not the bound's) | **new bucket** | + +Every form gets a refused instance — an in-place cycle written in that form — +and, where the form can also express a terminating schema, an accepted instance +that is the same reference form descending through a child applicator. **The +enumeration is checkable**: it is the reference-resolution surface of the +2020-12 core vocabulary, so the question "is this list complete?" reduces to +"is there a way to write a reference that is not F1–F12?" rather than to +reviewing the list itself. That is the property signoff 4 asked for, and it is +what the old corpus lacked: F1–F3 were the only forms present, and every crash +lived in F4–F11. + +Corpus: **12 → 20 refused, 14 → 22 accepted**, plus a new **`engineRefused`** +bucket of 4. + +### F12 gets its own bucket, because it pins the premise that was wrong + +`engineRefused` is the narrowed claim turned into a test. For each case both +sides assert **the bound does NOT claim it** and **the engine DOES refuse it**: + +```rust +// kernel: references_the_bound_leaves_opaque_are_refused_by_the_engine +assert!(bound_declaration(schema).is_ok(), "the bound must not claim a reference it cannot resolve"); +let error = jsonschema::validator_for(schema).expect_err("the engine must refuse this reference"); +assert!(!error.contains(UNBOUNDED_REF_CYCLE), "this must be the engine's refusal, not the bound's"); +assert!(compile(schema).is_err(), "the gate as a whole must refuse it"); +``` + +```ts +// sdk: 'leaves %s to the engine, which refuses it' +expect(jsonSchemaBoundError(entry.schema)).toBeUndefined(); +expect(fromEngine).toBeDefined(); +expect(fromEngine).not.toContain(UNBOUNDED_REF_CYCLE); +``` + +The old comment asserted this and was wrong. Now, if a future `jsonschema` or +Ajv starts *accepting* an unresolvable reference, these fail and the opaque +default has to be revisited rather than silently becoming the same hole again. + +The remaining opaque case is a meta-schema bundled with the engine. It cannot +reference back into the user's document, so it cannot close a cycle rooted +there — which is the narrow claim the comments now make, in place of the wide +one that was false. + +### Direct resolver unit test + +`in_document_uri_references_resolve_to_the_node_they_name` asserts the resolver +against a fixture with a nested `$id` and an `$anchor`, one row per form: + +``` +("", "https://ex.test/a", Some("/$defs/a")) +("https://ex.test/root", "a", Some("/$defs/a")) +("https://ex.test/root", "/a", Some("/$defs/a")) +("https://ex.test/root", "https://ex.test/a#nm", Some("/$defs/a")) +("https://ex.test/a", "#nm", Some("/$defs/a")) +("https://ex.test/a", "#", Some("/$defs/a")) +("", "https://ex.test/root", Some("")) +("", "https://elsewhere.test/a", None) +("https://ex.test/root", "#unknown-anchor", None) +``` + +Both `None` rows matter as much as the `Some` rows: the resolver must not +invent an edge for a reference that names nothing here. + +### No regressions + +- All 20 refused corpus schemas still **compile cleanly** in `jsonschema` + before the bound refuses them, so the "bound, not mechanism" property holds + for the new cases too — `every_refused_corpus_schema_compiles_but_is_refused_by_the_bound` + now asserts `cases.len() >= 20`. +- Original crash families re-run on the final build: `# -> #`, `allOf`, + `anyOf`, `not`, anchor-mutual all still `REFUSED`, `cmd_ran=` empty. +- Legitimate recursion re-run: `items:{$ref:"#"}`, recursive tree, `$defs` + tree, mutual-with-bodies, terminating `anyOf`, plain object, vacuous `{}`, + boolean `true` — all still `ACCEPTED`. +- External `http://` still refused by the engine, message unchanged + (`Resource 'http://example.com/s.json' is not present`). +- Shipped-fixture sweep unchanged: every `testdata/*.flow.yaml` still exits 0 + except the intentional negative fixture. + +### Gates after the fix + +``` +tsc --noEmit G1=0 +tsc -p tsconfig.type-tests.json G2=0 +tsc -p tsconfig.tests.json G3=0 +vitest run Test Files 25 passed | 1 skipped (26) + Tests 485 passed | 3 skipped (488) G4=0 +cargo test --workspace 22 0 22 1 1 3 3 40 5 17 0 0 0 = 114 tests, 13 binaries + failures: 0 never-executed: 0 G5=0 +``` + +SDK 457 → 485 (+28): 8 new refused cases × 2 paths (direct and `output` sugar) += 16, 8 new accepted = 8, 4 `engineRefused` = 4. Kernel 112 → 114 (+2): the +`engineRefused` test and the resolver unit test. No test removed, no assertion +weakened, no `.skip`/`.only`. + +The exit-101 anomaly did not recur across any run in this pass either. + +## 13. What I did NOT verify + +- **`3da71e2` as the base.** §0. The branch is on `512723c`, main's head as of + this rebase, pinned by SHA and confirmed by the lead. +- **Mutation verification of the new tests** in the AGENTS.md sense — I did not + revert each hunk individually, capture the failure, restore byte-for-byte and + re-capture. What I did instead is structural and stated as such: every refused + corpus case is asserted to *compile cleanly*, so the tests provably cannot + pass without the bound. Nothing here is labelled "mutation-verified". +- **`main` aborting identically.** I did not build a `main` kernel to + demonstrate it. I built and ran `e6210a2`, which is what §1 shows. +- **A 10 MB schema**, and schema-size limits generally. +- **A gate that exceeds `timeoutMs`.** Every gate evaluated in under 400 ms. +- **`output_contains` on `llm`/`agent` steps** — the `output.to_string()` + fallback at `verify.rs:23-27`. Pre-existing, unchanged, still only suspected. +- **Concurrency** — two clients racing `run.start`, or a poisoned run + interleaving with other in-flight runs' journal writes. +- **The `internal` error code** on a legacy poisoned journal (§4). Correct + behaviour, imprecise taxonomy; I did not widen the protocol to fix it. +- ~~**`$id` scoping of plain-name anchors.**~~ **Now demonstrated and fixed.** + I wrote that a document reusing one anchor name under two `$id`s "could + resolve to the wrong node", that I had not constructed one, and that the + failure mode "would be a false refusal, not a crash". Signoff 4 constructed + it and it was a **crash**, not a false refusal. Anchors are now keyed + `(base URI, name)` and the case is in the corpus as F11. Recording the miss: + I labelled it suspected-not-demonstrated, which was honest, but I also + guessed at its severity in the safe direction and the guess was wrong. A + failure mode I have not constructed does not have a known blast radius. +- **CI jobs** `linux-x64-artifact` and `packed-consumer` — not run locally, and + neither exercises either suite anyway. +- **`timeoutMs` at runtime.** The four-path proof (§10) shows the declaration + reaching the kernel step as `timeout_ms: 5000`; I did not run a deterministic + step that actually exceeds its timeout and observe the kernel enforce it. + That is #138's behaviour, not this PR's, and #138 carries its own signoff. +- **Whether `origin/main` moves a third time.** It was `512723c` at the moment + of this push; #137 and #134 rebase onto whatever follows. diff --git a/sdk/package-lock.json b/sdk/package-lock.json index 44ace6f11..45bb9247e 100644 --- a/sdk/package-lock.json +++ b/sdk/package-lock.json @@ -10,6 +10,8 @@ "license": "UNLICENSED", "dependencies": { "@types/js-yaml": "^4.0.9", + "ajv": "^8.17.1", + "ajv-draft-04": "^1.0.0", "js-yaml": "^5.4.1", "yaml": "^2.5.1" }, @@ -810,6 +812,36 @@ "undici-types": "~6.21.0" } }, + "node_modules/ajv": { + "version": "8.17.1", + "resolved": "https://registry.npmjs.org/ajv/-/ajv-8.17.1.tgz", + "integrity": "sha512-B/gBuNg5SiMTrPkC+A2+cW0RszwxYmn6VYxB/inlBStS5nx6xHIt/ehKRhIMhqusl7a8LjQoZnjCs5vhwxOQ1g==", + "license": "MIT", + "dependencies": { + "fast-deep-equal": "^3.1.3", + "fast-uri": "^3.0.1", + "json-schema-traverse": "^1.0.0", + "require-from-string": "^2.0.2" + }, + "funding": { + "type": "github", + "url": "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/sponsors/epoberezkin" + } + }, + "node_modules/ajv-draft-04": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/ajv-draft-04/-/ajv-draft-04-1.0.0.tgz", + "integrity": "sha512-mv00Te6nmYbRp5DCwclxtt7yV/joXJPGS7nM+97GdxvuttCOfgI3K4U25zboyeX0O+myI8ERluxQe5wljMmVIw==", + "license": "MIT", + "peerDependencies": { + "ajv": "^8.5.0" + }, + "peerDependenciesMeta": { + "ajv": { + "optional": true + } + } + }, "node_modules/@vitest/expect": { "version": "2.1.9", "resolved": "https://registry.npmjs.org/@vitest/expect/-/expect-2.1.9.tgz", @@ -1004,6 +1036,28 @@ "node": ">=6" } }, + "node_modules/fast-deep-equal": { + "version": "3.1.3", + "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz", + "integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==", + "license": "MIT" + }, + "node_modules/fast-uri": { + "version": "3.1.7", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.7.tgz", + "integrity": "sha512-dOvZVzjdZdz7phd9v6jCbwxrBW3fK6n8Rc0CtdmM4bumzMnxywBYhuph6J819RRw/ku+rLbelwfMunktuzVVHg==", + "license": "BSD-3-Clause", + "funding": [ + { + "type": "github", + "url": "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/sponsors/fastify" + }, + { + "type": "opencollective", + "url": "https://opencollective.com/fastify" + } + ] + }, "node_modules/es-module-lexer": { "version": "1.7.0", "resolved": "https://registry.npmjs.org/es-module-lexer/-/es-module-lexer-1.7.0.tgz", @@ -1107,6 +1161,12 @@ "js-yaml": "bin/js-yaml.mjs" } }, + "node_modules/json-schema-traverse": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-1.0.0.tgz", + "integrity": "sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==", + "license": "MIT" + }, "node_modules/loupe": { "version": "3.2.1", "resolved": "https://registry.npmjs.org/loupe/-/loupe-3.2.1.tgz", @@ -1494,6 +1554,15 @@ } } }, + "node_modules/require-from-string": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/require-from-string/-/require-from-string-2.0.2.tgz", + "integrity": "sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==", + "license": "MIT", + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/why-is-node-running": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/why-is-node-running/-/why-is-node-running-2.3.0.tgz", diff --git a/sdk/package.json b/sdk/package.json index d905764ce..d9eba9a69 100644 --- a/sdk/package.json +++ b/sdk/package.json @@ -32,6 +32,8 @@ "private": true, "dependencies": { "@types/js-yaml": "^4.0.9", + "ajv": "^8.17.1", + "ajv-draft-04": "^1.0.0", "js-yaml": "^5.4.1", "yaml": "^2.5.1" }, diff --git a/sdk/src/canonical.ts b/sdk/src/canonical.ts index 8a7fbe37d..e78e70e89 100644 --- a/sdk/src/canonical.ts +++ b/sdk/src/canonical.ts @@ -8,29 +8,21 @@ // assumed. import { createHash } from 'node:crypto'; +import { snapshotJsonValue, type JsonValue } from './json-value.js'; /** * Serialize a value as canonical JSON: object keys sorted recursively, * arrays in order, no whitespace. Numbers are kept as-is (tokens are * integers; money is a decimal string, never a float — so no float drift). + * + * This is an exported boundary, so runtime input is snapshotted into frozen, + * behavior-free data before a single byte is serialized — the same guard + * `validateSpec` and `kernelToAuthoring` use. Without it a Proxy or a plain + * getter could return one value while the identity is computed and another + * afterwards, and `specHash` would stamp a spec nobody ever declared. */ export function canonicalize(value: unknown): string { - if (value === null || typeof value !== 'object') { - return JSON.stringify(value); - } - if (Array.isArray(value)) { - return '[' + (value as unknown[]).map(canonicalize).join(',') + ']'; - } - const obj = value as Record; - const keys = Object.keys(obj).sort(); - return ( - '{' + - keys - .filter((k) => obj[k] !== undefined) - .map((k) => JSON.stringify(k) + ':' + canonicalize(obj[k])) - .join(',') + - '}' - ); + return serialize(snapshotJsonValue(value, 'value')); } /** @@ -38,5 +30,28 @@ export function canonicalize(value: unknown): string { * (`toKernelSpec`) to get the identity the kernel stamps as `spec_hash`. */ export function specHash(spec: unknown): string { - return createHash('sha256').update(canonicalize(spec)).digest('hex'); + return createHash('sha256').update(serialize(snapshotJsonValue(spec, 'spec'))).digest('hex'); +} + +/** + * Serialize an already-snapshotted value. Each key is read EXACTLY ONCE: the + * previous `keys.filter(k => obj[k] !== undefined).map(k => … obj[k])` read + * every property twice, which on a live getter is a TOCTOU — the filter and + * the serialize could disagree, and two `canonicalize` calls on one object + * could return different strings. + */ +function serialize(value: JsonValue): string { + if (value === null || typeof value !== 'object') { + return JSON.stringify(value); + } + if (Array.isArray(value)) { + return '[' + value.map(serialize).join(',') + ']'; + } + const parts: string[] = []; + for (const key of Object.keys(value).sort()) { + const child = value[key]; + if (child === undefined) continue; + parts.push(JSON.stringify(key) + ':' + serialize(child)); + } + return '{' + parts.join(',') + '}'; } diff --git a/sdk/src/cli.ts b/sdk/src/cli.ts index 6c393ea14..77f89d165 100644 --- a/sdk/src/cli.ts +++ b/sdk/src/cli.ts @@ -168,6 +168,12 @@ function emitCheckReport(report: CheckReport, json: boolean, io: CliIo): void { io.stdout(JSON.stringify(report)); return; } + for (const gate of report.gates) { + // A gate that accepts every output is legal, but it must not read like a + // gate that judges something. + const vacuous = gate.acceptsAnyOutput === true ? ' [json_schema accepts any output]' : ''; + io.stdout(`GATE step "${gate.stepId}" ${gate.checks.join('+')} from data (kernel, journal-replayable)${vacuous}`); + } for (const resolution of report.resolutions) { const config = resolution.source === 'project' && report.projectConfigPath !== undefined ? ` (${report.projectConfigPath})` diff --git a/sdk/src/cli/check.ts b/sdk/src/cli/check.ts index 8c644929b..27a0826d0 100644 --- a/sdk/src/cli/check.ts +++ b/sdk/src/cli/check.ts @@ -14,6 +14,7 @@ import { import { MODEL_ENV } from '../worker-cli.js'; import { modelNameError } from '../model-name.js'; import type { FlowSpec } from '../spec.js'; +import type { StepGateInspection } from '../gate-contract.js'; import type { CheckFailureKind } from '../failure-kinds.js'; import { preflight, @@ -36,6 +37,7 @@ export interface CheckReport { ok: boolean; path?: string; projectConfigPath?: string; + gates: StepGateInspection[]; resolutions: CliResolution[]; diagnostics: Array; } @@ -85,6 +87,7 @@ export function checkFlow(path: string): CheckExecution { ok: result.ok, path, ...(config.path !== undefined ? { projectConfigPath: config.path } : {}), + gates: result.gates, resolutions: result.resolutions, diagnostics: result.diagnostics, }, @@ -105,6 +108,7 @@ export function inputFailureReport( return { ok: false, ...(path !== undefined ? { path } : {}), + gates: [], resolutions: [], diagnostics: [{ severity: 'refusal', kind: failure.kind, message: failure.message }], }; diff --git a/sdk/src/compile.ts b/sdk/src/compile.ts index 5ed22b684..4534b87d5 100644 --- a/sdk/src/compile.ts +++ b/sdk/src/compile.ts @@ -27,14 +27,17 @@ import type { KernelVerificationSpec, LlmStepSpec, NamedAgentSpec, + OutputVerificationSpec, StepSpec, StepType, TriggerSpec, + VerificationSpec, } from './spec.js'; import { SPEC_SCHEMA_VERSION } from './spec.js'; import { canonicalize, specHash } from './canonical.js'; import { validateOutputDeclaration } from './output-schema.js'; import { validateSpec, type ValidationResult } from './validate.js'; +import { snapshotJsonValue } from './json-value.js'; export class CompileError extends Error { readonly errors: string[]; @@ -67,10 +70,18 @@ export function compileYamlToCanonicalJson(yaml: string): string { * normalized `FlowSpec`. Throws `CompileError` on validation failure. */ export function compileSpec(spec: unknown): FlowSpec { - const validation: ValidationResult = validateSpec(spec); + let snapshot: unknown; + try { + snapshot = snapshotJsonValue(spec, 'spec'); + } catch (error) { + throw new CompileError([ + error instanceof Error ? error.message : 'spec: expected JSON-compatible data', + ]); + } + const validation: ValidationResult = validateSpec(snapshot); if (!validation.ok) throw new CompileError(validation.errors); - const input = spec as FlowSpec; + const input = snapshot as FlowSpec; // Preserve named declarations and selectors through authoring normalization. // They are resolved exactly once at the kernel boundary, after public // preflight has validated every declaration with truthful provenance. @@ -97,20 +108,26 @@ function compileStep(step: StepSpec): StepSpec { id: step.id, type: step.type, ...(step.dependsOn !== undefined ? { dependsOn: step.dependsOn } : {}), - ...(verification !== undefined ? { verification } : {}), maxIterations, }; switch (step.type as StepType) { case 'deterministic': { const s = step as DeterministicStepSpec; - // A deterministic step with no verification gets the implicit exit_code gate. - const verification = s.verification ?? { type: 'exit_code' as const }; + // A deterministic step with no verification gets the implicit exit_code + // gate. Read the SHARED `verification`, never `s.verification`: for this + // verb the two are equal today (typedOutputVerification returns + // `step.verification` unchanged, because `output` is not authorable on a + // deterministic step), but reading the shared binding is what keeps every + // branch of this switch on the lowered gate rather than the raw authored + // one. See ops/reviews/20260903-pr139-repair-0903.md §10, trap 2. return { ...base, type: 'deterministic', command: s.command, - verification, + verification: verification ?? { type: 'exit_code' as const }, + // #138: `timeoutMs` is deterministic-only — worker-backed verbs own + // their dispatch timeout. It must be spread HERE and nowhere in `base`. ...(s.timeoutMs !== undefined ? { timeoutMs: s.timeoutMs } : {}), }; } @@ -120,6 +137,10 @@ function compileStep(step: StepSpec): StepSpec { ...base, type: 'llm', prompt: s.prompt, + // From the shared `verification` — which may have been lowered from an + // `output` declaration — never from `s.verification`, which would drop + // that lowering. + ...(verification !== undefined ? { verification: outputGate(verification, s.id) } : {}), ...(s.model !== undefined ? { model: s.model } : {}), ...(s.cli !== undefined ? { cli: s.cli } : {}), }; @@ -131,6 +152,7 @@ function compileStep(step: StepSpec): StepSpec { ...base, type: 'agent', instruction: s.instruction, + ...(verification !== undefined ? { verification: outputGate(verification, s.id) } : {}), ...(s.agent !== undefined ? { agent: s.agent } : {}), ...(s.cli !== undefined ? { cli: s.cli } : {}), ...(s.model !== undefined ? { model: s.model } : {}), @@ -145,6 +167,20 @@ function compileStep(step: StepSpec): StepSpec { } } +/** + * Narrow a gate to the ones an `llm`/`agent` step may carry. `validateSpec` + * already refuses `exit_code` off a deterministic step, so this is the + * fail-closed backstop for a runtime value cast past the authoring types. + */ +function outputGate(gate: VerificationSpec, stepId: string): OutputVerificationSpec { + if (gate.type === 'exit_code') { + throw new CompileError([ + `step "${stepId}": exit_code is supported only on deterministic steps`, + ]); + } + return gate; +} + function typedOutputVerification(step: StepSpec): StepSpec['verification'] { if (step.type !== 'deterministic' && step.output !== undefined) { const errors = validateOutputDeclaration(step, `step "${step.id}"`); @@ -195,21 +231,32 @@ const KERNEL_RETRY_DEFAULTS = { * sugar that the dialect cannot carry is a `CompileError`, never a silent drop. */ export function toKernelSpec(flow: FlowSpec): KernelRunSpec { - const validation = validateSpec(flow); - if (!validation.ok) throw new CompileError(validation.errors); + // This public boundary is callable without compileSpec. Compile again so + // runtime casts are validated and all returned schema data is snapshotted. + const compiled = compileSpec(flow); + return { - version: flow.version, - ...(flow.name !== undefined ? { name: flow.name } : {}), - ...(flow.description !== undefined ? { description: flow.description } : {}), - ...(flow.cli !== undefined ? { cli: flow.cli } : {}), - ...(flow.triggers?.length ? { triggers: flow.triggers.map(toKernelTrigger) } : {}), - steps: flow.steps.map((step) => toKernelStep(resolveNamedAgent(step, flow.agents))), - ...(flow.budget !== undefined + version: compiled.version, + ...(compiled.name !== undefined ? { name: compiled.name } : {}), + ...(compiled.description !== undefined ? { description: compiled.description } : {}), + ...(compiled.cli !== undefined ? { cli: compiled.cli } : {}), + // Both halves are load-bearing, and this hunk is trap 2's shape a second + // time. `compiled.*` is #139's snapshot guard: this boundary is callable + // without compileSpec, so it recompiles and reads the validated, snapshotted + // spec rather than the caller's raw object. `.map(toKernelTrigger)` is + // #151's LOWERING of authoring trigger keys into the kernel dialect -- + // authoring sugar that becomes a different object at the boundary, exactly + // like `output:`. Taking either side of this conflict wholesale silently + // reverts the other, and `validateSpec` and `flows check` would both still + // look correct. See ops/reviews/20260903-pr139-repair-0903.md section 10. + ...(compiled.triggers?.length ? { triggers: compiled.triggers.map(toKernelTrigger) } : {}), + steps: compiled.steps.map((step) => toKernelStep(resolveNamedAgent(step, compiled.agents))), + ...(compiled.budget !== undefined ? { budget: { - ...(flow.budget.maxTokensIn !== undefined ? { max_tokens_in: flow.budget.maxTokensIn } : {}), - ...(flow.budget.maxTokensOut !== undefined ? { max_tokens_out: flow.budget.maxTokensOut } : {}), - ...(flow.budget.maxDollars !== undefined ? { max_dollars: flow.budget.maxDollars } : {}), + ...(compiled.budget.maxTokensIn !== undefined ? { max_tokens_in: compiled.budget.maxTokensIn } : {}), + ...(compiled.budget.maxTokensOut !== undefined ? { max_tokens_out: compiled.budget.maxTokensOut } : {}), + ...(compiled.budget.maxDollars !== undefined ? { max_dollars: compiled.budget.maxDollars } : {}), }, } : {}), @@ -222,8 +269,16 @@ export function toKernelSpec(flow: FlowSpec): KernelRunSpec { * Kernel-only values with no authoring representation are refused. */ export function kernelToAuthoring(value: unknown): unknown { + let snapshot: unknown; + try { + snapshot = snapshotJsonValue(value, 'spec'); + } catch (error) { + throw new CompileError([ + error instanceof Error ? error.message : 'spec: expected JSON-compatible data', + ]); + } const root = requireKernelObject( - value, + snapshot, ['version', 'name', 'description', 'cli', 'triggers', 'steps', 'budget'], 'spec', ); @@ -495,11 +550,14 @@ function toKernelVerification(step: StepSpec): KernelVerificationSpec { return { json_schema: output }; } const gate = step.verification; - // No gate / explicit exit_code both compile to {}: exit_code == 0 is the - // kernel's implicit gate for deterministic steps (kernel DESIGN.md §4). + // Validation permits explicit exit_code only on deterministic steps, where + // {} selects the kernel's implicit exit_code == 0 gate (DESIGN.md §4). if (gate === undefined || gate.type === 'exit_code') return {}; if (gate.type === 'output_contains') return { output_contains: gate.value }; - return { json_schema: gate.schema }; + if (gate.type === 'json_schema') return { json_schema: gate.schema }; + throw new CompileError([ + `step "${step.id}".verification.type: expected exit_code | output_contains | json_schema`, + ]); } /** diff --git a/sdk/src/failure-kinds.ts b/sdk/src/failure-kinds.ts index ef36508e1..c60f63789 100644 --- a/sdk/src/failure-kinds.ts +++ b/sdk/src/failure-kinds.ts @@ -37,11 +37,16 @@ export const CHECK_FAILURE_KINDS = [ * anything it cannot prove, so a deterministic step always leaves exactly one * of these: its command resolved (effects still unknowable), it did not * resolve, or it could not be probed at all. Silence is not one of the states. + * + * `vacuous_gate` is the same principle applied to a declared gate that judges + * nothing: `schema: {}` and `schema: true` are legal and accepted, but a gate + * accepting every output must not be reported as if it constrained one. */ export const PREFLIGHT_WARNING_KINDS = [ 'unprovable_effects', 'command_unresolved', 'command_unprovable', + 'vacuous_gate', ] as const; /** Closed outcome taxonomy owned by the `flows run` / `flows resume` surface. */ diff --git a/sdk/src/gate-contract.ts b/sdk/src/gate-contract.ts new file mode 100644 index 000000000..9596b2c7a --- /dev/null +++ b/sdk/src/gate-contract.ts @@ -0,0 +1,72 @@ +import type { StepSpec } from './spec.js'; + +export type JournalGateCheck = + | 'completion' + | 'exit_code' + | 'output_contains' + | 'json_schema'; + +export interface DataGateClassification { + kind: 'data'; + checks: JournalGateCheck[]; + evaluator: 'kernel'; + preflightable: true; + replayable: true; +} + +export interface StepGateInspection extends DataGateClassification { + stepId: string; + /** + * Present only when the declared `json_schema` accepts every possible + * output. `{}` and `true` are legal JSON Schema and the kernel accepts both, + * so this is not a refusal — but a gate that judges nothing must not read + * identically to a gate that judges something. + */ + acceptsAnyOutput?: true; +} + +/** Annotation-only keywords: present, they still constrain no instance. */ +const ANNOTATIONS: ReadonlySet = new Set([ + '$anchor', + '$comment', + '$defs', + '$dynamicAnchor', + '$id', + '$schema', + '$vocabulary', + 'default', + 'definitions', + 'deprecated', + 'description', + 'examples', + 'readOnly', + 'title', + 'writeOnly', +]); + +/** A schema that accepts every output: `true`, `{}`, or annotations only. */ +export function acceptsAnyOutput(schema: unknown): boolean { + if (schema === true) return true; + if (typeof schema !== 'object' || schema === null || Array.isArray(schema)) return false; + return Object.keys(schema).every((key) => ANNOTATIONS.has(key)); +} + +/** Describe the exact named checks the existing kernel applies to a step. */ +export function inspectStepGate(step: StepSpec): StepGateInspection { + const checks: JournalGateCheck[] = []; + if (step.type === 'deterministic') checks.push('exit_code'); + if (step.verification?.type === 'output_contains') checks.push('output_contains'); + if (step.verification?.type === 'json_schema') checks.push('json_schema'); + if (checks.length === 0) checks.push('completion'); + const vacuous = step.verification?.type === 'json_schema' + && acceptsAnyOutput(step.verification.schema); + return { + stepId: step.id, + kind: 'data', + checks, + evaluator: 'kernel', + preflightable: true, + replayable: true, + ...(vacuous ? { acceptsAnyOutput: true as const } : {}), + }; +} diff --git a/sdk/src/index.ts b/sdk/src/index.ts index a684f80bc..27798eda8 100644 --- a/sdk/src/index.ts +++ b/sdk/src/index.ts @@ -27,6 +27,7 @@ export type { LlmStepSpec, NamedAgentSpec, OutputContainsGate, + OutputVerificationSpec, PermissionsSpec, RecoveryMode, StreamSurface, @@ -63,6 +64,11 @@ export { type PreflightResult, type PreflightWarning, } from './preflight.js'; +export { + type DataGateClassification, + type JournalGateCheck, + type StepGateInspection, +} from './gate-contract.js'; export { CHECK_FAILURE_KINDS, CHECK_INPUT_FAILURE_KINDS, diff --git a/sdk/src/json-schema-bound.ts b/sdk/src/json-schema-bound.ts new file mode 100644 index 000000000..bcabc3396 --- /dev/null +++ b/sdk/src/json-schema-bound.ts @@ -0,0 +1,357 @@ +// Termination bound for JSON Schema declarations — the SDK half. +// +// A schema that COMPILES is not a schema whose VALIDATION terminates. The +// kernel learned this the expensive way: `jsonschema` compiles +// `$defs.a -> $defs.b -> $defs.a` happily and then recurses until the process +// aborts with SIGABRT, after the step's command has already run. +// +// This is a line-for-line mirror of `kernel/relayflowd-core/src/schema.rs`, and +// `testdata/json-schema-bound-cases.json` is the corpus both sides are pinned +// to. Kernel and SDK therefore agree on which schemas are legal by +// construction, rather than by coincidence of Ajv's catchable RangeError and +// Rust's uncatchable abort. +// +// The rule: follow only the IN-PLACE applicators, which re-apply a subschema to +// the SAME instance. A cycle among those makes no progress and cannot +// terminate. Cycles that pass through a CHILD applicator (`properties`, +// `items`, ...) consume one level of the instance per step, so they terminate, +// and they stay legal. + +export const UNBOUNDED_REF_CYCLE = 'unbounded $ref cycle'; + +const REFERENCE_KEYWORDS = ['$ref', '$dynamicRef', '$recursiveRef'] as const; +const IN_PLACE_SINGLE = ['not', 'if', 'then', 'else'] as const; +const IN_PLACE_ARRAY = ['allOf', 'anyOf', 'oneOf'] as const; +const IN_PLACE_MAP = ['dependentSchemas', 'dependencies'] as const; +const CHILD_SINGLE = [ + 'additionalItems', + 'additionalProperties', + 'contains', + 'items', + 'propertyNames', + 'unevaluatedItems', + 'unevaluatedProperties', +] as const; +const CHILD_MAP = ['properties', 'patternProperties'] as const; +const CHILD_ARRAY = ['prefixItems'] as const; + +type Node = Record; + +const escape = (segment: string): string => segment.replace(/~/g, '~0').replace(/\//g, '~1'); +const unescape = (segment: string): string => segment.replace(/~1/g, '/').replace(/~0/g, '~'); +const childPointer = (pointer: string, key: string): string => `${pointer}/${escape(key)}`; +const displayPointer = (pointer: string): string => (pointer === '' ? '#' : `#${pointer}`); + +const isNode = (value: unknown): value is Node => + typeof value === 'object' && value !== null && !Array.isArray(value); + +/** Resolve a JSON pointer against the document root. */ +function at(root: unknown, pointer: string): unknown { + if (pointer === '') return root; + let current: unknown = root; + for (const raw of pointer.split('/').slice(1)) { + const segment = unescape(raw); + if (Array.isArray(current)) { + const index = Number(segment); + if (!Number.isInteger(index) || index < 0 || index >= current.length) return undefined; + current = current[index]; + } else if (isNode(current)) { + if (!Object.prototype.hasOwnProperty.call(current, segment)) return undefined; + current = current[segment]; + } else { + return undefined; + } + } + return current; +} + +function percentDecode(input: string): string { + try { + return decodeURIComponent(input); + } catch { + return input; + } +} + +/** Split `#`. `undefined` means no `#` at all, which is + * distinct from an empty fragment. */ +function splitFragment(reference: string): [string, string | undefined] { + const index = reference.indexOf('#'); + return index === -1 + ? [reference, undefined] + : [reference.slice(0, index), reference.slice(index + 1)]; +} + +function stripFragment(uri: string): string { + return splitFragment(uri)[0]; +} + +/** RFC 3986 section 3.1 scheme detection: absolute URI vs relative reference. */ +function hasScheme(reference: string): boolean { + return /^[A-Za-z][A-Za-z0-9+\-.]*:/.test(reference); +} + +/** RFC 3986 section 5.2.4. */ +function removeDotSegments(path: string): string { + const absolute = path.startsWith('/'); + const trailing = path.endsWith('/') || path.endsWith('/.') || path.endsWith('/..'); + const out: string[] = []; + for (const segment of path.split('/')) { + if (segment === '' || segment === '.') continue; + if (segment === '..') out.pop(); + else out.push(segment); + } + let resolved = absolute ? '/' : ''; + resolved += out.join('/'); + if (trailing && !resolved.endsWith('/')) resolved += '/'; + return resolved; +} + +/** Split a URI into the part a rooted path replaces and the path itself. */ +function splitAuthority(uri: string): [string, string] { + const index = uri.indexOf('://'); + if (index !== -1) { + const after = uri.slice(index + 3); + const slash = after.indexOf('/'); + if (slash === -1) return [uri, '']; + return [uri.slice(0, index + 3 + slash), uri.slice(index + 3 + slash)]; + } + const last = uri.lastIndexOf('/'); + return last === -1 ? [uri, ''] : [uri.slice(0, last + 1), uri.slice(last + 1)]; +} + +/** + * RFC 3986 section 5.3 reference resolution, enough of it for schema + * identifiers. Exactness matters less than *consistency*: every `$id` is + * registered through this function and every `$ref` looked up through it, and + * `collectScopes` also registers each resource under its raw `$id`, so a + * bundled document matches regardless of normalization. Mirrors + * `kernel/relayflowd-core/src/schema.rs::resolve_uri`. + */ +function resolveUri(base: string, reference: string): string { + if (reference === '') return base; + if (hasScheme(reference)) return reference; + if (base === '') return reference; + if (reference.startsWith('//')) { + const scheme = base.split(':')[0] ?? ''; + return `${scheme}://${reference.slice(2)}`; + } + const [root, path] = splitAuthority(base); + if (reference.startsWith('/')) return `${root}${removeDotSegments(reference)}`; + const slash = path.lastIndexOf('/'); + const merged = slash === -1 ? `/${reference}` : `${path.slice(0, slash + 1)}${reference}`; + return `${root}${removeDotSegments(merged)}`; +} + +interface Scopes { + /** Base URI of every resource declared in this document -> its pointer, + * registered under both the resolved and the raw `$id`. */ + resources: Map; + /** ` ` -> pointer. Anchors are scoped to the resource + * that declares them, so the same name under two different `$id`s does not + * shadow. */ + anchors: Map; + /** Base URI in effect at each node pointer. */ + baseAt: Map; + hasIds: boolean; +} + +const anchorKey = (base: string, name: string): string => `${base} ${name}`; + +function collectScopes(root: unknown): Scopes { + const resources = new Map([['', '']]); + const anchors = new Map(); + const baseAt = new Map(); + let hasIds = false; + const stack: Array<[string, string, unknown]> = [['', '', root]]; + while (stack.length > 0) { + const [pointer, inherited, node] = stack.pop() as [string, string, unknown]; + if (Array.isArray(node)) { + baseAt.set(pointer, inherited); + node.forEach((value, index) => stack.push([`${pointer}/${index}`, inherited, value])); + } else if (isNode(node)) { + let base = inherited; + const declared = node['$id'] ?? node['id']; + if (typeof declared === 'string') { + hasIds = true; + const raw = stripFragment(declared); + const resolved = resolveUri(base, raw); + if (!resources.has(resolved)) resources.set(resolved, pointer); + if (!resources.has(raw)) resources.set(raw, pointer); + base = resolved; + } + baseAt.set(pointer, base); + for (const keyword of ['$anchor', '$dynamicAnchor', '$recursiveAnchor'] as const) { + const name = node[keyword]; + if (typeof name === 'string' && !anchors.has(anchorKey(base, name))) { + anchors.set(anchorKey(base, name), pointer); + } + } + for (const [key, value] of Object.entries(node)) { + stack.push([childPointer(pointer, key), base, value]); + } + } else { + baseAt.set(pointer, inherited); + } + } + return { resources, anchors, baseAt, hasIds }; +} + +/** + * Resolve a reference to the pointer of the node it names, or `undefined` when + * it names nothing inside this document. + * + * A reference naming no in-document resource is left opaque, on a claim + * narrower than this function used to make. It is either remote -- refused by + * the engine, which has no retriever -- or a bundled meta-schema, which cannot + * reference back into this document and so cannot close a cycle rooted here. + * The older, wider claim was false for an in-document `$id`, which is the gap + * this resolver closes. + */ +function resolve( + base: string, + reference: string, + resources: Map, + anchors: Map, +): string | undefined { + const [uri, fragment] = splitFragment(reference); + let targetBase: string; + let targetPointer: string | undefined; + if (uri === '') { + targetBase = base; + targetPointer = resources.get(base); + } else { + const resolved = resolveUri(base, uri); + targetPointer = resources.get(resolved); + if (targetPointer !== undefined) { + targetBase = resolved; + } else { + // Literal fallback, in case this resolver and the `$id` that registered + // the resource normalized differently. + targetBase = uri; + targetPointer = resources.get(uri); + } + } + if (targetPointer === undefined) return undefined; + if (fragment === undefined || fragment === '') return targetPointer; + if (fragment.startsWith('/')) return `${targetPointer}${percentDecode(fragment)}`; + return anchors.get(anchorKey(targetBase, percentDecode(fragment))); +} + +function collectArray(node: Node, pointer: string, keyword: string, out: string[]): void { + const value = node[keyword]; + if (Array.isArray(value)) { + for (let index = 0; index < value.length; index += 1) { + out.push(`${childPointer(pointer, keyword)}/${index}`); + } + } +} + +function collectMap(node: Node, pointer: string, keyword: string, out: string[]): void { + const value = node[keyword]; + if (!isNode(value)) return; + for (const [name, entry] of Object.entries(value)) { + // draft-07 `dependencies` values may be a property-name array. + if (isNode(entry) || typeof entry === 'boolean') { + out.push(`${childPointer(pointer, keyword)}/${escape(name)}`); + } + } +} + +/** Iterative DFS — the checker must not recurse, or it inherits the very + * unbounded recursion it exists to refuse. */ +function findCycle(edges: Map): string[] | undefined { + const color = new Map(); + for (const start of edges.keys()) { + if (color.has(start)) continue; + const stack: Array<{ node: string; index: number }> = [{ node: start, index: 0 }]; + const path: string[] = [start]; + color.set(start, 'gray'); + while (stack.length > 0) { + const top = stack[stack.length - 1] as { node: string; index: number }; + const successors = edges.get(top.node) ?? []; + if (top.index < successors.length) { + const next = successors[top.index] as string; + top.index += 1; + const seen = color.get(next); + if (seen === 'gray') { + const from = path.indexOf(next); + return [...path.slice(from === -1 ? 0 : from), next]; + } + if (seen === undefined) { + color.set(next, 'gray'); + path.push(next); + stack.push({ node: next, index: 0 }); + } + } else { + color.set(top.node, 'black'); + path.pop(); + stack.pop(); + } + } + } + return undefined; +} + +/** + * Refuse a declaration whose validation is not guaranteed to terminate. + * Returns the named refusal, or `undefined` when the schema is bounded. + */ +export function jsonSchemaBoundError(schema: unknown): string | undefined { + if (!isNode(schema)) return undefined; // boolean schemas carry no references + const { resources, anchors, baseAt, hasIds } = collectScopes(schema); + const inPlace = new Map(); + const seen = new Set(['']); + const queue: string[] = ['']; + + while (queue.length > 0) { + const pointer = queue.pop() as string; + const node = at(schema, pointer); + if (!isNode(node)) continue; + const here: string[] = []; + const children: string[] = []; + + for (const keyword of REFERENCE_KEYWORDS) { + const reference = node[keyword]; + if (typeof reference !== 'string') continue; + const base = hasIds ? (baseAt.get(pointer) ?? '') : ''; + const target = resolve(base, reference, resources, anchors); + if (target !== undefined && at(schema, target) !== undefined) here.push(target); + } + for (const keyword of IN_PLACE_SINGLE) { + if (keyword in node) here.push(childPointer(pointer, keyword)); + } + for (const keyword of IN_PLACE_ARRAY) collectArray(node, pointer, keyword, here); + for (const keyword of IN_PLACE_MAP) collectMap(node, pointer, keyword, here); + for (const keyword of CHILD_SINGLE) { + const value = node[keyword]; + if (Array.isArray(value)) { + // draft-04/07 tuple form: `items` may be an array of schemas. + for (let index = 0; index < value.length; index += 1) { + children.push(`${childPointer(pointer, keyword)}/${index}`); + } + } else if (value !== undefined) { + children.push(childPointer(pointer, keyword)); + } + } + for (const keyword of CHILD_MAP) collectMap(node, pointer, keyword, children); + for (const keyword of CHILD_ARRAY) collectArray(node, pointer, keyword, children); + + for (const next of [...here, ...children]) { + if (!seen.has(next)) { + seen.add(next); + queue.push(next); + } + } + // `$defs`/`definitions` are containers, not applicators: their members are + // reachable only through a `$ref`, so an unused degenerate definition is + // never validated and stays legal. + if (here.length > 0) inPlace.set(pointer, here); + } + + const cycle = findCycle(inPlace); + if (cycle === undefined) return undefined; + return `${UNBOUNDED_REF_CYCLE}: ${cycle + .map(displayPointer) + .join(' -> ')} — this cycle re-applies to the same instance, so validation would not terminate`; +} diff --git a/sdk/src/json-schema.ts b/sdk/src/json-schema.ts new file mode 100644 index 000000000..54104c974 --- /dev/null +++ b/sdk/src/json-schema.ts @@ -0,0 +1,65 @@ +import Ajv from 'ajv'; +import AjvDraft4 from 'ajv-draft-04'; +import Ajv2019 from 'ajv/dist/2019.js'; +import Ajv2020 from 'ajv/dist/2020.js'; +import draft6MetaSchema from 'ajv/dist/refs/json-schema-draft-06.json' with { type: 'json' }; +import { jsonSchemaBoundError } from './json-schema-bound.js'; +import { snapshotJsonValue, type JsonValue } from './json-value.js'; + +const DRAFT_4 = 'http://json-schema.org/draft-04/schema'; +const DRAFT_6 = 'http://json-schema.org/draft-06/schema'; +const DRAFT_7 = 'http://json-schema.org/draft-07/schema'; +const DRAFT_2019_09 = 'https://json-schema.org/draft/2019-09/schema'; +const DRAFT_2020_12 = 'https://json-schema.org/draft/2020-12/schema'; + +/** Compile a declaration with the same drafts accepted by the kernel. */ +export function jsonSchemaError(schema: boolean | Record): string | undefined { + // Termination first: a schema whose $ref graph cycles without consuming + // input compiles fine here and aborts the kernel at verification time. Ajv's + // own overflow is a catchable RangeError, so the authoring path happened to + // hold for one shape of this bug and not for others; the explicit bound is + // what makes SDK and kernel agree. See sdk/src/json-schema-bound.ts. + const unbounded = jsonSchemaBoundError(schema); + if (unbounded !== undefined) return unbounded; + try { + const dialect = typeof schema === 'object' && typeof schema['$schema'] === 'string' + ? schema['$schema'].replace(/#$/, '') + : DRAFT_2020_12; + const options = { strict: false, allErrors: true } as const; + if (dialect === DRAFT_4) { + new AjvDraft4(options).compile(schema); + } else if (dialect === DRAFT_6) { + const validator = new Ajv(options); + validator.addMetaSchema(draft6MetaSchema); + validator.compile(schema); + } else if (dialect === DRAFT_7) { + new Ajv(options).compile(schema); + } else if (dialect === DRAFT_2019_09) { + new Ajv2019(options).compile(schema); + } else { + // Ajv reports unknown dialect identifiers instead of guessing. + new Ajv2020(options).compile(schema); + } + return undefined; + } catch (error) { + // A RangeError here is Ajv exhausting its own JS stack while COMPILING, + // not a verdict about the schema. The bound above has already proved this + // declaration terminates, and the kernel -- which is the engine that + // actually validates outputs -- accepts and runs it. Reporting Ajv's stack + // as `invalid JSON Schema: Maximum call stack size exceeded` would refuse a + // legal schema, leak an engine-internal message as if it were a named + // refusal kind, and put legality back in the hands of an engine's + // accidental overflow behaviour -- which is the precise defect the shared + // rule and corpus exist to remove. The rule decides legality; Ajv decides + // only well-formedness, and a stack overflow is neither verdict. + if (error instanceof RangeError) return undefined; + return error instanceof Error ? error.message : String(error); + } +} + +export function snapshotJsonSchema(schema: unknown, at: string): boolean | Record { + const snapshot = snapshotJsonValue(schema, at); + if (typeof snapshot === 'boolean') return snapshot; + if (snapshot !== null && !Array.isArray(snapshot) && typeof snapshot === 'object') return snapshot; + throw new Error(`${at}: expected a JSON Schema object or boolean`); +} diff --git a/sdk/src/json-value.ts b/sdk/src/json-value.ts new file mode 100644 index 000000000..ec49f8c09 --- /dev/null +++ b/sdk/src/json-value.ts @@ -0,0 +1,110 @@ +import { isProxy } from 'node:util/types'; + +export type JsonValue = + | null + | boolean + | number + | string + | JsonValue[] + | { [key: string]: JsonValue }; + +/** Copy runtime input into frozen, behavior-free JSON data. */ +export function snapshotJsonValue(value: unknown, at: string): JsonValue { + return snapshot(value, at, new WeakSet()); +} + +function snapshot(value: unknown, at: string, ancestors: WeakSet): JsonValue { + if (value === null || typeof value === 'string' || typeof value === 'boolean') return value; + if (typeof value === 'number') { + if (Number.isFinite(value)) return value; + throw nonJson(at, 'numbers must be finite'); + } + if (typeof value !== 'object') { + throw nonJson(at, `${typeof value} values are not allowed`); + } + // Every ordinary reflective operation on a Proxy can execute author code. + // Node and Bun expose this trap-free brand check, so reject before touching + // its prototype, keys, descriptors, or identity collection. + if (isProxy(value)) throw nonJson(at, 'Proxy objects are not allowed'); + if (ancestors.has(value)) throw nonJson(at, 'cycles are not allowed'); + ancestors.add(value); + try { + return Array.isArray(value) + ? snapshotArray(value, at, ancestors) + : snapshotObject(value, at, ancestors); + } finally { + ancestors.delete(value); + } +} + +function snapshotArray( + value: unknown[], + at: string, + ancestors: WeakSet, +): JsonValue[] { + const keys = Reflect.ownKeys(value); + for (const key of keys) { + if (key === 'length') continue; + if (typeof key !== 'string' || !isArrayIndex(key, value.length)) { + throw nonJson(at, 'arrays may contain only indexed data'); + } + } + const out: JsonValue[] = []; + for (let index = 0; index < value.length; index += 1) { + const descriptor = Object.getOwnPropertyDescriptor(value, String(index)); + if (descriptor === undefined) throw nonJson(`${at}[${index}]`, 'array holes are not allowed'); + out.push(snapshotDescriptor(descriptor, `${at}[${index}]`, ancestors)); + } + return Object.freeze(out) as unknown as JsonValue[]; +} + +function snapshotObject( + value: object, + at: string, + ancestors: WeakSet, +): { [key: string]: JsonValue } { + const prototype = Object.getPrototypeOf(value); + if (prototype !== Object.prototype && prototype !== null) { + throw nonJson(at, 'only plain objects are allowed'); + } + const out = Object.create(null) as { [key: string]: JsonValue }; + for (const key of Reflect.ownKeys(value)) { + if (typeof key !== 'string') throw nonJson(at, 'symbol keys are not allowed'); + const childAt = propertyPath(at, key); + const descriptor = Object.getOwnPropertyDescriptor(value, key); + if (descriptor === undefined) throw nonJson(childAt, 'missing property descriptor'); + const child = descriptorValue(descriptor, childAt); + // JSON.stringify and the pre-existing compiler omit undefined object + // optionals. Arrays remain strict because undefined there becomes null. + if (child === undefined) continue; + out[key] = snapshot(child, childAt, ancestors); + } + return Object.freeze(out); +} + +function snapshotDescriptor( + descriptor: PropertyDescriptor, + at: string, + ancestors: WeakSet, +): JsonValue { + return snapshot(descriptorValue(descriptor, at), at, ancestors); +} + +function descriptorValue(descriptor: PropertyDescriptor, at: string): unknown { + if (!descriptor.enumerable) throw nonJson(at, 'non-enumerable properties are not allowed'); + if (!('value' in descriptor)) throw nonJson(at, 'accessors are not allowed'); + return descriptor.value; +} + +function isArrayIndex(key: string, length: number): boolean { + const index = Number(key); + return Number.isInteger(index) && index >= 0 && index < length && String(index) === key; +} + +function propertyPath(at: string, key: string): string { + return /^[A-Za-z_$][A-Za-z0-9_$]*$/.test(key) ? `${at}.${key}` : `${at}[${JSON.stringify(key)}]`; +} + +function nonJson(at: string, detail: string): Error { + return new Error(`${at}: expected JSON-compatible data; ${detail}`); +} diff --git a/sdk/src/output-schema.ts b/sdk/src/output-schema.ts index bf448b13a..d846e384d 100644 --- a/sdk/src/output-schema.ts +++ b/sdk/src/output-schema.ts @@ -1,3 +1,5 @@ +import { jsonSchemaError } from './json-schema.js'; + /** JSON Schema accepted by the `output` authoring declaration. */ export type JsonOutputSchema = Record; @@ -10,6 +12,15 @@ export function validateOutputDeclaration( const errors: string[] = []; if (!isObject(step.output)) { errors.push(`${at}.output: expected a JSON Schema object`); + } else { + // `output` lowers to a `json_schema` gate at compile time, so it must clear + // exactly the gate a hand-written `verification: {type: json_schema}` does. + // Without this the kernel refuses at `run.start` a declaration `flows check` + // had just reported as a gate — the divergence this PR exists to close. + const invalid = jsonSchemaError(step.output); + if (invalid !== undefined) { + errors.push(`${at}.output: invalid JSON Schema: ${invalid}`); + } } if (step.verification !== undefined) { errors.push(`${at}: output already declares json_schema verification; remove verification`); diff --git a/sdk/src/preflight.ts b/sdk/src/preflight.ts index 4195f340f..a16125617 100644 --- a/sdk/src/preflight.ts +++ b/sdk/src/preflight.ts @@ -1,9 +1,10 @@ import type { FlowSpec, StepSpec, TriggerSpec } from './spec.js'; +import { acceptsAnyOutput, inspectStepGate, type StepGateInspection } from './gate-contract.js'; +import { compileSpec, CompileError } from './compile.js'; import type { PreflightFailureKind, PreflightWarningKind, } from './failure-kinds.js'; -import { validateSpec } from './validate.js'; export type CliResolutionSource = 'step' | 'named' | 'flow' | 'project'; @@ -94,21 +95,35 @@ export type PreflightDiagnostic = PreflightRefusal | PreflightWarning; export interface PreflightResult { ok: boolean; + gates: StepGateInspection[]; resolutions: CliResolution[]; diagnostics: PreflightDiagnostic[]; } export function preflight(flow: FlowSpec, options: PreflightOptions): PreflightResult { - const validation = validateSpec(flow); - if (!validation.ok) { + // Compile before touching any environment fact. `compileSpec` snapshots raw + // input into inert data, validates it against the closed authoring schema, + // and lowers `output` sugar into its json_schema gate — so the gate plan + // below describes what the kernel will actually judge, and no probe or gate + // inspection ever reads a live accessor. The failure is a named refusal + // rather than a thrown error (RFC covenant 2), which is the contract main + // settled for this boundary. + let compiled: FlowSpec; + try { + compiled = compileSpec(flow); + } catch (error) { + const errors = error instanceof CompileError + ? error.errors + : [error instanceof Error ? error.message : 'spec: expected JSON-compatible data']; return { ok: false, + gates: [], resolutions: [], diagnostics: [{ severity: 'refusal', kind: 'invalid_spec', - message: `Relayflow spec is invalid: ${validation.errors.join('; ')}`, - errors: validation.errors, + message: `Relayflow spec is invalid: ${errors.join('; ')}`, + errors, }], }; } @@ -117,13 +132,13 @@ export function preflight(flow: FlowSpec, options: PreflightOptions): PreflightR const resolutionByStep = new Map(); const cliProbeResults = new Map(); - diagnostics.push(...unknownModelDiagnostics(flow, options)); + diagnostics.push(...unknownModelDiagnostics(compiled, options)); // Resolve the complete flow before touching any environment fact. A later // statically unresolved CLI makes the whole submission impossible, so no // earlier command, provider/model, or trigger probe may run first. - for (const step of flow.steps) { + for (const step of compiled.steps) { if (step.type === 'deterministic') continue; - const resolution = resolveCli(step, flow, options.projectCli); + const resolution = resolveCli(step, compiled, options.projectCli); if (resolution === undefined) { diagnostics.push({ severity: 'refusal', @@ -136,21 +151,25 @@ export function preflight(flow: FlowSpec, options: PreflightOptions): PreflightR resolutionByStep.set(step.id, resolution); } } - if (diagnostics.length > 0) return { ok: false, resolutions, diagnostics }; + if (diagnostics.length > 0) { + return { ok: false, gates: compiled.steps.map(inspectStepGate), resolutions, diagnostics }; + } - for (const step of flow.steps) { + for (const step of compiled.steps) { + warnOnVacuousGate(step, diagnostics); warnOnUnprovableEffects(step, options.probes, diagnostics); if (step.type === 'deterministic') continue; const resolution = resolutionByStep.get(step.id)!; probeResolvedCli(resolution, options.probes, cliProbeResults, diagnostics); } - for (const trigger of flow.triggers ?? []) { + for (const trigger of compiled.triggers ?? []) { probeTrigger(trigger, options.probes, diagnostics); } return { ok: !diagnostics.some((diagnostic) => diagnostic.severity === 'refusal'), + gates: compiled.steps.map(inspectStepGate), resolutions, diagnostics, }; @@ -373,6 +392,23 @@ function probeTrigger( * containing `/` names a path rather than relying on shell resolution, so a * failed existence probe refuses the flow. */ +/** + * A declared `json_schema` gate that accepts every output is legal and stays + * legal — but it is indistinguishable in the gate plan from one that judges + * something, which is exactly the confusion AGENTS.md's "never edit a gate that + * judges your own work" rail exists to prevent. + */ +function warnOnVacuousGate(step: StepSpec, diagnostics: PreflightDiagnostic[]): void { + if (step.verification?.type !== 'json_schema') return; + if (!acceptsAnyOutput(step.verification.schema)) return; + diagnostics.push({ + severity: 'warning', + kind: 'vacuous_gate', + stepId: step.id, + message: `Step "${step.id}" declares a json_schema gate that accepts every possible output, so it judges nothing.`, + }); +} + function warnOnUnprovableEffects( step: StepSpec, probes: PreflightProbes, diff --git a/sdk/src/spec.ts b/sdk/src/spec.ts index 3e3f8df75..410d1fa40 100644 --- a/sdk/src/spec.ts +++ b/sdk/src/spec.ts @@ -40,10 +40,11 @@ export interface OutputContainsGate { /** Step output validates against a JSON Schema. Used for `llm` structured output. */ export interface JsonSchemaGate { type: 'json_schema'; - schema: Record; + schema: boolean | Record; } export type VerificationSpec = ExitCodeGate | OutputContainsGate | JsonSchemaGate; +export type OutputVerificationSpec = OutputContainsGate | JsonSchemaGate; /** * Agent-step recovery modes (RFC Appendix A rule 4). Default is `reset`. @@ -100,8 +101,6 @@ export interface BaseStepSpec { type: StepType; /** Step dependencies — a step runs only after these complete. */ dependsOn?: string[]; - /** Verification gate. Omit on a deterministic step to get the implicit `exit_code` gate. */ - verification?: VerificationSpec; /** Semantic retry bound (kernel DESIGN.md §1.2 `max_iterations`). Default 1. */ maxIterations?: number; } @@ -116,6 +115,8 @@ export interface DeterministicStepSpec extends BaseStepSpec { command: string; /** Wall-clock command timeout; worker-backed verbs own their dispatch timeout. */ timeoutMs?: number; + /** Omit to get the implicit `exit_code` gate. */ + verification?: VerificationSpec; } /** @@ -126,6 +127,7 @@ export interface DeterministicStepSpec extends BaseStepSpec { export interface LlmStepSpec extends BaseStepSpec { type: 'llm'; prompt: string; + verification?: OutputVerificationSpec; model?: string; /** Inert preflight declaration; overrides the flow/project CLI default. */ cli?: string; @@ -145,6 +147,7 @@ export interface LlmStepSpec extends BaseStepSpec { export interface AgentStepSpec extends BaseStepSpec { type: 'agent'; instruction: string; + verification?: OutputVerificationSpec; /** Named authoring declaration selected from `FlowSpec.agents`. Compiled away. */ agent?: string; /** Inert preflight declaration; overrides the flow/project CLI default. */ @@ -265,7 +268,7 @@ export interface KernelRetryPolicy { */ export interface KernelVerificationSpec { output_contains?: string; - json_schema?: Record; + json_schema?: boolean | Record; } export interface KernelStepCommon { diff --git a/sdk/src/validate.ts b/sdk/src/validate.ts index b11e4f875..08591a55b 100644 --- a/sdk/src/validate.ts +++ b/sdk/src/validate.ts @@ -27,6 +27,8 @@ import { STEP_COMMON_FIELDS, STEP_FIELDS_BY_TYPE, } from './step-fields.js'; +import { jsonSchemaError, snapshotJsonSchema } from './json-schema.js'; +import { snapshotJsonValue } from './json-value.js'; export interface ValidationResult { ok: boolean; @@ -310,7 +312,7 @@ class Validator { } if (st['verification'] !== undefined) { - this.validateVerification(st['verification'], `${at}.verification`); + this.validateVerification(st['verification'], `${at}.verification`, type); } if (st['maxIterations'] !== undefined && !isPosInt(st['maxIterations'])) { @@ -328,7 +330,7 @@ class Validator { } } - private validateVerification(v: unknown, at: string): void { + private validateVerification(v: unknown, at: string, stepType: StepType): void { if (!isObject(v)) { this.fail(`${at}: expected an object`); return; @@ -339,6 +341,9 @@ class Validator { this.checkKeys(v, gateKeys, at); } if (gate.type === 'exit_code') { + if (stepType !== 'deterministic') { + this.fail(`${at}: exit_code is supported only on deterministic steps`); + } // v0 judges exit_code == 0 exactly (kernel DESIGN.md §4). Fail closed // rather than compile a spec whose gate the kernel cannot enforce. if (gate.expect !== undefined && gate.expect !== 0) { @@ -349,8 +354,14 @@ class Validator { this.fail(`${at}.value: expected a non-empty string`); } } else if (gate.type === 'json_schema') { - if (!isObject(gate.schema)) { - this.fail(`${at}.schema: expected a JSON Schema object`); + try { + const schema = snapshotJsonSchema(gate.schema, `${at}.schema`); + const error = jsonSchemaError(schema); + if (error !== undefined) { + this.fail(`${at}.schema: invalid JSON Schema: ${error}`); + } + } catch (error) { + this.fail(error instanceof Error ? error.message : `${at}.schema: expected JSON-compatible data`); } } else { this.fail(`${at}.type: expected exit_code | output_contains | json_schema`); @@ -463,7 +474,14 @@ class Validator { /** Validate a parsed spec object. Returns `{ok, errors}`; never throws. */ export function validateSpec(spec: unknown): ValidationResult { - return new Validator().run(spec); + try { + return new Validator().run(snapshotJsonValue(spec, 'spec')); + } catch (error) { + return { + ok: false, + errors: [error instanceof Error ? error.message : 'spec: expected JSON-compatible data'], + }; + } } // --- predicates ------------------------------------------------------------- diff --git a/sdk/tests/cli.test.ts b/sdk/tests/cli.test.ts index 04f46b8cb..d8a2497f2 100644 --- a/sdk/tests/cli.test.ts +++ b/sdk/tests/cli.test.ts @@ -471,6 +471,14 @@ steps: ok: true, path: passPath, projectConfigPath: configPath, + gates: [{ + stepId: 'answer', + kind: 'data', + checks: ['completion'], + evaluator: 'kernel', + preflightable: true, + replayable: true, + }], resolutions: [{ stepId: 'answer', cli: './authenticated-cli', source: 'step' }], diagnostics: [], }); @@ -483,6 +491,14 @@ steps: ok: false, path: refusalPath, projectConfigPath: configPath, + gates: [{ + stepId: 'answer', + kind: 'data', + checks: ['completion'], + evaluator: 'kernel', + preflightable: true, + replayable: true, + }], resolutions: [{ stepId: 'answer', cli: './missing-cli', source: 'step' }], diagnostics: [{ severity: 'refusal', diff --git a/sdk/tests/dependency-validation.test.ts b/sdk/tests/dependency-validation.test.ts index 8dea5e92e..0f8c25d50 100644 --- a/sdk/tests/dependency-validation.test.ts +++ b/sdk/tests/dependency-validation.test.ts @@ -77,6 +77,8 @@ describe('dependency validation', () => { expect(validateSpec(flow)).toEqual({ ok: false, errors: [expected] }); expect(preflight(flow, { probes })).toEqual({ ok: false, + // A refused spec has no gate plan: nothing compiled, so nothing is judged. + gates: [], resolutions: [], diagnostics: [{ severity: 'refusal', diff --git a/sdk/tests/gate-contract.test.ts b/sdk/tests/gate-contract.test.ts new file mode 100644 index 000000000..b201ef0f6 --- /dev/null +++ b/sdk/tests/gate-contract.test.ts @@ -0,0 +1,415 @@ +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; +import { checkFlow } from '../src/cli/check.js'; +import { runCli } from '../src/cli.js'; +import { compileSpec, toKernelSpec } from '../src/compile.js'; +import { acceptsAnyOutput, inspectStepGate } from '../src/gate-contract.js'; +import { preflight } from '../src/index.js'; +import type { FlowSpec } from '../src/spec.js'; +import { validateSpec } from '../src/validate.js'; + +const TESTDATA = join(dirname(fileURLToPath(import.meta.url)), '..', '..', 'testdata'); + +function schemaFixture(name: 'valid' | 'invalid'): unknown { + return JSON.parse(readFileSync(join(TESTDATA, `json-schema-${name}.json`), 'utf8')); +} + +describe('data/code gate contract', () => { + it('describes the implicit and explicit checks the kernel will journal', () => { + expect(inspectStepGate({ + id: 'render', + type: 'deterministic', + command: 'printf ready', + verification: { type: 'output_contains', value: 'ready' }, + })).toEqual({ + stepId: 'render', + kind: 'data', + checks: ['exit_code', 'output_contains'], + evaluator: 'kernel', + preflightable: true, + replayable: true, + }); + }); + + it('makes the preflightable gate plan visible through flows check', () => { + const checked = checkFlow(join(TESTDATA, 'hello-deterministic.flow.yaml')); + + expect(checked.report.gates).toEqual([ + expect.objectContaining({ + stepId: 'greet', + kind: 'data', + checks: ['exit_code', 'output_contains'], + preflightable: true, + replayable: true, + }), + expect.objectContaining({ + stepId: 'shout', + kind: 'data', + checks: ['exit_code', 'output_contains'], + preflightable: true, + replayable: true, + }), + ]); + }); + + it('prints the gate plan in the human flows check report', async () => { + const stdout: string[] = []; + const code = await runCli( + ['check', join(TESTDATA, 'hello-deterministic.flow.yaml')], + { stdout: (line) => stdout.push(line), stderr: () => {} }, + ); + + expect(code).toBe(0); + expect(stdout).toContain( + 'GATE step "greet" exit_code+output_contains from data (kernel, journal-replayable)', + ); + }); + + it('preserves v1 verification at the unchanged kernel boundary', () => { + const flow = compileSpec({ + version: '0.1.0', + steps: [{ + id: 'render', + type: 'deterministic', + command: 'printf ready', + verification: { type: 'output_contains', value: 'ready' }, + }], + }); + + expect(toKernelSpec(flow).steps[0]?.verification).toEqual({ + output_contains: 'ready', + }); + }); + + it('does not admit an expression language into serializable verification', () => { + const candidate = { + version: '0.1.0', + steps: [{ + id: 'render', + type: 'deterministic', + command: 'printf ready', + verification: { type: 'expression', expression: 'length < 200' }, + }], + }; + + expect(() => compileSpec(candidate)).toThrow( + /expected exit_code \| output_contains \| json_schema/, + ); + expect(() => toKernelSpec(candidate as unknown as FlowSpec)).toThrow( + /expected exit_code \| output_contains \| json_schema/, + ); + }); + + it('rejects author callbacks at both serializable compiler boundaries', () => { + const candidate = { + version: '0.1.0', + steps: [{ + id: 'render', + type: 'deterministic', + command: 'printf ready', + verification: (value: string): boolean => value.length < 200, + }], + }; + + expect(() => compileSpec(candidate)).toThrow(/verification: expected JSON-compatible data/); + expect(() => toKernelSpec(candidate as unknown as FlowSpec)).toThrow( + /verification: expected JSON-compatible data/, + ); + }); + + it('rejects explicit exit_code gates the kernel cannot apply to llm or agent steps', () => { + for (const step of [ + { id: 'llm', type: 'llm', prompt: 'answer', verification: { type: 'exit_code' } }, + { id: 'agent', type: 'agent', instruction: 'answer', verification: { type: 'exit_code' } }, + ]) { + const candidate = { version: '0.1.0', steps: [step] }; + expect(() => compileSpec(candidate)).toThrow(/exit_code.*deterministic/); + expect(() => toKernelSpec(candidate as unknown as FlowSpec)).toThrow( + /exit_code.*deterministic/, + ); + } + }); + + it('snapshots and freezes schema data before returning a compiled or kernel spec', () => { + const schema = { type: 'string' }; + const candidate = { + version: '0.1.0', + steps: [{ + id: 'schema', + type: 'llm', + prompt: 'answer', + verification: { type: 'json_schema', schema }, + }], + }; + + const compiled = compileSpec(candidate); + schema.type = 'number'; + const compiledGate = compiled.steps[0]?.verification; + expect(compiledGate?.type).toBe('json_schema'); + if (compiledGate?.type !== 'json_schema') throw new Error('expected schema gate'); + expect(compiledGate.schema).toEqual({ type: 'string' }); + expect(Object.isFrozen(compiledGate.schema)).toBe(true); + + const loweredGate = toKernelSpec(compiled).steps[0]?.verification.json_schema; + expect(loweredGate).toEqual({ type: 'string' }); + expect(Object.isFrozen(loweredGate)).toBe(true); + }); + + it('rejects behavioral and non-JSON values inside schema declarations', () => { + let accessorReads = 0; + let toJsonCalls = 0; + const accessor = {}; + Object.defineProperty(accessor, 'value', { + enumerable: true, + get: () => { + accessorReads += 1; + return 'secret'; + }, + }); + const cyclic: Record = {}; + cyclic['self'] = cyclic; + + const schemas = [ + { type: 'string', toJSON: () => { toJsonCalls += 1; return true; } }, + { type: 'object', x_runtime: { callback: () => true } }, + { type: 'object', x_runtime: 1n }, + { type: 'object', x_runtime: accessor }, + { type: 'object', x_runtime: cyclic }, + ]; + for (const schema of schemas) { + const candidate = { + version: '0.1.0', + steps: [{ + id: 'schema', + type: 'llm', + prompt: 'answer', + verification: { type: 'json_schema', schema }, + }], + }; + expect(() => compileSpec(candidate)).toThrow(/JSON-compatible data/); + expect(() => toKernelSpec(candidate as unknown as FlowSpec)).toThrow( + /JSON-compatible data/, + ); + } + expect(accessorReads).toBe(0); + expect(toJsonCalls).toBe(0); + }); + + // Boundaries report a refusal in two shapes: `compileSpec`/`toKernelSpec` + // throw, while `preflight` returns a named `invalid_spec` diagnostic. Both + // are refusals; anything that returns a usable result is not, and the + // fallback string below cannot match the assertion. + const captureRefusal = (run: () => unknown): string => { + try { + const result = run() as { + ok?: boolean; + errors?: string[]; + diagnostics?: Array<{ message?: string }>; + }; + if (result !== null && typeof result === 'object' && result.ok === false) { + return [ + ...(result.errors ?? []), + ...(result.diagnostics ?? []).map((diagnostic) => diagnostic.message ?? ''), + ].join('; '); + } + return 'NOT REFUSED: the boundary accepted the input'; + } catch (error) { + return error instanceof Error ? error.message : String(error); + } + }; + + it('rejects proxy schemas without executing traps at any public data boundary', () => { + for (const boundary of [ + (flow: FlowSpec) => compileSpec(flow), + (flow: FlowSpec) => toKernelSpec(flow), + (flow: FlowSpec) => preflight(flow, { + probes: { + command: () => true, + cli: () => ({ exists: true, authenticated: true }), + executor: () => true, + }, + }), + ]) { + let proxyTraps = 0; + const sourceSchema = { type: 'string' }; + const proxySchema = new Proxy(sourceSchema, { + getPrototypeOf(target) { + proxyTraps += 1; + return Reflect.getPrototypeOf(target); + }, + ownKeys(target) { + proxyTraps += 1; + return Reflect.ownKeys(target); + }, + getOwnPropertyDescriptor(target, key) { + proxyTraps += 1; + const descriptor = Reflect.getOwnPropertyDescriptor(target, key); + return key === 'type' && descriptor !== undefined + ? { ...descriptor, value: 'number' } + : descriptor; + }, + }); + const candidate = { + version: '0.1.0', + steps: [{ + id: 'schema', + type: 'deterministic', + command: 'printf ok', + verification: { type: 'json_schema', schema: proxySchema }, + }], + } as FlowSpec; + + expect(captureRefusal(() => boundary(candidate))).toMatch(/proxy/i); + expect(proxyTraps).toBe(0); + expect(sourceSchema.type).toBe('string'); + } + }); + + it('omits explicit undefined object properties accepted by the public types and validator', () => { + const candidate: FlowSpec = { + version: '0.1.0', + description: undefined, + steps: [{ + id: 'defined', + type: 'deterministic', + command: 'true', + verification: undefined, + }], + }; + + expect(validateSpec(candidate)).toEqual({ ok: true, errors: [] }); + const compiled = compileSpec(candidate); + expect(compiled).not.toHaveProperty('description'); + expect(compiled.steps[0]).toMatchObject({ + id: 'defined', + verification: { type: 'exit_code' }, + }); + expect(preflight(candidate, { + probes: { + command: () => true, + cli: () => ({ exists: true, authenticated: true }), + executor: () => true, + }, + }).ok).toBe(true); + }); + + it('accepts both boolean JSON Schemas exactly as the kernel does', () => { + for (const schema of [true, false]) { + const candidate = { + version: '0.1.0', + steps: [{ + id: 'schema', + type: 'llm', + prompt: 'answer', + verification: { type: 'json_schema', schema }, + }], + }; + const compiled = compileSpec(candidate); + expect(toKernelSpec(compiled).steps[0]?.verification.json_schema).toBe(schema); + } + }); + + it('preflights the same valid and invalid JSON Schemas as the kernel', () => { + const candidate = (schema: unknown) => ({ + version: '0.1.0', + steps: [{ + id: 'schema', + type: 'deterministic', + command: 'printf ok', + verification: { type: 'json_schema', schema }, + }], + }); + + expect(() => compileSpec(candidate(schemaFixture('valid')))).not.toThrow(); + expect(() => compileSpec(candidate(schemaFixture('invalid')))).toThrow( + /invalid JSON Schema/, + ); + + const checked = checkFlow(join(TESTDATA, 'json-schema-invalid.flow.yaml')); + expect(checked.report).toEqual(expect.objectContaining({ + ok: false, + gates: [], + diagnostics: [expect.objectContaining({ + kind: 'invalid_spec', + message: expect.stringMatching(/invalid JSON Schema/), + })], + })); + }); + + // A schema that accepts every output is legal and stays legal — the kernel + // accepts `{}` and `true` too — but it must not be reported as a gate that + // judges something. + describe('vacuous gates', () => { + it.each([ + ['true', true], + ['empty object', {}], + ['annotations only', { $schema: 'https://json-schema.org/draft/2020-12/schema', title: 'anything' }], + ])('classifies %s as accepting any output', (_name, schema) => { + expect(acceptsAnyOutput(schema)).toBe(true); + expect(inspectStepGate({ + id: 'vacuous', + type: 'deterministic', + command: 'printf ok', + verification: { type: 'json_schema', schema }, + } as never)).toEqual(expect.objectContaining({ acceptsAnyOutput: true })); + }); + + it.each([ + ['a typed schema', { type: 'object' }], + ['false', false], + ['a schema with required', { $schema: 'https://json-schema.org/draft/2020-12/schema', required: ['a'] }], + ])('does not flag %s', (_name, schema) => { + expect(acceptsAnyOutput(schema)).toBe(false); + expect(inspectStepGate({ + id: 'real', + type: 'deterministic', + command: 'printf ok', + verification: { type: 'json_schema', schema }, + } as never)).not.toHaveProperty('acceptsAnyOutput'); + }); + + it('warns through preflight and marks the line in flows check', async () => { + const directory = mkdtempSync(join(tmpdir(), 'flows-vacuous-')); + const path = join(directory, 'vacuous.flow.yaml'); + writeFileSync(path, [ + "version: '0.1.0'", + 'name: vacuous', + 'steps:', + ' - id: judges-nothing', + ' type: deterministic', + ' command: printf ok', + ' verification:', + ' type: json_schema', + ' schema: true', + '', + ].join('\n')); + try { + const checked = checkFlow(path); + expect(checked.report.gates[0]).toEqual( + expect.objectContaining({ stepId: 'judges-nothing', acceptsAnyOutput: true }), + ); + expect(checked.report.diagnostics).toContainEqual(expect.objectContaining({ + severity: 'warning', + kind: 'vacuous_gate', + stepId: 'judges-nothing', + })); + + const stdout: string[] = []; + const code = await runCli(['check', path], { + stdout: (line) => stdout.push(line), + stderr: () => {}, + }); + expect(code).toBe(0); + expect(stdout).toContain( + 'GATE step "judges-nothing" exit_code+json_schema from data (kernel, journal-replayable)' + + ' [json_schema accepts any output]', + ); + } finally { + rmSync(directory, { recursive: true, force: true }); + } + }); + }); +}); diff --git a/sdk/tests/json-schema-bound.test.ts b/sdk/tests/json-schema-bound.test.ts new file mode 100644 index 000000000..42e3c8914 --- /dev/null +++ b/sdk/tests/json-schema-bound.test.ts @@ -0,0 +1,151 @@ +// Kernel/SDK agreement on which JSON Schema declarations are legal. +// +// Both sides read `testdata/json-schema-bound-cases.json`. The kernel half is +// `kernel/relayflowd-core/src/schema.rs` +// (`every_refused_corpus_schema_compiles_but_is_refused_by_the_bound` and +// `every_accepted_corpus_schema_is_accepted`) plus the protocol-level +// `kernel/relayflowd/tests/invalid_schema_preflight.rs`. A schema added to the +// corpus is enforced on both sides or one of the two suites goes red. + +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; +import { compileSpec } from '../src/compile.js'; +import { jsonSchemaBoundError, UNBOUNDED_REF_CYCLE } from '../src/json-schema-bound.js'; +import { jsonSchemaError } from '../src/json-schema.js'; +import type { FlowSpec } from '../src/spec.js'; + +const TESTDATA = join(dirname(fileURLToPath(import.meta.url)), '..', '..', 'testdata'); + +interface Case { + name: string; + before?: string; + schema: boolean | Record; +} +const corpus = JSON.parse( + readFileSync(join(TESTDATA, 'json-schema-bound-cases.json'), 'utf8'), +) as { marker: string; refused: Case[]; accepted: Case[]; engineRefused: Case[] }; + +const flowWith = (schema: unknown): FlowSpec => + ({ + version: '0.1.0', + name: 'bound', + steps: [ + { + id: 's', + type: 'deterministic', + command: 'true', + verification: { type: 'json_schema', schema }, + }, + ], + }) as unknown as FlowSpec; + +describe('JSON Schema termination bound', () => { + it('shares its refusal marker with the kernel', () => { + expect(UNBOUNDED_REF_CYCLE).toBe(corpus.marker); + }); + + // The bound, not the mechanism: this asserts the BOUND function itself, so + // neutering the bound while leaving Ajv compilation in place fails here. + it.each(corpus.refused.map((entry) => [entry.name, entry] as const))( + 'refuses %s', + (_name, entry) => { + expect(jsonSchemaBoundError(entry.schema)).toContain(UNBOUNDED_REF_CYCLE); + expect(jsonSchemaError(entry.schema)).toContain(UNBOUNDED_REF_CYCLE); + expect(() => compileSpec(flowWith(entry.schema))).toThrow( + new RegExp(UNBOUNDED_REF_CYCLE.replace('$', '\\$')), + ); + }, + ); + + // The other half: without this the bound could pass by refusing everything. + it.each(corpus.accepted.map((entry) => [entry.name, entry] as const))( + 'accepts %s', + (_name, entry) => { + expect(jsonSchemaBoundError(entry.schema)).toBeUndefined(); + expect(jsonSchemaError(entry.schema)).toBeUndefined(); + expect(() => compileSpec(flowWith(entry.schema))).not.toThrow(); + }, + ); + + // The narrowed premise, tested rather than asserted in a comment. + // + // The resolver leaves a reference that names no in-document resource opaque. + // That is only safe because such a reference is refused by the ENGINE, and + // the previous, wider version of this claim -- "the engine refuses anything + // the bound cannot resolve" -- was false for an in-document `$id`, which is + // the whole of signoff-4's P0. So both halves are pinned here: the bound must + // NOT claim these schemas, and the engine MUST refuse them. If a future + // change makes the engine accept an unresolvable reference, this fails and + // the opaque default has to be revisited. + it.each(corpus.engineRefused.map((entry) => [entry.name, entry] as const))( + 'leaves %s to the engine, which refuses it', + (_name, entry) => { + expect(jsonSchemaBoundError(entry.schema)).toBeUndefined(); + const fromEngine = jsonSchemaError(entry.schema); + expect(fromEngine).toBeDefined(); + expect(fromEngine).not.toContain(UNBOUNDED_REF_CYCLE); + expect(() => compileSpec(flowWith(entry.schema))).toThrow(); + }, + ); + + // `output:` sugar lowers to a json_schema gate at compile time, so it must + // clear the same bound. Before this, `flows check` reported an unbounded + // `output` schema as a gate and the kernel refused it at run.start. + it.each(corpus.refused.map((entry) => [entry.name, entry] as const))( + 'refuses %s when declared through output sugar', + (_name, entry) => { + if (typeof entry.schema !== 'object') return; + const flow = { + version: '0.1.0', + name: 'out', + steps: [{ id: 's', type: 'llm', prompt: 'p', cli: 'x', output: entry.schema }], + } as unknown as FlowSpec; + expect(() => compileSpec(flow)).toThrow( + new RegExp(UNBOUNDED_REF_CYCLE.replace('$', '\\$')), + ); + }, + ); + + it('refuses an uncompilable schema declared through output sugar', () => { + const flow = { + version: '0.1.0', + name: 'out', + steps: [{ + id: 's', + type: 'llm', + prompt: 'p', + cli: 'x', + output: { type: 'definitely-not-a-json-schema-type' }, + }], + } as unknown as FlowSpec; + expect(() => compileSpec(flow)).toThrow(/output: invalid JSON Schema/); + }); + + it('names the cycle it found', () => { + const message = jsonSchemaBoundError({ + $defs: { a: { $ref: '#/$defs/b' }, b: { $ref: '#/$defs/a' } }, + $ref: '#/$defs/a', + }); + expect(message).toContain('#/$defs/a'); + expect(message).toContain('#/$defs/b'); + }); + + it('does not mistake a property named $ref for a reference', () => { + expect( + jsonSchemaBoundError({ + type: 'object', + properties: { $ref: { type: 'string' }, allOf: { type: 'string' } }, + }), + ).toBeUndefined(); + }); + + it('walks a deep schema with an explicit stack rather than recursion', () => { + let schema: Record = { type: 'string' }; + for (let depth = 0; depth < 5_000; depth += 1) { + schema = { type: 'array', items: schema }; + } + expect(jsonSchemaBoundError(schema)).toBeUndefined(); + }); +}); diff --git a/sdk/tests/preflight.test.ts b/sdk/tests/preflight.test.ts index d64bd4414..cafeacefb 100644 --- a/sdk/tests/preflight.test.ts +++ b/sdk/tests/preflight.test.ts @@ -5,10 +5,10 @@ import { } from '../src/failure-kinds.js'; import { CliProbeError, - preflight, type CliProbeResult, type PreflightProbes, } from '../src/preflight.js'; +import { preflight } from '../src/index.js'; import type { FlowSpec } from '../src/spec.js'; import { compileSpec, toKernelSpec } from '../src/compile.js'; @@ -82,6 +82,41 @@ describe('preflight: CLI resolution and refusal predicates', () => { expect(calls).toEqual([]); }); + // Same guarantee for gate data specifically: a callback gate, an unknown gate + // type, and an uncompilable schema each refuse with a named `invalid_spec` + // diagnostic, and nothing in the environment is touched first. + it('validates raw public input before any probe or gate inspection', () => { + let probeCount = 0; + const injected = probes({ + command: () => { probeCount += 1; return true; }, + cli: () => { probeCount += 1; return { exists: true, authenticated: true }; }, + }); + const candidate = (verification: unknown): FlowSpec => ({ + version: '0.1.0', + steps: [{ + id: 'raw', + type: 'deterministic', + command: 'printf ok', + verification, + } as FlowSpec['steps'][number]], + }); + + for (const verification of [ + (value: unknown) => value, + { type: 'expression', expression: 'length < 200' }, + { type: 'json_schema', schema: { type: 'definitely-not-a-json-schema-type' } }, + ]) { + const result = preflight(candidate(verification), { probes: injected }); + expect(result).toMatchObject({ + ok: false, + gates: [], + resolutions: [], + diagnostics: [{ severity: 'refusal', kind: 'invalid_spec' }], + }); + } + expect(probeCount).toBe(0); + }); + it('resolves step, then flow, then project without guessing a platform default', () => { const seen: string[] = []; const check = (spec: FlowSpec, projectCli?: string) => preflight(spec, { @@ -297,6 +332,17 @@ describe('preflight: CLI resolution and refusal predicates', () => { preflight(flow({ id: 'a', type: 'deterministic', command: 'x' }), { probes: probes() }), preflight(flow({ id: 'a', type: 'deterministic', command: 'x' }), { probes: probes({ command: () => false }) }), preflight(flow({ id: 'a', type: 'deterministic', command: 'x' }), { probes: probes({ command: () => { throw new Error('raw secret'); } }) }), + // A declared gate that accepts every output is legal, and silence about + // it is exactly the covenant-2 silence this test forbids. + preflight( + flow({ + id: 'a', + type: 'deterministic', + command: 'x', + verification: { type: 'json_schema', schema: true }, + } as never), + { probes: probes() }, + ), ]; const warningKinds = scenarios.flatMap((result) => result.diagnostics) .filter((diagnostic) => diagnostic.severity === 'warning') diff --git a/sdk/tests/spec-parity.test.ts b/sdk/tests/spec-parity.test.ts index 662f47b85..32804f94d 100644 --- a/sdk/tests/spec-parity.test.ts +++ b/sdk/tests/spec-parity.test.ts @@ -6,9 +6,9 @@ import { compileAndHash, compileYaml, compileYamlToCanonicalJson, - kernelToAuthoring, toKernelSpec, } from '../src/compile.js'; +import { canonicalize, kernelToAuthoring, specHash } from '../src/index.js'; // The SDK half of the cross-boundary spec-parity gate. The shared fixture in // testdata/ pins one spec dialect at the SDK<->kernel seam: this test proves @@ -95,6 +95,87 @@ describe('spec parity: one dialect at the SDK<->kernel boundary', () => { expect(() => kernelToAuthoring(kernel)).toThrow('spec.triggers[0]'); }); + it('refuses nested proxy data before executing any trap', () => { + const flow = compileYaml(fixture('hello-deterministic.flow.yaml')); + const kernel = toKernelSpec(flow); + let proxyTraps = 0; + kernel.steps = new Proxy(kernel.steps, { + getPrototypeOf(target) { + proxyTraps += 1; + return Reflect.getPrototypeOf(target); + }, + ownKeys(target) { + proxyTraps += 1; + return Reflect.ownKeys(target); + }, + getOwnPropertyDescriptor(target, key) { + proxyTraps += 1; + return Reflect.getOwnPropertyDescriptor(target, key); + }, + }); + + expect(() => kernelToAuthoring(kernel)).toThrow(/proxy/i); + expect(proxyTraps).toBe(0); + }); + + // `canonicalize` and `specHash` are exported unknown-input helpers, so they + // carry the same snapshot guard as `validateSpec` and `kernelToAuthoring`. + // A signoff demonstrated the gap: a raw Proxy fired 10 traps and a plain + // getter was read twice per call, so two `canonicalize` calls on one object + // could disagree — and `specHash` would then stamp a spec nobody declared. + it('refuses a proxy at canonicalize and specHash before executing any trap', () => { + let proxyTraps = 0; + const handler = { + getPrototypeOf(target: object) { + proxyTraps += 1; + return Reflect.getPrototypeOf(target); + }, + ownKeys(target: object) { + proxyTraps += 1; + return Reflect.ownKeys(target); + }, + getOwnPropertyDescriptor(target: object, key: string | symbol) { + proxyTraps += 1; + return Reflect.getOwnPropertyDescriptor(target, key); + }, + get(target: object, key: string | symbol, receiver: unknown) { + proxyTraps += 1; + return Reflect.get(target, key, receiver); + }, + }; + + expect(() => canonicalize(new Proxy({ a: 1, b: 2 }, handler))).toThrow(/proxy/i); + expect(() => specHash(new Proxy({ a: 1, b: 2 }, handler))).toThrow(/proxy/i); + expect(proxyTraps).toBe(0); + }); + + it('refuses a getter at canonicalize and never reads it', () => { + let getterReads = 0; + const build = (): Record => { + const object = {}; + Object.defineProperty(object, 'k', { + enumerable: true, + configurable: true, + get() { + getterReads += 1; + return getterReads; + }, + }); + return object as Record; + }; + + expect(() => canonicalize(build())).toThrow(/accessors are not allowed/); + expect(() => specHash(build())).toThrow(/accessors are not allowed/); + expect(getterReads).toBe(0); + }); + + it('canonicalizes inert data identically on every call', () => { + const value = { b: 2, a: 1, c: [3, { z: 1, y: 2 }] }; + expect(canonicalize(value)).toBe('{"a":1,"b":2,"c":[3,{"y":2,"z":1}]}'); + expect(canonicalize(value)).toBe(canonicalize(value)); + expect(specHash(value)).toBe(specHash(value)); + }); + it('round-trips flow, trigger, and step CLI declarations', () => { const flow = compileYaml(` version: '0.1.0' diff --git a/sdk/tests/validate.test.ts b/sdk/tests/validate.test.ts index 4c4a1dd49..343dcfd65 100644 --- a/sdk/tests/validate.test.ts +++ b/sdk/tests/validate.test.ts @@ -1,12 +1,39 @@ import { describe, expect, it } from 'vitest'; -import { validateSpec } from '../src/validate.js'; import { compileSpec, compileYaml, CompileError } from '../src/compile.js'; +import { validateSpec } from '../src/index.js'; import type { FlowSpec } from '../src/spec.js'; // Fail-closed (AGENTS.md rule 4): a malformed spec is rejected with a concrete // error, never silently coerced. Every case below must produce a non-ok result. describe('validate: rejects malformed specs', () => { + it('returns a failure for proxy input without executing traps or throwing', () => { + let proxyTraps = 0; + const candidate = new Proxy({ + version: '0.1.0', + steps: [{ id: 'a', type: 'deterministic', command: 'true' }], + }, { + get(target, key, receiver) { + proxyTraps += 1; + return Reflect.get(target, key, receiver); + }, + ownKeys(target) { + proxyTraps += 1; + return Reflect.ownKeys(target); + }, + getOwnPropertyDescriptor(target, key) { + proxyTraps += 1; + return Reflect.getOwnPropertyDescriptor(target, key); + }, + }); + + let result: ReturnType | undefined; + expect(() => { result = validateSpec(candidate); }).not.toThrow(); + expect(result?.ok).toBe(false); + expect(result?.errors.join(' ')).toMatch(/proxy/i); + expect(proxyTraps).toBe(0); + }); + it('rejects a non-object spec', () => { expect(validateSpec(null).ok).toBe(false); expect(validateSpec('hello').ok).toBe(false); diff --git a/sdk/tests/verb-field-lint.test.ts b/sdk/tests/verb-field-lint.test.ts index 470e88e90..bb2d3e150 100644 --- a/sdk/tests/verb-field-lint.test.ts +++ b/sdk/tests/verb-field-lint.test.ts @@ -233,6 +233,8 @@ describe('closed per-verb step fields', () => { }); expect(result).toEqual({ ok: false, + // A refused spec has no gate plan: nothing compiled, so nothing is judged. + gates: [], resolutions: [], diagnostics: [{ severity: 'refusal', @@ -295,6 +297,8 @@ describe('closed per-verb step fields', () => { expect(result).toEqual({ ok: false, + // A refused spec has no gate plan: nothing compiled, so nothing is judged. + gates: [], resolutions: [], diagnostics: [{ severity: 'refusal', diff --git a/testdata/json-schema-bound-cases.json b/testdata/json-schema-bound-cases.json new file mode 100644 index 000000000..1b22676f7 --- /dev/null +++ b/testdata/json-schema-bound-cases.json @@ -0,0 +1,768 @@ +{ + "_comment": "Shared kernel/SDK corpus for the JSON Schema termination bound. kernel/relayflowd-core/src/schema.rs and sdk/src/json-schema-bound.ts are pinned to this file, so a case added here is enforced on both sides or a suite goes red. Cases are enumerated along two axes: the 2020-12 APPLICATOR vocabulary (which keywords can close a cycle) and the 2020-12 REFERENCE FORMS F1-F12 (how a reference can name its target: same-resource fragment, pointer, anchor; absolute, relative, rooted and opaque URI naming an in-document $id, with and without a fragment; nested $id bases; repeated anchor names across $id scopes; and the external URI the engine refuses rather than the bound). Derive from the specification, not from this list.", + "marker": "unbounded $ref cycle", + "refused": [ + { + "name": "mutual $defs cycle with no body", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "a": { + "$ref": "#/$defs/b" + }, + "b": { + "$ref": "#/$defs/a" + } + }, + "$ref": "#/$defs/a" + } + }, + { + "name": "self $defs cycle with no body", + "before": "latent", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "a": { + "$ref": "#/$defs/a" + } + }, + "$ref": "#/$defs/a" + } + }, + { + "name": "root self reference", + "before": "latent", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$ref": "#" + } + }, + { + "name": "allOf self cycle", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "a": { + "allOf": [ + { + "$ref": "#/$defs/a" + } + ] + } + }, + "$ref": "#/$defs/a" + } + }, + { + "name": "anyOf mutual cycle", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "a": { + "anyOf": [ + { + "$ref": "#/$defs/b" + } + ] + }, + "b": { + "anyOf": [ + { + "$ref": "#/$defs/a" + } + ] + } + }, + "$ref": "#/$defs/a" + } + }, + { + "name": "oneOf cycle", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "a": { + "oneOf": [ + { + "$ref": "#/$defs/a" + } + ] + } + }, + "$ref": "#/$defs/a" + } + }, + { + "name": "not cycle", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "a": { + "not": { + "$ref": "#/$defs/a" + } + } + }, + "$ref": "#/$defs/a" + } + }, + { + "name": "if cycle", + "before": "latent", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "a": { + "if": { + "$ref": "#/$defs/a" + } + } + }, + "$ref": "#/$defs/a" + } + }, + { + "name": "dependentSchemas cycle", + "before": "latent", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "a": { + "dependentSchemas": { + "x": { + "$ref": "#/$defs/a" + } + } + } + }, + "$ref": "#/$defs/a" + } + }, + { + "name": "anchor mutual cycle", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "a": { + "$anchor": "A", + "$ref": "#B" + }, + "b": { + "$anchor": "B", + "$ref": "#A" + } + }, + "$ref": "#A" + } + }, + { + "name": "$id-scoped mutual cycle", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://example.com/root", + "$defs": { + "sub": { + "$id": "https://example.com/sub", + "$defs": { + "a": { + "$ref": "#/$defs/b" + }, + "b": { + "$ref": "#/$defs/a" + } + }, + "$ref": "#/$defs/a" + } + }, + "$ref": "#/$defs/sub" + } + }, + { + "name": "cycle reachable only under properties", + "before": "latent", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "p": { + "$defs": { + "a": { + "$ref": "#/properties/p/$defs/b" + }, + "b": { + "$ref": "#/properties/p/$defs/a" + } + }, + "$ref": "#/properties/p/$defs/a" + } + } + } + }, + { + "name": "F4 absolute-URI $ref to an in-document $id", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/root", + "$defs": { + "a": { + "$id": "https://ex.test/a", + "$ref": "https://ex.test/b" + }, + "b": { + "$id": "https://ex.test/b", + "$ref": "https://ex.test/a" + } + }, + "$ref": "https://ex.test/a" + } + }, + { + "name": "F5 absolute-URI with pointer fragment", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/root", + "$defs": { + "s": { + "$id": "https://ex.test/s", + "$defs": { + "a": { + "$ref": "https://ex.test/s#/$defs/a" + } + } + } + }, + "$ref": "https://ex.test/s#/$defs/a" + } + }, + { + "name": "F6 absolute-URI with anchor fragment", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/root", + "$defs": { + "s": { + "$id": "https://ex.test/s", + "$defs": { + "a": { + "$anchor": "loop", + "$ref": "https://ex.test/s#loop" + } + } + } + }, + "$ref": "https://ex.test/s#loop" + } + }, + { + "name": "F7 relative-URI reference to an in-document $id", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/root", + "$defs": { + "a": { + "$id": "a", + "$ref": "b" + }, + "b": { + "$id": "b", + "$ref": "a" + } + }, + "$ref": "a" + } + }, + { + "name": "F8 rooted relative reference", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/dir/root", + "$defs": { + "a": { + "$id": "/x", + "$ref": "/y" + }, + "b": { + "$id": "/y", + "$ref": "/x" + } + }, + "$ref": "/x" + } + }, + { + "name": "F9 opaque absolute URI (urn) self cycle", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "urn:ex:root", + "$defs": { + "a": { + "$id": "urn:ex:a", + "$ref": "urn:ex:a" + } + }, + "$ref": "urn:ex:a" + } + }, + { + "name": "F10 reference resolved against a nested $id base", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/root", + "$defs": { + "s": { + "$id": "https://ex.test/sub/", + "$defs": { + "a": { + "$ref": "https://ex.test/sub/#/$defs/b" + }, + "b": { + "$ref": "#/$defs/a" + } + } + } + }, + "$ref": "https://ex.test/sub/#/$defs/a" + } + }, + { + "name": "F11 same anchor name under two $id scopes", + "before": "abort", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/root", + "$defs": { + "x": { + "$id": "https://ex.test/x", + "$defs": { + "a": { + "$anchor": "nm", + "$ref": "https://ex.test/y#nm" + } + } + }, + "y": { + "$id": "https://ex.test/y", + "$defs": { + "b": { + "$anchor": "nm", + "$ref": "https://ex.test/x#nm" + } + } + } + }, + "$ref": "https://ex.test/x#nm" + } + } + ], + "accepted": [ + { + "name": "recursive array items", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "array", + "items": { + "$ref": "#" + } + } + }, + { + "name": "recursive tree through properties", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "children": { + "type": "array", + "items": { + "$ref": "#" + } + } + } + } + }, + { + "name": "recursive $defs tree", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "node": { + "type": "object", + "properties": { + "kids": { + "type": "array", + "items": { + "$ref": "#/$defs/node" + } + } + } + } + }, + "$ref": "#/$defs/node" + } + }, + { + "name": "mutual recursion with bodies", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "a": { + "type": "object", + "properties": { + "b": { + "$ref": "#/$defs/b" + } + } + }, + "b": { + "type": "object", + "properties": { + "a": { + "$ref": "#/$defs/a" + } + } + } + }, + "$ref": "#/$defs/a" + } + }, + { + "name": "terminating anyOf recursion", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "n": { + "anyOf": [ + { + "type": "null" + }, + { + "type": "array", + "items": { + "$ref": "#/$defs/n" + } + } + ] + } + }, + "$ref": "#/$defs/n" + } + }, + { + "name": "recursion through prefixItems", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "array", + "prefixItems": [ + { + "type": "string" + }, + { + "$ref": "#" + } + ] + } + }, + { + "name": "unused degenerate definition", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "$defs": { + "a": { + "$ref": "#/$defs/b" + }, + "b": { + "$ref": "#/$defs/a" + } + } + } + }, + { + "name": "anchor recursion through properties", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "n": { + "$anchor": "N", + "type": "object", + "properties": { + "kid": { + "$ref": "#N" + } + } + } + }, + "$ref": "#N" + } + }, + { + "name": "draft-07 recursion", + "schema": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "kid": { + "$ref": "#" + } + } + } + }, + { + "name": "draft-07 dependencies property-name array", + "schema": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "dependencies": { + "a": [ + "b" + ] + } + } + }, + { + "name": "plain object schema", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "required": [ + "exit_code" + ] + } + }, + { + "name": "vacuous object schema", + "schema": {} + }, + { + "name": "boolean true schema", + "schema": true + }, + { + "name": "boolean false schema", + "schema": false + }, + { + "name": "F4 absolute-URI reference through a child applicator", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/r", + "$defs": { + "a": { + "$id": "https://ex.test/a", + "type": "object", + "properties": { + "n": { + "$ref": "https://ex.test/a" + } + } + } + }, + "$ref": "https://ex.test/a" + } + }, + { + "name": "F4 bundled compound document, terminating", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/bundle", + "$defs": { + "node": { + "$id": "https://ex.test/node", + "type": "object", + "properties": { + "child": { + "$ref": "https://ex.test/node" + } + } + } + }, + "$ref": "https://ex.test/node" + } + }, + { + "name": "F4 two distinct $id resources, no cycle", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/r", + "$defs": { + "a": { + "$id": "https://ex.test/a", + "$ref": "https://ex.test/b" + }, + "b": { + "$id": "https://ex.test/b", + "type": "string" + } + }, + "$ref": "https://ex.test/a" + } + }, + { + "name": "F5 pointer fragment into an $id scope, terminating", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "s": { + "$id": "https://ex.test/s", + "$defs": { + "a": { + "type": "object", + "properties": { + "p": { + "$ref": "#/$defs/a" + } + } + } + }, + "$ref": "#/$defs/a" + } + }, + "$ref": "#/$defs/s" + } + }, + { + "name": "F6 anchor fragment through a child applicator", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/root", + "$defs": { + "s": { + "$id": "https://ex.test/s", + "$defs": { + "a": { + "$anchor": "node", + "type": "object", + "properties": { + "p": { + "$ref": "https://ex.test/s#node" + } + } + } + } + } + }, + "$ref": "https://ex.test/s#node" + } + }, + { + "name": "F7 relative-URI reference through a child applicator", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/root", + "$defs": { + "a": { + "$id": "a", + "type": "object", + "properties": { + "n": { + "$ref": "a" + } + } + } + }, + "$ref": "a" + } + }, + { + "name": "F9 opaque absolute URI (urn) through a child applicator", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "urn:ex:root", + "$defs": { + "a": { + "$id": "urn:ex:a", + "type": "object", + "properties": { + "n": { + "$ref": "urn:ex:a" + } + } + } + }, + "$ref": "urn:ex:a" + } + }, + { + "name": "F11 two anchors of the same name, neither cyclic", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/root", + "$defs": { + "x": { + "$id": "https://ex.test/x", + "$defs": { + "a": { + "$anchor": "nm", + "type": "string" + } + } + }, + "y": { + "$id": "https://ex.test/y", + "$defs": { + "b": { + "$anchor": "nm", + "$ref": "https://ex.test/x#nm" + } + } + } + }, + "$ref": "https://ex.test/y#nm" + } + } + ], + "engineRefused": [ + { + "name": "F12 external absolute URI, no fragment", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$ref": "https://ex.test/not-in-this-document" + } + }, + { + "name": "F12 external absolute URI with a pointer fragment", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$ref": "https://ex.test/other#/$defs/a" + } + }, + { + "name": "F12 external absolute URI reachable only through a child applicator", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "p": { + "$ref": "https://ex.test/not-in-this-document" + } + } + } + }, + { + "name": "F12 relative reference resolving outside the document", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://ex.test/dir/root", + "$ref": "sibling" + } + } + ] +} diff --git a/testdata/json-schema-invalid.flow.yaml b/testdata/json-schema-invalid.flow.yaml new file mode 100644 index 000000000..795bb92e4 --- /dev/null +++ b/testdata/json-schema-invalid.flow.yaml @@ -0,0 +1,10 @@ +version: '0.1.0' +steps: + - id: schema + type: deterministic + command: printf ok + verification: + type: json_schema + schema: + $schema: https://json-schema.org/draft/2020-12/schema + type: definitely-not-a-json-schema-type diff --git a/testdata/json-schema-invalid.json b/testdata/json-schema-invalid.json new file mode 100644 index 000000000..af66defcd --- /dev/null +++ b/testdata/json-schema-invalid.json @@ -0,0 +1,4 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "definitely-not-a-json-schema-type" +} diff --git a/testdata/json-schema-valid.json b/testdata/json-schema-valid.json new file mode 100644 index 000000000..c5d81e1a9 --- /dev/null +++ b/testdata/json-schema-valid.json @@ -0,0 +1,11 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "answer": { + "type": "integer" + } + }, + "required": ["answer"], + "additionalProperties": false +}