diff --git a/crates/aura-cli/src/graph_construct.rs b/crates/aura-cli/src/graph_construct.rs index 39133b4..480865f 100644 --- a/crates/aura-cli/src/graph_construct.rs +++ b/crates/aura-cli/src/graph_construct.rs @@ -8,9 +8,9 @@ use std::collections::BTreeMap; use std::path::Path; use aura_engine::{ - blueprint_from_json, blueprint_identity_json, blueprint_to_json, replay, ArgOpError, - BindOpError, BlueprintDoc, CompileError, Composite, GraphSession, LoadError, Op, OpError, - Scalar, ScalarKind, + blueprint_from_json, blueprint_identity_json, blueprint_to_json, name_gate, replay, ArgOpError, + BindOpError, BlueprintDoc, CompileError, Composite, GraphSession, LoadError, NameGateFault, Op, + OpError, Scalar, ScalarKind, }; use aura_runner::runner::render_value; use serde::Deserialize; @@ -22,9 +22,10 @@ use crate::research_docs::resolve_id_prefix; #[cfg(test)] use aura_vocabulary::std_vocabulary; -/// The op-list reference `aura graph build --help` appends (#323): the ten -/// op kinds with their fields and one worked element each. Lives beside -/// [`OpDoc`] so a new op variant is one screen away from its help line. +/// The op-list reference `aura graph build --help` appends (#323): the eleven +/// op kinds with their fields and one worked element each (#331: `name` joins +/// the roster, ten -> eleven). Lives beside [`OpDoc`] so a new op variant is +/// one screen away from its help line. pub const OP_REFERENCE: &str = r#"Op-list reference (stdin: a JSON array of op objects, applied in order): {"op":"source","role":"price","kind":"F64"} declare a bound root input role of a scalar kind @@ -50,6 +51,9 @@ pub const OP_REFERENCE: &str = r#"Op-list reference (stdin: a JSON array of op o {"op":"use","ref":{"name":"agree"},"name":"gate","bind":{"sma.length":{"I64":9}}} splice a registered blueprint (by "content_id" or "name") under an instance name ("bind" path-qualifies the spliced instance's params) + {"op":"name","name":"ny_momentum"} + set the composite's render name, at most once per script (default + "graph" if omitted) Node types and their ports: aura graph introspect --vocabulary | --node "#; @@ -111,6 +115,9 @@ enum OpDoc { #[serde(default)] bind: BTreeMap, }, + /// Set the composite's render name (#331) — script-level, at most once; + /// the op-script twin of `replay`'s seeded default name (`"graph"`). + Name { name: String }, } /// A `use` op's reference (#317) — exactly one of a store content id (full @@ -143,6 +150,7 @@ impl OpDoc { OpDoc::Doc { .. } => "doc".to_string(), OpDoc::Use { name: Some(n), .. } => format!("use {n:?}"), OpDoc::Use { name: None, .. } => "use".to_string(), + OpDoc::Name { .. } => "name".to_string(), } } } @@ -182,6 +190,7 @@ impl From for Op { name, bind: bind.into_iter().collect(), }, + OpDoc::Name { name } => Op::Name { name }, } } } @@ -257,9 +266,25 @@ fn format_op_error(e: &OpError) -> String { OpError::UnknownSubgraph { ref_id } => { format!("use: no subgraph for content id {ref_id:?}") } + OpError::DuplicateName => "a script names its blueprint at most once".to_string(), + OpError::BadName { name, fault } => name_gate_fault_prose(name, fault), } } +/// Phrase a `name_gate` shape violation as prose (#331): shared by this +/// module's op-intake `format_op_error` `BadName` arm and the +/// blueprint-envelope intake's root-name gate (`composite_from_authored_text` +/// below) — the shape rule has exactly one seam-independent cause per fault, +/// so both data-borne birth routes for a name read identically. +fn name_gate_fault_prose(name: &str, fault: &NameGateFault) -> String { + let cause = match fault { + NameGateFault::Empty => "must be non-empty", + NameGateFault::ContainsSeparator => "must not contain '/' or '\\'", + NameGateFault::DotSegment => "must not be \".\" or \"..\"", + }; + format!("blueprint name {name:?} is invalid: {cause} (a single path segment)") +} + /// Resolve one `use` op's [`UseRef`] to the full store content id (#317): /// verbatim if it already IS a 64-hex content id, else a unique content-id /// prefix (#302 semantics, reusing [`resolve_id_prefix`]) or a registry name @@ -649,7 +674,7 @@ pub fn introspect_cmd(cmd: crate::GraphIntrospectCmd, env: &aura_runner::project std::process::exit(1); } }; - match composite_from_any(&text, env) { + match composite_from_authored_text(&text, env) { Ok(c) => c, Err(m) => { eprintln!("aura: {m}"); @@ -819,6 +844,18 @@ pub(crate) fn blueprint_slot_prose(doc: &str, env: &aura_runner::project::Env) - /// envelope (a JSON object: `format_version` + `blueprint`) OR a construction /// op-list (a JSON array) — shape-discriminated on the top-level JSON type, /// each canonicalized by its own rules. +/// +/// **Ungated by design (#331 review finding).** This is the STORE READ-BACK +/// shape: `params_lines`'s content-id fetch (`resolve_blueprint_text`'s +/// non-file branch) — `introspect --params ` and, by the same +/// convention, `validate_campaign_refs`'s already-ungated `blueprint_from_json` +/// call — reads whatever is already sitting in the store, and C29 says a +/// registered artifact is never retroactively invalidated. Freshly authored +/// FILE text (a hand-edited document that has not yet passed through a gated +/// intake) goes through [`composite_from_authored_text`] instead, never here +/// — `params_lines`'s OWN file branch is such a case (#331 delta re-review: +/// it used to call straight through to this fn, missing the gate; fixed by +/// branching on `resolve_blueprint_text`'s file-vs-store flag). pub(crate) fn composite_from_any(text: &str, env: &aura_runner::project::Env) -> Result { let value: serde_json::Value = serde_json::from_str(text).map_err(|e| format!("invalid document: {e}"))?; @@ -836,12 +873,68 @@ pub(crate) fn composite_from_any(text: &str, env: &aura_runner::project::Env) -> } } +/// The FILE-intake counterpart of [`composite_from_any`] (#331 review +/// finding): identical parse, plus the blueprint-envelope root-name shape +/// gate. Every call site that builds a `Composite` from text freshly read +/// off disk (`graph register`, `graph introspect --content-id `, the +/// bare `aura graph ` viewer, and `graph introspect --params `'s +/// file branch) goes through this wrapper — never the ungated +/// `composite_from_any` — because a hand-edited envelope with +/// `"name":"../x"` would otherwise register/build cleanly and write +/// `traces/../x/` at run time (`trace_store.rs` joins the name unsanitized). +/// Gating the composite's name unconditionally (not just on the Object +/// branch) is harmless: an op-script-built composite's name already passed +/// the op-intake gate (`GraphSession::set_name`), so re-checking it here is +/// redundant, not restrictive — it keeps this wrapper a single shared choke +/// point rather than one that has to re-discriminate the JSON shape. Only +/// the ROOT name is checked: the filesystem seam consumes exclusively the +/// root name. Store read-back (reproduce, `use`-splice resolution from the +/// registry, and `params_lines`'s content-id branch) never reaches this +/// function — C29's "registered artifacts are never retroactively +/// invalidated" stays intact. +/// +/// **One deliberate exception:** `aura run `'s loaded-blueprint +/// branch (`dispatch_run`, main.rs) also reads a fresh FILE and reaches the +/// same unsanitized `traces//` seam (via `run_signal_r`/`run_measurement` +/// -> `bind_tap_plan` -> `TraceStore::begin_run`), so it too must gate the root +/// name — but it does NOT route through this wrapper, because its grammar is +/// deliberately narrower than `composite_from_any`'s (envelope-only, no +/// op-script array fallback; a bare `blueprint_from_json` call with its own +/// load-error prose/hint convention). Routing it through this wrapper would +/// silently loosen that grammar to also accept op-scripts. It instead calls +/// [`gate_authored_root_name`] directly, sharing the identical `name_gate` + +/// `name_gate_fault_prose` primitives this wrapper uses — so the refusal wording +/// is byte-identical across all five authored-file-intake routes even though +/// the shape-discrimination step is not shared by `dispatch_run`. +pub(crate) fn composite_from_authored_text( + text: &str, + env: &aura_runner::project::Env, +) -> Result { + let composite = composite_from_any(text, env)?; + gate_authored_root_name(composite.name())?; + Ok(composite) +} + +/// The root-name shape gate itself (#331 delta re-review), factored out of +/// [`composite_from_authored_text`] so `dispatch_run`'s narrower-grammar file +/// intake (see that fn's doc comment) can share the exact `name_gate` call and +/// `name_gate_fault_prose` wording without going through the shape-discriminating +/// wrapper. +pub(crate) fn gate_authored_root_name(name: &str) -> Result<(), String> { + name_gate(name).map_err(|fault| name_gate_fault_prose(name, &fault)) +} + /// Resolve a blueprint document's bytes from a file path or a 64-hex content /// id in the project store (the campaign-run target-addressing convention). /// The id shape is `aura_runner::axes::is_content_id` — the same predicate /// `campaign run`'s target resolution uses, so the two FILE-or-id surfaces -/// cannot drift apart on what counts as a store address. -fn resolve_blueprint_text(target: &str, env: &aura_runner::project::Env) -> Result { +/// cannot drift apart on what counts as a store address. The `bool` names +/// which branch fired: `true` for a FRESH FILE read, `false` for a STORE +/// (content-id) fetch — `params_lines` (#331 delta re-review) uses it to +/// decide whether the root-name gate applies (a fresh file must gate; a +/// store read-back must not, C29) without re-deriving the FILE-vs-id +/// distinction a second time. +fn resolve_blueprint_text(target: &str, env: &aura_runner::project::Env) -> Result<(String, bool), String> { // A CLI arg tolerates the `content:` display prefix (#194); doc ref // fields stay bare-only. let target = target @@ -851,11 +944,12 @@ fn resolve_blueprint_text(target: &str, env: &aura_runner::project::Env) -> Resu let path = Path::new(target); if path.is_file() { return std::fs::read_to_string(path) + .map(|text| (text, true)) .map_err(|e| format!("cannot read {}: {e}", path.display())); } if aura_runner::axes::is_content_id(target) { return match env.registry().get_blueprint(target) { - Ok(Some(json)) => Ok(json), + Ok(Some(json)) => Ok((json, false)), Ok(None) => Err(format!("no blueprint {target} in the project store")), Err(e) => Err(e.to_string()), }; @@ -872,12 +966,16 @@ fn resolve_blueprint_text(target: &str, env: &aura_runner::project::Env) -> Resu /// `bound_param_space()`'s names are already RAW (#203), so no blueprint-name /// concatenation is needed — line-identical to the reconciled `--list-axes` /// bound pass (`list_blueprint_axes`, main.rs), same `render_value` lexicon. +/// #331 delta re-review: the FILE target is a FOURTH freshly-authored-file +/// intake this fn's own `composite_from_any` call had missed gating — a FILE +/// routes through [`composite_from_authored_text`] instead; a content id +/// (STORE read-back) keeps the ungated `composite_from_any` (C29). fn params_lines(target: &str, env: &aura_runner::project::Env) -> Result { use std::fmt::Write as _; - let text = resolve_blueprint_text(target, env)?; + let (text, is_file) = resolve_blueprint_text(target, env)?; // Shape-discriminated like file-mode --content-id: an op-script (array) // builds through the one-build tail, an envelope (object) loads (#202). - let composite = composite_from_any(&text, env)?; + let composite = if is_file { composite_from_authored_text(&text, env)? } else { composite_from_any(&text, env)? }; let mut out = String::new(); for p in composite.param_space() { let _ = writeln!(out, "{}:{:?}", p.name, p.kind); @@ -918,7 +1016,9 @@ fn register_blueprint( .map_err(|e| format!("cannot read {}: {e}", file.display()))?; // Shape-discriminated like file-mode --content-id (#202): either shape // canonicalizes to the envelope form, so the stored id is shape-invariant. - let composite = composite_from_any(&text, env)?; + // Gated (#331 review finding): this text is freshly authored FILE bytes, + // not a store read-back, so `composite_from_authored_text` applies. + let composite = composite_from_authored_text(&text, env)?; let canonical = blueprint_to_json(&composite).map_err(|e| format!("serialize error: {e:?}"))?; let id = crate::content_id(&canonical); let registry = env.registry(); diff --git a/crates/aura-cli/src/main.rs b/crates/aura-cli/src/main.rs index 37573f6..844441b 100644 --- a/crates/aura-cli/src/main.rs +++ b/crates/aura-cli/src/main.rs @@ -1884,6 +1884,19 @@ fn dispatch_run(a: RunCmd, env: &aura_runner::project::Env) { eprintln!("aura: {path}: {msg}"); std::process::exit(2); }); + // #331 delta re-review: this is the third freshly-authored-FILE + // intake (`register`, `introspect --content-id `, and this + // one) that must gate the root name before it ever reaches the + // filesystem — both run arms below feed `signal.name()` straight + // into `bind_tap_plan`'s `TraceStore::begin_run`, which joins it + // unsanitized (`trace_store.rs`). Not routed through + // `composite_from_authored_text` itself (see that fn's doc + // comment on why); same `name_gate`/`name_gate_fault_prose` + // primitives, this arm's own load-error exit/prose convention. + if let Err(msg) = graph_construct::gate_authored_root_name(signal.name()) { + eprintln!("aura: {path}: {msg}"); + std::process::exit(2); + } // Shape dispatch (C28 phase 3): a `bias` output → the strategy path // (wrap_r + R evaluation), byte-identical; else ≥1 declared tap → a // bare measurement run (no wrap_r); else an inert blueprint → refuse. @@ -1981,7 +1994,7 @@ fn dispatch_graph(a: GraphCmd, env: &aura_runner::project::Env) { eprintln!("aura: {path}: {e}"); std::process::exit(2); }); - let bp = graph_construct::composite_from_any(&doc, env).unwrap_or_else(|msg| { + let bp = graph_construct::composite_from_authored_text(&doc, env).unwrap_or_else(|msg| { eprintln!("aura: {path}: {msg}"); std::process::exit(2); }); diff --git a/crates/aura-cli/tests/graph_construct.rs b/crates/aura-cli/tests/graph_construct.rs index 7c3575e..7d6c175 100644 --- a/crates/aura-cli/tests/graph_construct.rs +++ b/crates/aura-cli/tests/graph_construct.rs @@ -88,6 +88,43 @@ const SIGNAL_DOC_NAME_RESTATED: &str = r#"[ {"op":"expose","from":"bias.bias","as":"bias"} ]"#; +/// SIGNAL_DOC with a leading `{"op":"name",...}` (#331) — authors a render +/// name distinct from the CLI's own default seed ("graph"), otherwise +/// structurally identical to SIGNAL_DOC. +const SIGNAL_DOC_NAMED: &str = r#"[ + {"op":"name","name":"ny_momentum"}, + {"op":"source","role":"price","kind":"F64"}, + {"op":"add","type":"SMA","name":"fast","bind":{"length":{"I64":2}}}, + {"op":"add","type":"SMA","name":"slow","bind":{"length":{"I64":4}}}, + {"op":"add","type":"Sub"}, + {"op":"add","type":"Bias"}, + {"op":"feed","role":"price","into":["fast.series","slow.series"]}, + {"op":"connect","from":"fast.value","to":"sub.lhs"}, + {"op":"connect","from":"slow.value","to":"sub.rhs"}, + {"op":"connect","from":"sub.value","to":"bias.signal"}, + {"op":"expose","from":"bias.bias","as":"bias"} +]"#; + +/// SIGNAL_DOC_NAME_RESTATED with an authored name (#331 spec test 7) instead +/// of the CLI's default seed: the `doc` op's text restates the *authored* +/// name ("ny_momentum") rather than the literal "graph" — the same +/// RestatesName arm of the C29 shape gate must fire against whichever name +/// the composite actually carries, not a hardcoded "graph" comparison. +const SIGNAL_DOC_NAMED_NAME_RESTATED: &str = r#"[ + {"op":"name","name":"ny_momentum"}, + {"op":"doc","text":"Ny Momentum"}, + {"op":"source","role":"price","kind":"F64"}, + {"op":"add","type":"SMA","name":"fast","bind":{"length":{"I64":2}}}, + {"op":"add","type":"SMA","name":"slow","bind":{"length":{"I64":4}}}, + {"op":"add","type":"Sub"}, + {"op":"add","type":"Bias"}, + {"op":"feed","role":"price","into":["fast.series","slow.series"]}, + {"op":"connect","from":"fast.value","to":"sub.lhs"}, + {"op":"connect","from":"slow.value","to":"sub.rhs"}, + {"op":"connect","from":"sub.value","to":"bias.signal"}, + {"op":"expose","from":"bias.bias","as":"bias"} +]"#; + /// Every op below applies cleanly (all eager checks pass), but `sub.rhs` is /// never wired — a 0-cover input slot only the holistic finalize gate can /// reject. The smallest document that is op-valid yet whole-document-invalid. @@ -929,6 +966,120 @@ fn graph_register_refuses_an_op_script_doc_that_restates_the_composite_name() { assert!(err.contains("C29"), "stderr cites the contract: {err}"); } +/// (#331 spec test 7) The C29 restates-name gate checks against the +/// *authored* name, not a hardcoded default: a `doc` op restating the name +/// set by a `name` op ("ny_momentum") refuses at register exactly like the +/// default-name case above (`graph_register_refuses_an_op_script_doc_that_ +/// restates_the_composite_name`, which must stay green byte-identical). +#[test] +fn graph_register_refuses_a_doc_that_restates_an_authored_name() { + let dir = temp_cwd("register-op-script-restated-authored-name-doc"); + let restated_script = dir.join("restated-authored-name-doc-script.json"); + std::fs::write(&restated_script, SIGNAL_DOC_NAMED_NAME_RESTATED) + .expect("write op-script fixture"); + let (out, err, code) = run_in(&dir, &["graph", "register", restated_script.to_str().unwrap()]); + assert_eq!(code, Some(1), "authored-name-restating doc refuses: {out} {err}"); + assert!(err.contains("merely restates"), "stderr names the rule: {err}"); + assert!(err.contains("C29"), "stderr cites the contract: {err}"); +} + +/// (#331 spec test 6) `graph build` on an ops doc with a `name` op emits +/// blueprint JSON carrying the authored name, not the CLI's own seeded +/// default (`"graph"`, `replay("graph", ...)` in `composite_from_str`). +#[test] +fn graph_build_emits_the_authored_name_from_a_name_op() { + let (stdout, stderr, ok) = run(&["graph", "build"], SIGNAL_DOC_NAMED); + assert!(ok, "build succeeds: {stderr}"); + assert!(stdout.contains("\"name\":\"ny_momentum\""), "authored name carries into the blueprint: {stdout}"); + assert!(!stdout.contains("\"name\":\"graph\""), "the default seed name must not leak in: {stdout}"); +} + +/// (#331 spec test 8) Refusal prose for a duplicate `name` op and a +/// shape-violating name is op-indexed (`op N (name): ...`) — the same +/// by-identifier labeling every other op fault gets. +#[test] +fn graph_build_name_op_refusals_are_op_indexed() { + let twice_named = r#"[ + {"op":"source","role":"price","kind":"F64"}, + {"op":"add","type":"SMA","name":"fast","bind":{"length":{"I64":2}}}, + {"op":"name","name":"a"}, + {"op":"name","name":"b"}, + {"op":"feed","role":"price","into":["fast.series"]}, + {"op":"expose","from":"fast.value","as":"out"} + ]"#; + let (_out, stderr, ok) = run(&["graph", "build"], twice_named); + assert!(!ok, "a second name op refuses"); + assert!(stderr.contains("op 3 (name):"), "op-indexed at the second name op: {stderr}"); + + let slash_named = r#"[{"op":"name","name":"a/b"}]"#; + let (_out2, stderr2, ok2) = run(&["graph", "build"], slash_named); + assert!(!ok2, "a shape-violating name refuses"); + assert!(stderr2.contains("op 0 (name):"), "op-indexed at the offending op: {stderr2}"); +} + +/// (#331 spec test 9, at the binary seam — mirrors +/// `graph_introspect_identity_id_bridges_renamed_op_scripts`'s pattern for +/// the debug-name field): a named and an unnamed build of the SAME +/// structural script share the identity id (the name is stripped as a +/// debug symbol before identity hashing, C23) and differ in the content id +/// (the name is part of the serialized bytes). +#[test] +fn graph_named_and_unnamed_twins_share_identity_id_and_differ_content_id() { + let (ia, _e, ok) = run(&["graph", "introspect", "--identity-id"], SIGNAL_DOC); + assert!(ok, "exit success"); + let (ib, _e2, ok2) = run(&["graph", "introspect", "--identity-id"], SIGNAL_DOC_NAMED); + assert!(ok2, "exit success"); + assert_eq!(ia.trim(), ib.trim(), "the name is stripped before identity hashing (C23)"); + + let (ca, _e3, ok3) = run(&["graph", "introspect", "--content-id"], SIGNAL_DOC); + let (cb, _e4, ok4) = run(&["graph", "introspect", "--content-id"], SIGNAL_DOC_NAMED); + assert!(ok3 && ok4, "exit success"); + assert_ne!(ca.trim(), cb.trim(), "the name is part of the serialized bytes -> different content id"); +} + +/// (#331 spec test 10, the skeptic's bypass route): the op intake +/// (`GraphSession::set_name`) is not the name's only data-borne birth +/// route — a hand-edited blueprint ENVELOPE (not an op-script) can carry a +/// shape-violating root name directly. `aura graph register` on such a file +/// is refused naming the offending name (exit 1, the refusal convention +/// every sibling test in this module asserts), not silently stored under a +/// name that would write `traces/../x/` at run time. Shipped envelope +/// fixtures with plain root names (`doc_blueprint.json`'s "sig", +/// `nested_doc_blueprint.json`'s "root", every op-script's default "graph") +/// stay accepted — the surrounding suite is that regression gate. +/// +/// Spec test 10 also names `build` as a gated envelope-intake route +/// alongside `register`. The `graph build` SUBCOMMAND takes only a stdin +/// op-script (`Vec`, never a JSON object) — it cannot ingest this +/// envelope fixture at all (a shape mismatch fires before any name is ever +/// read). The file-consuming envelope-intake sibling `build` route is +/// `graph introspect --content-id ` (its own doc comment: "one +/// build, then each requested id…") — the other call site this iteration +/// moved onto the same `composite_from_authored_text` choke point as +/// `register`, so it is the leg exercised here, sharing the identical +/// `name_gate_fault_prose` text. +#[test] +fn graph_build_and_register_refuse_a_hand_crafted_envelope_with_a_bad_root_name() { + let dir = temp_cwd("register-bad-root-name"); + let (envelope_bytes, _e, built) = run(&["graph", "build"], SIGNAL_DOC_DESCRIBED); + assert!(built, "graph build produces the envelope"); + let bad = envelope_bytes.replacen("\"name\":\"graph\"", "\"name\":\"../x\"", 1); + assert!(bad.contains("\"name\":\"../x\""), "the root-name replace landed: {bad}"); + let file = dir.join("bad-root-name.json"); + std::fs::write(&file, &bad).expect("write envelope fixture"); + + let (out, err, code) = run_in(&dir, &["graph", "register", file.to_str().unwrap()]); + assert_eq!(code, Some(1), "a shape-violating root name refuses (not a panic): {out} {err}"); + assert!(err.contains("../x"), "names the offending root name: {err}"); + + // The other file-consuming envelope intake (same fixture, same choke + // point) refuses identically. + let (out2, err2, code2) = + run_in(&dir, &["graph", "introspect", "--content-id", file.to_str().unwrap()]); + assert_eq!(code2, Some(1), "the sibling envelope intake refuses too: {out2} {err2}"); + assert_eq!(err2, err, "both file-consuming envelope intakes share the identical fault prose"); +} + /// Property (#202, the same on-ramp seam at the sibling verb): `aura graph /// introspect --params` accepts an op-script exactly as it accepts a blueprint /// envelope — the raw axis namespace it prints for the op-script equals the one it @@ -954,6 +1105,35 @@ fn graph_params_accepts_an_op_script_matching_its_envelope() { assert_eq!(op_out, env_out, "op-script and envelope agree on the raw axis namespace"); } +/// (#331 review finding, the C29 exemption's ratifying pin): the root-name +/// shape gate lives at the CLI's file-consuming envelope intakes (`register`, +/// `introspect --content-id `) — never at STORE read-back. A blueprint +/// with a shape-violating root name registered DIRECTLY through the +/// `aura-registry` store API (bypassing the CLI's gate entirely, the way a +/// pre-#331 artifact would already sit in a project's store) must still be +/// introspectable: `introspect --params ` reads the store, not a file, so +/// it must never re-punish an artifact the store already accepted — C29, +/// "registered artifacts are never retroactively invalidated". +#[test] +fn graph_introspect_params_accepts_a_pre_331_store_artifact_with_a_bad_root_name() { + let dir = temp_cwd("introspect-store-readback-bad-root-name"); + let (envelope_bytes, _e, built) = run(&["graph", "build"], SIGNAL_DOC_DESCRIBED); + assert!(built, "graph build produces the envelope"); + let bad = envelope_bytes.replacen("\"name\":\"graph\"", "\"name\":\"../x\"", 1); + assert!(bad.contains("\"name\":\"../x\""), "the root-name replace landed: {bad}"); + + // Bypass the CLI's gate entirely: the registry's own store API is the + // simulated pre-#331 write path. + let id = aura_research::content_id_of(&bad); + let registry = aura_registry::Registry::open(dir.join("runs/runs.jsonl")); + registry + .put_blueprint(&id, &bad) + .expect("the registry's own store API has no root-name gate to bypass"); + + let (out, err, code) = run_in(&dir, &["graph", "introspect", "--params", &id]); + assert_eq!(code, Some(0), "store read-back stays ungated (C29): {out} {err}"); +} + /// Property (#226, the render positional closes the #202 on-ramp family): /// `aura graph ` (the render positional) accepts an op-script /// document (a JSON array) exactly as it accepts a #155 blueprint envelope (a diff --git a/crates/aura-cli/tests/help_self_description.rs b/crates/aura-cli/tests/help_self_description.rs index 54477e5..72ab508 100644 --- a/crates/aura-cli/tests/help_self_description.rs +++ b/crates/aura-cli/tests/help_self_description.rs @@ -58,7 +58,8 @@ fn sugar_verbs_name_their_document_shape() { /// #323: `graph build --help` carries the op-list reference — the op kinds /// and fields are learnable from the binary, not only from serde refusals. -/// #317: the `use` op joins the roster (nine -> ten). +/// #317: the `use` op joins the roster (nine -> ten). #331: the `name` op +/// joins the roster (ten -> eleven). #[test] fn graph_build_help_carries_the_op_reference() { let (out, err, ok) = run(&["graph", "build", "--help"]); @@ -74,6 +75,7 @@ fn graph_build_help_carries_the_op_reference() { r#"{"op":"gang""#, r#"{"op":"doc""#, r#"{"op":"use""#, + r#"{"op":"name""#, ] { assert!(out.contains(op), "op reference carries {op}: {out}"); } diff --git a/crates/aura-cli/tests/run_measurement.rs b/crates/aura-cli/tests/run_measurement.rs index 375da92..a11b8e6 100644 --- a/crates/aura-cli/tests/run_measurement.rs +++ b/crates/aura-cli/tests/run_measurement.rs @@ -81,3 +81,60 @@ fn measurement_run_honours_the_tap_selector() { let expected = (1.0097_f64 + 1.0092) / 2.0; assert!((got - expected).abs() < 1e-9, "last row: got {got}, expected {expected}"); } + +/// #331 delta re-review, the third authored-file intake route: `dispatch_run` +/// loads a FRESH FILE via `blueprint_from_json` and, on this measurement arm, +/// feeds `signal.name()` straight into `bind_tap_plan` -> `TraceStore::begin_run` +/// (`trace_store.rs` joins the name unsanitized) — a hand-crafted envelope with +/// `"name":"../x"` used to reach `--tap fast_tap=record` and escape +/// `runs/traces//` for `runs/x/` (one level up, since `traces/../x` +/// normalizes there) instead of refusing at the same root-name choke point +/// `register`/`introspect --content-id ` already gate through. This pins +/// the closed route: exit nonzero (dispatch_run's own bad-input-file +/// convention, exit 2), the shared `name_gate_fault_prose` wording, and — the +/// property that actually matters — the escaped directory is never created. +#[test] +fn run_refuses_a_hand_crafted_envelope_with_a_bad_root_name_before_any_trace_write() { + let cwd = temp_cwd("bad-root-name"); + let mut v: serde_json::Value = + serde_json::from_str(&measurement_blueprint_json()).expect("parse measurement blueprint"); + v["blueprint"]["name"] = serde_json::json!("../x"); + let bad = serde_json::to_string(&v).expect("re-serialize with a shape-violating root name"); + let bp_path = cwd.join("bad-root-name.json"); + std::fs::write(&bp_path, &bad).expect("write bad-root-name blueprint"); + + let out = Command::new(BIN) + .args(["run", bp_path.to_str().expect("utf-8 path"), "--tap", "fast_tap=record"]) + .current_dir(&cwd) + .output() + .expect("spawn aura run"); + assert_eq!( + out.status.code(), + Some(2), + "a shape-violating root name refuses at dispatch_run's own bad-file exit code: {:?} stderr={}", + out.status, + String::from_utf8_lossy(&out.stderr) + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains(r#"blueprint name "../x" is invalid"#), + "the shared name_gate_fault_prose wording names the offending root name: {stderr}" + ); + assert!( + out.stdout.is_empty(), + "a refused run must not print a report: {:?}", + String::from_utf8_lossy(&out.stdout) + ); + + // The property that actually matters: the escaped write target (one level + // up from `traces/`, since `trace_store.rs` joins the name unsanitized) + // was never created — the gate fires before `begin_run` ever touches disk. + assert!( + !cwd.join("runs/x").exists(), + "the escaped trace directory must never be created" + ); + assert!( + !cwd.join("runs/traces").exists(), + "no trace directory of any shape is written on a refused run" + ); +} diff --git a/crates/aura-engine/src/blueprint.rs b/crates/aura-engine/src/blueprint.rs index d4513ec..a034729 100644 --- a/crates/aura-engine/src/blueprint.rs +++ b/crates/aura-engine/src/blueprint.rs @@ -195,6 +195,38 @@ pub struct GangMember { pub name: String, } +/// Deterministic shape gate for authored composite render names (#331) — +/// shape only, never content judgement (C29 discipline): non-empty, no path +/// separator (`/` or `\`), and not the special segments `.` or `..`. The rule +/// exists because the name keys a trace directory on disk unsanitized +/// (`traces//`, `crates/aura-registry/src/trace_store.rs`). Applied at +/// both data-borne birth routes for a name (the skeptic's two-seam finding): +/// the `Op::Name` op intake (`construction.rs`) and the CLI's +/// blueprint-envelope root-name intake — never at store read-back (C29: +/// registered artifacts are never retroactively invalidated), and never on +/// the Rust builder API (`GraphBuilder::new`, role-2 native authoring). +pub fn name_gate(name: &str) -> Result<(), NameGateFault> { + if name.is_empty() { + return Err(NameGateFault::Empty); + } + if name.contains('/') || name.contains('\\') { + return Err(NameGateFault::ContainsSeparator); + } + if name == "." || name == ".." { + return Err(NameGateFault::DotSegment); + } + Ok(()) +} + +/// The [`name_gate`] shape violation — WHY the name was refused; the caller +/// already holds the offending name (WHAT) to build a message from. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum NameGateFault { + Empty, + ContainsSeparator, + DotSegment, +} + /// A reusable sub-graph fragment compiled away by inlining (C9/C23). It is **not** /// a [`Node`]: it is never `eval`'d. It holds interior items (local indices), /// interior edges (local indices), input roles (role `r` fans into the interior @@ -1519,6 +1551,19 @@ mod tests { use aura_strategy::Bias; use std::sync::mpsc; + /// `name_gate` (#331) unit table: an ordinary name passes; an empty + /// name, one containing either path separator, and the two dot segments + /// each fault with their own fault variant. + #[test] + fn name_gate_accepts_ordinary_names_and_refuses_shape_violations() { + assert_eq!(name_gate("ny_momentum"), Ok(())); + assert_eq!(name_gate(""), Err(NameGateFault::Empty)); + assert_eq!(name_gate("a/b"), Err(NameGateFault::ContainsSeparator)); + assert_eq!(name_gate("a\\b"), Err(NameGateFault::ContainsSeparator)); + assert_eq!(name_gate("."), Err(NameGateFault::DotSegment)); + assert_eq!(name_gate(".."), Err(NameGateFault::DotSegment)); + } + /// One knob fanning into two sibling open params passes the gate; the /// value carries the gang table. #[test] diff --git a/crates/aura-engine/src/construction.rs b/crates/aura-engine/src/construction.rs index 3190baa..cd1dd1b 100644 --- a/crates/aura-engine/src/construction.rs +++ b/crates/aura-engine/src/construction.rs @@ -16,8 +16,8 @@ use std::collections::{HashMap, HashSet}; use aura_core::{ArgOpError, BindOpError, NodeSchema, PrimitiveBuilder, Scalar, ScalarKind}; use crate::blueprint::{ - check_param_namespace_injective, edge_kind_check, validate_wiring, BlueprintNode, Composite, - Gang, GangMember, OutField, Role, Tap, TapWire, + check_param_namespace_injective, edge_kind_check, name_gate, validate_wiring, BlueprintNode, + Composite, Gang, GangMember, NameGateFault, OutField, Role, Tap, TapWire, }; use crate::builder::{resolve_input_slot, resolve_output_field, PortResolveError}; use crate::harness::{Edge, Target}; @@ -64,6 +64,8 @@ pub enum Op { /// `bind` is applied path-qualified (e.g. `"sma.length"`), after the /// splice. Use { ref_id: String, name: Option, bind: Vec<(String, Scalar)> }, + /// Set the composite's render name (#331). At most once per script. + Name { name: String }, } /// A per-op construction fault, by-identifier so the cause names the op. @@ -124,6 +126,13 @@ pub enum OpError { /// resolves and fetches before replay, refusing there on a miss), but /// total: a bare `replay`/`GraphSession` caller can still hit it. UnknownSubgraph { ref_id: String }, + /// A second `name` op — the render name is declared at most once (mirrors + /// `DuplicateDoc`). + DuplicateName, + /// A `name` op's `name` (or, at the CLI's blueprint-envelope intake, an + /// envelope's root name) failed the deterministic `name_gate` shape check + /// (#331). Carries the offending name and the shape fault detail. + BadName { name: String, fault: NameGateFault }, } /// A per-op-fallible blueprint accumulator. Holds the same interior data a @@ -153,6 +162,11 @@ pub struct GraphSession<'v> { coverage: HashMap<(usize, usize), usize>, gangs: Vec, doc: Option, + /// At-most-once guard for `Op::Name` (#331), mirroring `doc`'s own + /// uniqueness posture but tracked separately (`name` itself is always + /// populated, seeded from `::new`'s `name` argument, so it cannot double + /// as its own "was it authored" flag). + name_authored: bool, } impl<'v> GraphSession<'v> { @@ -182,6 +196,7 @@ impl<'v> GraphSession<'v> { coverage: HashMap::new(), gangs: Vec::new(), doc: None, + name_authored: false, } } @@ -205,9 +220,29 @@ impl<'v> GraphSession<'v> { Ok(()) } Op::Use { ref_id, name, bind } => self.use_subgraph(ref_id, name, bind), + Op::Name { name } => self.set_name(name), } } + /// `Op::Name` (#331): set the composite's render name — the op-script's + /// data-borne twin of `replay`'s seeded default name. Guarded by (a) + /// at-most-once (mirrors the `doc` op's uniqueness guard just above) and + /// (b) the shared deterministic shape gate (`name_gate`) — the SAME gate + /// the CLI's blueprint-envelope intake applies to a hand-edited + /// envelope's root name (the other data-borne birth route, skeptic + /// finding minuted on #331). + fn set_name(&mut self, name: String) -> Result<(), OpError> { + if self.name_authored { + return Err(OpError::DuplicateName); + } + if let Err(fault) = name_gate(&name) { + return Err(OpError::BadName { name, fault }); + } + self.name = name; + self.name_authored = true; + Ok(()) + } + /// `Op::Use` (#317): fetch a registered blueprint through the injected /// `subgraph` resolver, rename it to the instance identifier, apply its /// path-qualified `bind` entries, and push it as an ordinary @@ -1002,6 +1037,72 @@ mod tests { ); } + // ---- Op::Name (#331) ----------------------------------------------- + + /// A `name` op sets the finished composite's render name — the property + /// the whole op exists for. + #[test] + fn name_op_sets_the_finished_composite_name() { + let mut s = scaffold(); + s.apply(Op::Name { name: "x".into() }).unwrap(); + s.apply(Op::Feed { role: "price".into(), into: vec!["fast.series".into(), "slow.series".into()] }) + .unwrap(); + s.apply(Op::Connect { from: "fast.value".into(), to: "sub.lhs".into() }).unwrap(); + s.apply(Op::Connect { from: "slow.value".into(), to: "sub.rhs".into() }).unwrap(); + s.apply(Op::Expose { from: "sub.value".into(), as_name: "out".into() }).unwrap(); + let c = s.finish().expect("finishes"); + assert_eq!(c.name(), "x"); + } + + /// Byte-compat (#331 AC1): a script that never authors a `name` op keeps + /// the seed name `replay`/`GraphSession::new` was given — the CLI's own + /// seed stays `"graph"`, unchanged by this op's addition. + #[test] + fn no_name_op_keeps_the_seeded_replay_name() { + let ops = vec![ + Op::Source { role: "price".into(), kind: ScalarKind::F64 }, + Op::Add { type_id: "SMA".into(), as_name: Some("a".into()), args: vec![], bind: vec![] }, + Op::Feed { role: "price".into(), into: vec!["a.series".into()] }, + Op::Expose { from: "a.value".into(), as_name: "out".into() }, + ]; + let c = super::replay("graph", ops, &std_vocabulary, &|_: &str| None).expect("replays"); + assert_eq!(c.name(), "graph"); + } + + /// A second `name` op is a fault, not a silent last-wins overwrite — + /// mirrors the `doc` op's own at-most-once guard. + #[test] + fn duplicate_name_op_is_refused() { + let mut s = session("g"); + s.apply(Op::Name { name: "x".into() }).unwrap(); + assert_eq!( + s.apply(Op::Name { name: "y".into() }), + Err(OpError::DuplicateName) + ); + } + + /// Shape violations (#331): empty, containing a path separator, and the + /// dot segments all fault at the op via the shared `name_gate`. + #[test] + fn name_op_shape_violations_are_refused() { + use crate::blueprint::NameGateFault; + let mut s = session("g"); + assert_eq!( + s.apply(Op::Name { name: "".into() }), + Err(OpError::BadName { name: "".into(), fault: NameGateFault::Empty }) + ); + let mut s = session("g"); + assert_eq!( + s.apply(Op::Name { name: "a/b".into() }), + Err(OpError::BadName { name: "a/b".into(), fault: NameGateFault::ContainsSeparator }) + ); + let mut s = session("g"); + assert_eq!( + s.apply(Op::Name { name: "..".into() }), + Err(OpError::BadName { name: "..".into(), fault: NameGateFault::DotSegment }) + ); + } + #[test] fn finish_reports_incomplete_on_unwired_slot() { // a fully-named scaffold missing the sub.rhs connection -> holistic 0-arm @@ -1501,6 +1602,30 @@ mod tests { ); } + /// #331 ratifying pin (spec test 5): `use_subgraph`'s instance-identifier + /// default (`name: None` -> the fetched composite's own `name()`, + /// `construction.rs:221-230` — code-inspected, previously untested since + /// every other test in this section passes an explicit instance name) + /// actually reaches the session's identifier namespace: `use_fixture()`'s + /// own name is "fixture", so with no instance name the spliced ports + /// resolve under "fixture.*", not some other placeholder. + #[test] + fn use_op_with_no_name_defaults_instance_identifier_to_the_fetched_composites_name() { + let subgraph = |_: &str| Some(use_fixture()); + let mut s = GraphSession::new("g", &std_vocabulary, &subgraph); + s.apply(Op::Source { role: "price".into(), kind: ScalarKind::F64 }).unwrap(); + assert_eq!( + s.apply(Op::Use { ref_id: "fixture-id".into(), name: None, bind: vec![] }), + Ok(()) + ); + s.apply(Op::Feed { role: "price".into(), into: vec!["fixture.price".into()] }).unwrap(); + assert_eq!( + s.apply(Op::Expose { from: "fixture.out".into(), as_name: "bias".into() }), + Ok(()), + "the spliced instance resolves under \"fixture\" — the fetched composite's own name" + ); + } + /// A post-splice `bind` path with no matching open param anywhere in the /// instance faults through the existing bind-fault shape (`BadParam`), /// naming the instance and the qualified within-instance path (#317). diff --git a/crates/aura-engine/src/lib.rs b/crates/aura-engine/src/lib.rs index 820977f..98b1f39 100644 --- a/crates/aura-engine/src/lib.rs +++ b/crates/aura-engine/src/lib.rs @@ -52,8 +52,9 @@ mod sweep; mod walkforward; pub use blueprint::{ - BindError, Binder, BlueprintNode, BoundSpec, CompileError, Composite, Gang, GangFault, - GangMember, OutField, RandomBinder, ReopenError, Role, SweepBinder, Tap, TapWire, + name_gate, BindError, Binder, BlueprintNode, BoundSpec, CompileError, Composite, Gang, + GangFault, GangMember, NameGateFault, OutField, RandomBinder, ReopenError, Role, SweepBinder, + Tap, TapWire, }; pub use blueprint_serde::{ blueprint_from_json, blueprint_identity_json, blueprint_to_json, ArgData, BlueprintDoc,