From c424e2e9725a3c98603c39d855c0ae8b94511273 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Wed, 8 Jul 2026 14:17:17 +1000 Subject: [PATCH 01/15] build: harden v3 dep ordering (LC_ALL=C sort, cross-platform cycle gate, drop tac, anchored strip) --- tasks/build.sh | 54 +++++++++++++---------- tasks/build/ordering.sh | 51 +++++++++++++++++++++ tasks/test/build_ordering_helpers_test.sh | 36 +++++++++++++++ 3 files changed, 118 insertions(+), 23 deletions(-) create mode 100644 tasks/build/ordering.sh create mode 100644 tasks/test/build_ordering_helpers_test.sh diff --git a/tasks/build.sh b/tasks/build.sh index 3b3625605..7c3ed4f05 100755 --- a/tasks/build.sh +++ b/tasks/build.sh @@ -9,10 +9,14 @@ set -euo pipefail +source tasks/build/ordering.sh + # Regenerate encrypted-domain SQL from the Rust catalog before building. -# Generated files (src/v3/scalars//_*.sql) are gitignored; the -# catalog at crates/eql-domains/src (eql-domains::CATALOG) is the source of -# truth, rendered by the eql-codegen binary. +# The generated files (src/v3/scalars//_*.sql) are COMMITTED in place and +# drift-gated by `mise run codegen:parity`; only src/v3/version.sql and the +# src/deps*-v3.txt build intermediates are gitignored. The catalog at +# crates/eql-domains/src (eql-domains::CATALOG) is the source of truth, rendered +# by the eql-codegen binary. # # eql-codegen owns orphan removal: it writes every current file first (each via # an atomic temp+rename), then prunes stale generated SQL across ALL @@ -49,13 +53,13 @@ verify_deps_exist() { # must be self-contained (no eql_v2 coupling); a stray `-- REQUIRE: src/...` # edge to a non-v3 file would silently pull eql_v2 SQL into the v3 artefact (or # tsort would drop it), breaking self-containment. Each line in deps-v3.txt is -# " "; self-edges (file == dep) are skipped, every other dep target -# must start with src/v3/. +# " " (dependency FIRST); self-edges (dep == file) are skipped, every +# other dep target (field 1) must start with src/v3/. verify_v3_self_contained() { local dep_file=$1 local offending=0 - while IFS=' ' read -r src dep; do - [[ -z "$dep" ]] && continue + while IFS=' ' read -r dep src; do + [[ -z "$src" ]] && continue [[ "$src" == "$dep" ]] && continue if [[ "$dep" != src/v3/* ]]; then echo "ERROR: v3 REQUIRE edge points outside src/v3: $src -- REQUIRE: $dep" >&2 @@ -92,27 +96,31 @@ sed "s/\$RELEASE_VERSION/$RELEASE_VERSION/g" src/v3/version.template > src/v3/ve # dependency (CI-gated by verify_v3_self_contained below + test:self_contained_v3), # and it is written under the canonical release name now that the combined v2 # build that previously produced that name is gone. -find src/v3 -type f -path "*.sql" ! -path "*_test.sql" | while IFS= read -r sql_file; do - echo "$sql_file" - - echo "$sql_file $sql_file" >> src/deps-v3.txt - - while IFS= read -r line; do - if [[ "$line" == *"-- REQUIRE:"* ]]; then - deps=${line#*-- REQUIRE: } - for dep in $deps; do - echo "$sql_file $dep" >> src/deps-v3.txt - done +find src/v3 -type f -path "*.sql" ! -path "*_test.sql" -print0 \ + | LC_ALL=C sort -z \ + | while IFS= read -r -d '' sql_file; do + # self-edge: isolated files still appear in tsort output. A self-edge is a + # tsort no-op, not a cycle (see run_tsort_or_die), so it is safe here. + echo "$sql_file $sql_file" >> src/deps-v3.txt + while IFS= read -r line; do + if [[ "$line" =~ ^[[:space:]]*--\ REQUIRE: ]]; then + deps=${line#*-- REQUIRE: } + for dep in $deps; do + echo "$dep $sql_file" >> src/deps-v3.txt # dependency FIRST (no tac) + done fi - done < "$sql_file" -done + done < "$sql_file" + done verify_v3_self_contained src/deps-v3.txt - -cat src/deps-v3.txt | tsort | tac > src/deps-ordered-v3.txt +run_tsort_or_die src/deps-v3.txt src/deps-ordered-v3.txt verify_deps_exist src/deps-ordered-v3.txt +verify_linearization src/deps-v3.txt src/deps-ordered-v3.txt -cat src/deps-ordered-v3.txt | xargs cat | grep -v REQUIRE >> release/cipherstash-encrypt.sql +: > release/cipherstash-encrypt.sql +while IFS= read -r f; do + strip_require_lines "$f" >> release/cipherstash-encrypt.sql +done < src/deps-ordered-v3.txt cat tasks/pin_search_path_v3.sql >> release/cipherstash-encrypt.sql cat tasks/uninstall-v3.sql >> release/cipherstash-encrypt-uninstall.sql diff --git a/tasks/build/ordering.sh b/tasks/build/ordering.sh new file mode 100644 index 000000000..e94ec3ae1 --- /dev/null +++ b/tasks/build/ordering.sh @@ -0,0 +1,51 @@ +#!/usr/bin/env bash +# Sourceable dependency-ordering helpers for the eql_v3 build. No side effects on +# source; each function is pure w.r.t. its args. Shared with the staged-installer +# refactor (do not fork strip_require_lines). + +# Emit a file's body with anchored `-- REQUIRE:` directive lines removed. Anchored +# (allows leading whitespace) so a body line that merely contains the substring +# "REQUIRE" survives — unlike the old unanchored `grep -v REQUIRE`. +strip_require_lines() { + grep -vE '^[[:space:]]*-- REQUIRE:' "$1" || true +} + +# tsort with a cross-platform cycle gate. Input edges are " " (one per +# line, dependency FIRST) so plain tsort yields dependency-before-file order with +# no `tac`. BSD/macOS tsort exits 0 on a cycle but writes "tsort: cycle in data" +# to stderr; GNU exits 1. We fail on ANY tsort stderr, catching cycles on both. +# NOTE: the extraction emits a self-edge " " for every file so an +# isolated file still appears in the output. A self-edge is a tsort NO-OP, NOT a +# cycle — `printf 'a a\n' | tsort` prints `a` with empty stderr and exit 0 on +# both BSD and GNU, so self-edges never trip this stderr-based cycle-fail. +run_tsort_or_die() { + local edges=$1 out=$2 err + err="$(mktemp)" + tsort "$edges" > "$out" 2> "$err" || true + if [[ -s "$err" ]]; then + echo "ERROR: tsort reported a problem ordering $edges (cycle or malformed edge):" >&2 + cat "$err" >&2 + rm -f "$err" + return 1 + fi + rm -f "$err" +} + +# Assert the assembled order is a valid linearization: for every " " +# edge, dep must appear at or before file in . Self-edges and edges +# whose endpoints are absent from the order are skipped (absence is caught by +# verify_deps_exist). Complements run_tsort_or_die's cycle check with a direct +# check on the FINAL order (the two-phase concat is not produced by one tsort). +verify_linearization() { + local edges=$1 ordered=$2 + awk ' + NR==FNR { pos[$0]=FNR; next } + { if ($1=="" || $1==$2) next + if (!($1 in pos) || !($2 in pos)) next + if (pos[$1] > pos[$2]) { + printf("ERROR: %s is required by %s but is ordered AFTER it\n", $1, $2) > "/dev/stderr" + bad=1 + } } + END { exit bad ? 1 : 0 } + ' "$ordered" "$edges" +} diff --git a/tasks/test/build_ordering_helpers_test.sh b/tasks/test/build_ordering_helpers_test.sh new file mode 100644 index 000000000..b86294d70 --- /dev/null +++ b/tasks/test/build_ordering_helpers_test.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +#MISE description="DB-free unit tests for tasks/build/ordering.sh (cycle gate, edge reversal, anchored strip)" +set -euo pipefail +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +cd "$REPO_ROOT" +source tasks/build/ordering.sh + +tmp="$(mktemp -d)"; trap 'rm -rf "$tmp"' EXIT + +# 1. Cycle gate must FAIL on both BSD and GNU tsort (BSD exits 0 but prints to stderr). +printf 'a b\nb a\n' > "$tmp/cyc.txt" +if run_tsort_or_die "$tmp/cyc.txt" "$tmp/cyc.out" 2>/dev/null; then + echo "FAIL: cycle not rejected"; exit 1 +fi +echo "ok: cycle rejected" + +# 2. Edge reversal: dependency-first edges yield dependency-before-file order (no tac). +printf 'schema.sql types.sql\ntypes.sql ops.sql\n' > "$tmp/ok.txt" +run_tsort_or_die "$tmp/ok.txt" "$tmp/ok.out" +[[ "$(tr '\n' ' ' < "$tmp/ok.out")" == "schema.sql types.sql ops.sql " ]] || { echo "FAIL: order $(cat "$tmp/ok.out")"; exit 1; } +echo "ok: dependency-first order without tac" + +# 3. Anchored strip keeps a body line that merely contains the substring REQUIRE. +printf -- '-- REQUIRE: src/v3/schema.sql\nSELECT 1; -- the REQUIRE keyword in prose\n' > "$tmp/body.sql" +out="$(strip_require_lines "$tmp/body.sql")" +[[ "$out" == "SELECT 1; -- the REQUIRE keyword in prose" ]] || { echo "FAIL: strip removed non-directive line: [$out]"; exit 1; } +echo "ok: anchored strip preserves non-directive REQUIRE substring" + +# 4. verify_linearization fails when a dep is ordered AFTER its dependent. +printf 'types.sql ops.sql\n' > "$tmp/edges.txt" +printf 'ops.sql\ntypes.sql\n' > "$tmp/badorder.txt" +if verify_linearization "$tmp/edges.txt" "$tmp/badorder.txt" 2>/dev/null; then + echo "FAIL: bad linearization accepted"; exit 1 +fi +echo "ok: linearization violation detected" +echo "ALL build-ordering helper tests passed" From 7cf5f4053b708df15db3101cd2f957280c2c166e Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Wed, 8 Jul 2026 14:29:03 +1000 Subject: [PATCH 02/15] codegen: emit deterministic topo-ordered manifest for the generated SQL surface --- .gitignore | 4 + crates/eql-codegen/src/generate.rs | 71 ++++++++++++++ crates/eql-codegen/src/lib.rs | 1 + crates/eql-codegen/src/ordering.rs | 150 +++++++++++++++++++++++++++++ crates/eql-codegen/src/writer.rs | 2 + 5 files changed, 228 insertions(+) create mode 100644 crates/eql-codegen/src/ordering.rs diff --git a/.gitignore b/.gitignore index af3f11193..76c9468a9 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,10 @@ deps-ordered-supabase.txt src/deps-v3.txt src/deps-ordered-v3.txt +src/generated-order-v3.txt +src/generated-deps-v3.txt +src/handwritten-deps-v3.txt +src/handwritten-ordered-v3.txt # Generated by tasks/build.sh from src/v3/version.template (eql_v3.version()). src/v3/version.sql diff --git a/crates/eql-codegen/src/generate.rs b/crates/eql-codegen/src/generate.rs index 2b66bd88b..fb7608cef 100644 --- a/crates/eql-codegen/src/generate.rs +++ b/crates/eql-codegen/src/generate.rs @@ -908,11 +908,44 @@ pub fn render_ore_fallback_file() -> String { use std::fs; +use crate::ordering::{requires_of, topo_order}; use crate::writer::{ ensure_generated_paths_writable, normalized_set, remove_generated_orphans, write_generated_file, GeneratedKind, WriteError, }; +/// Ordered generated-file paths plus their dependency edges, both repo-relative. +pub struct GeneratedOrdering { + pub order: Vec, + pub edges: Vec<(String, String)>, // (dep, file) +} + +/// Render every scalar family (pure, no fs writes), read back each file's +/// `-- REQUIRE:` edges, and topologically order the generated surface. Paths are +/// repo-relative (matching the `-- REQUIRE:` and build.sh conventions). +pub fn generated_manifest(out_root: &Path) -> Result { + let scalars_root = out_root.join(V3_SCALARS_DIR); + let mut files: Vec<(String, Vec)> = Vec::new(); + let mut edges: Vec<(String, String)> = Vec::new(); + for spec in eql_domains::scalar_families() { + let out_dir = scalars_root.join(spec.name); + for (path, body) in render_type(spec, &out_dir) { + let rel = path + .strip_prefix(out_root) + .unwrap_or(&path) + .to_string_lossy() + .replace('\\', "/"); + let reqs = requires_of(&body); + for dep in &reqs { + edges.push((dep.clone(), rel.clone())); + } + files.push((rel, reqs)); + } + } + let order = topo_order(&files).map_err(|e| WriteError::Codegen(e.to_string()))?; + Ok(GeneratedOrdering { order, edges }) +} + /// Render every generated file for one type into memory, paired with its output /// path under `out_dir`. Mirrors `bindings::render_bindings`: rendering happens /// before any filesystem mutation, so a render `.expect` panic aborts the run @@ -1087,6 +1120,17 @@ pub fn generate_all(out_root: &Path) -> Result { } } + // Emit the ordering intermediates the build consumes: the topo-ordered + // generated manifest and its dep-edge file. Both are repo-relative, gitignored + // build intermediates (mirroring src/deps-ordered-v3.txt). + let manifest = generated_manifest(out_root)?; + let order_path = out_root.join("src/generated-order-v3.txt"); + fs::write(&order_path, format!("{}\n", manifest.order.join("\n")))?; + let deps_path = out_root.join("src/generated-deps-v3.txt"); + let deps_body: String = manifest.edges.iter().map(|(d, f)| format!("{d} {f}\n")).collect(); + fs::write(&deps_path, deps_body)?; + println!("wrote src/generated-order-v3.txt ({} files)", manifest.order.len()); + let names: Vec<&str> = eql_domains::families_with_scalar_domains() .map(|s| s.name) .collect(); @@ -1133,6 +1177,33 @@ mod tests { use super::*; use eql_domains::CATALOG; + #[test] + fn generated_manifest_is_valid_linearization_of_real_catalog() { + let d = crate::writer::test_support::tempdir(); + generate_all(d.path()).unwrap(); + let m = generated_manifest(d.path()).unwrap(); + + // Every generated file appears exactly once. (219 committed scalar files: + // the codegen manifest covers scalar_families() only — version.sql and the + // hand-written scalars/functions.sql are NOT part of it.) + assert_eq!(m.order.len(), 219, "expected 219 generated scalar files"); + let set: std::collections::BTreeSet<&String> = m.order.iter().collect(); + assert_eq!(set.len(), m.order.len(), "duplicate path in manifest"); + + // Every intra-generated edge is respected (dep before file). + let pos: std::collections::HashMap<&str, usize> = + m.order.iter().enumerate().map(|(i, p)| (p.as_str(), i)).collect(); + for (dep, file) in &m.edges { + if let (Some(di), Some(fi)) = (pos.get(dep.as_str()), pos.get(file.as_str())) { + assert!(di <= fi, "generated dep {dep} ordered after {file}"); + } + } + + // Determinism: a second call is byte-identical. + let m2 = generated_manifest(d.path()).unwrap(); + assert_eq!(m.order, m2.order); + } + fn spec(family_name: &str) -> &'static DomainFamily { CATALOG .iter() diff --git a/crates/eql-codegen/src/lib.rs b/crates/eql-codegen/src/lib.rs index 1242e08d7..f05c70bfa 100644 --- a/crates/eql-codegen/src/lib.rs +++ b/crates/eql-codegen/src/lib.rs @@ -13,6 +13,7 @@ pub mod context; pub mod dump; pub mod generate; pub mod operator_surface; +pub mod ordering; pub mod writer; /// The repository root, derived from this crate's manifest dir (the generator diff --git a/crates/eql-codegen/src/ordering.rs b/crates/eql-codegen/src/ordering.rs new file mode 100644 index 000000000..06110576b --- /dev/null +++ b/crates/eql-codegen/src/ordering.rs @@ -0,0 +1,150 @@ +//! Deterministic topological ordering of the generated SQL surface. + +use std::cmp::Reverse; +use std::collections::{BTreeMap, BTreeSet, BinaryHeap}; + +/// A dependency cycle among generated files — the topo-sort could not linearize. +#[derive(Debug)] +pub struct CycleError { + /// The nodes that never reached in-degree 0 (participate in / are blocked by a cycle). + pub remaining: Vec, +} + +impl std::fmt::Display for CycleError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "dependency cycle among generated files: {}", self.remaining.join(", ")) + } +} +impl std::error::Error for CycleError {} + +/// Read back the anchored `-- REQUIRE:` targets from a rendered SQL body. The +/// body was produced in-process from the typed `requires` vec via the template, +/// so this is a deterministic readback of the same data — not the fragile +/// cross-platform shell glob of 220 on-disk files this refactor removes. +pub fn requires_of(body: &str) -> Vec { + body.lines() + .filter_map(|l| l.trim_start().strip_prefix("-- REQUIRE:")) + .flat_map(|rest| rest.split_whitespace().map(str::to_string)) + .collect() +} + +/// Deterministic topological order of generated files. `files` is +/// `(repo-relative path, its REQUIRE targets)`. Edges whose target is NOT a key +/// in `files` (hand-written prerequisites) are ignored: the generated block is +/// emitted wholesale AFTER the hand-written block, so those edges are satisfied +/// by construction. Kahn's algorithm with a min-heap keyed by path string gives +/// name-sorted tie-breaking ⇒ byte-reproducible output. +pub fn topo_order(files: &[(String, Vec)]) -> Result, CycleError> { + let nodes: BTreeSet<&str> = files.iter().map(|(p, _)| p.as_str()).collect(); + let mut indeg: BTreeMap<&str, usize> = nodes.iter().map(|n| (*n, 0usize)).collect(); + let mut dependents: BTreeMap<&str, Vec<&str>> = BTreeMap::new(); + for (p, reqs) in files { + for dep in reqs { + let dep = dep.as_str(); + if dep != p.as_str() && nodes.contains(dep) { + *indeg.get_mut(p.as_str()).unwrap() += 1; + dependents.entry(dep).or_default().push(p.as_str()); + } + } + } + let mut ready: BinaryHeap> = indeg + .iter() + .filter(|(_, d)| **d == 0) + .map(|(p, _)| Reverse(*p)) + .collect(); + let mut order: Vec = Vec::with_capacity(files.len()); + while let Some(Reverse(n)) = ready.pop() { + order.push(n.to_string()); + if let Some(deps) = dependents.get(n) { + let mut ds = deps.clone(); + ds.sort_unstable(); + for d in ds { + let e = indeg.get_mut(d).unwrap(); + *e -= 1; + if *e == 0 { + ready.push(Reverse(d)); + } + } + } + } + if order.len() != nodes.len() { + let done: BTreeSet<&str> = order.iter().map(|s| s.as_str()).collect(); + let remaining = nodes.iter().filter(|n| !done.contains(**n)).map(|s| s.to_string()).collect(); + return Err(CycleError { remaining }); + } + Ok(order) +} + +#[cfg(test)] +mod tests { + use super::*; + + // Two independent nodes must come out in byte (name) order — reproducible. + #[test] + fn topo_order_is_name_sorted_for_independent_nodes() { + let files = vec![ + ("b.sql".to_string(), vec![]), + ("a.sql".to_string(), vec![]), + ]; + assert_eq!(topo_order(&files).unwrap(), vec!["a.sql", "b.sql"]); + } + + // A dependency edge (file requires dep) orders dep first. + #[test] + fn topo_order_respects_intra_generated_edges() { + let files = vec![ + ("ops.sql".to_string(), vec!["types.sql".to_string()]), + ("types.sql".to_string(), vec![]), + ("agg.sql".to_string(), vec!["ops.sql".to_string(), "types.sql".to_string()]), + ]; + let out = topo_order(&files).unwrap(); + let pos = |n: &str| out.iter().position(|x| x == n).unwrap(); + assert!(pos("types.sql") < pos("ops.sql")); + assert!(pos("ops.sql") < pos("agg.sql")); + } + + // Edges to files NOT in the set (hand-written prerequisites) are ignored: + // they never block ordering and never appear in the output. + #[test] + fn topo_order_ignores_external_edges() { + let files = vec![ + ("t.sql".to_string(), vec!["src/v3/schema.sql".to_string()]), + ]; + assert_eq!(topo_order(&files).unwrap(), vec!["t.sql"]); + } + + // Identical input twice ⇒ identical output (determinism invariant). + #[test] + fn topo_order_is_deterministic() { + let files = vec![ + ("c.sql".to_string(), vec!["a.sql".to_string()]), + ("a.sql".to_string(), vec![]), + ("b.sql".to_string(), vec!["a.sql".to_string()]), + ]; + assert_eq!(topo_order(&files).unwrap(), topo_order(&files).unwrap()); + // a first, then b, c by name. + assert_eq!(topo_order(&files).unwrap(), vec!["a.sql", "b.sql", "c.sql"]); + } + + // A cycle is a hard error naming the stuck nodes. + #[test] + fn topo_order_detects_cycle() { + let files = vec![ + ("a.sql".to_string(), vec!["b.sql".to_string()]), + ("b.sql".to_string(), vec!["a.sql".to_string()]), + ]; + let err = topo_order(&files).unwrap_err(); + assert!(err.remaining.contains(&"a.sql".to_string())); + assert!(err.remaining.contains(&"b.sql".to_string())); + } + + // requires_of reads back anchored `-- REQUIRE:` lines from a rendered body. + #[test] + fn requires_of_reads_anchored_directives() { + let body = "-- AUTOMATICALLY GENERATED FILE.\n-- REQUIRE: src/v3/schema.sql\n-- REQUIRE: a.sql b.sql\nSELECT 1; -- REQUIRE in prose\n"; + assert_eq!( + requires_of(body), + vec!["src/v3/schema.sql".to_string(), "a.sql".to_string(), "b.sql".to_string()] + ); + } +} diff --git a/crates/eql-codegen/src/writer.rs b/crates/eql-codegen/src/writer.rs index de3f4cf01..ec59fe08e 100644 --- a/crates/eql-codegen/src/writer.rs +++ b/crates/eql-codegen/src/writer.rs @@ -44,6 +44,8 @@ pub enum WriteError { Ownership(String), #[error("io error: {0}")] Io(#[from] io::Error), + #[error("{0}")] + Codegen(String), } fn first_line(path: &Path) -> io::Result { From 0cb8c6312789119d33419b2300a1967d149d237f Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Wed, 8 Jul 2026 14:33:31 +1000 Subject: [PATCH 03/15] build: order generated surface from codegen manifest; tsort now a whole-surface cycle+linearization verifier --- tasks/build.sh | 34 +++++++++++++---- tasks/test/verify_monolith_reorder_only.sh | 43 ++++++++++++++++++++++ 2 files changed, 70 insertions(+), 7 deletions(-) create mode 100755 tasks/test/verify_monolith_reorder_only.sh diff --git a/tasks/build.sh b/tasks/build.sh index 7c3ed4f05..73398adf3 100755 --- a/tasks/build.sh +++ b/tasks/build.sh @@ -77,8 +77,9 @@ mkdir -p release rm -f release/cipherstash-encrypt.sql rm -f release/cipherstash-encrypt-uninstall.sql -rm -f src/deps-v3.txt -rm -f src/deps-ordered-v3.txt +# Truncate the build intermediates we APPEND to below. The generated-*.txt files +# are (re)written wholesale by eql-codegen above, so they are NOT removed here. +rm -f src/deps-v3.txt src/deps-ordered-v3.txt src/handwritten-deps-v3.txt src/handwritten-ordered-v3.txt rm -f src/v3/version.sql @@ -99,21 +100,40 @@ sed "s/\$RELEASE_VERSION/$RELEASE_VERSION/g" src/v3/version.template > src/v3/ve find src/v3 -type f -path "*.sql" ! -path "*_test.sql" -print0 \ | LC_ALL=C sort -z \ | while IFS= read -r -d '' sql_file; do - # self-edge: isolated files still appear in tsort output. A self-edge is a - # tsort no-op, not a cycle (see run_tsort_or_die), so it is safe here. - echo "$sql_file $sql_file" >> src/deps-v3.txt + IFS= read -r first < "$sql_file" || first="" + # Generated scalar files are ordered by eql-codegen (src/generated-order-v3.txt); + # only hand-written files are parsed here. The classifier is the EXACT + # period-terminated marker ("-- AUTOMATICALLY GENERATED FILE."): generated + # scalars carry it, but src/v3/version.sql carries a period-LESS marker and + # IS hand-written (it has an authored -- REQUIRE: edge to schema.sql and is + # not part of the codegen manifest), so it must be parsed, not skipped. + [[ "$first" == "-- AUTOMATICALLY GENERATED FILE."* ]] && continue + echo "$sql_file $sql_file" >> src/handwritten-deps-v3.txt # self-edge while IFS= read -r line; do if [[ "$line" =~ ^[[:space:]]*--\ REQUIRE: ]]; then deps=${line#*-- REQUIRE: } for dep in $deps; do - echo "$dep $sql_file" >> src/deps-v3.txt # dependency FIRST (no tac) + echo "$dep $sql_file" >> src/handwritten-deps-v3.txt # dependency first done fi done < "$sql_file" done +# Union edge set for the whole-surface verifiers (hand-written + generated). +cat src/handwritten-deps-v3.txt src/generated-deps-v3.txt > src/deps-v3.txt + +# Whole-surface cycle gate (verification only) — tsort output discarded. verify_v3_self_contained src/deps-v3.txt -run_tsort_or_die src/deps-v3.txt src/deps-ordered-v3.txt +run_tsort_or_die src/deps-v3.txt /dev/null + +# Phase A: order the ~25 hand-written files from their authored edges. +run_tsort_or_die src/handwritten-deps-v3.txt src/handwritten-ordered-v3.txt + +# Phase B: hand-written order, then the codegen-emitted generated order. Valid +# because NO hand-written file depends on a generated file (verified), so every +# generated->hand-written edge points backward into the already-emitted block. +cat src/handwritten-ordered-v3.txt src/generated-order-v3.txt > src/deps-ordered-v3.txt + verify_deps_exist src/deps-ordered-v3.txt verify_linearization src/deps-v3.txt src/deps-ordered-v3.txt diff --git a/tasks/test/verify_monolith_reorder_only.sh b/tasks/test/verify_monolith_reorder_only.sh new file mode 100755 index 000000000..8e2e511d6 --- /dev/null +++ b/tasks/test/verify_monolith_reorder_only.sh @@ -0,0 +1,43 @@ +#!/usr/bin/env bash +#MISE description="Prove the build-ordering refactor changed ONLY statement order: the LC_ALL=C-sorted monolith is byte-identical to a pre-refactor baseline build" +set -euo pipefail +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +cd "$REPO_ROOT" + +# The pre-refactor baseline is the commit immediately BEFORE Task 1 of the +# build-ordering refactor. On this long-lived branch that is the branch tip at +# plan-execution start (c8d50efb) — NOT `git merge-base HEAD main`, which points +# 594 feature commits back and would diff on content, not order. Pass it +# explicitly. +BASELINE_REF="${1:?usage: verify_monolith_reorder_only.sh (the pre-refactor commit, e.g. c8d50efb)}" +VERSION="${EQL_VERSION:-DEV}" # BOTH builds must bake the SAME version string, else version.sql diffs spuriously. + +OUT="$(mktemp -d)" +WT="$(mktemp -d)/eql-baseline" +cleanup() { git worktree remove --force "$WT" 2>/dev/null || true; rm -rf "$OUT" "$(dirname "$WT")"; } +trap cleanup EXIT + +# 1. Current branch: build, then LC_ALL=C sort (locale-stable set view). +mise run build --version "$VERSION" >/dev/null +LC_ALL=C sort release/cipherstash-encrypt.sql > "$OUT/current-sorted.sql" + +# 2. Baseline: build in an ISOLATED detached worktree at the pre-refactor ref so +# the working tree is untouched; sort identically. `mise trust` is required — +# a fresh worktree's mise.toml is untrusted and `mise run` refuses to run it. +git worktree add --detach "$WT" "$BASELINE_REF" >/dev/null +( + cd "$WT" + mise trust >/dev/null 2>&1 || true + mise trust mise.toml >/dev/null 2>&1 || true + mise run build --version "$VERSION" >/dev/null +) +LC_ALL=C sort "$WT/release/cipherstash-encrypt.sql" > "$OUT/baseline-sorted.sql" + +# 3. HARD GATE: sorted views must be byte-identical. +if cmp -s "$OUT/baseline-sorted.sql" "$OUT/current-sorted.sql"; then + echo "PASS: sorted monolith is byte-identical to baseline $BASELINE_REF — the refactor changed ONLY statement order (nothing added/dropped/mutated)." +else + echo "FAIL: sorted monolith DIFFERS from baseline $BASELINE_REF — a line was added, dropped, or mutated (not merely reordered):" >&2 + diff "$OUT/baseline-sorted.sql" "$OUT/current-sorted.sql" | head -40 >&2 + exit 1 +fi From 0eabc471dd4858a42bc944ebf6563ba9ef438968 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Wed, 8 Jul 2026 14:39:55 +1000 Subject: [PATCH 04/15] build: add referenced-vs-defined symbol cross-check over the installer order --- tasks/build.sh | 1 + tasks/test/symbol_order_allowlist.txt | 4 + tasks/test/symbol_order_selftest.sh | 63 +++++++++++++++ tasks/test/verify_symbol_order_v3.sh | 107 ++++++++++++++++++++++++++ 4 files changed, 175 insertions(+) create mode 100644 tasks/test/symbol_order_allowlist.txt create mode 100755 tasks/test/symbol_order_selftest.sh create mode 100755 tasks/test/verify_symbol_order_v3.sh diff --git a/tasks/build.sh b/tasks/build.sh index 73398adf3..7520329f9 100755 --- a/tasks/build.sh +++ b/tasks/build.sh @@ -136,6 +136,7 @@ cat src/handwritten-ordered-v3.txt src/generated-order-v3.txt > src/deps-ordered verify_deps_exist src/deps-ordered-v3.txt verify_linearization src/deps-v3.txt src/deps-ordered-v3.txt +bash tasks/test/verify_symbol_order_v3.sh src/deps-ordered-v3.txt : > release/cipherstash-encrypt.sql while IFS= read -r f; do diff --git a/tasks/test/symbol_order_allowlist.txt b/tasks/test/symbol_order_allowlist.txt new file mode 100644 index 000000000..95e267ef1 --- /dev/null +++ b/tasks/test/symbol_order_allowlist.txt @@ -0,0 +1,4 @@ +# One fully-qualified symbol per line (e.g. eql_v3.foo) whose "defined earlier" +# check should be skipped. Use ONLY for genuine false positives (a reference the +# regex sees but that is not a real dependency — e.g. a symbol assembled by +# dynamic SQL). Keep this list minimal and comment WHY each entry is here. diff --git a/tasks/test/symbol_order_selftest.sh b/tasks/test/symbol_order_selftest.sh new file mode 100755 index 000000000..e7905d9f9 --- /dev/null +++ b/tasks/test/symbol_order_selftest.sh @@ -0,0 +1,63 @@ +#!/usr/bin/env bash +#MISE description="DB-free self-test for the symbol-order cross-check (good passes, mis-ordered fails)" +set -euo pipefail +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +cd "$REPO_ROOT" +tmp="$(mktemp -d)"; trap 'rm -rf "$tmp"' EXIT + +# GOOD: definer ordered before user. (a.sql RETURNS a non-owned type — the +# hmac_256 domain-capture branch is exercised separately by d.sql/e.sql below, so +# this pair isolates the eql_v3.eq_term define-before-use ordering it is testing.) +printf 'CREATE FUNCTION eql_v3.eq_term(a public.integer_eq) RETURNS text ...\n' > "$tmp/a.sql" +printf 'CREATE OPERATOR = ( FUNCTION = eql_v3.eq_term );\n' > "$tmp/b.sql" +printf '%s\n%s\n' "$tmp/a.sql" "$tmp/b.sql" > "$tmp/good_order.txt" +bash tasks/test/verify_symbol_order_v3.sh "$tmp/good_order.txt" \ + || { echo "FAIL: good order rejected"; exit 1; } +echo "ok: good order accepted" + +# BAD: user ordered before definer. +printf '%s\n%s\n' "$tmp/b.sql" "$tmp/a.sql" > "$tmp/bad_order.txt" +if bash tasks/test/verify_symbol_order_v3.sh "$tmp/bad_order.txt" 2>/dev/null; then + echo "FAIL: mis-ordered reference accepted"; exit 1 +fi +echo "ok: mis-ordered reference rejected" + +# COMMENT-ONLY reference must NOT trip the gate (doxygen @see). +printf -- '--! @see eql_v3.eq_term\nSELECT 1;\n' > "$tmp/c.sql" +printf '%s\n' "$tmp/c.sql" > "$tmp/comment_order.txt" +bash tasks/test/verify_symbol_order_v3.sh "$tmp/comment_order.txt" \ + || { echo "FAIL: comment-only reference tripped the gate"; exit 1; } +echo "ok: comment-only reference ignored" + +# CREATE DOMAIN eql_v3_internal.* form (SEM index-term types hmac_256/ope_cllw/ +# bloom_filter). A domain-form definer ordered before a function returning it +# must be ACCEPTED — pins the domain-capture branch's eql_v3_internal arm so the +# real surface (~165 refs to these three types) can never be misread as "defined +# nowhere" (which would tempt an allowlist entry). +printf 'CREATE DOMAIN eql_v3_internal.hmac_256 AS text;\n' > "$tmp/d.sql" +printf 'CREATE FUNCTION eql_v3.eq_term(a public.integer_eq) RETURNS eql_v3_internal.hmac_256 ...\n' > "$tmp/e.sql" +printf '%s\n%s\n' "$tmp/d.sql" "$tmp/e.sql" > "$tmp/domain_good.txt" +bash tasks/test/verify_symbol_order_v3.sh "$tmp/domain_good.txt" \ + || { echo "FAIL: CREATE DOMAIN eql_v3_internal.* definer not recognised"; exit 1; } +echo "ok: CREATE DOMAIN eql_v3_internal.* definition form recognised" + +# And the same domain-form type used BEFORE it is created must be REJECTED +# (defined-later ordering violation on a SEM index-term type — the exact rot +# this gate exists to catch). +printf '%s\n%s\n' "$tmp/e.sql" "$tmp/d.sql" > "$tmp/domain_bad.txt" +if bash tasks/test/verify_symbol_order_v3.sh "$tmp/domain_bad.txt" 2>/dev/null; then + echo "FAIL: eql_v3_internal.hmac_256 used before its CREATE DOMAIN accepted"; exit 1 +fi +echo "ok: domain-form type used before definition rejected" + +# CREATE OPERATOR CLASS|FAMILY eql_v3_internal.* form (the conditional SEM +# ordered-index opclasses). A file that both creates the opclass and mentions it +# in a RAISE NOTICE (same file) must be ACCEPTED — pins the operator-class +# definition-capture branch so the real ore_block_256/ore_cllw operator_class.sql +# files (self-contained: def + NOTICE prose only) never read as "defined nowhere". +printf "CREATE OPERATOR FAMILY eql_v3_internal.ore_cllw_ops USING btree;\nCREATE OPERATOR CLASS eql_v3_internal.ore_cllw_ops USING btree FAMILY eql_v3_internal.ore_cllw_ops AS STORAGE text;\nRAISE NOTICE 'created operator class eql_v3_internal.ore_cllw_ops';\n" > "$tmp/opclass.sql" +printf '%s\n' "$tmp/opclass.sql" > "$tmp/opclass_order.txt" +bash tasks/test/verify_symbol_order_v3.sh "$tmp/opclass_order.txt" \ + || { echo "FAIL: CREATE OPERATOR CLASS/FAMILY definer not recognised"; exit 1; } +echo "ok: CREATE OPERATOR CLASS/FAMILY definition form recognised" +echo "symbol-order self-test passed" diff --git a/tasks/test/verify_symbol_order_v3.sh b/tasks/test/verify_symbol_order_v3.sh new file mode 100755 index 000000000..b2b28438e --- /dev/null +++ b/tasks/test/verify_symbol_order_v3.sh @@ -0,0 +1,107 @@ +#!/usr/bin/env bash +#MISE description="Cross-check that every eql_v3/eql_v3_internal/public-domain symbol referenced in a file is defined by a file ordered earlier" +set -euo pipefail +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +cd "$REPO_ROOT" + +ORDERED="${1:-src/deps-ordered-v3.txt}" +ALLOW="tasks/test/symbol_order_allowlist.txt" +test -f "$ORDERED" || { echo "ERROR: ordered file $ORDERED missing (run mise run build)" >&2; exit 2; } + +awk -v allowfile="$ALLOW" ' + BEGIN { + idx = 0 + while ((getline a < allowfile) > 0) { + sub(/#.*/, "", a); gsub(/[ \t]+/, "", a) + if (a != "") allow[a] = 1 + } + } + # $0 here is a path from the ordered list. + { + idx++ + file = $0 + # First pass over the file: record DEFINITIONS with this index (min index kept). + while ((getline line < file) > 0) { + # Strip trailing line comments so prose/doxygen never counts as code. + sub(/--.*/, "", line) + # CREATE [OR REPLACE] FUNCTION|AGGREGATE eql_v3(_internal). + if (match(line, /CREATE[ \t]+(OR[ \t]+REPLACE[ \t]+)?(FUNCTION|AGGREGATE)[ \t]+(eql_v3_internal|eql_v3)\.("[^"]+"|[a-z0-9_]+)/)) { + s = substr(line, RSTART, RLENGTH); sub(/.*(eql_v3_internal|eql_v3)\./, "", s) + schema = (index(substr(line,RSTART,RLENGTH), "eql_v3_internal.") ? "eql_v3_internal." : "eql_v3.") + key = schema s + if (!(key in defined)) defined[key] = idx + } + # CREATE DOMAIN (eql_v3_internal|public).. Both schemas: the SEM + # index-term types split across DDL forms — hmac_256/ope_cllw/bloom_filter + # are `CREATE DOMAIN eql_v3_internal.` (over text/bytea/smallint[]), + # NOT `CREATE TYPE`. Capturing only `public.` here would leave the three + # most-referenced foundational types (~165 refs) reporting "defined + # nowhere" — a real gap, not an allowlist case. Only `public.` domains feed + # isdomain[] (that gates which `public.*` REFERENCES are checked). + if (match(line, /CREATE[ \t]+DOMAIN[ \t]+(eql_v3_internal|public)\.[a-z0-9_]+/)) { + seg = substr(line, RSTART, RLENGTH) + if (seg ~ /eql_v3_internal\./) { sub(/.*eql_v3_internal\./, "", seg); key = "eql_v3_internal." seg } + else { sub(/.*public\./, "", seg); key = "public." seg; isdomain[seg] = 1 } + if (!(key in defined)) defined[key] = idx + } + # CREATE TYPE eql_v3_internal. (the composite SEM types: ore_block_256, ore_cllw) + if (match(line, /CREATE[ \t]+TYPE[ \t]+eql_v3_internal\.[a-z0-9_]+/)) { + s = substr(line, RSTART, RLENGTH); sub(/.*eql_v3_internal\./, "", s) + key = "eql_v3_internal." s; if (!(key in defined)) defined[key] = idx + } + # CREATE OPERATOR CLASS|FAMILY (eql_v3_internal|eql_v3).. The conditional + # SEM ordered-index opclasses (ore_block_256_operator_class/_family, + # ore_cllw_ops), created via EXECUTE / plpgsql for superusers. Each is fully + # self-contained in its own operator_class.sql — the only other mentions are + # RAISE NOTICE string-literal prose in the SAME file — so recognising this + # definition form (like CREATE TYPE/DOMAIN above) keeps them from reading as + # "defined nowhere", while still catching a genuine cross-file mis-order. + if (match(line, /CREATE[ \t]+OPERATOR[ \t]+(CLASS|FAMILY)[ \t]+(eql_v3_internal|eql_v3)\.[a-z0-9_]+/)) { + s = substr(line, RSTART, RLENGTH) + schema = (index(s, "eql_v3_internal.") ? "eql_v3_internal." : "eql_v3.") + sub(/.*(eql_v3_internal|eql_v3)\./, "", s) + key = schema s; if (!(key in defined)) defined[key] = idx + } + } + close(file) + order[idx] = file + } + END { + # Second pass: for every file, collect REFERENCES (code only) and check them. + for (i = 1; i <= idx; i++) { + file = order[i] + while ((getline line < file) > 0) { + sub(/--.*/, "", line) # drop comments + rest = line + # eql_v3. and eql_v3_internal. + while (match(rest, /(eql_v3_internal|eql_v3)\.("[^"]+"|[a-z0-9_]+)/)) { + tok = substr(rest, RSTART, RLENGTH) + rest = substr(rest, RSTART + RLENGTH) + check(tok, i, file) + } + # public. — ONLY names we saw defined as a domain (avoids public tables/builtins). + rest = line + while (match(rest, /public\.[a-z0-9_]+/)) { + tok = substr(rest, RSTART, RLENGTH); rest = substr(rest, RSTART + RLENGTH) + name = tok; sub(/public\./, "", name) + if (name in isdomain) check(tok, i, file) + } + } + close(file) + } + if (bad) { print "symbol-order cross-check FAILED" > "/dev/stderr"; exit 1 } + print "symbol-order cross-check OK (" idx " files)" + } + function check(tok, i, file) { + if (tok in allow) return + if (!(tok in defined)) { + # Referenced owned-schema symbol never defined anywhere: a real hole. + printf("ERROR: %s references %s which is defined nowhere in the installer\n", file, tok) > "/dev/stderr" + bad = 1; return + } + if (defined[tok] > i) { + printf("ERROR: %s references %s defined later (at #%d, used at #%d)\n", file, tok, defined[tok], i) > "/dev/stderr" + bad = 1 + } + } +' "$ORDERED" From 76fcdc2ef6aba2d0d4935726698bc137ffc23181 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Wed, 8 Jul 2026 14:42:57 +1000 Subject: [PATCH 05/15] ci/docs: gate symbol-order + build-ordering helpers; document codegen-emitted ordering --- .github/workflows/test-eql.yml | 4 ++++ .../adding-a-scalar-encrypted-domain-type.md | 8 ++++++++ mise.toml | 14 ++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/.github/workflows/test-eql.yml b/.github/workflows/test-eql.yml index cceea834a..e1898c120 100644 --- a/.github/workflows/test-eql.yml +++ b/.github/workflows/test-eql.yml @@ -409,6 +409,10 @@ jobs: run: mise run clean && mise run --force build - name: Assert eql_v3 is self-contained run: mise run test:self_contained_v3 + - name: Symbol-order cross-check (v3) + run: mise run test:symbol_order_v3 + - name: Build-ordering helper unit tests + run: mise run test:build_ordering_helpers matrix-coverage: name: "Matrix coverage inventory" diff --git a/docs/reference/adding-a-scalar-encrypted-domain-type.md b/docs/reference/adding-a-scalar-encrypted-domain-type.md index dfc5952a9..5d4e3b826 100644 --- a/docs/reference/adding-a-scalar-encrypted-domain-type.md +++ b/docs/reference/adding-a-scalar-encrypted-domain-type.md @@ -56,6 +56,14 @@ To add a scalar type `` (e.g. `bigint`), with Rust type `` (e.g. `i64`): no per-type codegen task. The generated `*_{types,functions,operators,aggregates}.sql` are committed in place under `src/v3/scalars//` — regenerate and commit the SQL diff alongside the catalog change. + - **Ordering is emitted by the codegen — you do nothing.** The topological + order of the generated scalar files is produced by `eql-codegen` + (name-sorted, cycle-checked) into `src/generated-order-v3.txt`; + `tasks/build.sh` concatenates it after the hand-written files. Adding a + catalog row needs no `-- REQUIRE:` edits to any generated file. Only + hand-written files under `src/v3/` (SEM types, `jsonb/`, `schema.sql`, + `crypto.sql`, `common.sql`, `scalars/functions.sql`, `lint/lints.sql`, + `*_extensions.sql`) carry authored `-- REQUIRE:` edges. - The catalog row ALSO drives the **Rust payload bindings**: `eql-codegen bindings` (run first by `mise run types:generate`) regenerates the committed `crates/eql-bindings/src/v3/.rs` struct + `DomainType` diff --git a/mise.toml b/mise.toml index 844e913bc..08d091c45 100644 --- a/mise.toml +++ b/mise.toml @@ -225,6 +225,20 @@ run = """ cargo test -p eql-domains -p eql-codegen """ +[tasks."test:symbol_order_v3"] +description = "Cross-check installer symbol definition order (DB-free)" +depends = ["build"] +dir = "{{config_root}}" +run = """ +bash tasks/test/symbol_order_selftest.sh +bash tasks/test/verify_symbol_order_v3.sh src/deps-ordered-v3.txt +""" + +[tasks."test:build_ordering_helpers"] +description = "Unit tests for tasks/build/ordering.sh (DB-free)" +dir = "{{config_root}}" +run = "bash tasks/test/build_ordering_helpers_test.sh" + [tasks."test:crates"] description = "Compile, lint and test the std-only Rust workspace crates (no database)" dir = "{{config_root}}" From 3455561c58a43cb4eea73717efca8ba287033d05 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Wed, 8 Jul 2026 15:03:43 +1000 Subject: [PATCH 06/15] =?UTF-8?q?build:=20address=20review=20=E2=80=94=20f?= =?UTF-8?q?ail-loud=20strip=5Frequire=5Flines=20(propagate=20grep=20errors?= =?UTF-8?q?)=20and=20unreadable-path=20guard=20in=20symbol=20checker?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tasks/build/ordering.sh | 8 +++++++- tasks/test/build_ordering_helpers_test.sh | 14 ++++++++++++++ tasks/test/symbol_order_selftest.sh | 8 ++++++++ tasks/test/verify_symbol_order_v3.sh | 9 +++++++++ 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/tasks/build/ordering.sh b/tasks/build/ordering.sh index e94ec3ae1..3acdd5c53 100644 --- a/tasks/build/ordering.sh +++ b/tasks/build/ordering.sh @@ -7,7 +7,13 @@ # (allows leading whitespace) so a body line that merely contains the substring # "REQUIRE" survives — unlike the old unanchored `grep -v REQUIRE`. strip_require_lines() { - grep -vE '^[[:space:]]*-- REQUIRE:' "$1" || true + local rc=0 + grep -vE '^[[:space:]]*-- REQUIRE:' "$1" || rc=$? + # grep exits 1 when EVERY line matched the exclude (nothing left) — not an + # error. Exit codes >= 2 (missing file, unreadable, bad regex) are real + # failures: propagate so `set -e` aborts assembly instead of silently emitting + # a truncated monolith. (The blanket `|| true` this replaces hid exit 2.) + (( rc <= 1 )) } # tsort with a cross-platform cycle gate. Input edges are " " (one per diff --git a/tasks/test/build_ordering_helpers_test.sh b/tasks/test/build_ordering_helpers_test.sh index b86294d70..4a0a67881 100644 --- a/tasks/test/build_ordering_helpers_test.sh +++ b/tasks/test/build_ordering_helpers_test.sh @@ -26,6 +26,20 @@ out="$(strip_require_lines "$tmp/body.sql")" [[ "$out" == "SELECT 1; -- the REQUIRE keyword in prose" ]] || { echo "FAIL: strip removed non-directive line: [$out]"; exit 1; } echo "ok: anchored strip preserves non-directive REQUIRE substring" +# 3b. strip_require_lines must FAIL (not silently succeed) on an unreadable file — +# a real grep error (exit >= 2) propagates so `set -e` aborts a truncated build. +if strip_require_lines "$tmp/does-not-exist.sql" 2>/dev/null; then + echo "FAIL: strip_require_lines swallowed a missing-file error"; exit 1 +fi +echo "ok: strip_require_lines propagates a real grep failure" + +# 3c. A file that is ENTIRELY -- REQUIRE: lines (grep exit 1, nothing left) is +# NOT an error — it contributes no body and must succeed with empty output. +printf -- '-- REQUIRE: a\n-- REQUIRE: b\n' > "$tmp/allreq.sql" +out="$(strip_require_lines "$tmp/allreq.sql")" || { echo "FAIL: all-REQUIRE file treated as error"; exit 1; } +[[ -z "$out" ]] || { echo "FAIL: all-REQUIRE file produced output: [$out]"; exit 1; } +echo "ok: all-REQUIRE file succeeds with empty output" + # 4. verify_linearization fails when a dep is ordered AFTER its dependent. printf 'types.sql ops.sql\n' > "$tmp/edges.txt" printf 'ops.sql\ntypes.sql\n' > "$tmp/badorder.txt" diff --git a/tasks/test/symbol_order_selftest.sh b/tasks/test/symbol_order_selftest.sh index e7905d9f9..7abeea345 100755 --- a/tasks/test/symbol_order_selftest.sh +++ b/tasks/test/symbol_order_selftest.sh @@ -60,4 +60,12 @@ printf '%s\n' "$tmp/opclass.sql" > "$tmp/opclass_order.txt" bash tasks/test/verify_symbol_order_v3.sh "$tmp/opclass_order.txt" \ || { echo "FAIL: CREATE OPERATOR CLASS/FAMILY definer not recognised"; exit 1; } echo "ok: CREATE OPERATOR CLASS/FAMILY definition form recognised" + +# An UNREADABLE path in the ordered list must FAIL the gate, not be silently +# skipped as an empty file (a skipped file's definitions/references go unchecked). +printf '%s\n' "$tmp/does-not-exist.sql" > "$tmp/missing_order.txt" +if bash tasks/test/verify_symbol_order_v3.sh "$tmp/missing_order.txt" 2>/dev/null; then + echo "FAIL: unreadable path silently accepted"; exit 1 +fi +echo "ok: unreadable path rejected" echo "symbol-order self-test passed" diff --git a/tasks/test/verify_symbol_order_v3.sh b/tasks/test/verify_symbol_order_v3.sh index b2b28438e..0951ff50a 100755 --- a/tasks/test/verify_symbol_order_v3.sh +++ b/tasks/test/verify_symbol_order_v3.sh @@ -20,6 +20,15 @@ awk -v allowfile="$ALLOW" ' { idx++ file = $0 + # Fail loudly on an UNREADABLE path rather than silently treating it as an + # empty (zero-definition) file: getline returns -1 on error but 0 at EOF for + # a genuinely empty file, so only -1 is a fault. This guards both passes — a + # file flagged here sets bad=1, and the END block exits non-zero. + if ((getline probe < file) < 0) { + printf("ERROR: cannot read %s (listed in the ordered file)\n", file) > "/dev/stderr" + bad = 1 + } + close(file) # First pass over the file: record DEFINITIONS with this index (min index kept). while ((getline line < file) > 0) { # Strip trailing line comments so prose/doxygen never counts as code. From 0e72905360e22a8ee61d371ebe6b37992f6cd140 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 13:53:28 +1000 Subject: [PATCH 07/15] style: cargo fmt + silence clippy::type_complexity in property test substrate Reformat generate.rs/ordering.rs to satisfy `cargo fmt --check`, and add an EqRow type alias alongside the existing OrdRow for the equality oracle's result tuple. --- crates/eql-codegen/src/generate.rs | 19 ++++++++++++++---- crates/eql-codegen/src/ordering.rs | 32 ++++++++++++++++++++---------- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/crates/eql-codegen/src/generate.rs b/crates/eql-codegen/src/generate.rs index fb7608cef..c68dbe65b 100644 --- a/crates/eql-codegen/src/generate.rs +++ b/crates/eql-codegen/src/generate.rs @@ -1127,9 +1127,16 @@ pub fn generate_all(out_root: &Path) -> Result { let order_path = out_root.join("src/generated-order-v3.txt"); fs::write(&order_path, format!("{}\n", manifest.order.join("\n")))?; let deps_path = out_root.join("src/generated-deps-v3.txt"); - let deps_body: String = manifest.edges.iter().map(|(d, f)| format!("{d} {f}\n")).collect(); + let deps_body: String = manifest + .edges + .iter() + .map(|(d, f)| format!("{d} {f}\n")) + .collect(); fs::write(&deps_path, deps_body)?; - println!("wrote src/generated-order-v3.txt ({} files)", manifest.order.len()); + println!( + "wrote src/generated-order-v3.txt ({} files)", + manifest.order.len() + ); let names: Vec<&str> = eql_domains::families_with_scalar_domains() .map(|s| s.name) @@ -1191,8 +1198,12 @@ mod tests { assert_eq!(set.len(), m.order.len(), "duplicate path in manifest"); // Every intra-generated edge is respected (dep before file). - let pos: std::collections::HashMap<&str, usize> = - m.order.iter().enumerate().map(|(i, p)| (p.as_str(), i)).collect(); + let pos: std::collections::HashMap<&str, usize> = m + .order + .iter() + .enumerate() + .map(|(i, p)| (p.as_str(), i)) + .collect(); for (dep, file) in &m.edges { if let (Some(di), Some(fi)) = (pos.get(dep.as_str()), pos.get(file.as_str())) { assert!(di <= fi, "generated dep {dep} ordered after {file}"); diff --git a/crates/eql-codegen/src/ordering.rs b/crates/eql-codegen/src/ordering.rs index 06110576b..20fdde2e9 100644 --- a/crates/eql-codegen/src/ordering.rs +++ b/crates/eql-codegen/src/ordering.rs @@ -12,7 +12,11 @@ pub struct CycleError { impl std::fmt::Display for CycleError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "dependency cycle among generated files: {}", self.remaining.join(", ")) + write!( + f, + "dependency cycle among generated files: {}", + self.remaining.join(", ") + ) } } impl std::error::Error for CycleError {} @@ -69,7 +73,11 @@ pub fn topo_order(files: &[(String, Vec)]) -> Result, CycleE } if order.len() != nodes.len() { let done: BTreeSet<&str> = order.iter().map(|s| s.as_str()).collect(); - let remaining = nodes.iter().filter(|n| !done.contains(**n)).map(|s| s.to_string()).collect(); + let remaining = nodes + .iter() + .filter(|n| !done.contains(**n)) + .map(|s| s.to_string()) + .collect(); return Err(CycleError { remaining }); } Ok(order) @@ -82,10 +90,7 @@ mod tests { // Two independent nodes must come out in byte (name) order — reproducible. #[test] fn topo_order_is_name_sorted_for_independent_nodes() { - let files = vec![ - ("b.sql".to_string(), vec![]), - ("a.sql".to_string(), vec![]), - ]; + let files = vec![("b.sql".to_string(), vec![]), ("a.sql".to_string(), vec![])]; assert_eq!(topo_order(&files).unwrap(), vec!["a.sql", "b.sql"]); } @@ -95,7 +100,10 @@ mod tests { let files = vec![ ("ops.sql".to_string(), vec!["types.sql".to_string()]), ("types.sql".to_string(), vec![]), - ("agg.sql".to_string(), vec!["ops.sql".to_string(), "types.sql".to_string()]), + ( + "agg.sql".to_string(), + vec!["ops.sql".to_string(), "types.sql".to_string()], + ), ]; let out = topo_order(&files).unwrap(); let pos = |n: &str| out.iter().position(|x| x == n).unwrap(); @@ -107,9 +115,7 @@ mod tests { // they never block ordering and never appear in the output. #[test] fn topo_order_ignores_external_edges() { - let files = vec![ - ("t.sql".to_string(), vec!["src/v3/schema.sql".to_string()]), - ]; + let files = vec![("t.sql".to_string(), vec!["src/v3/schema.sql".to_string()])]; assert_eq!(topo_order(&files).unwrap(), vec!["t.sql"]); } @@ -144,7 +150,11 @@ mod tests { let body = "-- AUTOMATICALLY GENERATED FILE.\n-- REQUIRE: src/v3/schema.sql\n-- REQUIRE: a.sql b.sql\nSELECT 1; -- REQUIRE in prose\n"; assert_eq!( requires_of(body), - vec!["src/v3/schema.sql".to_string(), "a.sql".to_string(), "b.sql".to_string()] + vec![ + "src/v3/schema.sql".to_string(), + "a.sql".to_string(), + "b.sql".to_string() + ] ); } } From a7651bed088f9d34d175701408e2d2d45d044413 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 16:59:29 +1000 Subject: [PATCH 08/15] build: teach the symbol-order checker the eql_v3 CREATE DOMAIN form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cross-check recognised `CREATE DOMAIN` only in `eql_v3_internal` and `public`, but its reference scanner matches every `eql_v3.`. Since CIP-3442 the query-operand twins are `CREATE DOMAIN eql_v3.query__` (plus the hand-written `eql_v3.query_jsonb`) — a query operand is never a column type, so it lives in `eql_v3` rather than `public`. None of those definitions were recorded, so every reference to them, including the `CREATE DOMAIN` line itself, reported "defined nowhere". This branch passes on its own, where the operands are still `public._query`. It only fails once merged with `eql_v3`, which is the tree CI builds for a pull_request — a semantic merge conflict that git resolves cleanly and the checker then rejects. Add the eql_v3 arm, ordered after the eql_v3_internal test since `eql_v3.` is a prefix of `eql_v3_internal.`. `isdomain[]` stays public-only: it gates which bare `public.*` tokens are checked, while `eql_v3*.*` references are checked unconditionally. Self-test gains accept and reject cases for the new form, so recognising the schema cannot silently blunt the ordering check. --- tasks/test/symbol_order_selftest.sh | 19 +++++++++++++++++++ tasks/test/verify_symbol_order_v3.sh | 26 +++++++++++++++++--------- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/tasks/test/symbol_order_selftest.sh b/tasks/test/symbol_order_selftest.sh index 7abeea345..a6226d021 100755 --- a/tasks/test/symbol_order_selftest.sh +++ b/tasks/test/symbol_order_selftest.sh @@ -50,6 +50,25 @@ if bash tasks/test/verify_symbol_order_v3.sh "$tmp/domain_bad.txt" 2>/dev/null; fi echo "ok: domain-form type used before definition rejected" +# CREATE DOMAIN eql_v3.* form (the query-operand twins `eql_v3.query__` +# and the hand-written `eql_v3.query_jsonb`, CIP-3442). Pins the domain-capture +# branch's eql_v3 arm: without it every query operand — including the line that +# creates it, which is also a reference — reads as "defined nowhere". +printf 'CREATE DOMAIN eql_v3.query_integer_eq AS jsonb;\n' > "$tmp/q.sql" +printf 'CREATE FUNCTION eql_v3.eq(a public.integer_eq, b eql_v3.query_integer_eq) ...\n' > "$tmp/qf.sql" +printf '%s\n%s\n' "$tmp/q.sql" "$tmp/qf.sql" > "$tmp/query_good.txt" +bash tasks/test/verify_symbol_order_v3.sh "$tmp/query_good.txt" \ + || { echo "FAIL: CREATE DOMAIN eql_v3.* definer not recognised"; exit 1; } +echo "ok: CREATE DOMAIN eql_v3.* definition form recognised" + +# And the same query operand used BEFORE it is created must still be REJECTED — +# recognising the schema must not blunt the ordering check. +printf '%s\n%s\n' "$tmp/qf.sql" "$tmp/q.sql" > "$tmp/query_bad.txt" +if bash tasks/test/verify_symbol_order_v3.sh "$tmp/query_bad.txt" 2>/dev/null; then + echo "FAIL: eql_v3.query_integer_eq used before its CREATE DOMAIN accepted"; exit 1 +fi +echo "ok: eql_v3 query operand used before definition rejected" + # CREATE OPERATOR CLASS|FAMILY eql_v3_internal.* form (the conditional SEM # ordered-index opclasses). A file that both creates the opclass and mentions it # in a RAISE NOTICE (same file) must be ACCEPTED — pins the operator-class diff --git a/tasks/test/verify_symbol_order_v3.sh b/tasks/test/verify_symbol_order_v3.sh index 0951ff50a..f74b2e5a2 100755 --- a/tasks/test/verify_symbol_order_v3.sh +++ b/tasks/test/verify_symbol_order_v3.sh @@ -40,17 +40,25 @@ awk -v allowfile="$ALLOW" ' key = schema s if (!(key in defined)) defined[key] = idx } - # CREATE DOMAIN (eql_v3_internal|public).. Both schemas: the SEM - # index-term types split across DDL forms — hmac_256/ope_cllw/bloom_filter - # are `CREATE DOMAIN eql_v3_internal.` (over text/bytea/smallint[]), - # NOT `CREATE TYPE`. Capturing only `public.` here would leave the three - # most-referenced foundational types (~165 refs) reporting "defined - # nowhere" — a real gap, not an allowlist case. Only `public.` domains feed - # isdomain[] (that gates which `public.*` REFERENCES are checked). - if (match(line, /CREATE[ \t]+DOMAIN[ \t]+(eql_v3_internal|public)\.[a-z0-9_]+/)) { + # CREATE DOMAIN (eql_v3_internal|eql_v3|public).. All three schemas + # define domains, across DDL forms: + # - eql_v3_internal: the SEM index-term types hmac_256/ope_cllw/ + # bloom_filter are `CREATE DOMAIN` (over text/bytea/smallint[]), NOT + # `CREATE TYPE`. Omitting this schema would leave the three most- + # referenced foundational types (~165 refs) reporting "defined nowhere". + # - eql_v3: the query-operand twins `eql_v3.query__` and the + # hand-written `eql_v3.query_jsonb` (CIP-3442 — a query operand is + # never a column type, so it lives here rather than in `public`). + # - public: the user-column domains. + # The eql_v3_internal test must precede the eql_v3 one: `eql_v3.` is a + # prefix of `eql_v3_internal.`. Only `public.` domains feed isdomain[] + # (that gates which `public.*` REFERENCES are checked); `eql_v3*.*` + # references are checked unconditionally. + if (match(line, /CREATE[ \t]+DOMAIN[ \t]+(eql_v3_internal|eql_v3|public)\.[a-z0-9_]+/)) { seg = substr(line, RSTART, RLENGTH) if (seg ~ /eql_v3_internal\./) { sub(/.*eql_v3_internal\./, "", seg); key = "eql_v3_internal." seg } - else { sub(/.*public\./, "", seg); key = "public." seg; isdomain[seg] = 1 } + else if (seg ~ /eql_v3\./) { sub(/.*eql_v3\./, "", seg); key = "eql_v3." seg } + else { sub(/.*public\./, "", seg); key = "public." seg; isdomain[seg] = 1 } if (!(key in defined)) defined[key] = idx } # CREATE TYPE eql_v3_internal. (the composite SEM types: ore_block_256, ore_cllw) From 965dce2b2cccc600e102ed90d5431333c0fac4f5 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 22:51:49 +1000 Subject: [PATCH 09/15] build: order the whole v3 surface from one walk, not two enumerations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tasks/build.sh` enumerated the SQL surface twice and nothing reconciled the two. The shell glob classified a file as generated by its first line (`-- AUTOMATICALLY GENERATED FILE.`) and skipped it; `generated_manifest()` classified a file as generated by re-running `render_type()` over `scalar_families()`. `src/v3/scalars/ore_fallback.sql` — rendered outside `render_type` — matched the first predicate and not the second, so it landed in neither block and was dropped from `release/cipherstash-encrypt.sql`. The ORE poison constraints never installed and `v3_ore_fallback_tests` failed on the merge with eql_v3, while the build itself reported success. Nothing could have caught it. `verify_deps_exist` checked that every *listed* file exists on disk, never that every file on disk is listed. `verify_linearization` skipped edges whose endpoints were absent from the order, commented "absence is caught by verify_deps_exist" — it wasn't. And the manifest test asserted a hardcoded per-family count of 219, structurally blind to cross-family files. Replace both enumerations with one. `eql-codegen order` walks src/v3 once, parses `-- REQUIRE:` from every file (generated and hand-written alike), and linearizes via the existing tested `topo_order`. You order exactly the set you walk, so a dropped file is unrepresentable rather than merely detectable. The new `surface_order` wrapper hard-errors on a dangling REQUIRE target and on any edge leaving src/v3, subsuming `verify_deps_exist` and `verify_v3_self_contained`. Ordering is name-sorted in Rust rather than left to `tsort`'s platform-dependent tie-break, which is what the manifest was introduced to dodge in the first place. Verified against CI's merge commit: 241 files on disk, 241 ordered (was 240), 38 hits for `eql_ore_unavailable` in the installer (was 0). ore_fallback.sql sorts immediately after the operator_class.sql whose outcome it reads. Every SQL statement in the merged installer is identical to the base branch's own build; only statement order among independent files changes. Removed: the marker classifier, the two-phase concat, verify_deps_exist, verify_v3_self_contained, run_tsort_or_die, verify_linearization, generated_manifest(), and five gitignored intermediates. strip_require_lines stays in tasks/build/ordering.sh. Tests: install_order_contains_every_v3_sql_file (parity.rs) compares the order against an independent walk by set equality, replacing the 219-count assertion; surface_order unit tests cover dangling targets, out-of-surface edges, cycles and determinism; order_subcommand_fails_on_a_dangling_require pins that the build aborts with no partial order on stdout. --- .gitignore | 6 +- CLAUDE.md | 2 +- crates/eql-codegen/src/generate.rs | 85 +---- crates/eql-codegen/src/main.rs | 32 ++ crates/eql-codegen/src/ordering.rs | 290 +++++++++++++++++- crates/eql-codegen/tests/cli.rs | 66 ++++ crates/eql-codegen/tests/parity.rs | 62 ++++ .../adding-a-scalar-encrypted-domain-type.md | 19 +- mise.toml | 2 +- tasks/build.sh | 122 ++------ tasks/build/ordering.sh | 51 +-- tasks/test/build_ordering_helpers_test.sh | 32 +- 12 files changed, 505 insertions(+), 264 deletions(-) diff --git a/.gitignore b/.gitignore index 76c9468a9..28d40fb2d 100644 --- a/.gitignore +++ b/.gitignore @@ -10,12 +10,8 @@ deps-ordered.txt deps-supabase.txt deps-ordered-supabase.txt -src/deps-v3.txt src/deps-ordered-v3.txt -src/generated-order-v3.txt -src/generated-deps-v3.txt -src/handwritten-deps-v3.txt -src/handwritten-ordered-v3.txt +src/deps-ordered-v3.txt.tmp # Generated by tasks/build.sh from src/v3/version.template (eql_v3.version()). src/v3/version.sql diff --git a/CLAUDE.md b/CLAUDE.md index cc21f3b4b..682c6898e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -189,7 +189,7 @@ HTML output is also generated in `docs/api/html/` for local preview only. - SQL files are modular - put operator wrappers in `operators.sql`, implementation in `functions.sql` - All SQL files must have `-- REQUIRE:` dependency declarations -- Build system uses `tsort` to resolve dependency order +- Build system resolves dependency order with `cargo run -p eql-codegen -- order`, which walks the whole `src/v3` surface once and topologically sorts it from the `-- REQUIRE:` edges. Dangling targets, edges leaving `src/v3`, and cycles all fail the build. - **Documentation**: All functions/types must have Doxygen comments (see Documentation Standards above) ### Function Language Choice (SQL vs PL/pgSQL) diff --git a/crates/eql-codegen/src/generate.rs b/crates/eql-codegen/src/generate.rs index c68dbe65b..2b3bdef1f 100644 --- a/crates/eql-codegen/src/generate.rs +++ b/crates/eql-codegen/src/generate.rs @@ -908,44 +908,11 @@ pub fn render_ore_fallback_file() -> String { use std::fs; -use crate::ordering::{requires_of, topo_order}; use crate::writer::{ ensure_generated_paths_writable, normalized_set, remove_generated_orphans, write_generated_file, GeneratedKind, WriteError, }; -/// Ordered generated-file paths plus their dependency edges, both repo-relative. -pub struct GeneratedOrdering { - pub order: Vec, - pub edges: Vec<(String, String)>, // (dep, file) -} - -/// Render every scalar family (pure, no fs writes), read back each file's -/// `-- REQUIRE:` edges, and topologically order the generated surface. Paths are -/// repo-relative (matching the `-- REQUIRE:` and build.sh conventions). -pub fn generated_manifest(out_root: &Path) -> Result { - let scalars_root = out_root.join(V3_SCALARS_DIR); - let mut files: Vec<(String, Vec)> = Vec::new(); - let mut edges: Vec<(String, String)> = Vec::new(); - for spec in eql_domains::scalar_families() { - let out_dir = scalars_root.join(spec.name); - for (path, body) in render_type(spec, &out_dir) { - let rel = path - .strip_prefix(out_root) - .unwrap_or(&path) - .to_string_lossy() - .replace('\\', "/"); - let reqs = requires_of(&body); - for dep in &reqs { - edges.push((dep.clone(), rel.clone())); - } - files.push((rel, reqs)); - } - } - let order = topo_order(&files).map_err(|e| WriteError::Codegen(e.to_string()))?; - Ok(GeneratedOrdering { order, edges }) -} - /// Render every generated file for one type into memory, paired with its output /// path under `out_dir`. Mirrors `bindings::render_bindings`: rendering happens /// before any filesystem mutation, so a render `.expect` panic aborts the run @@ -1120,24 +1087,9 @@ pub fn generate_all(out_root: &Path) -> Result { } } - // Emit the ordering intermediates the build consumes: the topo-ordered - // generated manifest and its dep-edge file. Both are repo-relative, gitignored - // build intermediates (mirroring src/deps-ordered-v3.txt). - let manifest = generated_manifest(out_root)?; - let order_path = out_root.join("src/generated-order-v3.txt"); - fs::write(&order_path, format!("{}\n", manifest.order.join("\n")))?; - let deps_path = out_root.join("src/generated-deps-v3.txt"); - let deps_body: String = manifest - .edges - .iter() - .map(|(d, f)| format!("{d} {f}\n")) - .collect(); - fs::write(&deps_path, deps_body)?; - println!( - "wrote src/generated-order-v3.txt ({} files)", - manifest.order.len() - ); - + // No ordering manifest is emitted here. The installer order is derived by + // `eql-codegen order` from a single walk of the whole src/v3 surface, so the + // generator has no say in — and cannot disagree with — what gets ordered. let names: Vec<&str> = eql_domains::families_with_scalar_domains() .map(|s| s.name) .collect(); @@ -1184,37 +1136,6 @@ mod tests { use super::*; use eql_domains::CATALOG; - #[test] - fn generated_manifest_is_valid_linearization_of_real_catalog() { - let d = crate::writer::test_support::tempdir(); - generate_all(d.path()).unwrap(); - let m = generated_manifest(d.path()).unwrap(); - - // Every generated file appears exactly once. (219 committed scalar files: - // the codegen manifest covers scalar_families() only — version.sql and the - // hand-written scalars/functions.sql are NOT part of it.) - assert_eq!(m.order.len(), 219, "expected 219 generated scalar files"); - let set: std::collections::BTreeSet<&String> = m.order.iter().collect(); - assert_eq!(set.len(), m.order.len(), "duplicate path in manifest"); - - // Every intra-generated edge is respected (dep before file). - let pos: std::collections::HashMap<&str, usize> = m - .order - .iter() - .enumerate() - .map(|(i, p)| (p.as_str(), i)) - .collect(); - for (dep, file) in &m.edges { - if let (Some(di), Some(fi)) = (pos.get(dep.as_str()), pos.get(file.as_str())) { - assert!(di <= fi, "generated dep {dep} ordered after {file}"); - } - } - - // Determinism: a second call is byte-identical. - let m2 = generated_manifest(d.path()).unwrap(); - assert_eq!(m.order, m2.order); - } - fn spec(family_name: &str) -> &'static DomainFamily { CATALOG .iter() diff --git a/crates/eql-codegen/src/main.rs b/crates/eql-codegen/src/main.rs index 8d200d681..1fc7eba0a 100644 --- a/crates/eql-codegen/src/main.rs +++ b/crates/eql-codegen/src/main.rs @@ -69,6 +69,37 @@ fn main() -> ExitCode { } } + // `order`: print the install order of the whole src/v3 SQL surface, one + // repo-relative path per line, dependency before dependent. Consumed by + // tasks/build.sh (`> src/deps-ordered-v3.txt`), which concatenates the files + // in this order into release/cipherstash-encrypt.sql. + // + // The walk is the ONLY enumeration of the surface — hand-written and + // generated files are ordered together from their `-- REQUIRE:` edges, with + // no marker classifier and no separate codegen manifest to fall out of sync + // with. Missing REQUIRE targets, targets outside src/v3, and cycles are all + // hard errors here, so build.sh needs no post-hoc verification of the order. + if args.len() == 2 && args[1] == "order" { + let root = out_root(); + let result = eql_codegen::ordering::walk_v3_surface(&root) + .map_err(|e| format!("walking {}/src/v3: {e}", root.display())) + .and_then(|files| { + eql_codegen::ordering::surface_order(&files).map_err(|e| e.to_string()) + }); + match result { + Ok(order) => { + for path in &order { + println!("{path}"); + } + return ExitCode::SUCCESS; + } + Err(e) => { + eprintln!("error: {e}"); + return ExitCode::FAILURE; + } + } + } + // `clean`: remove the generated SQL surface (marker-aware) under every // src/v3/scalars/* type dir. Replaces build.sh's filename-pattern sweep; // hand-written files (no AUTO-GENERATED marker) are preserved. @@ -102,6 +133,7 @@ fn main() -> ExitCode { } eprintln!("Usage: eql-codegen (generate all types)"); + eprintln!(" eql-codegen order (print the src/v3 install order, one path per line)"); eprintln!(" eql-codegen clean (remove the generated SQL surface)"); eprintln!(" eql-codegen list-types (print catalog tokens)"); eprintln!(" eql-codegen list-schemas (print owned schemas, public first)"); diff --git a/crates/eql-codegen/src/ordering.rs b/crates/eql-codegen/src/ordering.rs index 20fdde2e9..d2fb208e5 100644 --- a/crates/eql-codegen/src/ordering.rs +++ b/crates/eql-codegen/src/ordering.rs @@ -1,7 +1,25 @@ -//! Deterministic topological ordering of the generated SQL surface. +//! Deterministic topological ordering of the whole `src/v3` SQL surface. +//! +//! One enumeration orders every file: hand-written and generated alike are +//! walked off disk, their `-- REQUIRE:` edges parsed, and the result linearized +//! by [`surface_order`]. There is deliberately no generated/hand-written +//! classifier here. An earlier design split the surface into two blocks — a +//! shell glob that skipped the `-- AUTOMATICALLY GENERATED FILE.` marker, and a +//! codegen-emitted manifest of `render_type` output — and a cross-family +//! generated file (`scalars/ore_fallback.sql`, rendered outside `render_type`) +//! matched neither predicate and was silently dropped from the installer. You +//! order exactly the set you walk, so that class of bug is unrepresentable. use std::cmp::Reverse; use std::collections::{BTreeMap, BTreeSet, BinaryHeap}; +use std::fs; +use std::io; +use std::path::Path; + +/// The surface root, relative to the repo root. Every node and every +/// `-- REQUIRE:` target must live under it — the eql_v3 installer is +/// self-contained and owns no edge pointing outside this tree. +pub const SURFACE_ROOT: &str = "src/v3"; /// A dependency cycle among generated files — the topo-sort could not linearize. #[derive(Debug)] @@ -21,10 +39,118 @@ impl std::fmt::Display for CycleError { } impl std::error::Error for CycleError {} -/// Read back the anchored `-- REQUIRE:` targets from a rendered SQL body. The -/// body was produced in-process from the typed `requires` vec via the template, -/// so this is a deterministic readback of the same data — not the fragile -/// cross-platform shell glob of 220 on-disk files this refactor removes. +/// A `-- REQUIRE:` target that is not a node in the surface, or points outside it. +#[derive(Debug)] +pub enum OrderError { + /// Targets naming a file that does not exist in the walked surface. Subsumes + /// the old `verify_deps_exist` shell gate, which only checked the converse + /// (every *listed* file exists on disk) and so never noticed a file on disk + /// that no block listed. + UnknownTargets(Vec<(String, String)>), + /// Targets outside `src/v3`. The v3 installer is self-contained: an edge to + /// (say) `src/v2/foo.sql` would pull non-v3 SQL into the artefact. Subsumes + /// the old `verify_v3_self_contained` shell gate. + OutsideSurface(Vec<(String, String)>), + /// The edges do not linearize. + Cycle(CycleError), +} + +impl std::fmt::Display for OrderError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // Every offender is listed, not just the first: a REQUIRE typo tends to + // come in batches (a renamed file breaks every dependent at once), and + // one-at-a-time diagnostics turn that into one build per typo. + match self { + Self::UnknownTargets(v) => { + writeln!(f, "-- REQUIRE: target does not exist:")?; + for (file, dep) in v { + writeln!(f, " {file} requires {dep}")?; + } + write!(f, "check the -- REQUIRE: directives above for typos") + } + Self::OutsideSurface(v) => { + writeln!(f, "-- REQUIRE: target outside {SURFACE_ROOT}:")?; + for (file, dep) in v { + writeln!(f, " {file} requires {dep}")?; + } + write!( + f, + "the eql_v3 surface must be self-contained — no edge may leave {SURFACE_ROOT}" + ) + } + Self::Cycle(e) => write!(f, "{e}"), + } + } +} +impl std::error::Error for OrderError {} + +/// Linearize the whole surface. `files` is `(repo-relative path, its REQUIRE +/// targets)` for EVERY `.sql` file in the surface. +/// +/// Unlike [`topo_order`], which tolerates edges to non-nodes, this validates +/// first: every target must be a node, and must live under [`SURFACE_ROOT`]. +/// Both gates ran in shell before; keeping them here means the invariant travels +/// with the sort rather than with whoever remembers to call the checker. +pub fn surface_order(files: &[(String, Vec)]) -> Result, OrderError> { + let nodes: BTreeSet<&str> = files.iter().map(|(p, _)| p.as_str()).collect(); + let (mut outside, mut unknown) = (Vec::new(), Vec::new()); + for (file, deps) in files { + for dep in deps { + let prefix = format!("{SURFACE_ROOT}/"); + if !dep.starts_with(&prefix) { + outside.push((file.clone(), dep.clone())); + } else if !nodes.contains(dep.as_str()) { + unknown.push((file.clone(), dep.clone())); + } + } + } + // Outside-surface first: such a target is also "unknown" (it is not a node), + // and the self-containment breach is the more actionable diagnosis. + if !outside.is_empty() { + return Err(OrderError::OutsideSurface(outside)); + } + if !unknown.is_empty() { + return Err(OrderError::UnknownTargets(unknown)); + } + topo_order(files).map_err(OrderError::Cycle) +} + +/// Walk `/src/v3` for the surface: every `.sql` file paired with its +/// `-- REQUIRE:` targets, sorted by path. `*_test.sql` is excluded (it is not +/// part of the installer). +/// +/// Symlinked subdirectories are NOT followed — `file_type()` reports the link +/// itself, where `Path::is_dir()` would resolve it and could walk out of the +/// tree. Mirrors the orphan-sweep guard in `generate.rs`. +pub fn walk_v3_surface(root: &Path) -> io::Result)>> { + let mut files = Vec::new(); + let mut stack = vec![root.join(SURFACE_ROOT)]; + while let Some(dir) = stack.pop() { + for entry in fs::read_dir(&dir)? { + let entry = entry?; + let path = entry.path(); + if entry.file_type()?.is_dir() { + stack.push(path); + continue; + } + let name = entry.file_name().to_string_lossy().into_owned(); + if !name.ends_with(".sql") || name.ends_with("_test.sql") { + continue; + } + let rel = path + .strip_prefix(root) + .unwrap_or(&path) + .to_string_lossy() + .replace('\\', "/"); + files.push((rel, requires_of(&fs::read_to_string(&path)?))); + } + } + files.sort(); + Ok(files) +} + +/// Read back the anchored `-- REQUIRE:` targets from a SQL body — either a body +/// rendered in-process from the typed `requires` vec, or one read off disk. pub fn requires_of(body: &str) -> Vec { body.lines() .filter_map(|l| l.trim_start().strip_prefix("-- REQUIRE:")) @@ -144,6 +270,160 @@ mod tests { assert!(err.remaining.contains(&"b.sql".to_string())); } + fn f(path: &str, deps: &[&str]) -> (String, Vec) { + ( + path.to_string(), + deps.iter().map(|d| d.to_string()).collect(), + ) + } + + // The happy path: every target is a node under src/v3, and edges are honoured. + #[test] + fn surface_order_linearizes_a_valid_surface() { + let files = vec![ + f("src/v3/ops.sql", &["src/v3/types.sql"]), + f("src/v3/types.sql", &["src/v3/schema.sql"]), + f("src/v3/schema.sql", &[]), + f("src/v3/orphan.sql", &[]), // no edges: must still be emitted + ]; + let out = surface_order(&files).unwrap(); + let pos = |n: &str| out.iter().position(|x| x == n).unwrap(); + assert!(pos("src/v3/schema.sql") < pos("src/v3/types.sql")); + assert!(pos("src/v3/types.sql") < pos("src/v3/ops.sql")); + assert!(out.contains(&"src/v3/orphan.sql".to_string())); + assert_eq!(out.len(), 4); + } + + // A REQUIRE naming a file that is not in the surface is a hard error. This is + // the gate the old shell `verify_deps_exist` could not express. + #[test] + fn surface_order_rejects_unknown_target() { + let files = vec![f("src/v3/a.sql", &["src/v3/missing.sql"])]; + let err = surface_order(&files).unwrap_err(); + let OrderError::UnknownTargets(v) = &err else { + panic!("expected UnknownTargets, got {err:?}"); + }; + assert_eq!( + v.as_slice(), + &[("src/v3/a.sql".into(), "src/v3/missing.sql".into())] + ); + assert!(err.to_string().contains("src/v3/missing.sql")); + } + + // An edge leaving src/v3 breaks self-containment, and is reported as such + // rather than as a generic unknown target. + #[test] + fn surface_order_rejects_target_outside_the_surface() { + let files = vec![f("src/v3/a.sql", &["src/v2/x.sql"]), f("src/v2/x.sql", &[])]; + let err = surface_order(&files).unwrap_err(); + assert!( + matches!(err, OrderError::OutsideSurface(_)), + "expected OutsideSurface, got {err:?}" + ); + assert!(err.to_string().contains("self-contained")); + } + + // `src/v3xyz/` must not pass the prefix check on a bare `starts_with("src/v3")`. + #[test] + fn surface_order_prefix_check_is_path_segment_exact() { + let files = vec![f("src/v3/a.sql", &["src/v3suffix/x.sql"])]; + assert!(matches!( + surface_order(&files).unwrap_err(), + OrderError::OutsideSurface(_) + )); + } + + // A cycle surfaces as a cycle, not as a silently truncated order. + #[test] + fn surface_order_propagates_cycles() { + let files = vec![ + f("src/v3/a.sql", &["src/v3/b.sql"]), + f("src/v3/b.sql", &["src/v3/a.sql"]), + ]; + assert!(matches!( + surface_order(&files).unwrap_err(), + OrderError::Cycle(_) + )); + } + + // Identical input twice => identical output. The build's byte-reproducibility + // rests on this (the monolith is concatenated in this order). + #[test] + fn surface_order_is_deterministic() { + let files = vec![ + f("src/v3/c.sql", &["src/v3/a.sql"]), + f("src/v3/a.sql", &[]), + f("src/v3/b.sql", &["src/v3/a.sql"]), + ]; + assert_eq!( + surface_order(&files).unwrap(), + surface_order(&files).unwrap() + ); + assert_eq!( + surface_order(&files).unwrap(), + vec!["src/v3/a.sql", "src/v3/b.sql", "src/v3/c.sql"] + ); + } + + // The walk finds nested files, reads their edges, skips `*_test.sql`, and is + // blind to the generated/hand-written marker (both kinds are ordered together). + #[test] + fn walk_v3_surface_collects_every_sql_file_with_its_edges() { + let d = crate::writer::test_support::tempdir(); + let v3 = d.path().join("src/v3"); + fs::create_dir_all(v3.join("scalars/integer")).unwrap(); + fs::write(v3.join("schema.sql"), "CREATE SCHEMA eql_v3;\n").unwrap(); + fs::write( + v3.join("scalars/ore_fallback.sql"), + "-- AUTOMATICALLY GENERATED FILE.\n-- REQUIRE: src/v3/schema.sql\n", + ) + .unwrap(); + fs::write( + v3.join("scalars/integer/integer_types.sql"), + "-- REQUIRE: src/v3/schema.sql\n", + ) + .unwrap(); + fs::write(v3.join("scalars/integer/x_test.sql"), "SELECT 1;\n").unwrap(); + fs::write(v3.join("notes.md"), "not sql\n").unwrap(); + + let files = walk_v3_surface(d.path()).unwrap(); + let paths: Vec<&str> = files.iter().map(|(p, _)| p.as_str()).collect(); + // Path-sorted, so `scalars/` precedes `schema.sql`. The walk order carries + // no dependency meaning — surface_order supplies that, below. + assert_eq!( + paths, + vec![ + "src/v3/scalars/integer/integer_types.sql", + "src/v3/scalars/ore_fallback.sql", + "src/v3/schema.sql", + ], + "walk must be sorted, skip *_test.sql and non-sql, and include the \ + cross-family generated file the two-block build used to drop" + ); + assert_eq!(files[1].1, vec!["src/v3/schema.sql"]); + // And the walked surface linearizes: schema.sql moves ahead of its dependents. + assert_eq!(surface_order(&files).unwrap()[0], "src/v3/schema.sql"); + } + + // A symlinked subdirectory is not followed: `file_type()` reports the link, + // where `Path::is_dir()` would resolve it and walk outside the tree. + #[cfg(unix)] + #[test] + fn walk_v3_surface_does_not_follow_symlinked_subdir() { + let d = crate::writer::test_support::tempdir(); + let v3 = d.path().join("src/v3"); + fs::create_dir_all(&v3).unwrap(); + fs::write(v3.join("schema.sql"), "SELECT 1;\n").unwrap(); + let outside = d.path().join("outside"); + fs::create_dir_all(&outside).unwrap(); + fs::write(outside.join("stray.sql"), "SELECT 2;\n").unwrap(); + std::os::unix::fs::symlink(&outside, v3.join("linked")).unwrap(); + + let files = walk_v3_surface(d.path()).unwrap(); + let paths: Vec<&str> = files.iter().map(|(p, _)| p.as_str()).collect(); + assert_eq!(paths, vec!["src/v3/schema.sql"]); + } + // requires_of reads back anchored `-- REQUIRE:` lines from a rendered body. #[test] fn requires_of_reads_anchored_directives() { diff --git a/crates/eql-codegen/tests/cli.rs b/crates/eql-codegen/tests/cli.rs index 3a87548d2..cbf7c0f4f 100644 --- a/crates/eql-codegen/tests/cli.rs +++ b/crates/eql-codegen/tests/cli.rs @@ -86,6 +86,72 @@ fn list_schemas_subcommand_prints_owned_schemas() { ); } +/// `order` exits 0 and prints the real surface's install order, one repo-relative +/// path per line, dependency first. `tasks/build.sh` redirects this straight into +/// `src/deps-ordered-v3.txt` and concatenates the files in this order, so the +/// stdout contract is pinned here: nothing but paths (no banner, no progress). +#[test] +fn order_subcommand_prints_the_install_order() { + let out = Command::new(bin()) + .arg("order") + .output() + .expect("run eql-codegen order"); + assert!( + out.status.success(), + "order should exit 0; stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + let lines: Vec<&str> = stdout.lines().collect(); + assert_eq!( + lines.first(), + Some(&"src/v3/schema.sql"), + "schema.sql creates the schemas everything else requires, so it must come first" + ); + assert!( + lines + .iter() + .all(|l| l.starts_with("src/v3/") && l.ends_with(".sql")), + "stdout must be paths only — build.sh feeds it to `strip_require_lines` unfiltered" + ); +} + +/// A `-- REQUIRE:` naming a file that does not exist fails the build loudly +/// instead of emitting a short order. The two-block scheme this replaced could +/// omit a file and still exit 0, which is how `scalars/ore_fallback.sql` reached +/// a green build while missing from `release/cipherstash-encrypt.sql`. +#[test] +fn order_subcommand_fails_on_a_dangling_require() { + let root = tempdir(); + let v3 = root.0.join("src/v3"); + std::fs::create_dir_all(&v3).unwrap(); + std::fs::write(v3.join("schema.sql"), "CREATE SCHEMA eql_v3;\n").unwrap(); + std::fs::write( + v3.join("broken.sql"), + "-- REQUIRE: src/v3/typo.sql\nSELECT 1;\n", + ) + .unwrap(); + + let out = Command::new(bin()) + .arg("order") + .env("EQL_CODEGEN_OUT_ROOT", root.0.as_os_str()) + .output() + .expect("run eql-codegen order"); + assert!( + !out.status.success(), + "a dangling REQUIRE must fail the build, not emit a partial order" + ); + assert!( + out.stdout.is_empty(), + "a failed order must print no partial order to stdout" + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("src/v3/typo.sql") && stderr.contains("src/v3/broken.sql"), + "the error must name both the dangling target and the file requiring it, got:\n{stderr}" + ); +} + /// An unrecognised argument prints usage and exits 2 (the `ExitCode::from(2)` /// fall-through in `main.rs`). #[test] diff --git a/crates/eql-codegen/tests/parity.rs b/crates/eql-codegen/tests/parity.rs index e3fb90a04..45bd2a901 100644 --- a/crates/eql-codegen/tests/parity.rs +++ b/crates/eql-codegen/tests/parity.rs @@ -115,6 +115,68 @@ fn every_generated_sql_file_starts_with_marker() { ); } +/// Every `.sql` file in the real `src/v3` tree is in the install order — the +/// completeness invariant, checked against an independent walk rather than a +/// hardcoded count. +/// +/// This is the gate that a two-block build could not express. When the surface +/// was ordered as "hand-written files (globbed, minus the AUTO-GENERATED marker)" +/// plus "generated files (from a codegen manifest of `render_type` output)", the +/// cross-family `scalars/ore_fallback.sql` — marker-bearing, but rendered outside +/// `render_type` — matched neither and was silently dropped from the installer, +/// taking the ORE poison constraints with it. Set equality, not a count: a new +/// cross-family generated file is required here the moment it lands on disk. +/// +/// Runs against the real tree, not a tempdir: `surface_order` validates that +/// every `-- REQUIRE:` target is a node, and a generate-only tempdir has no +/// `schema.sql` or `sem/**` for the generated files to point at. +#[test] +fn install_order_contains_every_v3_sql_file() { + let root = repo_root(); + + // Independent of walk_v3_surface: if the walker under-collects, this diverges. + let mut on_disk: BTreeSet = BTreeSet::new(); + let mut stack = vec![root.join("src/v3")]; + while let Some(dir) = stack.pop() { + for entry in fs::read_dir(&dir).unwrap() { + let entry = entry.unwrap(); + let path = entry.path(); + if entry.file_type().unwrap().is_dir() { + stack.push(path); + continue; + } + let name = entry.file_name().to_string_lossy().into_owned(); + if name.ends_with(".sql") && !name.ends_with("_test.sql") { + on_disk.insert( + path.strip_prefix(&root) + .unwrap() + .to_string_lossy() + .replace('\\', "/"), + ); + } + } + } + + let files = eql_codegen::ordering::walk_v3_surface(&root).expect("walk src/v3"); + let ordered: BTreeSet = eql_codegen::ordering::surface_order(&files) + .expect( + "src/v3 surface must linearize: every REQUIRE target a node under src/v3, no cycles", + ) + .into_iter() + .collect(); + + assert_eq!( + ordered, on_disk, + "the install order must contain exactly the src/v3 SQL files on disk — a file \ + present on disk but absent from the order is silently missing from \ + release/cipherstash-encrypt.sql" + ); + assert!( + on_disk.contains("src/v3/schema.sql"), + "sanity: the walk found no schema.sql, so it is not seeing the real tree" + ); +} + #[test] fn generate_all_skips_non_scalar_families() { let tmp = tempdir("skip-non-scalar"); diff --git a/docs/reference/adding-a-scalar-encrypted-domain-type.md b/docs/reference/adding-a-scalar-encrypted-domain-type.md index 5d4e3b826..d59440f0d 100644 --- a/docs/reference/adding-a-scalar-encrypted-domain-type.md +++ b/docs/reference/adding-a-scalar-encrypted-domain-type.md @@ -56,14 +56,17 @@ To add a scalar type `` (e.g. `bigint`), with Rust type `` (e.g. `i64`): no per-type codegen task. The generated `*_{types,functions,operators,aggregates}.sql` are committed in place under `src/v3/scalars//` — regenerate and commit the SQL diff alongside the catalog change. - - **Ordering is emitted by the codegen — you do nothing.** The topological - order of the generated scalar files is produced by `eql-codegen` - (name-sorted, cycle-checked) into `src/generated-order-v3.txt`; - `tasks/build.sh` concatenates it after the hand-written files. Adding a - catalog row needs no `-- REQUIRE:` edits to any generated file. Only - hand-written files under `src/v3/` (SEM types, `jsonb/`, `schema.sql`, - `crypto.sql`, `common.sql`, `scalars/functions.sql`, `lint/lints.sql`, - `*_extensions.sql`) carry authored `-- REQUIRE:` edges. + - **Ordering is resolved by the codegen — you do nothing.** `eql-codegen order` + walks the whole `src/v3` surface once and topologically sorts it from the + `-- REQUIRE:` edges every file declares (name-sorted tie-break, cycle- and + dangling-target-checked); `tasks/build.sh` concatenates the files in that + order. Generated and hand-written files are ordered together — there is no + separate generated block, so a generated file cannot fall between the two. + Adding a catalog row needs no `-- REQUIRE:` edits to any generated file: the + renderers emit each file's edges. Only hand-written files under `src/v3/` + (SEM types, `jsonb/`, `schema.sql`, `crypto.sql`, `common.sql`, + `scalars/functions.sql`, `lint/lints.sql`, `*_extensions.sql`) carry + authored `-- REQUIRE:` edges. - The catalog row ALSO drives the **Rust payload bindings**: `eql-codegen bindings` (run first by `mise run types:generate`) regenerates the committed `crates/eql-bindings/src/v3/.rs` struct + `DomainType` diff --git a/mise.toml b/mise.toml index 08d091c45..51cbf1923 100644 --- a/mise.toml +++ b/mise.toml @@ -235,7 +235,7 @@ bash tasks/test/verify_symbol_order_v3.sh src/deps-ordered-v3.txt """ [tasks."test:build_ordering_helpers"] -description = "Unit tests for tasks/build/ordering.sh (DB-free)" +description = "Unit tests for tasks/build/ordering.sh strip_require_lines (DB-free)" dir = "{{config_root}}" run = "bash tasks/test/build_ordering_helpers_test.sh" diff --git a/tasks/build.sh b/tasks/build.sh index 7520329f9..a33b702f6 100755 --- a/tasks/build.sh +++ b/tasks/build.sh @@ -14,7 +14,7 @@ source tasks/build/ordering.sh # Regenerate encrypted-domain SQL from the Rust catalog before building. # The generated files (src/v3/scalars//_*.sql) are COMMITTED in place and # drift-gated by `mise run codegen:parity`; only src/v3/version.sql and the -# src/deps*-v3.txt build intermediates are gitignored. The catalog at +# src/deps-ordered-v3.txt build intermediate are gitignored. The catalog at # crates/eql-domains/src (eql-domains::CATALOG) is the source of truth, rendered # by the eql-codegen binary. # @@ -29,113 +29,45 @@ source tasks/build/ordering.sh # # The plaintext fixture lists are not generated — the SQLx tests read them # straight from the catalog (eql_domains::INT4_VALUES / …). -cargo run -p eql-codegen - -# Fail loudly if any file referenced in a tsorted dep list doesn't exist. -# Without this, `xargs cat` would print `cat: foo.sql: No such file or directory` -# and continue — silently producing an incomplete release artefact. -verify_deps_exist() { - local dep_file=$1 - local missing=0 - while IFS= read -r f; do - if [[ ! -f "$f" ]]; then - echo "ERROR: $dep_file references missing file: $f" >&2 - missing=1 - fi - done < "$dep_file" - if [[ $missing -ne 0 ]]; then - echo "ERROR: dependency graph references missing files (see above). Check -- REQUIRE: directives." >&2 - exit 1 - fi -} - -# Fail loudly if any v3 REQUIRE edge points OUTSIDE src/v3. The v3-only build -# must be self-contained (no eql_v2 coupling); a stray `-- REQUIRE: src/...` -# edge to a non-v3 file would silently pull eql_v2 SQL into the v3 artefact (or -# tsort would drop it), breaking self-containment. Each line in deps-v3.txt is -# " " (dependency FIRST); self-edges (dep == file) are skipped, every -# other dep target (field 1) must start with src/v3/. -verify_v3_self_contained() { - local dep_file=$1 - local offending=0 - while IFS=' ' read -r dep src; do - [[ -z "$src" ]] && continue - [[ "$src" == "$dep" ]] && continue - if [[ "$dep" != src/v3/* ]]; then - echo "ERROR: v3 REQUIRE edge points outside src/v3: $src -- REQUIRE: $dep" >&2 - offending=1 - fi - done < "$dep_file" - if [[ $offending -ne 0 ]]; then - echo "ERROR: v3-only build is not self-contained — a -- REQUIRE: target lives outside src/v3 (see above)." >&2 - exit 1 - fi -} +cargo run -q -p eql-codegen mkdir -p release rm -f release/cipherstash-encrypt.sql rm -f release/cipherstash-encrypt-uninstall.sql - -# Truncate the build intermediates we APPEND to below. The generated-*.txt files -# are (re)written wholesale by eql-codegen above, so they are NOT removed here. -rm -f src/deps-v3.txt src/deps-ordered-v3.txt src/handwritten-deps-v3.txt src/handwritten-ordered-v3.txt +rm -f src/deps-ordered-v3.txt src/deps-ordered-v3.txt.tmp rm -f src/v3/version.sql -# Bake the release version into eql_v3.version() (and the eql_v3 schema -# comment) before the glob below picks it up. The version is supplied via -# `mise run build --version ` (the `usage_version` env var mise derives -# from the #USAGE flag); local builds with no flag fall back to DEV. The -# generated src/v3/version.sql is gitignored, like the other generated v3 SQL. +# Bake the release version into eql_v3.version() (and the eql_v3 schema comment). +# The version is supplied via `mise run build --version ` (the +# `usage_version` env var mise derives from the #USAGE flag); local builds with +# no flag fall back to DEV. The generated src/v3/version.sql is gitignored. +# +# This MUST precede `eql-codegen order` below: the ordering walks the surface on +# disk, so version.sql has to exist to be ordered into the installer. RELEASE_VERSION=${usage_version:-DEV} sed "s/\$RELEASE_VERSION/$RELEASE_VERSION/g" src/v3/version.template > src/v3/version.sql -# The self-contained eql_v3 surface — schema, SEM types, scalar domains — -# globbed from src/v3 ONLY. This is the sole EQL artifact: it owns no eql_v2 -# dependency (CI-gated by verify_v3_self_contained below + test:self_contained_v3), -# and it is written under the canonical release name now that the combined v2 -# build that previously produced that name is gone. -find src/v3 -type f -path "*.sql" ! -path "*_test.sql" -print0 \ - | LC_ALL=C sort -z \ - | while IFS= read -r -d '' sql_file; do - IFS= read -r first < "$sql_file" || first="" - # Generated scalar files are ordered by eql-codegen (src/generated-order-v3.txt); - # only hand-written files are parsed here. The classifier is the EXACT - # period-terminated marker ("-- AUTOMATICALLY GENERATED FILE."): generated - # scalars carry it, but src/v3/version.sql carries a period-LESS marker and - # IS hand-written (it has an authored -- REQUIRE: edge to schema.sql and is - # not part of the codegen manifest), so it must be parsed, not skipped. - [[ "$first" == "-- AUTOMATICALLY GENERATED FILE."* ]] && continue - echo "$sql_file $sql_file" >> src/handwritten-deps-v3.txt # self-edge - while IFS= read -r line; do - if [[ "$line" =~ ^[[:space:]]*--\ REQUIRE: ]]; then - deps=${line#*-- REQUIRE: } - for dep in $deps; do - echo "$dep $sql_file" >> src/handwritten-deps-v3.txt # dependency first - done - fi - done < "$sql_file" - done - -# Union edge set for the whole-surface verifiers (hand-written + generated). -cat src/handwritten-deps-v3.txt src/generated-deps-v3.txt > src/deps-v3.txt - -# Whole-surface cycle gate (verification only) — tsort output discarded. -verify_v3_self_contained src/deps-v3.txt -run_tsort_or_die src/deps-v3.txt /dev/null - -# Phase A: order the ~25 hand-written files from their authored edges. -run_tsort_or_die src/handwritten-deps-v3.txt src/handwritten-ordered-v3.txt - -# Phase B: hand-written order, then the codegen-emitted generated order. Valid -# because NO hand-written file depends on a generated file (verified), so every -# generated->hand-written edge points backward into the already-emitted block. -cat src/handwritten-ordered-v3.txt src/generated-order-v3.txt > src/deps-ordered-v3.txt +# Resolve the install order of the whole eql_v3 surface — schema, SEM types, +# hand-written jsonb, generated scalars, version.sql — in ONE walk of src/v3, +# topologically sorted from the `-- REQUIRE:` edges every file declares. +# +# `eql-codegen order` is the sole enumeration of the surface, and it fails the +# build on a missing REQUIRE target, on an edge leaving src/v3 (self-containment, +# also gated by test:self_contained_v3), and on a dependency cycle. It replaces a +# two-block scheme — shell-globbed hand-written files, plus a codegen manifest of +# the generated ones — whose two enumerations could disagree about a file and +# silently drop it from the installer. Ordering what you walk makes that +# unrepresentable; `install_order_contains_every_v3_sql_file` in the codegen +# crate's parity tests pins the invariant. +# +# Written via a temp file so an aborted order leaves no truncated list behind for +# the downstream tasks (test:self_contained_v3, test:symbol_order_v3) to read. +cargo run -q -p eql-codegen -- order > src/deps-ordered-v3.txt.tmp +mv src/deps-ordered-v3.txt.tmp src/deps-ordered-v3.txt -verify_deps_exist src/deps-ordered-v3.txt -verify_linearization src/deps-v3.txt src/deps-ordered-v3.txt bash tasks/test/verify_symbol_order_v3.sh src/deps-ordered-v3.txt : > release/cipherstash-encrypt.sql diff --git a/tasks/build/ordering.sh b/tasks/build/ordering.sh index 3acdd5c53..99d5b27cf 100644 --- a/tasks/build/ordering.sh +++ b/tasks/build/ordering.sh @@ -1,7 +1,12 @@ #!/usr/bin/env bash -# Sourceable dependency-ordering helpers for the eql_v3 build. No side effects on -# source; each function is pure w.r.t. its args. Shared with the staged-installer -# refactor (do not fork strip_require_lines). +# Sourceable helpers for the eql_v3 build. No side effects on source; each +# function is pure w.r.t. its args. Shared with the staged-installer refactor +# (do not fork strip_require_lines). +# +# Dependency ordering itself now lives in `eql-codegen order` (see +# crates/eql-codegen/src/ordering.rs), which walks src/v3 once and topologically +# sorts the whole surface. The shell tsort wrapper and linearization checker this +# file used to carry are gone with the two-block build they served. # Emit a file's body with anchored `-- REQUIRE:` directive lines removed. Anchored # (allows leading whitespace) so a body line that merely contains the substring @@ -15,43 +20,3 @@ strip_require_lines() { # a truncated monolith. (The blanket `|| true` this replaces hid exit 2.) (( rc <= 1 )) } - -# tsort with a cross-platform cycle gate. Input edges are " " (one per -# line, dependency FIRST) so plain tsort yields dependency-before-file order with -# no `tac`. BSD/macOS tsort exits 0 on a cycle but writes "tsort: cycle in data" -# to stderr; GNU exits 1. We fail on ANY tsort stderr, catching cycles on both. -# NOTE: the extraction emits a self-edge " " for every file so an -# isolated file still appears in the output. A self-edge is a tsort NO-OP, NOT a -# cycle — `printf 'a a\n' | tsort` prints `a` with empty stderr and exit 0 on -# both BSD and GNU, so self-edges never trip this stderr-based cycle-fail. -run_tsort_or_die() { - local edges=$1 out=$2 err - err="$(mktemp)" - tsort "$edges" > "$out" 2> "$err" || true - if [[ -s "$err" ]]; then - echo "ERROR: tsort reported a problem ordering $edges (cycle or malformed edge):" >&2 - cat "$err" >&2 - rm -f "$err" - return 1 - fi - rm -f "$err" -} - -# Assert the assembled order is a valid linearization: for every " " -# edge, dep must appear at or before file in . Self-edges and edges -# whose endpoints are absent from the order are skipped (absence is caught by -# verify_deps_exist). Complements run_tsort_or_die's cycle check with a direct -# check on the FINAL order (the two-phase concat is not produced by one tsort). -verify_linearization() { - local edges=$1 ordered=$2 - awk ' - NR==FNR { pos[$0]=FNR; next } - { if ($1=="" || $1==$2) next - if (!($1 in pos) || !($2 in pos)) next - if (pos[$1] > pos[$2]) { - printf("ERROR: %s is required by %s but is ordered AFTER it\n", $1, $2) > "/dev/stderr" - bad=1 - } } - END { exit bad ? 1 : 0 } - ' "$ordered" "$edges" -} diff --git a/tasks/test/build_ordering_helpers_test.sh b/tasks/test/build_ordering_helpers_test.sh index 4a0a67881..bcc7324ef 100644 --- a/tasks/test/build_ordering_helpers_test.sh +++ b/tasks/test/build_ordering_helpers_test.sh @@ -1,50 +1,34 @@ #!/usr/bin/env bash -#MISE description="DB-free unit tests for tasks/build/ordering.sh (cycle gate, edge reversal, anchored strip)" +#MISE description="DB-free unit tests for tasks/build/ordering.sh (anchored REQUIRE strip)" set -euo pipefail REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" cd "$REPO_ROOT" source tasks/build/ordering.sh -tmp="$(mktemp -d)"; trap 'rm -rf "$tmp"' EXIT - -# 1. Cycle gate must FAIL on both BSD and GNU tsort (BSD exits 0 but prints to stderr). -printf 'a b\nb a\n' > "$tmp/cyc.txt" -if run_tsort_or_die "$tmp/cyc.txt" "$tmp/cyc.out" 2>/dev/null; then - echo "FAIL: cycle not rejected"; exit 1 -fi -echo "ok: cycle rejected" +# strip_require_lines is all that remains in ordering.sh: the tsort cycle gate and +# the linearization checker moved into `eql-codegen order`, and are covered by the +# eql-codegen crate's ordering:: unit tests. -# 2. Edge reversal: dependency-first edges yield dependency-before-file order (no tac). -printf 'schema.sql types.sql\ntypes.sql ops.sql\n' > "$tmp/ok.txt" -run_tsort_or_die "$tmp/ok.txt" "$tmp/ok.out" -[[ "$(tr '\n' ' ' < "$tmp/ok.out")" == "schema.sql types.sql ops.sql " ]] || { echo "FAIL: order $(cat "$tmp/ok.out")"; exit 1; } -echo "ok: dependency-first order without tac" +tmp="$(mktemp -d)"; trap 'rm -rf "$tmp"' EXIT -# 3. Anchored strip keeps a body line that merely contains the substring REQUIRE. +# 1. Anchored strip keeps a body line that merely contains the substring REQUIRE. printf -- '-- REQUIRE: src/v3/schema.sql\nSELECT 1; -- the REQUIRE keyword in prose\n' > "$tmp/body.sql" out="$(strip_require_lines "$tmp/body.sql")" [[ "$out" == "SELECT 1; -- the REQUIRE keyword in prose" ]] || { echo "FAIL: strip removed non-directive line: [$out]"; exit 1; } echo "ok: anchored strip preserves non-directive REQUIRE substring" -# 3b. strip_require_lines must FAIL (not silently succeed) on an unreadable file — +# 2. strip_require_lines must FAIL (not silently succeed) on an unreadable file — # a real grep error (exit >= 2) propagates so `set -e` aborts a truncated build. if strip_require_lines "$tmp/does-not-exist.sql" 2>/dev/null; then echo "FAIL: strip_require_lines swallowed a missing-file error"; exit 1 fi echo "ok: strip_require_lines propagates a real grep failure" -# 3c. A file that is ENTIRELY -- REQUIRE: lines (grep exit 1, nothing left) is +# 3. A file that is ENTIRELY -- REQUIRE: lines (grep exit 1, nothing left) is # NOT an error — it contributes no body and must succeed with empty output. printf -- '-- REQUIRE: a\n-- REQUIRE: b\n' > "$tmp/allreq.sql" out="$(strip_require_lines "$tmp/allreq.sql")" || { echo "FAIL: all-REQUIRE file treated as error"; exit 1; } [[ -z "$out" ]] || { echo "FAIL: all-REQUIRE file produced output: [$out]"; exit 1; } echo "ok: all-REQUIRE file succeeds with empty output" -# 4. verify_linearization fails when a dep is ordered AFTER its dependent. -printf 'types.sql ops.sql\n' > "$tmp/edges.txt" -printf 'ops.sql\ntypes.sql\n' > "$tmp/badorder.txt" -if verify_linearization "$tmp/edges.txt" "$tmp/badorder.txt" 2>/dev/null; then - echo "FAIL: bad linearization accepted"; exit 1 -fi -echo "ok: linearization violation detected" echo "ALL build-ordering helper tests passed" From fccd712ab4a8b90cf8a1cc2c2989a9534b14602a Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 23:11:36 +1000 Subject: [PATCH 10/15] build: harden the symbol-order gate and correct the ordering diagnostics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the build-ordering refactor. The installer is byte-identical before and after; every change is to a gate, a diagnostic, or a comment. The symbol-order cross-check runs inside `mise run build`, and the release workflow reaches it via release-eql.yml -> _build-sql.yml -> `mise run build`. That makes it the sole symbol-order gate on the release path — the separate `test:symbol_order_v3` CI step only guards PRs. Two consequences, both fixed: - The allowlist was read with an unguarded `getline < file`, which cannot distinguish EOF from an unreadable file. A bad ALLOW path silently yielded an empty allowlist. That fails safe today (nothing to suppress), but the moment a real entry lands, a typo'd path resurrects the false positive the entry exists to suppress — during a release. Guard with `test -r`, and make the path overridable via SYMBOL_ORDER_ALLOWLIST so the self-test can exercise it. - The gate is stricter than PostgreSQL: a `LANGUAGE plpgsql` body resolves its callees at execution time, so Postgres accepts a forward reference the gate rejects. Unfixable by a line-oriented scan, which cannot tell a plpgsql body from a `LANGUAGE sql` one (whose callees Postgres DOES resolve at CREATE time, which is what the ordering is for). Pin the trade instead: tests for both the rejection and the allowlist escape hatch, and document the divergence in the script header and the allowlist, warning never to allowlist a `LANGUAGE sql` body — that yields a green build and a failed install. Diagnostics and dead code: - CycleError said "dependency cycle among generated files", inherited from the two-block design this module replaced. The sort draws no generated/hand-written distinction, so a cycle through a hand-authored `-- REQUIRE:` edge sent readers into the codegen. Now names the stuck files; a test asserts the word "generated" never appears. topo_order's doc described the same dead two-block scheme. - topo_order is pub(crate). It tolerates edges to non-nodes; on the real surface a non-node target IS the bug surface_order exists to catch. No external callers. - Drop the never-constructed WriteError::Codegen variant. Clippy does not flag an unused pub enum variant in a library, so it would have rotted silently. - Four comments narrated `scalars/ore_fallback.sql` as a file this build once dropped. It is not in the tree and its adding commit is unreachable from this branch. The bug class is real; the file is another lineage's. Describe the class, and rename the synthetic tempdir fixture to cross_family.sql. Also hoist the per-edge format!("{SURFACE_ROOT}/") out of surface_order's inner loop. --- crates/eql-codegen/src/ordering.rs | 64 ++++++++++++++++++++------- crates/eql-codegen/src/writer.rs | 2 - crates/eql-codegen/tests/cli.rs | 4 +- crates/eql-codegen/tests/parity.rs | 10 ++--- tasks/test/symbol_order_allowlist.txt | 15 ++++++- tasks/test/symbol_order_selftest.sh | 54 +++++++++++++++++----- tasks/test/verify_symbol_order_v3.sh | 55 ++++++++++++++++------- 7 files changed, 149 insertions(+), 55 deletions(-) diff --git a/crates/eql-codegen/src/ordering.rs b/crates/eql-codegen/src/ordering.rs index d2fb208e5..8c679ef63 100644 --- a/crates/eql-codegen/src/ordering.rs +++ b/crates/eql-codegen/src/ordering.rs @@ -5,10 +5,12 @@ //! by [`surface_order`]. There is deliberately no generated/hand-written //! classifier here. An earlier design split the surface into two blocks — a //! shell glob that skipped the `-- AUTOMATICALLY GENERATED FILE.` marker, and a -//! codegen-emitted manifest of `render_type` output — and a cross-family -//! generated file (`scalars/ore_fallback.sql`, rendered outside `render_type`) -//! matched neither predicate and was silently dropped from the installer. You -//! order exactly the set you walk, so that class of bug is unrepresentable. +//! codegen-emitted manifest of `render_type` output. Two enumerations means two +//! predicates, and a file matching neither — a generated one rendered outside +//! `render_type`, say — is silently dropped from the installer while the build +//! stays green. You order exactly the set you walk, so that class of bug is +//! unrepresentable. `install_order_contains_every_v3_sql_file` (parity tests) +//! pins it against an independent walk. use std::cmp::Reverse; use std::collections::{BTreeMap, BTreeSet, BinaryHeap}; @@ -21,7 +23,7 @@ use std::path::Path; /// self-contained and owns no edge pointing outside this tree. pub const SURFACE_ROOT: &str = "src/v3"; -/// A dependency cycle among generated files — the topo-sort could not linearize. +/// A dependency cycle in the surface — the topo-sort could not linearize. #[derive(Debug)] pub struct CycleError { /// The nodes that never reached in-degree 0 (participate in / are blocked by a cycle). @@ -30,9 +32,12 @@ pub struct CycleError { impl std::fmt::Display for CycleError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // Names the files, not their provenance: the sort draws no + // generated/hand-written distinction, and a cycle is as likely to run + // through a hand-authored `-- REQUIRE:` edge as a rendered one. write!( f, - "dependency cycle among generated files: {}", + "-- REQUIRE: dependency cycle, these files never linearize: {}", self.remaining.join(", ") ) } @@ -93,10 +98,10 @@ impl std::error::Error for OrderError {} /// with the sort rather than with whoever remembers to call the checker. pub fn surface_order(files: &[(String, Vec)]) -> Result, OrderError> { let nodes: BTreeSet<&str> = files.iter().map(|(p, _)| p.as_str()).collect(); + let prefix = format!("{SURFACE_ROOT}/"); let (mut outside, mut unknown) = (Vec::new(), Vec::new()); for (file, deps) in files { for dep in deps { - let prefix = format!("{SURFACE_ROOT}/"); if !dep.starts_with(&prefix) { outside.push((file.clone(), dep.clone())); } else if !nodes.contains(dep.as_str()) { @@ -158,13 +163,16 @@ pub fn requires_of(body: &str) -> Vec { .collect() } -/// Deterministic topological order of generated files. `files` is -/// `(repo-relative path, its REQUIRE targets)`. Edges whose target is NOT a key -/// in `files` (hand-written prerequisites) are ignored: the generated block is -/// emitted wholesale AFTER the hand-written block, so those edges are satisfied -/// by construction. Kahn's algorithm with a min-heap keyed by path string gives +/// Deterministic topological order of `files`, each `(repo-relative path, its +/// REQUIRE targets)`. Kahn's algorithm with a min-heap keyed by path string gives /// name-sorted tie-breaking ⇒ byte-reproducible output. -pub fn topo_order(files: &[(String, Vec)]) -> Result, CycleError> { +/// +/// Edges whose target is not itself a key in `files` are ignored rather than +/// rejected. That tolerance is why this is not public: on the real surface a +/// non-node target means a typo'd or escaping `-- REQUIRE:`, and silently +/// ignoring it would drop the very check [`surface_order`] exists to make. Go +/// through [`surface_order`], which validates the targets before sorting. +pub(crate) fn topo_order(files: &[(String, Vec)]) -> Result, CycleError> { let nodes: BTreeSet<&str> = files.iter().map(|(p, _)| p.as_str()).collect(); let mut indeg: BTreeMap<&str, usize> = nodes.iter().map(|n| (*n, 0usize)).collect(); let mut dependents: BTreeMap<&str, Vec<&str>> = BTreeMap::new(); @@ -333,6 +341,27 @@ mod tests { )); } + // The cycle diagnostic must describe the surface it actually sorts. It once + // said "among generated files", inherited from the two-block design this + // module replaced — which sent a reader chasing the codegen when the fix was + // a `-- REQUIRE:` line in their own hand-written file. + #[test] + fn cycle_error_names_the_stuck_files_not_a_generated_block() { + let files = vec![ + f("src/v3/jsonb/a.sql", &["src/v3/jsonb/b.sql"]), + f("src/v3/jsonb/b.sql", &["src/v3/jsonb/a.sql"]), + ]; + let msg = surface_order(&files).unwrap_err().to_string(); + assert!( + !msg.contains("generated"), + "the sort has no generated/hand-written distinction; the message must not imply one: {msg}" + ); + assert!( + msg.contains("src/v3/jsonb/a.sql") && msg.contains("src/v3/jsonb/b.sql"), + "the message must name the stuck files: {msg}" + ); + } + // A cycle surfaces as a cycle, not as a silently truncated order. #[test] fn surface_order_propagates_cycles() { @@ -374,7 +403,7 @@ mod tests { fs::create_dir_all(v3.join("scalars/integer")).unwrap(); fs::write(v3.join("schema.sql"), "CREATE SCHEMA eql_v3;\n").unwrap(); fs::write( - v3.join("scalars/ore_fallback.sql"), + v3.join("scalars/cross_family.sql"), "-- AUTOMATICALLY GENERATED FILE.\n-- REQUIRE: src/v3/schema.sql\n", ) .unwrap(); @@ -393,14 +422,15 @@ mod tests { assert_eq!( paths, vec![ + "src/v3/scalars/cross_family.sql", "src/v3/scalars/integer/integer_types.sql", - "src/v3/scalars/ore_fallback.sql", "src/v3/schema.sql", ], "walk must be sorted, skip *_test.sql and non-sql, and include the \ - cross-family generated file the two-block build used to drop" + cross-family generated file a two-block build would drop" ); - assert_eq!(files[1].1, vec!["src/v3/schema.sql"]); + // The cross-family file's edges came back with it, not just its path. + assert_eq!(files[0].1, vec!["src/v3/schema.sql"]); // And the walked surface linearizes: schema.sql moves ahead of its dependents. assert_eq!(surface_order(&files).unwrap()[0], "src/v3/schema.sql"); } diff --git a/crates/eql-codegen/src/writer.rs b/crates/eql-codegen/src/writer.rs index ec59fe08e..de3f4cf01 100644 --- a/crates/eql-codegen/src/writer.rs +++ b/crates/eql-codegen/src/writer.rs @@ -44,8 +44,6 @@ pub enum WriteError { Ownership(String), #[error("io error: {0}")] Io(#[from] io::Error), - #[error("{0}")] - Codegen(String), } fn first_line(path: &Path) -> io::Result { diff --git a/crates/eql-codegen/tests/cli.rs b/crates/eql-codegen/tests/cli.rs index cbf7c0f4f..4beced971 100644 --- a/crates/eql-codegen/tests/cli.rs +++ b/crates/eql-codegen/tests/cli.rs @@ -118,8 +118,8 @@ fn order_subcommand_prints_the_install_order() { /// A `-- REQUIRE:` naming a file that does not exist fails the build loudly /// instead of emitting a short order. The two-block scheme this replaced could -/// omit a file and still exit 0, which is how `scalars/ore_fallback.sql` reached -/// a green build while missing from `release/cipherstash-encrypt.sql`. +/// omit a file and still exit 0 — a green build with the file missing from +/// `release/cipherstash-encrypt.sql`. #[test] fn order_subcommand_fails_on_a_dangling_require() { let root = tempdir(); diff --git a/crates/eql-codegen/tests/parity.rs b/crates/eql-codegen/tests/parity.rs index 45bd2a901..0482b7d80 100644 --- a/crates/eql-codegen/tests/parity.rs +++ b/crates/eql-codegen/tests/parity.rs @@ -121,11 +121,11 @@ fn every_generated_sql_file_starts_with_marker() { /// /// This is the gate that a two-block build could not express. When the surface /// was ordered as "hand-written files (globbed, minus the AUTO-GENERATED marker)" -/// plus "generated files (from a codegen manifest of `render_type` output)", the -/// cross-family `scalars/ore_fallback.sql` — marker-bearing, but rendered outside -/// `render_type` — matched neither and was silently dropped from the installer, -/// taking the ORE poison constraints with it. Set equality, not a count: a new -/// cross-family generated file is required here the moment it lands on disk. +/// plus "generated files (from a codegen manifest of `render_type` output)", a +/// cross-family generated file — marker-bearing, but rendered outside +/// `render_type` — matched neither predicate and would be silently dropped from +/// the installer. Set equality, not a count: a new cross-family generated file is +/// required here the moment it lands on disk. /// /// Runs against the real tree, not a tempdir: `surface_order` validates that /// every `-- REQUIRE:` target is a node, and a generate-only tempdir has no diff --git a/tasks/test/symbol_order_allowlist.txt b/tasks/test/symbol_order_allowlist.txt index 95e267ef1..69a2f16e1 100644 --- a/tasks/test/symbol_order_allowlist.txt +++ b/tasks/test/symbol_order_allowlist.txt @@ -1,4 +1,15 @@ # One fully-qualified symbol per line (e.g. eql_v3.foo) whose "defined earlier" # check should be skipped. Use ONLY for genuine false positives (a reference the -# regex sees but that is not a real dependency — e.g. a symbol assembled by -# dynamic SQL). Keep this list minimal and comment WHY each entry is here. +# regex sees but that is not a real dependency). Keep this list minimal and +# comment WHY each entry is here. +# +# The two cases that are genuinely not dependencies: +# - a symbol assembled by dynamic SQL (EXECUTE format(...)), which the scan +# reads as a reference but Postgres never resolves at install time; +# - a symbol forward-referenced from a `LANGUAGE plpgsql` body, whose callees +# Postgres resolves at execution time, not at CREATE time. (A `LANGUAGE sql` +# body IS resolved at CREATE time — never allowlist one of those; fix the +# `-- REQUIRE:` edge instead, or the install will fail.) +# +# Adding an entry here weakens the gate that keeps the concatenated installer +# in define-before-use order. Prefer a `-- REQUIRE:` edge whenever one exists. diff --git a/tasks/test/symbol_order_selftest.sh b/tasks/test/symbol_order_selftest.sh index a6226d021..f7bb8c0bd 100755 --- a/tasks/test/symbol_order_selftest.sh +++ b/tasks/test/symbol_order_selftest.sh @@ -50,24 +50,25 @@ if bash tasks/test/verify_symbol_order_v3.sh "$tmp/domain_bad.txt" 2>/dev/null; fi echo "ok: domain-form type used before definition rejected" -# CREATE DOMAIN eql_v3.* form (the query-operand twins `eql_v3.query__` -# and the hand-written `eql_v3.query_jsonb`, CIP-3442). Pins the domain-capture -# branch's eql_v3 arm: without it every query operand — including the line that -# creates it, which is also a reference — reads as "defined nowhere". +# CREATE DOMAIN eql_v3.* form (the query-operand domains CIP-3442 moved out of +# `public`: eql_v3.query__ and eql_v3.query_jsonb). Pins the domain-capture +# branch's eql_v3 arm. Without it the whole surface's 39 query domains read as +# "defined nowhere" — the regression that reddened every build-dependent CI job. printf 'CREATE DOMAIN eql_v3.query_integer_eq AS jsonb;\n' > "$tmp/q.sql" printf 'CREATE FUNCTION eql_v3.eq(a public.integer_eq, b eql_v3.query_integer_eq) ...\n' > "$tmp/qf.sql" -printf '%s\n%s\n' "$tmp/q.sql" "$tmp/qf.sql" > "$tmp/query_good.txt" -bash tasks/test/verify_symbol_order_v3.sh "$tmp/query_good.txt" \ +printf '%s\n%s\n' "$tmp/q.sql" "$tmp/qf.sql" > "$tmp/qdomain_good.txt" +bash tasks/test/verify_symbol_order_v3.sh "$tmp/qdomain_good.txt" \ || { echo "FAIL: CREATE DOMAIN eql_v3.* definer not recognised"; exit 1; } echo "ok: CREATE DOMAIN eql_v3.* definition form recognised" -# And the same query operand used BEFORE it is created must still be REJECTED — -# recognising the schema must not blunt the ordering check. -printf '%s\n%s\n' "$tmp/qf.sql" "$tmp/q.sql" > "$tmp/query_bad.txt" -if bash tasks/test/verify_symbol_order_v3.sh "$tmp/query_bad.txt" 2>/dev/null; then +# And a query-operand domain used BEFORE it is created must still be REJECTED — +# proves the eql_v3 arm records a definition rather than silently suppressing the +# symbol (a check that never fires would pass the case above too). +printf '%s\n%s\n' "$tmp/qf.sql" "$tmp/q.sql" > "$tmp/qdomain_bad.txt" +if bash tasks/test/verify_symbol_order_v3.sh "$tmp/qdomain_bad.txt" 2>/dev/null; then echo "FAIL: eql_v3.query_integer_eq used before its CREATE DOMAIN accepted"; exit 1 fi -echo "ok: eql_v3 query operand used before definition rejected" +echo "ok: query-operand domain used before definition rejected" # CREATE OPERATOR CLASS|FAMILY eql_v3_internal.* form (the conditional SEM # ordered-index opclasses). A file that both creates the opclass and mentions it @@ -87,4 +88,35 @@ if bash tasks/test/verify_symbol_order_v3.sh "$tmp/missing_order.txt" 2>/dev/nul echo "FAIL: unreadable path silently accepted"; exit 1 fi echo "ok: unreadable path rejected" + +# An UNREADABLE ALLOWLIST must FAIL the gate. awk's `getline < file` returns <= 0 +# both at EOF and on error, so an unguarded read loop silently yields an empty +# allowlist. That is fail-safe today only because the committed allowlist has no +# active entries; the moment one is added, a path typo would resurrect the very +# false positive the entry exists to suppress — and it would surface inside +# `mise run build`, i.e. inside a release. +if SYMBOL_ORDER_ALLOWLIST="$tmp/no-such-allowlist.txt" \ + bash tasks/test/verify_symbol_order_v3.sh "$tmp/good_order.txt" 2>/dev/null; then + echo "FAIL: unreadable allowlist silently accepted"; exit 1 +fi +echo "ok: unreadable allowlist rejected" + +# The gate is STRICTER than PostgreSQL, deliberately, and the allowlist is the +# escape hatch. A `LANGUAGE plpgsql` body resolves its callees at execution time, +# so Postgres accepts a forward reference that this gate rejects. Pin both halves: +# the rejection (so the strictness is a choice, not an accident) and the release +# valve (so a real forward reference has a documented, reviewable way out). +printf 'CREATE FUNCTION eql_v3.caller() RETURNS int LANGUAGE plpgsql AS $$ BEGIN RETURN eql_v3.callee(); END; $$;\n' > "$tmp/caller.sql" +printf 'CREATE FUNCTION eql_v3.callee() RETURNS int LANGUAGE sql AS $$ SELECT 1 $$;\n' > "$tmp/callee.sql" +printf '%s\n%s\n' "$tmp/caller.sql" "$tmp/callee.sql" > "$tmp/plpgsql_order.txt" +if bash tasks/test/verify_symbol_order_v3.sh "$tmp/plpgsql_order.txt" 2>/dev/null; then + echo "FAIL: plpgsql forward reference accepted — the gate's strictness is unpinned"; exit 1 +fi +echo "ok: plpgsql forward reference rejected (documented strictness)" + +printf 'eql_v3.callee # forward-referenced from a plpgsql body\n' > "$tmp/allow.txt" +SYMBOL_ORDER_ALLOWLIST="$tmp/allow.txt" \ + bash tasks/test/verify_symbol_order_v3.sh "$tmp/plpgsql_order.txt" \ + || { echo "FAIL: allowlist did not release the plpgsql forward reference"; exit 1; } +echo "ok: allowlist releases a plpgsql forward reference" echo "symbol-order self-test passed" diff --git a/tasks/test/verify_symbol_order_v3.sh b/tasks/test/verify_symbol_order_v3.sh index f74b2e5a2..62332d59c 100755 --- a/tasks/test/verify_symbol_order_v3.sh +++ b/tasks/test/verify_symbol_order_v3.sh @@ -1,12 +1,37 @@ #!/usr/bin/env bash #MISE description="Cross-check that every eql_v3/eql_v3_internal/public-domain symbol referenced in a file is defined by a file ordered earlier" +# +# This gate is deliberately STRICTER than PostgreSQL, in one direction: it treats +# every owned-schema token as a reference, wherever it appears, including inside a +# function body. PostgreSQL resolves a `LANGUAGE plpgsql` body's callees at +# execution time, so it accepts a plpgsql function that forward-references a +# function defined later in the installer. This gate rejects it. +# +# That is the intended trade: a define-before-use order is what makes a +# single-transaction install of the concatenated monolith safe for `LANGUAGE sql` +# bodies (which Postgres DOES resolve at CREATE time), and the checker cannot tell +# the two languages apart from a line-oriented scan. The cost is that a genuine +# plpgsql forward reference — mutual recursion, say — needs an entry in +# tasks/test/symbol_order_allowlist.txt. +# +# Note this runs inside `mise run build`, so it gates the release build, not just +# CI. A false positive blocks a release until allowlisted. Both the rejection and +# the allowlist escape hatch are pinned by tasks/test/symbol_order_selftest.sh. set -euo pipefail REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" cd "$REPO_ROOT" ORDERED="${1:-src/deps-ordered-v3.txt}" -ALLOW="tasks/test/symbol_order_allowlist.txt" +# Overridable so the self-test can exercise the missing-allowlist path without +# disturbing the committed one. +ALLOW="${SYMBOL_ORDER_ALLOWLIST:-tasks/test/symbol_order_allowlist.txt}" test -f "$ORDERED" || { echo "ERROR: ordered file $ORDERED missing (run mise run build)" >&2; exit 2; } +# awk's `getline < file` cannot distinguish EOF from an unreadable file, so an +# unguarded read loop turns a bad ALLOW path into a silently empty allowlist. +# Today that fails safe (nothing to suppress), but this gate runs inside +# `mise run build` — including the release build — so a typo in a future entry +# would resurrect the false positive it was added to suppress, at release time. +test -r "$ALLOW" || { echo "ERROR: allowlist $ALLOW missing or unreadable" >&2; exit 2; } awk -v allowfile="$ALLOW" ' BEGIN { @@ -40,25 +65,23 @@ awk -v allowfile="$ALLOW" ' key = schema s if (!(key in defined)) defined[key] = idx } - # CREATE DOMAIN (eql_v3_internal|eql_v3|public).. All three schemas - # define domains, across DDL forms: - # - eql_v3_internal: the SEM index-term types hmac_256/ope_cllw/ - # bloom_filter are `CREATE DOMAIN` (over text/bytea/smallint[]), NOT - # `CREATE TYPE`. Omitting this schema would leave the three most- - # referenced foundational types (~165 refs) reporting "defined nowhere". - # - eql_v3: the query-operand twins `eql_v3.query__` and the - # hand-written `eql_v3.query_jsonb` (CIP-3442 — a query operand is - # never a column type, so it lives here rather than in `public`). - # - public: the user-column domains. - # The eql_v3_internal test must precede the eql_v3 one: `eql_v3.` is a - # prefix of `eql_v3_internal.`. Only `public.` domains feed isdomain[] - # (that gates which `public.*` REFERENCES are checked); `eql_v3*.*` - # references are checked unconditionally. + # CREATE DOMAIN (eql_v3_internal|eql_v3|public).. All three schemas: the + # SEM index-term types split across DDL forms — hmac_256/ope_cllw/bloom_filter + # are `CREATE DOMAIN eql_v3_internal.` (over text/bytea/smallint[]), + # NOT `CREATE TYPE`. Capturing only `public.` here would leave the three + # most-referenced foundational types (~165 refs) reporting "defined + # nowhere" — a real gap, not an allowlist case. `eql_v3.` owns the + # query-operand domains (`eql_v3.query__`, `eql_v3.query_jsonb`), + # which CIP-3442 moved out of `public`: omitting the schema here leaves all + # 39 of them reporting "defined nowhere". Only `public.` domains feed + # isdomain[] (that gates which `public.*` REFERENCES are checked). + # Test eql_v3_internal FIRST in both the alternation and the arms below, so + # the `eql_v3` prefix cannot shadow it. if (match(line, /CREATE[ \t]+DOMAIN[ \t]+(eql_v3_internal|eql_v3|public)\.[a-z0-9_]+/)) { seg = substr(line, RSTART, RLENGTH) if (seg ~ /eql_v3_internal\./) { sub(/.*eql_v3_internal\./, "", seg); key = "eql_v3_internal." seg } else if (seg ~ /eql_v3\./) { sub(/.*eql_v3\./, "", seg); key = "eql_v3." seg } - else { sub(/.*public\./, "", seg); key = "public." seg; isdomain[seg] = 1 } + else { sub(/.*public\./, "", seg); key = "public." seg; isdomain[seg] = 1 } if (!(key in defined)) defined[key] = idx } # CREATE TYPE eql_v3_internal. (the composite SEM types: ore_block_256, ore_cllw) From 8b5a116466149290b4c1227429cf87976ab877a5 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 23:21:21 +1000 Subject: [PATCH 11/15] test: gate the installer against the order, not just the order against disk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ordering refactor pinned the wrong half of the invariant. `eql-codegen order` guarantees the ORDER LIST names every .sql file on disk, and install_order_contains_every_v3_sql_file proves it against an independent walk. Nothing proved that build.sh's concat loop then emitted each ordered file's BODY into the installer. That is the same bug class one layer down, and it is the layer where a dropped file actually costs you something. It matters because 93 of the ~244 files under src/v3 are leaves: no other file `-- REQUIRE:`s them. Several define no object any inventory test enumerates — a bare DO block, functions in eql_v3_internal, a CREATE OPERATOR CLASS. Drop one and the monolith still applies cleanly and every symbol still resolves, so an install smoke test passes. verify_symbol_order_v3.sh is blind by construction: a dropped leaf removes its definition and, being a leaf, leaves no reference dangling to trip on. The only backstop is a DB behavioural test, which needs CipherStash credentials and is skipped on fork PRs. Add tasks/test/verify_installer_complete.sh — DB-free, creds-free, wired into the self-contained-v3 job (which runs on forks) and into `mise run build` itself: - Non-vacuity: the order names exactly the files find(1) sees under src/v3. - Trailing newline: no ordered file may lack one. build.sh assembles with `>>`, so a missing newline glues one file's last statement onto the next file's first line — different SQL, not a syntax error. Nothing checked this. - Line-count identity: Σ(lines − anchored REQUIRE lines) + pin script == installer. Exact today (43339). A dropped, truncated, or twice-emitted body breaks it. Each gate was proven to fail before being wired in: full order with two leaf bodies missing from the installer, a file dropped from the order, an empty order, a duplicated emit, and a file without a trailing newline. Also, from the same audit: - Restore the no-duplicate assertion lost when generated_manifest's test was deleted. parity.rs collapsed the order into a BTreeSet before comparing, so a repeated path was absorbed; build.sh concatenates with no `uniq`, so it would emit that file's DDL twice. Verified the assertion fires by temporarily pushing a duplicate. - verify_symbol_order_v3.sh reported "OK (0 files)" and exited 0 on an empty order — a pass meaning "I checked nothing", indistinguishable in CI from one meaning "I checked everything". Refuse the vacuous case; self-test both empty and whitespace-only. - walk_v3_surface's read error said "stream did not contain valid UTF-8" without naming which of 244 files. Name it. - Pin that a self-require is tolerated (topo_order's `dep != p` guard makes it reachable; the old shell build emitted one per file for tsort's benefit). - #MISE outputs omitted src/deps-ordered-v3.txt, so a cached build could skip while a consumed product was absent. Add it, and trap the order's temp file. Docs: CLAUDE.md and DEVELOPMENT.md still described verify_v3_self_contained, src/deps-v3.txt and tsort, all deleted. self_contained_v3.sh's file gate is now vacuous (the walk is rooted at src/v3, so every node is under it by construction) — kept as belt-and-braces, comment corrected. --- .github/workflows/README.md | 2 +- .github/workflows/test-eql.yml | 2 + CLAUDE.md | 2 +- DEVELOPMENT.md | 15 ++- crates/eql-codegen/src/ordering.rs | 35 +++++- crates/eql-codegen/tests/parity.rs | 20 +++- mise.toml | 6 + tasks/build.sh | 12 +- tasks/test/self_contained_v3.sh | 6 +- tasks/test/symbol_order_selftest.sh | 16 +++ tasks/test/verify_installer_complete.sh | 139 ++++++++++++++++++++++++ tasks/test/verify_symbol_order_v3.sh | 8 ++ 12 files changed, 246 insertions(+), 17 deletions(-) create mode 100644 tasks/test/verify_installer_complete.sh diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 68556daae..c3c8bbff9 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -133,7 +133,7 @@ All jobs run on `blacksmith-16vcpu-ubuntu-2204`. "PG set" follows the event | **schema** | `test:schema` | v2.2 / v2.3 payload JSON-schema validation | no | no | | **rust-crates** | `test:crates` + `types:check` | `cargo fmt --check`, clippy + `cargo test` for `eql-domains` / `eql-codegen` / `eql-tests-macros` / `eql-bindings`; verify TS bindings + JSON schemas are fresh | no | no | | **codegen** | `codegen:parity` | Regenerate encrypted-domain SQL in place + `git diff` drift gate (committed `src/v3/scalars/` matches the generator) | no | no | -| **self-contained-v3** | `test:self_contained_v3` | `eql_v3` surface has no `eql_v2` dependency | no | no | +| **self-contained-v3** | `test:self_contained_v3`, `test:installer_complete`, `test:symbol_order_v3`, `test:build_ordering_helpers` | `eql_v3` surface has no `eql_v2` dependency; installer contains every ordered file; symbols defined before use | no | no | | **matrix-coverage** | `test:matrix:inventory` (+`:jsonb_entry`, `:v3-jsonb`) + `test:matrix:catalog-coverage` | Scalar-matrix test-name snapshots are not silently dropped; catalog surface is covered | no | no | | **splinter** | `test:splinter` | Supabase/Splinter lints over the installed EQL | yes (PG17) | no | | **ci-required** | — | aggregator: every needed job is `success`/`skipped` | no | no | diff --git a/.github/workflows/test-eql.yml b/.github/workflows/test-eql.yml index e1898c120..8667f4767 100644 --- a/.github/workflows/test-eql.yml +++ b/.github/workflows/test-eql.yml @@ -409,6 +409,8 @@ jobs: run: mise run clean && mise run --force build - name: Assert eql_v3 is self-contained run: mise run test:self_contained_v3 + - name: Assert the installer contains every ordered file + run: mise run test:installer_complete - name: Symbol-order cross-check (v3) run: mise run test:symbol_order_v3 - name: Build-ordering helper unit tests diff --git a/CLAUDE.md b/CLAUDE.md index 682c6898e..5a20b0db2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,7 +37,7 @@ This project uses `mise` for task management. Common commands: - `cipherstash-encrypt.sql` - The sole installer: the self-contained `eql_v3` surface, globbed from `src/v3` only (no `eql_v2`; installable into a DB with no `eql_v2` present) - `cipherstash-encrypt-uninstall.sql` - Matching uninstaller -There are no longer separate Main / Supabase / Protect / v3-only build variants. The combined `eql_v2` build that previously produced multiple artefacts has been removed; the v3 surface now ships as one self-contained installer under the canonical `cipherstash-encrypt.sql` name (`tasks/build.sh` globs `src/v3` only). Because the surface owns no `eql_v2` dependency, it is already Supabase / managed-Postgres compatible (functional indexes over extractors, no superuser-only operator classes) without a dedicated subset build. Self-containment — no `-- REQUIRE:` edge pointing outside `src/v3`, no `eql_v2.` anywhere in the surface — is enforced at build time by `verify_v3_self_contained` in `tasks/build.sh` and CI-gated by `mise run test:self_contained_v3`. +There are no longer separate Main / Supabase / Protect / v3-only build variants. The combined `eql_v2` build that previously produced multiple artefacts has been removed; the v3 surface now ships as one self-contained installer under the canonical `cipherstash-encrypt.sql` name (`eql-codegen order` walks `src/v3` only). Because the surface owns no `eql_v2` dependency, it is already Supabase / managed-Postgres compatible (functional indexes over extractors, no superuser-only operator classes) without a dedicated subset build. Self-containment — no `-- REQUIRE:` edge pointing outside `src/v3`, no `eql_v2.` anywhere in the surface — is enforced at build time by `surface_order`'s `OutsideSurface` error (`crates/eql-codegen/src/ordering.rs`) and CI-gated by `mise run test:self_contained_v3`. ## Project Architecture diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 2b351ae03..910fea132 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -88,8 +88,7 @@ These are the important files and directories in the repo: │ │ │ └── / <-- e.g. integer/, text/, boolean/ (generated, committed in place) │ │ ├── jsonb/ <-- jsonb SteVec support │ │ └── lint/ <-- structural lints -│ ├── deps-v3.txt <-- REQUIRE edges for the v3 surface -│ ├── deps-ordered-v3.txt <-- tsorted build order +│ ├── deps-ordered-v3.txt <-- install order, emitted by `eql-codegen order` │ └── README.md ├── docs/ <-- reference, concept, and API documentation ├── tests/ <-- test framework and fixtures @@ -223,10 +222,14 @@ At minimum, a file references the schema: -- REQUIRE: src/v3/schema.sql ``` -The build collects these edges into `src/deps-v3.txt`, resolves them with -`tsort` into `src/deps-ordered-v3.txt`, and concatenates the files in -dependency order to produce a single installer. The build fails loudly if a -file referenced in the dependency list does not exist. +`cargo run -p eql-codegen -- order` walks the whole `src/v3` tree once, collects +these edges, and topologically sorts them into `src/deps-ordered-v3.txt`; the +build then concatenates the files in that order to produce a single installer. +Generated and hand-written files are ordered together, so nothing can fall +between two enumerations and be dropped. The build fails loudly if a `-- REQUIRE:` +target does not exist, if an edge leaves `src/v3`, or if the edges form a cycle. +`mise run test:installer_complete` then asserts the installer actually contains +every ordered file's body. The `eql_v3` surface is **self-contained**: no `eql_v2.` reference appears anywhere under `src/v3/`. This invariant is enforced in CI by diff --git a/crates/eql-codegen/src/ordering.rs b/crates/eql-codegen/src/ordering.rs index 8c679ef63..7aedcfd62 100644 --- a/crates/eql-codegen/src/ordering.rs +++ b/crates/eql-codegen/src/ordering.rs @@ -147,7 +147,12 @@ pub fn walk_v3_surface(root: &Path) -> io::Result)>> { .unwrap_or(&path) .to_string_lossy() .replace('\\', "/"); - files.push((rel, requires_of(&fs::read_to_string(&path)?))); + // Name the file. A bare `?` here surfaces as "stream did not contain + // valid UTF-8" with no indication of which of ~244 files is at fault. + let body = fs::read_to_string(&path).map_err(|e| { + io::Error::new(e.kind(), format!("reading {}: {e}", path.display())) + })?; + files.push((rel, requires_of(&body))); } } files.sort(); @@ -362,6 +367,18 @@ mod tests { ); } + // A file that requires ITSELF is tolerated, not a cycle. `topo_order`'s + // `dep != p` guard skips the self-edge, so this is reachable in production — + // the old shell build even emitted a self-edge per file, because `tsort` only + // prints tokens that appear in some edge. Pinned so a future rewrite of the + // guard turns a harmless typo into a build failure loudly, in this test, + // rather than quietly at release time. + #[test] + fn surface_order_tolerates_a_self_edge() { + let files = vec![f("src/v3/a.sql", &["src/v3/a.sql"])]; + assert_eq!(surface_order(&files).unwrap(), vec!["src/v3/a.sql"]); + } + // A cycle surfaces as a cycle, not as a silently truncated order. #[test] fn surface_order_propagates_cycles() { @@ -454,6 +471,22 @@ mod tests { assert_eq!(paths, vec!["src/v3/schema.sql"]); } + // An unreadable file names itself. `fs::read_to_string`'s own error is + // "stream did not contain valid UTF-8" — true, and useless across ~244 files. + #[test] + fn walk_v3_surface_names_the_file_it_could_not_read() { + let d = crate::writer::test_support::tempdir(); + let v3 = d.path().join("src/v3"); + fs::create_dir_all(&v3).unwrap(); + fs::write(v3.join("bad.sql"), [0xff, 0xfe, 0x00]).unwrap(); + + let err = walk_v3_surface(d.path()).unwrap_err(); + assert!( + err.to_string().contains("bad.sql"), + "the error must name the offending file, got: {err}" + ); + } + // requires_of reads back anchored `-- REQUIRE:` lines from a rendered body. #[test] fn requires_of_reads_anchored_directives() { diff --git a/crates/eql-codegen/tests/parity.rs b/crates/eql-codegen/tests/parity.rs index 0482b7d80..d9ba18da5 100644 --- a/crates/eql-codegen/tests/parity.rs +++ b/crates/eql-codegen/tests/parity.rs @@ -158,12 +158,20 @@ fn install_order_contains_every_v3_sql_file() { } let files = eql_codegen::ordering::walk_v3_surface(&root).expect("walk src/v3"); - let ordered: BTreeSet = eql_codegen::ordering::surface_order(&files) - .expect( - "src/v3 surface must linearize: every REQUIRE target a node under src/v3, no cycles", - ) - .into_iter() - .collect(); + let order = eql_codegen::ordering::surface_order(&files).expect( + "src/v3 surface must linearize: every REQUIRE target a node under src/v3, no cycles", + ); + + // Check for duplicates BEFORE collapsing into a set, which would absorb them. + // `tasks/build.sh` concatenates the order line by line with no `uniq`, so a + // repeated path emits that file's DDL twice into the installer. Kahn's + // algorithm cannot produce one today; this pins that it stays that way. + let ordered: BTreeSet = order.iter().cloned().collect(); + assert_eq!( + order.len(), + ordered.len(), + "the install order contains a duplicate path — build.sh would emit its DDL twice" + ); assert_eq!( ordered, on_disk, diff --git a/mise.toml b/mise.toml index 51cbf1923..1c9d7ef84 100644 --- a/mise.toml +++ b/mise.toml @@ -234,6 +234,12 @@ bash tasks/test/symbol_order_selftest.sh bash tasks/test/verify_symbol_order_v3.sh src/deps-ordered-v3.txt """ +[tasks."test:installer_complete"] +description = "Assert the installer contains every ordered src/v3 file's body (DB-free)" +depends = ["build"] +dir = "{{config_root}}" +run = "bash tasks/test/verify_installer_complete.sh src/deps-ordered-v3.txt release/cipherstash-encrypt.sql" + [tasks."test:build_ordering_helpers"] description = "Unit tests for tasks/build/ordering.sh strip_require_lines (DB-free)" dir = "{{config_root}}" diff --git a/tasks/build.sh b/tasks/build.sh index a33b702f6..89fdf9bc5 100755 --- a/tasks/build.sh +++ b/tasks/build.sh @@ -2,7 +2,7 @@ #MISE description="Build SQL into single release file" #MISE alias="b" #MISE sources=["src/v3/**/*.sql", "src/v3/version.template", "tasks/pin_search_path_v3.sql", "tasks/uninstall-v3.sql", "crates/eql-domains/src/**/*.rs", "crates/eql-codegen/src/**/*.rs"] -#MISE outputs=["release/cipherstash-encrypt.sql","release/cipherstash-encrypt-uninstall.sql"] +#MISE outputs=["release/cipherstash-encrypt.sql","release/cipherstash-encrypt-uninstall.sql","src/deps-ordered-v3.txt"] #USAGE flag "--version " help="Specify release version of EQL" default="DEV" #!/bin/bash @@ -11,6 +11,9 @@ set -euo pipefail source tasks/build/ordering.sh +# A failed `eql-codegen order` leaves its temp behind; don't strand it. +trap 'rm -f src/deps-ordered-v3.txt.tmp' EXIT + # Regenerate encrypted-domain SQL from the Rust catalog before building. # The generated files (src/v3/scalars//_*.sql) are COMMITTED in place and # drift-gated by `mise run codegen:parity`; only src/v3/version.sql and the @@ -76,6 +79,13 @@ while IFS= read -r f; do done < src/deps-ordered-v3.txt cat tasks/pin_search_path_v3.sql >> release/cipherstash-encrypt.sql +# `eql-codegen order` guarantees the ORDER contains every file on disk. This gate +# closes the layer below — that the concat loop above actually emitted each ordered +# file's body. 93 of the ~244 v3 files are leaves (required by nothing, defining +# nothing another file references), so dropping one yields an installer that applies +# cleanly and passes the symbol checker while silently shipping less than it should. +bash tasks/test/verify_installer_complete.sh src/deps-ordered-v3.txt release/cipherstash-encrypt.sql + cat tasks/uninstall-v3.sql >> release/cipherstash-encrypt-uninstall.sql diff --git a/tasks/test/self_contained_v3.sh b/tasks/test/self_contained_v3.sh index b9fa44c85..74acf907e 100755 --- a/tasks/test/self_contained_v3.sh +++ b/tasks/test/self_contained_v3.sh @@ -21,7 +21,11 @@ if grep -rnE 'eql_v2[._]' src/v3; then fi # File level (design goal 2): the v3-only dependency closure pulls in no file -# outside src/v3/. tsort output is one path per line. +# outside src/v3/. `eql-codegen order` emits one repo-relative path per line. +# +# Belt-and-braces: surface_order already rejects any `-- REQUIRE:` edge leaving +# src/v3, and the walk is rooted at src/v3, so every node is under it by +# construction. This gate would only fire again if that root ever widened. if [[ ! -f src/deps-ordered-v3.txt ]]; then echo "ERROR: src/deps-ordered-v3.txt missing — run 'mise run build' first" >&2 exit 2 diff --git a/tasks/test/symbol_order_selftest.sh b/tasks/test/symbol_order_selftest.sh index f7bb8c0bd..bcfb6f388 100755 --- a/tasks/test/symbol_order_selftest.sh +++ b/tasks/test/symbol_order_selftest.sh @@ -89,6 +89,22 @@ if bash tasks/test/verify_symbol_order_v3.sh "$tmp/missing_order.txt" 2>/dev/nul fi echo "ok: unreadable path rejected" +# An EMPTY ordered list must FAIL, not report "OK (0 files)". A vacuous pass is +# indistinguishable in CI from a real one, so an emptied src/v3 would clear this +# gate and ship an installer containing nothing but the pin script. +: > "$tmp/empty_order.txt" +if bash tasks/test/verify_symbol_order_v3.sh "$tmp/empty_order.txt" 2>/dev/null; then + echo "FAIL: empty ordered list passed vacuously"; exit 1 +fi +echo "ok: empty ordered list rejected" + +# Whitespace-only is empty too — the guard must not be fooled by a stray blank line. +printf '\n \n' > "$tmp/blank_order.txt" +if bash tasks/test/verify_symbol_order_v3.sh "$tmp/blank_order.txt" 2>/dev/null; then + echo "FAIL: whitespace-only ordered list passed vacuously"; exit 1 +fi +echo "ok: whitespace-only ordered list rejected" + # An UNREADABLE ALLOWLIST must FAIL the gate. awk's `getline < file` returns <= 0 # both at EOF and on error, so an unguarded read loop silently yields an empty # allowlist. That is fail-safe today only because the committed allowlist has no diff --git a/tasks/test/verify_installer_complete.sh b/tasks/test/verify_installer_complete.sh new file mode 100644 index 000000000..bad324f74 --- /dev/null +++ b/tasks/test/verify_installer_complete.sh @@ -0,0 +1,139 @@ +#!/usr/bin/env bash +#MISE description="Assert release/cipherstash-encrypt.sql contains the body of every ordered src/v3 file (DB-free)" +# +# The order→artefact gate. `eql-codegen order` guarantees the ORDER LIST contains +# every .sql file on disk (pinned by install_order_contains_every_v3_sql_file in +# the eql-codegen parity tests). This gate closes the layer below: that build.sh's +# concat loop actually emitted each ordered file's body into the installer. +# +# Why that layer needs its own gate: 93 of the ~244 files in src/v3 are LEAVES — +# no other file `-- REQUIRE:`s them, and several define no object that any +# inventory test enumerates (a bare `DO` block; functions in eql_v3_internal; a +# `CREATE OPERATOR CLASS`). Drop a leaf and the monolith still applies cleanly and +# every symbol still resolves, so an install smoke test passes. A +# referenced-vs-defined checker (verify_symbol_order_v3.sh) is blind to it by +# construction: a dropped leaf removes its definition AND, being a leaf, leaves no +# reference dangling to trip on. The loss surfaces only in a DB behavioural test — +# which needs CipherStash credentials and is skipped on fork PRs. +# +# So this gate does not look for symbols. It does arithmetic on lines, which no +# leaf can hide from. + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +cd "$REPO_ROOT" + +ORDERED="${1:-src/deps-ordered-v3.txt}" +INSTALLER="${2:-release/cipherstash-encrypt.sql}" +PIN="tasks/pin_search_path_v3.sql" + +for f in "$ORDERED" "$INSTALLER" "$PIN"; do + if [[ ! -f "$f" ]]; then + echo "ERROR: $f missing — run 'mise run build' first" >&2 + exit 2 + fi +done + +# `grep -c ''` counts a final line that lacks a trailing newline; `wc -l` does not. +# The trailing-newline gate below makes the two agree, but count the honest way. +count_lines() { grep -c '' "$1" || true; } + +fail=0 + +# --------------------------------------------------------------------------- +# Gate 1: non-vacuity. The ordered list must name every .sql file on disk. +# +# Without this, an empty or truncated order list sails through every other gate: +# verify_symbol_order_v3.sh prints "OK (0 files)", the self-containment file gate +# finds no offending path, and build.sh emits an installer holding nothing but the +# pin script. Every check green, nothing shipped. Compare against an INDEPENDENT +# find(1) rather than trusting the list's own length. +# --------------------------------------------------------------------------- +echo "==> Non-vacuity gate: the order names every src/v3 SQL file" +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT +find src/v3 -type f -name '*.sql' ! -name '*_test.sql' | LC_ALL=C sort > "$work/disk" +grep -v '^[[:space:]]*$' "$ORDERED" | LC_ALL=C sort > "$work/order" || true +n_disk=$(count_lines "$work/disk") + +if [[ "$n_disk" -eq 0 ]]; then + echo "ERROR: no .sql files found under src/v3 — refusing to validate an empty surface" >&2 + exit 1 +fi + +# Report at most 10 offenders per direction. An empty order list would otherwise +# print every file in the surface and bury the verdict under 244 lines. +# comm(1) needs real files, not process substitutions: the list is read twice. +report_missing() { + local label=$1 file=$2 n + n=$(count_lines "$file") + if [[ "$n" -gt 0 ]]; then + echo "ERROR: $n file(s) $label:" >&2 + head -10 "$file" | sed 's/^/ /' >&2 + if [[ "$n" -gt 10 ]]; then + echo " … and $(( n - 10 )) more" >&2 + fi + fail=1 + fi + return 0 +} +comm -23 "$work/disk" "$work/order" > "$work/only_disk" +comm -13 "$work/disk" "$work/order" > "$work/only_order" +report_missing "on disk but absent from $ORDERED (they will not ship)" "$work/only_disk" +report_missing "named in $ORDERED but absent from disk" "$work/only_order" +if [[ $fail -eq 0 ]]; then + echo " $n_disk files, order matches disk" +fi + +# --------------------------------------------------------------------------- +# Gate 2: every ordered file ends with a newline. +# +# build.sh assembles with `>>`. A file whose last line has no trailing newline +# would glue its final statement onto the first line of the next file — silently +# producing different SQL, not a syntax error. Nothing else checks this. +# --------------------------------------------------------------------------- +echo "==> Trailing-newline gate: no file can glue onto the next on concat" +while IFS= read -r f; do + [[ -z "$f" ]] && continue + if [[ -s "$f" && -n "$(tail -c1 "$f")" ]]; then + echo "ERROR: $f has no trailing newline — concatenation would merge it with the next file" >&2 + fail=1 + fi +done < "$ORDERED" + +# --------------------------------------------------------------------------- +# Gate 3: line-count identity. +# +# Σ_f (lines(f) − anchored REQUIRE lines(f)) + lines(pin script) == lines(installer) +# +# build.sh strips exactly the anchored `-- REQUIRE:` directives (strip_require_lines +# in tasks/build/ordering.sh) and appends the pin script. So the installer's line +# count is a pure function of the ordered inputs. A dropped file, a truncated body, +# or a duplicated file all break the arithmetic. The REQUIRE regex here MUST match +# strip_require_lines' — keep them in lockstep. +# --------------------------------------------------------------------------- +echo "==> Line-count identity: installer == Σ ordered bodies + pin script" +expected=0 +while IFS= read -r f; do + [[ -z "$f" ]] && continue + total=$(count_lines "$f") + reqs=$(grep -cE '^[[:space:]]*-- REQUIRE:' "$f" || true) + expected=$(( expected + total - reqs )) +done < "$ORDERED" +expected=$(( expected + $(count_lines "$PIN") )) +actual=$(count_lines "$INSTALLER") + +if [[ "$expected" -ne "$actual" ]]; then + echo "ERROR: installer has $actual lines, expected $expected from the ordered inputs" >&2 + echo " (difference of $(( actual - expected )) lines — a file's body was dropped, truncated, or emitted twice)" >&2 + fail=1 +else + echo " $actual lines accounted for" +fi + +if [[ $fail -ne 0 ]]; then + echo "installer completeness gate FAILED" >&2 + exit 1 +fi +echo "installer completeness gate OK" diff --git a/tasks/test/verify_symbol_order_v3.sh b/tasks/test/verify_symbol_order_v3.sh index 62332d59c..e8248d0a9 100755 --- a/tasks/test/verify_symbol_order_v3.sh +++ b/tasks/test/verify_symbol_order_v3.sh @@ -26,6 +26,14 @@ ORDERED="${1:-src/deps-ordered-v3.txt}" # disturbing the committed one. ALLOW="${SYMBOL_ORDER_ALLOWLIST:-tasks/test/symbol_order_allowlist.txt}" test -f "$ORDERED" || { echo "ERROR: ordered file $ORDERED missing (run mise run build)" >&2; exit 2; } +# Refuse a zero-file run. Without this the checker reports "OK (0 files)" and exits +# 0 on an empty order — a pass that means "I checked nothing", indistinguishable in +# CI from "I checked everything". An emptied surface would sail through here, and +# through the self-containment file gate, into an installer holding only the pin +# script. (A short-but-non-empty order is caught by verify_installer_complete.sh; +# this gate only has to refuse the vacuous case, since the self-test drives it with +# one- and two-file lists.) +grep -qv '^[[:space:]]*$' "$ORDERED" || { echo "ERROR: ordered file $ORDERED is empty — refusing a vacuous check" >&2; exit 2; } # awk's `getline < file` cannot distinguish EOF from an unreadable file, so an # unguarded read loop turns a bad ALLOW path into a silently empty allowlist. # Today that fails safe (nothing to suppress), but this gate runs inside From fb7c096409b98900a11493c9f92da92751ec0ca6 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 23:27:29 +1000 Subject: [PATCH 12/15] build: drop the one-shot monolith reorder-only check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit verify_monolith_reorder_only.sh proved the build-ordering refactor changed only statement order: build the installer at HEAD and at a pre-refactor baseline, sort both, demand byte-identical. It served that purpose and is now dead weight. It is referenced by no CI job, no mise task wiring, and no doc. It cannot run unattended — the baseline ref is a mandatory positional arg with no default, because `git merge-base HEAD main` sits 594 commits back on this branch and would diff on content rather than order. And its premise expires on merge: once eql_v3 lands and new files appear past c8d50efb, "a line was added" is the correct answer, and the script can only report failure. verify_installer_complete.sh is the standing form of the same intent. Rather than comparing against a baseline that goes stale, it asserts on every build that the installer's line count equals the sum of its ordered inputs — so a dropped, truncated, or duplicated file body fails immediately, with no baseline to pin. It also carried a live hazard. It builds in a second git worktree sharing CARGO_TARGET_DIR, which is the configuration that lets cargo serve a stale eql-domains rlib built from another worktree's catalog — silently rewriting src/v3/scalars in the tree you are standing in. --- tasks/test/verify_monolith_reorder_only.sh | 43 ---------------------- 1 file changed, 43 deletions(-) delete mode 100755 tasks/test/verify_monolith_reorder_only.sh diff --git a/tasks/test/verify_monolith_reorder_only.sh b/tasks/test/verify_monolith_reorder_only.sh deleted file mode 100755 index 8e2e511d6..000000000 --- a/tasks/test/verify_monolith_reorder_only.sh +++ /dev/null @@ -1,43 +0,0 @@ -#!/usr/bin/env bash -#MISE description="Prove the build-ordering refactor changed ONLY statement order: the LC_ALL=C-sorted monolith is byte-identical to a pre-refactor baseline build" -set -euo pipefail -REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" -cd "$REPO_ROOT" - -# The pre-refactor baseline is the commit immediately BEFORE Task 1 of the -# build-ordering refactor. On this long-lived branch that is the branch tip at -# plan-execution start (c8d50efb) — NOT `git merge-base HEAD main`, which points -# 594 feature commits back and would diff on content, not order. Pass it -# explicitly. -BASELINE_REF="${1:?usage: verify_monolith_reorder_only.sh (the pre-refactor commit, e.g. c8d50efb)}" -VERSION="${EQL_VERSION:-DEV}" # BOTH builds must bake the SAME version string, else version.sql diffs spuriously. - -OUT="$(mktemp -d)" -WT="$(mktemp -d)/eql-baseline" -cleanup() { git worktree remove --force "$WT" 2>/dev/null || true; rm -rf "$OUT" "$(dirname "$WT")"; } -trap cleanup EXIT - -# 1. Current branch: build, then LC_ALL=C sort (locale-stable set view). -mise run build --version "$VERSION" >/dev/null -LC_ALL=C sort release/cipherstash-encrypt.sql > "$OUT/current-sorted.sql" - -# 2. Baseline: build in an ISOLATED detached worktree at the pre-refactor ref so -# the working tree is untouched; sort identically. `mise trust` is required — -# a fresh worktree's mise.toml is untrusted and `mise run` refuses to run it. -git worktree add --detach "$WT" "$BASELINE_REF" >/dev/null -( - cd "$WT" - mise trust >/dev/null 2>&1 || true - mise trust mise.toml >/dev/null 2>&1 || true - mise run build --version "$VERSION" >/dev/null -) -LC_ALL=C sort "$WT/release/cipherstash-encrypt.sql" > "$OUT/baseline-sorted.sql" - -# 3. HARD GATE: sorted views must be byte-identical. -if cmp -s "$OUT/baseline-sorted.sql" "$OUT/current-sorted.sql"; then - echo "PASS: sorted monolith is byte-identical to baseline $BASELINE_REF — the refactor changed ONLY statement order (nothing added/dropped/mutated)." -else - echo "FAIL: sorted monolith DIFFERS from baseline $BASELINE_REF — a line was added, dropped, or mutated (not merely reordered):" >&2 - diff "$OUT/baseline-sorted.sql" "$OUT/current-sorted.sql" | head -40 >&2 - exit 1 -fi From e8b261b29239cb64988458d04c60556c96f19743 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Fri, 17 Jul 2026 11:02:46 +1000 Subject: [PATCH 13/15] =?UTF-8?q?build:=20address=20review=20=E2=80=94=20i?= =?UTF-8?q?nvalidate=20the=20build=20cache=20on=20gate=20edits,=20fail=20l?= =?UTF-8?q?oud=20on=20unreadable=20inputs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The build sources tasks/build/ordering.sh and shells out to two gate scripts under tasks/test/, but none were in the task's #MISE sources. Editing strip_require_lines left mise considering the build fresh, so it re-served an installer assembled by the old logic — and skipped the gates that would have caught it, since a cache hit skips the whole script. Add all four (the two gates and the allowlist included: they decide whether the build passes) so any edit invalidates. Verified the mechanism discriminates rather than always rebuilding: untouched reports "sources up-to-date, skipping", each new source re-runs, and a tasks/test file left out of sources still skips. count_lines in verify_installer_complete.sh swallowed every grep error, and the caller's arithmetic read the resulting empty string as 0 — a file could contribute nothing to the line-count identity while the gate stayed green. Gate 1 makes that unreachable today, but this script exists to fail loudly. Mirror the rc <= 1 idiom already in strip_require_lines: exit 2 naming the file on a real fault, tolerate grep's exit 1 on an empty file. Also: document that the symbol checker enforces cross-file order only (the opclass files name their own class in a RAISE NOTICE, and would otherwise need allowlisting); drop the redundant sort_unstable in topo_order, which read as load-bearing for determinism when the min-heap already owns ordering; make the two runnable gate scripts executable, matching verify_symbol_order_v3.sh. No CHANGELOG entry: build tooling only, nothing observable to a caller. --- crates/eql-codegen/src/ordering.rs | 10 +++++----- tasks/build.sh | 6 +++++- tasks/test/build_ordering_helpers_test.sh | 0 tasks/test/verify_installer_complete.sh | 22 +++++++++++++++++++++- tasks/test/verify_symbol_order_v3.sh | 11 +++++++++++ 5 files changed, 42 insertions(+), 7 deletions(-) mode change 100644 => 100755 tasks/test/build_ordering_helpers_test.sh mode change 100644 => 100755 tasks/test/verify_installer_complete.sh diff --git a/crates/eql-codegen/src/ordering.rs b/crates/eql-codegen/src/ordering.rs index 7aedcfd62..9f5ddd14b 100644 --- a/crates/eql-codegen/src/ordering.rs +++ b/crates/eql-codegen/src/ordering.rs @@ -199,13 +199,13 @@ pub(crate) fn topo_order(files: &[(String, Vec)]) -> Result, while let Some(Reverse(n)) = ready.pop() { order.push(n.to_string()); if let Some(deps) = dependents.get(n) { - let mut ds = deps.clone(); - ds.sort_unstable(); - for d in ds { - let e = indeg.get_mut(d).unwrap(); + // Push order does not matter: `ready` is a min-heap keyed by path, so + // it — not the insertion sequence — decides what comes out next. + for d in deps { + let e = indeg.get_mut(*d).unwrap(); *e -= 1; if *e == 0 { - ready.push(Reverse(d)); + ready.push(Reverse(*d)); } } } diff --git a/tasks/build.sh b/tasks/build.sh index 89fdf9bc5..edcd5009a 100755 --- a/tasks/build.sh +++ b/tasks/build.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash #MISE description="Build SQL into single release file" #MISE alias="b" -#MISE sources=["src/v3/**/*.sql", "src/v3/version.template", "tasks/pin_search_path_v3.sql", "tasks/uninstall-v3.sql", "crates/eql-domains/src/**/*.rs", "crates/eql-codegen/src/**/*.rs"] +#MISE sources=["src/v3/**/*.sql", "src/v3/version.template", "tasks/pin_search_path_v3.sql", "tasks/uninstall-v3.sql", "crates/eql-domains/src/**/*.rs", "crates/eql-codegen/src/**/*.rs", "tasks/build/ordering.sh", "tasks/test/verify_symbol_order_v3.sh", "tasks/test/verify_installer_complete.sh", "tasks/test/symbol_order_allowlist.txt"] #MISE outputs=["release/cipherstash-encrypt.sql","release/cipherstash-encrypt-uninstall.sql","src/deps-ordered-v3.txt"] #USAGE flag "--version " help="Specify release version of EQL" default="DEV" @@ -9,6 +9,10 @@ set -euo pipefail +# ordering.sh shapes the installer (strip_require_lines), and the two verify +# scripts below gate it. All four are in #MISE sources: a cache hit skips this +# script entirely, gates included, so an edit to any of them must invalidate the +# build rather than re-serve an installer built by the old logic. source tasks/build/ordering.sh # A failed `eql-codegen order` leaves its temp behind; don't strand it. diff --git a/tasks/test/build_ordering_helpers_test.sh b/tasks/test/build_ordering_helpers_test.sh old mode 100644 new mode 100755 diff --git a/tasks/test/verify_installer_complete.sh b/tasks/test/verify_installer_complete.sh old mode 100644 new mode 100755 index bad324f74..7f0263ef8 --- a/tasks/test/verify_installer_complete.sh +++ b/tasks/test/verify_installer_complete.sh @@ -37,7 +37,27 @@ done # `grep -c ''` counts a final line that lacks a trailing newline; `wc -l` does not. # The trailing-newline gate below makes the two agree, but count the honest way. -count_lines() { grep -c '' "$1" || true; } +# +# grep exits 1 on an empty file (zero lines — legitimate) and >= 2 on a real fault +# (missing, unreadable). Same rc <= 1 idiom as strip_require_lines in +# tasks/build/ordering.sh. A blanket `|| true` would print nothing on a fault, and +# the caller's `$(( expected + total - reqs ))` reads that empty string as 0 — the +# file silently contributes nothing to the identity. Gate 1 makes that unreachable +# today, but this script's whole job is to fail loudly. +# +# Every caller is `var=$(count_lines f)`, so this `exit 2` leaves only the +# command-substitution subshell; the parent aborts because `set -e` sees the +# failed assignment. Keep the callers as bare assignments — `local n=$(...)` or a +# `|| true` would mask the status and restore the silent-zero this replaces. +count_lines() { + local n rc=0 + n=$(grep -c '' "$1") || rc=$? + if (( rc > 1 )); then + echo "ERROR: cannot count lines in $1 (grep exit $rc)" >&2 + exit 2 + fi + echo "${n:-0}" +} fail=0 diff --git a/tasks/test/verify_symbol_order_v3.sh b/tasks/test/verify_symbol_order_v3.sh index e8248d0a9..ae4c9169b 100755 --- a/tasks/test/verify_symbol_order_v3.sh +++ b/tasks/test/verify_symbol_order_v3.sh @@ -14,6 +14,17 @@ # plpgsql forward reference — mutual recursion, say — needs an entry in # tasks/test/symbol_order_allowlist.txt. # +# Scope: this checks CROSS-FILE order only. A reference is compared against the +# index of the file that defines it (`defined[tok] > i`), so a symbol referenced +# in the same file that defines it always passes, regardless of line order within +# that file. That is deliberate: the conditional SEM opclass files define an +# operator class and then name it in a RAISE NOTICE in the same file, and several +# generated files reference a domain they just created. Enforcing intra-file order +# would flag all of them and push real definitions onto the allowlist, which is +# the opposite of what the allowlist is for. Postgres resolves within a single +# file's statements in statement order anyway, and that order comes from the +# renderers, not from the install order this gate exists to check. +# # Note this runs inside `mise run build`, so it gates the release build, not just # CI. A false positive blocks a release until allowlisted. Both the rejection and # the allowlist escape hatch are pinned by tasks/test/symbol_order_selftest.sh. From 2fd6bd31e374476f4eb7e742bd46ce75406f85d3 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Fri, 17 Jul 2026 11:39:36 +1000 Subject: [PATCH 14/15] build: report what the symbol gate cannot resolve; reject self-edges; widen build cache inputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the three CodeRabbit threads on #382. The symbol checker keyed definitions and references by schema+name with no argument list, and kept the minimum index. eql_v3.eq has 186 definitions across files #55..#242, so every eq reference from #55 on passed for free — and the gate still printed "OK (244 files)". Proven: swapping text_eq_operators ahead of text_eq_functions in the real order passes this gate, and fails a real install with `function eql_v3.eq(text_eq, text_eq) does not exist` (psql exit 3). Overloads cannot be resolved here: a call site is a bare eql_v3.eq(a, b) with no types, and CREATE OPERATOR supplies LEFTARG/RIGHTARG on other lines — that needs a type checker, not a line scan. Nor do they need to be: Postgres resolves them exactly at CREATE time via test:clean_install_v3, which runs on every relevant PR across PG 14-17, needs no credentials, and is not skipped on forks. So the defect was the silence, not the blindness — the same vacuous pass this script already refuses for the empty-list case ("a pass that means 'I checked nothing', indistinguishable from 'I checked everything'"). Track defcount/defmax alongside the min index and report the unresolvable set: 41 names on the current surface. Two catches are deliberately preserved rather than skipped wholesale: a reference preceding EVERY overload is still an error (wrong whichever was meant), and overloads whose definitions all precede the use stay soundly checked — that keeps 8 of 49 multi-def names, including the same-file eql_v3.ste_vec_contains and the OPERATOR FAMILY+CLASS pair sharing eql_v3_internal.ore_cllw_ops. Self-edges are now rejected. The tolerance was deliberate but its rationale died with the shell build: that build emitted a self-edge per file because tsort only prints tokens appearing in an edge. The walk enumerates nodes directly, so the only source now is a typo — and a typo'd edge meant to name another file, so the real dependency is missing and the order can be silently wrong. Reported as a dedicated SelfEdge, not folded into Cycle: "dependency cycle" would send the reader hunting a loop when the fix is one line in one file. Cargo.toml/Cargo.lock join #MISE sources for the same reason ordering.sh did: the cargo run steps render this artefact and eql-codegen's deps decide what they render (minijinja templates the SQL, prettyplease pins bindings formatting), so a dep bump changes output with no .rs file touched. No CHANGELOG entry: build tooling only, nothing observable to a caller. --- .github/workflows/README.md | 2 +- crates/eql-codegen/src/ordering.rs | 86 +++++++++++++++++++++++----- tasks/build.sh | 7 ++- tasks/test/symbol_order_selftest.sh | 46 +++++++++++++++ tasks/test/verify_symbol_order_v3.sh | 70 ++++++++++++++++++++-- 5 files changed, 191 insertions(+), 20 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index c3c8bbff9..cbe72a1c5 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -133,7 +133,7 @@ All jobs run on `blacksmith-16vcpu-ubuntu-2204`. "PG set" follows the event | **schema** | `test:schema` | v2.2 / v2.3 payload JSON-schema validation | no | no | | **rust-crates** | `test:crates` + `types:check` | `cargo fmt --check`, clippy + `cargo test` for `eql-domains` / `eql-codegen` / `eql-tests-macros` / `eql-bindings`; verify TS bindings + JSON schemas are fresh | no | no | | **codegen** | `codegen:parity` | Regenerate encrypted-domain SQL in place + `git diff` drift gate (committed `src/v3/scalars/` matches the generator) | no | no | -| **self-contained-v3** | `test:self_contained_v3`, `test:installer_complete`, `test:symbol_order_v3`, `test:build_ordering_helpers` | `eql_v3` surface has no `eql_v2` dependency; installer contains every ordered file; symbols defined before use | no | no | +| **self-contained-v3** | `test:self_contained_v3`, `test:installer_complete`, `test:symbol_order_v3`, `test:build_ordering_helpers` | `eql_v3` surface has no `eql_v2` dependency; installer contains every ordered file; singleton symbols defined before use (overloads are resolved exactly by the `clean-install` job's `test:clean_install_v3`) | no | no | | **matrix-coverage** | `test:matrix:inventory` (+`:jsonb_entry`, `:v3-jsonb`) + `test:matrix:catalog-coverage` | Scalar-matrix test-name snapshots are not silently dropped; catalog surface is covered | no | no | | **splinter** | `test:splinter` | Supabase/Splinter lints over the installed EQL | yes (PG17) | no | | **ci-required** | — | aggregator: every needed job is `success`/`skipped` | no | no | diff --git a/crates/eql-codegen/src/ordering.rs b/crates/eql-codegen/src/ordering.rs index 9f5ddd14b..7be34a449 100644 --- a/crates/eql-codegen/src/ordering.rs +++ b/crates/eql-codegen/src/ordering.rs @@ -56,6 +56,16 @@ pub enum OrderError { /// (say) `src/v2/foo.sql` would pull non-v3 SQL into the artefact. Subsumes /// the old `verify_v3_self_contained` shell gate. OutsideSurface(Vec<(String, String)>), + /// Files that `-- REQUIRE:` themselves. Always a typo — and a damaging one, + /// because the line almost certainly meant to name a *different* file, so the + /// real edge is missing and the order can be silently wrong. + /// + /// The old shell build emitted a self-edge for every file on purpose (`echo + /// "$sql_file $sql_file"`), because `tsort` only prints tokens that appear in + /// some edge; tolerating them was load-bearing then. The walk enumerates every + /// node directly, so nothing emits self-edges now and the tolerance protects + /// nothing but the typo. + SelfEdge(Vec), /// The edges do not linearize. Cycle(CycleError), } @@ -83,6 +93,20 @@ impl std::fmt::Display for OrderError { "the eql_v3 surface must be self-contained — no edge may leave {SURFACE_ROOT}" ) } + Self::SelfEdge(v) => { + // NOT reported as a cycle. It is one in graph terms, but "dependency + // cycle" sends the reader hunting a loop between files that does not + // exist, when the fix is one line in one file. + writeln!(f, "-- REQUIRE: file requires itself:")?; + for file in v { + writeln!(f, " {file} requires {file}")?; + } + write!( + f, + "a file cannot depend on itself — this line likely meant to name another file, \ + in which case the real dependency is missing" + ) + } Self::Cycle(e) => write!(f, "{e}"), } } @@ -92,20 +116,23 @@ impl std::error::Error for OrderError {} /// Linearize the whole surface. `files` is `(repo-relative path, its REQUIRE /// targets)` for EVERY `.sql` file in the surface. /// -/// Unlike [`topo_order`], which tolerates edges to non-nodes, this validates -/// first: every target must be a node, and must live under [`SURFACE_ROOT`]. -/// Both gates ran in shell before; keeping them here means the invariant travels -/// with the sort rather than with whoever remembers to call the checker. +/// Unlike [`topo_order`], which tolerates edges to non-nodes and self-edges, this +/// validates first: every target must be a node, must live under [`SURFACE_ROOT`], +/// and must not be the requiring file itself. The first two gates ran in shell +/// before; keeping them here means the invariant travels with the sort rather than +/// with whoever remembers to call the checker. pub fn surface_order(files: &[(String, Vec)]) -> Result, OrderError> { let nodes: BTreeSet<&str> = files.iter().map(|(p, _)| p.as_str()).collect(); let prefix = format!("{SURFACE_ROOT}/"); - let (mut outside, mut unknown) = (Vec::new(), Vec::new()); + let (mut outside, mut unknown, mut self_edges) = (Vec::new(), Vec::new(), Vec::new()); for (file, deps) in files { for dep in deps { if !dep.starts_with(&prefix) { outside.push((file.clone(), dep.clone())); } else if !nodes.contains(dep.as_str()) { unknown.push((file.clone(), dep.clone())); + } else if dep == file { + self_edges.push(file.clone()); } } } @@ -117,6 +144,10 @@ pub fn surface_order(files: &[(String, Vec)]) -> Result, Ord if !unknown.is_empty() { return Err(OrderError::UnknownTargets(unknown)); } + if !self_edges.is_empty() { + self_edges.dedup(); + return Err(OrderError::SelfEdge(self_edges)); + } topo_order(files).map_err(OrderError::Cycle) } @@ -367,16 +398,45 @@ mod tests { ); } - // A file that requires ITSELF is tolerated, not a cycle. `topo_order`'s - // `dep != p` guard skips the self-edge, so this is reachable in production — - // the old shell build even emitted a self-edge per file, because `tsort` only - // prints tokens that appear in some edge. Pinned so a future rewrite of the - // guard turns a harmless typo into a build failure loudly, in this test, - // rather than quietly at release time. + // A file that requires ITSELF is rejected, and NOT as a cycle. + // + // This tolerance used to be deliberate: the old shell build emitted a self-edge + // for every file (`echo "$sql_file $sql_file"`) because `tsort` only prints + // tokens appearing in some edge. That rationale died with the shell build — the + // walk enumerates nodes directly, so nothing emits self-edges now and the only + // way one appears is a hand-typo. A typo'd edge is not harmless: the line meant + // to name another file, so the real dependency is missing and the order can be + // silently wrong. + // + // Reported as SelfEdge, not Cycle: "dependency cycle" would send the reader + // hunting a loop between files when the fix is one line in one file. #[test] - fn surface_order_tolerates_a_self_edge() { + fn surface_order_rejects_a_self_edge() { let files = vec![f("src/v3/a.sql", &["src/v3/a.sql"])]; - assert_eq!(surface_order(&files).unwrap(), vec!["src/v3/a.sql"]); + let err = surface_order(&files).unwrap_err(); + let OrderError::SelfEdge(v) = &err else { + panic!("expected SelfEdge, got {err:?}"); + }; + assert_eq!(v.as_slice(), &["src/v3/a.sql".to_string()]); + let msg = err.to_string(); + assert!( + msg.contains("requires itself") && !msg.contains("cycle"), + "a self-require must not be reported as a cycle: {msg}" + ); + } + + // A self-edge is rejected even when the file has other, legitimate edges — the + // typo hides among real REQUIRE lines, which is exactly how it would ship. + #[test] + fn surface_order_rejects_a_self_edge_among_valid_edges() { + let files = vec![ + f("src/v3/schema.sql", &[]), + f("src/v3/a.sql", &["src/v3/schema.sql", "src/v3/a.sql"]), + ]; + assert!(matches!( + surface_order(&files).unwrap_err(), + OrderError::SelfEdge(_) + )); } // A cycle surfaces as a cycle, not as a silently truncated order. diff --git a/tasks/build.sh b/tasks/build.sh index edcd5009a..cf8cd28d1 100755 --- a/tasks/build.sh +++ b/tasks/build.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash #MISE description="Build SQL into single release file" #MISE alias="b" -#MISE sources=["src/v3/**/*.sql", "src/v3/version.template", "tasks/pin_search_path_v3.sql", "tasks/uninstall-v3.sql", "crates/eql-domains/src/**/*.rs", "crates/eql-codegen/src/**/*.rs", "tasks/build/ordering.sh", "tasks/test/verify_symbol_order_v3.sh", "tasks/test/verify_installer_complete.sh", "tasks/test/symbol_order_allowlist.txt"] +#MISE sources=["src/v3/**/*.sql", "src/v3/version.template", "tasks/pin_search_path_v3.sql", "tasks/uninstall-v3.sql", "crates/eql-domains/src/**/*.rs", "crates/eql-codegen/src/**/*.rs", "Cargo.toml", "Cargo.lock", "crates/eql-codegen/Cargo.toml", "crates/eql-domains/Cargo.toml", "tasks/build/ordering.sh", "tasks/test/verify_symbol_order_v3.sh", "tasks/test/verify_installer_complete.sh", "tasks/test/symbol_order_allowlist.txt"] #MISE outputs=["release/cipherstash-encrypt.sql","release/cipherstash-encrypt-uninstall.sql","src/deps-ordered-v3.txt"] #USAGE flag "--version " help="Specify release version of EQL" default="DEV" @@ -13,6 +13,11 @@ set -euo pipefail # scripts below gate it. All four are in #MISE sources: a cache hit skips this # script entirely, gates included, so an edit to any of them must invalidate the # build rather than re-serve an installer built by the old logic. +# +# The Cargo manifests and Cargo.lock are sources for the same reason: the two +# `cargo run` steps below render this artefact, and eql-codegen's own deps decide +# what they render — minijinja templates the SQL, prettyplease (=0.2.37) formats +# the bindings. A dep bump changes the output with no .rs file touched. source tasks/build/ordering.sh # A failed `eql-codegen order` leaves its temp behind; don't strand it. diff --git a/tasks/test/symbol_order_selftest.sh b/tasks/test/symbol_order_selftest.sh index bcfb6f388..a04783849 100755 --- a/tasks/test/symbol_order_selftest.sh +++ b/tasks/test/symbol_order_selftest.sh @@ -135,4 +135,50 @@ SYMBOL_ORDER_ALLOWLIST="$tmp/allow.txt" \ bash tasks/test/verify_symbol_order_v3.sh "$tmp/plpgsql_order.txt" \ || { echo "FAIL: allowlist did not release the plpgsql forward reference"; exit 1; } echo "ok: allowlist releases a plpgsql forward reference" + +# --------------------------------------------------------------------------- +# Overload resolution: the gate cannot tell overloads apart (a reference carries +# no argument types), so it must SAY so rather than report a bare OK. On the real +# surface eql_v3.eq has 186 definitions spanning files #55..#242 — keying on +# schema+name and keeping the MIN index means every eq reference after #55 passes +# for free. These three cases pin the boundary of what is still decidable. +# --------------------------------------------------------------------------- + +# (a) Overloaded, and a later overload is still ahead of the reference: NOT +# decidable. Must pass (it is a structural limit, not rot) but must report the +# name as unresolvable instead of claiming a clean check. +printf 'CREATE FUNCTION eql_v3.eq(a public.integer_eq, b public.integer_eq) RETURNS boolean ...\n' > "$tmp/eq_int.sql" +printf 'CREATE OPERATOR = ( FUNCTION = eql_v3.eq, LEFTARG = public.text_eq, RIGHTARG = public.text_eq );\n' > "$tmp/eq_use.sql" +printf 'CREATE FUNCTION eql_v3.eq(a public.text_eq, b public.text_eq) RETURNS boolean ...\n' > "$tmp/eq_text.sql" +printf '%s\n%s\n%s\n' "$tmp/eq_int.sql" "$tmp/eq_use.sql" "$tmp/eq_text.sql" > "$tmp/overload_order.txt" +out="$(bash tasks/test/verify_symbol_order_v3.sh "$tmp/overload_order.txt")" \ + || { echo "FAIL: ambiguous overload treated as an error"; exit 1; } +case "$out" in + *"unresolvable"*) echo "ok: ambiguous overload reported as unresolvable, not a bare OK" ;; + *) echo "FAIL: overload blindness went unreported: [$out]"; exit 1 ;; +esac + +# (b) The reference precedes EVERY definition of the name. Decidable without +# knowing which overload was meant — it is wrong either way. Pins that the +# ambiguity bail-out did not swallow this existing catch. +printf '%s\n%s\n%s\n' "$tmp/eq_use.sql" "$tmp/eq_int.sql" "$tmp/eq_text.sql" > "$tmp/overload_bad.txt" +if bash tasks/test/verify_symbol_order_v3.sh "$tmp/overload_bad.txt" 2>/dev/null; then + echo "FAIL: reference before ALL overloads accepted — the preserved catch is gone"; exit 1 +fi +echo "ok: reference before every overload still rejected" + +# (c) Overloaded but every definition sits in ONE file ordered before the use, so +# the answer is sound whichever overload was meant — must stay fully checked, not +# written off as unresolvable. Mirrors the real eql_v3.ste_vec_contains and the +# CREATE OPERATOR FAMILY + CLASS pair sharing eql_v3_internal.ore_cllw_ops. +printf 'CREATE FUNCTION eql_v3.selector(a public.jsonb_entry) RETURNS text ...\nCREATE FUNCTION eql_v3.selector(a public.jsonb_query) RETURNS text ...\n' > "$tmp/sel_defs.sql" +printf 'SELECT eql_v3.selector(x);\n' > "$tmp/sel_use.sql" +printf '%s\n%s\n' "$tmp/sel_defs.sql" "$tmp/sel_use.sql" > "$tmp/samefile_order.txt" +out="$(bash tasks/test/verify_symbol_order_v3.sh "$tmp/samefile_order.txt")" \ + || { echo "FAIL: same-file overloads rejected"; exit 1; } +case "$out" in + *"unresolvable"*) echo "FAIL: same-file overloads written off as unresolvable: [$out]"; exit 1 ;; + *) echo "ok: overloads all defined before use stay soundly checked" ;; +esac + echo "symbol-order self-test passed" diff --git a/tasks/test/verify_symbol_order_v3.sh b/tasks/test/verify_symbol_order_v3.sh index ae4c9169b..c49921291 100755 --- a/tasks/test/verify_symbol_order_v3.sh +++ b/tasks/test/verify_symbol_order_v3.sh @@ -14,6 +14,30 @@ # plpgsql forward reference — mutual recursion, say — needs an entry in # tasks/test/symbol_order_allowlist.txt. # +# Scope: OVERLOADS ARE NOT RESOLVED HERE. Definitions and references are both +# keyed by schema+name with no argument list, because a reference carries no types +# to key on: a call site is a bare `eql_v3.eq(a, b)`, and CREATE OPERATOR supplies +# LEFTARG/RIGHTARG on other lines. Resolving that needs a type checker, not a +# line-oriented scan. eql_v3.eq has 186 definitions across files #55..#242, so for +# the hot names this gate decides almost nothing — it reports the count of such +# names rather than implying it checked them. +# +# That is not a hole in coverage, because Postgres already resolves overloads +# exactly, at CREATE time, when the concatenated monolith is installed: +# +# mise run test:clean_install_v3 +# +# which runs in CI on every relevant PR across PG 14-17, needs no CipherStash +# credentials, and is not skipped on forks. Verified: swapping text_eq_operators +# ahead of text_eq_functions passes THIS gate and fails that one with +# `function eql_v3.eq(text_eq, text_eq) does not exist`. +# +# So this gate is the DB-free pre-flight; the clean install is the authority. What +# this gate uniquely adds is (1) singleton symbols — hmac_256, the eql_v3.query_* +# domains, the opclasses, version() — where name identifies the object and the +# check is sound, and (2) the plpgsql strictness described above, which the clean +# install cannot catch because Postgres defers those bodies to execution time. +# # Scope: this checks CROSS-FILE order only. A reference is compared against the # index of the file that defines it (`defined[tok] > i`), so a symbol referenced # in the same file that defines it always passes, regardless of line order within @@ -82,7 +106,7 @@ awk -v allowfile="$ALLOW" ' s = substr(line, RSTART, RLENGTH); sub(/.*(eql_v3_internal|eql_v3)\./, "", s) schema = (index(substr(line,RSTART,RLENGTH), "eql_v3_internal.") ? "eql_v3_internal." : "eql_v3.") key = schema s - if (!(key in defined)) defined[key] = idx + record_def(key, idx) } # CREATE DOMAIN (eql_v3_internal|eql_v3|public).. All three schemas: the # SEM index-term types split across DDL forms — hmac_256/ope_cllw/bloom_filter @@ -101,12 +125,12 @@ awk -v allowfile="$ALLOW" ' if (seg ~ /eql_v3_internal\./) { sub(/.*eql_v3_internal\./, "", seg); key = "eql_v3_internal." seg } else if (seg ~ /eql_v3\./) { sub(/.*eql_v3\./, "", seg); key = "eql_v3." seg } else { sub(/.*public\./, "", seg); key = "public." seg; isdomain[seg] = 1 } - if (!(key in defined)) defined[key] = idx + record_def(key, idx) } # CREATE TYPE eql_v3_internal. (the composite SEM types: ore_block_256, ore_cllw) if (match(line, /CREATE[ \t]+TYPE[ \t]+eql_v3_internal\.[a-z0-9_]+/)) { s = substr(line, RSTART, RLENGTH); sub(/.*eql_v3_internal\./, "", s) - key = "eql_v3_internal." s; if (!(key in defined)) defined[key] = idx + key = "eql_v3_internal." s; record_def(key, idx) } # CREATE OPERATOR CLASS|FAMILY (eql_v3_internal|eql_v3).. The conditional # SEM ordered-index opclasses (ore_block_256_operator_class/_family, @@ -119,7 +143,7 @@ awk -v allowfile="$ALLOW" ' s = substr(line, RSTART, RLENGTH) schema = (index(s, "eql_v3_internal.") ? "eql_v3_internal." : "eql_v3.") sub(/.*(eql_v3_internal|eql_v3)\./, "", s) - key = schema s; if (!(key in defined)) defined[key] = idx + key = schema s; record_def(key, idx) } } close(file) @@ -149,7 +173,26 @@ awk -v allowfile="$ALLOW" ' close(file) } if (bad) { print "symbol-order cross-check FAILED" > "/dev/stderr"; exit 1 } - print "symbol-order cross-check OK (" idx " files)" + # Report the unresolvable set rather than folding it into a bare "OK". A pass + # that says "OK (244 files)" while ~27 overloaded names went unchecked is the + # same lie as the "OK (0 files)" vacuous pass refused above: indistinguishable + # from having actually checked them. + n_unchecked = 0 + for (t in unchecked) n_unchecked++ + if (n_unchecked > 0) { + printf("symbol-order cross-check OK (%d files; %d overloaded name(s) unresolvable here — \ +overload define-before-use is proven exactly by: mise run test:clean_install_v3)\n", idx, n_unchecked) + } else { + print "symbol-order cross-check OK (" idx " files)" + } + } + # Record a definition of `key` at file index `i`. Tracks the min index (the + # ordering check), the max, and the count — the latter two are what let check() + # tell "resolvable" from "overloaded, and I cannot know which one". + function record_def(key, i) { + if (!(key in defined) || i < defined[key]) defined[key] = i + if (!(key in defmax) || i > defmax[key]) defmax[key] = i + defcount[key]++ } function check(tok, i, file) { if (tok in allow) return @@ -158,9 +201,26 @@ awk -v allowfile="$ALLOW" ' printf("ERROR: %s references %s which is defined nowhere in the installer\n", file, tok) > "/dev/stderr" bad = 1; return } + # Reference precedes even the EARLIEST definition of this name. Wrong whichever + # overload was meant, so it is decidable without knowing which one. Checked + # before the ambiguity bail-out below — dropping this would lose a real catch. if (defined[tok] > i) { printf("ERROR: %s references %s defined later (at #%d, used at #%d)\n", file, tok, defined[tok], i) > "/dev/stderr" bad = 1 + return + } + # Overloaded, and at least one overload is still ahead of this reference: the + # right one may or may not be defined yet, and a line-oriented scan cannot say + # which. Bare call sites (`eql_v3.eq(a, b)`) carry no types, and CREATE OPERATOR + # supplies them via LEFTARG/RIGHTARG on other lines. Record, do not guess. + # + # When defmax <= i every overload already precedes the reference, so the answer + # is sound regardless of which one was meant — that keeps the same-file overload + # pairs (eql_v3.ste_vec_contains, eql_v3_internal.compare_ore_block_256_terms) + # fully checked instead of written off. + if (defcount[tok] > 1 && defmax[tok] > i) { + unchecked[tok] = 1 + return } } ' "$ORDERED" From f9de11661fa894e7a2e868a5e1fc7bc47dd79ac9 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Fri, 24 Jul 2026 13:41:12 +1000 Subject: [PATCH 15/15] build: describe the query-domain schema split without a private tracker id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The symbol-order gate's comments cited a private issue identifier, which the test:public_identifiers gate on main now rejects. State the reason directly instead: query operands live in eql_v3 rather than public because a query operand is never a column type. Same pass corrects two staleness bugs in those comments: the containment needle is eql_v3.query_json (renamed with src/v3/jsonb -> src/v3/json), and the hardcoded "39 query domains" is now 40 — replaced with a count-free phrasing so it cannot rot again. --- tasks/test/symbol_order_selftest.sh | 9 +++++---- tasks/test/verify_symbol_order_v3.sh | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/tasks/test/symbol_order_selftest.sh b/tasks/test/symbol_order_selftest.sh index a04783849..c1c07dc55 100755 --- a/tasks/test/symbol_order_selftest.sh +++ b/tasks/test/symbol_order_selftest.sh @@ -50,10 +50,11 @@ if bash tasks/test/verify_symbol_order_v3.sh "$tmp/domain_bad.txt" 2>/dev/null; fi echo "ok: domain-form type used before definition rejected" -# CREATE DOMAIN eql_v3.* form (the query-operand domains CIP-3442 moved out of -# `public`: eql_v3.query__ and eql_v3.query_jsonb). Pins the domain-capture -# branch's eql_v3 arm. Without it the whole surface's 39 query domains read as -# "defined nowhere" — the regression that reddened every build-dependent CI job. +# CREATE DOMAIN eql_v3.* form. Query operands live in `eql_v3`, not `public`, +# because a query operand is never a column type: eql_v3.query__ and +# eql_v3.query_json. Pins the domain-capture branch's eql_v3 arm. Without it +# every query domain on the surface reads as "defined nowhere" — the regression +# that reddened every build-dependent CI job. printf 'CREATE DOMAIN eql_v3.query_integer_eq AS jsonb;\n' > "$tmp/q.sql" printf 'CREATE FUNCTION eql_v3.eq(a public.integer_eq, b eql_v3.query_integer_eq) ...\n' > "$tmp/qf.sql" printf '%s\n%s\n' "$tmp/q.sql" "$tmp/qf.sql" > "$tmp/qdomain_good.txt" diff --git a/tasks/test/verify_symbol_order_v3.sh b/tasks/test/verify_symbol_order_v3.sh index c49921291..1d859d527 100755 --- a/tasks/test/verify_symbol_order_v3.sh +++ b/tasks/test/verify_symbol_order_v3.sh @@ -114,10 +114,11 @@ awk -v allowfile="$ALLOW" ' # NOT `CREATE TYPE`. Capturing only `public.` here would leave the three # most-referenced foundational types (~165 refs) reporting "defined # nowhere" — a real gap, not an allowlist case. `eql_v3.` owns the - # query-operand domains (`eql_v3.query__`, `eql_v3.query_jsonb`), - # which CIP-3442 moved out of `public`: omitting the schema here leaves all - # 39 of them reporting "defined nowhere". Only `public.` domains feed - # isdomain[] (that gates which `public.*` REFERENCES are checked). + # query-operand domains (`eql_v3.query__`, `eql_v3.query_json`), + # which live outside `public` because a query operand is never a column + # type: omitting the schema here leaves every one of them reporting + # "defined nowhere". Only `public.` domains feed isdomain[] (that gates + # which `public.*` REFERENCES are checked). # Test eql_v3_internal FIRST in both the alternation and the arms below, so # the `eql_v3` prefix cannot shadow it. if (match(line, /CREATE[ \t]+DOMAIN[ \t]+(eql_v3_internal|eql_v3|public)\.[a-z0-9_]+/)) {