diff --git a/crates/omnigraph-cli/tests/parity_matrix.rs b/crates/omnigraph-cli/tests/parity_matrix.rs new file mode 100644 index 0000000..75ba49e --- /dev/null +++ b/crates/omnigraph-cli/tests/parity_matrix.rs @@ -0,0 +1,255 @@ +//! RFC-009 Phase 1 — the embedded/remote parity referee. +//! +//! For every CLI verb with an `is_remote` fork, run the identical +//! invocation against (a) the local graph directly and (b) a spawned +//! server on a twin copy of the same graph, with the SAME actor on both +//! arms (local `--as act-parity`; remote bearer token resolving to +//! `act-parity`). Scrub the declared-volatile allowlist +//! (`support::scrub_volatile` — ids, wall-clock, transport locations); +//! everything else must match exactly. +//! +//! This test PINS behavior; it does not idealize it. Genuine divergences +//! discovered here are recorded in `KNOWN_DIVERGENCES` below (and filed), +//! never silently repaired — repairs are Phase 3's job, gated by this +//! referee staying green through the refactor. + +use tempfile::TempDir; + +mod support; +use support::*; + +/// Divergences between the arms that exist today, pinned as expectations. +/// Removing an entry requires the corresponding behavior change to be a +/// deliberate, release-noted decision (RFC-009 Compatibility). +const KNOWN_DIVERGENCES: &[&str] = &[ + // populated by the rows below as they are written +]; + +/// One matched setup per row: twin graphs + the SAME Cedar bundle on both +/// arms (the local arm via --config top-level policy.file; the server via +/// its config). Returns everything a row needs. +struct Parity { + _temp: TempDir, + local: std::path::PathBuf, + local_cfg: std::path::PathBuf, + server: TestServer, +} + +fn parity() -> Parity { + let (temp, local, remote) = twin_graphs(); + let (local_cfg, server_cfg) = parity_configs(temp.path(), &local, &remote); + let server = spawn_server_with_config_env( + &server_cfg, + &[( + "OMNIGRAPH_SERVER_BEARER_TOKENS_JSON", + r#"{"act-parity":"parity-tok"}"#, + )], + ); + Parity { + _temp: temp, + local, + local_cfg, + server, + } +} + +impl Parity { + fn run(&self, args: &[&str]) -> (std::process::Output, std::process::Output) { + run_both_with_config(&self.local, Some(&self.local_cfg), &self.server.base_url, args) + } +} + +fn assert_parity(verb: &str, local: &std::process::Output, remote: &std::process::Output) { + assert_eq!( + local.status.code(), + remote.status.code(), + "{verb}: exit codes diverge\nlocal: {local:?}\nremote: {remote:?}" + ); + if local.status.success() { + let local_json = scrubbed_json(local); + let remote_json = scrubbed_json(remote); + assert_eq!( + local_json, remote_json, + "{verb}: scrubbed JSON diverges (left=local, right=remote)" + ); + } +} + +#[test] +fn parity_query() { + let p = parity(); + let query = fixture("test.gq"); + let (l, r) = p.run(&[ + "query", + "--query", + query.to_str().unwrap(), + "--name", + "get_person", + "--params", + r#"{"name":"Alice"}"#, + "--json", + ], + ); + assert_parity("query", &l, &r); +} + +#[test] +fn parity_schema_show() { + let p = parity(); + let (l, r) = p.run(&["schema", "show", "--json"]); + assert_parity("schema show", &l, &r); +} + +#[test] +fn parity_snapshot() { + let p = parity(); + let (l, r) = p.run(&["snapshot", "--json"]); + assert_parity("snapshot", &l, &r); +} + +#[test] +fn parity_branch_list() { + let p = parity(); + let (l, r) = p.run(&["branch", "list", "--json"]); + assert_parity("branch list", &l, &r); +} + +#[test] +fn parity_commit_list() { + let p = parity(); + let (l, r) = p.run(&["commit", "list", "--json"]); + assert_parity("commit list", &l, &r); +} + +#[test] +fn parity_mutate() { + let p = parity(); + let (l, r) = p.run(&[ + "mutate", + "-e", + "query add($name: String, $age: I32) { insert Person { name: $name, age: $age } }", + "--params", + r#"{"name":"Parity","age":7}"#, + "--json", + ], + ); + assert_parity("mutate", &l, &r); +} + +#[test] +fn parity_branch_create_delete() { + let p = parity(); + let (l, r) = p.run(&["branch", "create", "--from", "main", "parity-branch", "--json"], + ); + assert_parity("branch create", &l, &r); + let (l, r) = p.run(&["branch", "delete", "parity-branch", "--json"], + ); + assert_parity("branch delete", &l, &r); +} + +#[test] +fn parity_branch_merge() { + let p = parity(); + let (l, r) = p.run(&["branch", "create", "--from", "main", "feature", "--json"], + ); + assert_parity("branch create (merge setup)", &l, &r); + let (l, r) = p.run(&["branch", "merge", "feature", "--into", "main", "--json"], + ); + assert_parity("branch merge", &l, &r); +} + +#[test] +fn parity_load() { + let p = parity(); + let data = p.local.parent().unwrap().join("rows.jsonl"); + std::fs::write( + &data, + "{\"type\":\"Person\",\"data\":{\"name\":\"Loaded\",\"age\":1}}\n", + ) + .unwrap(); + let (l, r) = p.run(&[ + "load", + "--mode", + "merge", + "--data", + data.to_str().unwrap(), + "--json", + ], + ); + assert_parity("load", &l, &r); +} + +// ---- error parity: exit codes must match for shared failure cases ---- + +#[test] +fn parity_errors_share_exit_codes() { + let p = parity(); + + // unknown branch on merge + let (l, r) = p.run(&["branch", "merge", "no-such-branch", "--into", "main", "--json"], + ); + assert_eq!( + (l.status.success(), r.status.success()), + (false, false), + "merge of unknown branch must fail on both arms\nlocal {l:?}\nremote {r:?}" + ); + + // unknown query name in the source + let query = fixture("test.gq"); + let (l, r) = p.run(&[ + "query", + "--query", + query.to_str().unwrap(), + "--name", + "no_such_query", + "--json", + ], + ); + assert_eq!( + (l.status.success(), r.status.success()), + (false, false), + "unknown query name must fail on both arms\nlocal {l:?}\nremote {r:?}" + ); + + // Discovery (parity HOLDS, behavior surprising): an inline query run + // with a declared-but-unbound param does NOT error on either arm — it + // returns every row (the filter drops), while the stored-query invoke + // path hard-errors 'parameter not provided'. Pinned here as agreeing + // behavior; the cross-path asymmetry is filed separately. + let (l, r) = p.run(&[ + "query", + "--query", + query.to_str().unwrap(), + "--name", + "get_person", + "--json", + ], + ); + assert_eq!( + (l.status.success(), r.status.success()), + (true, true), + "unbound-param inline query currently SUCCEEDS on both arms (matches-all)" + ); +} + +// ---- documented exclusions (not bugs; the Phase 4 capability table) ---- +// +// - `graphs list`: server-only today; becomes Both-capability when the +// embedded arm enumerates the cluster catalog (RFC-009 open Q3, answered). +// - `ingest`: deprecated alias of load; the remote `load` arm itself rides +// the deprecated /ingest route today (RFC-009 Phase 5 flips it to /load — +// this matrix's `parity_load` row is where that flip becomes visible). +// - `init`, `optimize`, `repair`, `cleanup`, `cluster *`: storage-plane by +// design (must work with the server down); Phase 4 declares this. +#[allow(dead_code)] +const EXCLUSIONS_DOCUMENTED: () = (); + +#[test] +fn known_divergences_ledger_is_current() { + // The ledger exists so removals are deliberate: an empty list with all + // rows green means the arms agree everywhere the matrix looks. + assert!( + KNOWN_DIVERGENCES.is_empty(), + "divergences are pinned: {KNOWN_DIVERGENCES:?}" + ); +} diff --git a/crates/omnigraph-cli/tests/support/mod.rs b/crates/omnigraph-cli/tests/support/mod.rs index b11e94d..b36288c 100644 --- a/crates/omnigraph-cli/tests/support/mod.rs +++ b/crates/omnigraph-cli/tests/support/mod.rs @@ -688,3 +688,170 @@ pub fn queries_test_config(graph_uri: &str, entry: &str, gq_file: &str) -> Strin graph_uri.replace('\'', "''") ) } + +// ---- RFC-009 Phase 1: parity-matrix harness ---- + +/// Twin graphs for embedded-vs-remote comparison: the same loaded fixture +/// copied to two roots, so write verbs can run once per arm on identical +/// state. Returns (tempdir-guard, local_graph, remote_graph). +pub fn twin_graphs() -> (TempDir, PathBuf, PathBuf) { + let temp = tempdir().unwrap(); + let seed = temp.path().join("seed"); + fs::create_dir_all(&seed).unwrap(); + let graph = seed.join("server.omni"); + init_graph(&graph); + load_fixture(&graph); + let local = temp.path().join("local.omni"); + let remote = temp.path().join("remote.omni"); + copy_dir(&graph, &local); + copy_dir(&graph, &remote); + (temp, local, remote) +} + +pub fn copy_dir(from: &Path, to: &Path) { + fs::create_dir_all(to).unwrap(); + for entry in fs::read_dir(from).unwrap() { + let entry = entry.unwrap(); + let target = to.join(entry.file_name()); + if entry.file_type().unwrap().is_dir() { + copy_dir(&entry.path(), &target); + } else { + fs::copy(entry.path(), &target).unwrap(); + } + } +} + +/// Scrub declared-volatile fields (RFC-009 Phase 1 allowlist) so the rest +/// of the JSON must match exactly. Key-based, recursive; both arms get the +/// same placeholders. Everything NOT listed here is contract. +pub fn scrub_volatile(value: &mut serde_json::Value) { + const VOLATILE_KEYS: &[&str] = &[ + // identity-bearing per-instance values + "commit_id", "id", "parent_id", "merge_parent_id", "snapshot", + // wall-clock + "committed_at", "created_at", "timestamp", + // transport / location + "uri", "path", + ]; + match value { + serde_json::Value::Object(map) => { + for (key, val) in map.iter_mut() { + if VOLATILE_KEYS.contains(&key.as_str()) && !val.is_null() { + *val = serde_json::Value::String(format!("")); + } else { + scrub_volatile(val); + } + } + } + serde_json::Value::Array(items) => { + for item in items { + scrub_volatile(item); + } + } + _ => {} + } +} + +pub const PARITY_ACTOR: &str = "act-parity"; +pub const PARITY_TOKEN: &str = "parity-tok"; + +/// Identical Cedar bundle for BOTH arms — like-for-like enforcement is part +/// of the parity contract (a bare local arm is permissive while a +/// tokens-only server is default-deny; comparing those would measure +/// configuration, not the fork). +pub fn parity_policy_yaml() -> String { + r#"version: 1 +groups: + parity: ["act-parity"] +protected_branches: [] +rules: + - id: reads + allow: + actors: { group: parity } + actions: [read, export, invoke_query] + - id: read-scope + allow: + actors: { group: parity } + actions: [read, export] + branch_scope: any + - id: writes + allow: + actors: { group: parity } + actions: [change] + branch_scope: any + - id: branching + allow: + actors: { group: parity } + actions: [schema_apply, branch_create, branch_delete, branch_merge] + target_branch_scope: any +"# + .to_string() +} + +/// Per-arm config files carrying the same policy. Both arms address the +/// graph by positional URI, so the TOP-LEVEL policy.file applies on each +/// side (single-graph semantics). +pub fn parity_configs(root: &Path, _local_graph: &Path, remote_graph: &Path) -> (PathBuf, PathBuf) { + let policy = root.join("parity.policy.yaml"); + fs::write(&policy, parity_policy_yaml()).unwrap(); + let local_cfg = root.join("local.omnigraph.yaml"); + fs::write( + &local_cfg, + format!("policy:\n file: {}\n", policy.display()), + ) + .unwrap(); + let server_cfg = root.join("server.omnigraph.yaml"); + fs::write( + &server_cfg, + format!( + "server:\n graph: parity\ngraphs:\n parity:\n uri: {}\n policy:\n file: {}\n", + remote_graph.display(), + policy.display() + ), + ) + .unwrap(); + (local_cfg, server_cfg) +} + +/// Run one CLI invocation per arm with identical verb args: locally against +/// `local_graph` (--as actor) and remotely against a server URL whose token +/// resolves to the same actor. Returns raw Outputs for exit-code + JSON +/// comparison by the caller. +pub fn run_both( + local_graph: &Path, + server_url: &str, + args: &[&str], +) -> (std::process::Output, std::process::Output) { + run_both_with_config(local_graph, None, server_url, args) +} + +pub fn run_both_with_config( + local_graph: &Path, + local_config: Option<&Path>, + server_url: &str, + args: &[&str], +) -> (std::process::Output, std::process::Output) { + let mut local = cli(); + local.arg(args[0]).arg(local_graph).args(&args[1..]).arg("--as").arg(PARITY_ACTOR); + if let Some(config) = local_config { + local.arg("--config").arg(config); + } + let local_out = local.output().unwrap(); + + let mut remote = cli(); + remote + .env("OMNIGRAPH_BEARER_TOKEN", PARITY_TOKEN) + .arg(args[0]) + .arg(server_url) + .args(&args[1..]); + let remote_out = remote.output().unwrap(); + (local_out, remote_out) +} + +/// Parse, scrub, and pretty-print for diffable assertion messages. +pub fn scrubbed_json(output: &std::process::Output) -> String { + let mut value: serde_json::Value = serde_json::from_slice(&output.stdout) + .unwrap_or_else(|e| panic!("non-JSON stdout ({e}): {output:?}")); + scrub_volatile(&mut value); + serde_json::to_string_pretty(&value).unwrap() +} diff --git a/docs/dev/rfc-009-unify-access-paths.md b/docs/dev/rfc-009-unify-access-paths.md index 43dcdf2..cada723 100644 --- a/docs/dev/rfc-009-unify-access-paths.md +++ b/docs/dev/rfc-009-unify-access-paths.md @@ -68,7 +68,7 @@ anything moves — mirroring the storage collapse, where the pinned contract tests gated the swap, and the test-monolith modularization (#192/#193), which makes Phase 3 tractable: the CLI dispatch is 1,184 lines today, not 4,200. -### Phase 1 — Parity matrix (the referee; do first, no refactor) +### Phase 1 — Parity matrix (the referee; do first, no refactor) *(landed)* A CLI integration test (extend the `system_local.rs` harness, which already spawns both binaries): one fixture graph; for every forked verb, run the @@ -81,6 +81,15 @@ This pins today's behavior so Phase 3 can't silently change it, and catches every future fork drift. It also incidentally covers utoipa annotation↔route mismatches (a lying `#[utoipa::path]` makes the remote leg 404). +**Phase 1 outcome (landed):** `crates/omnigraph-cli/tests/parity_matrix.rs` +— 11 rows green with an **empty divergence ledger**: with matched Cedar +policy on both arms, embedded and remote agree on every forked verb's +scrubbed JSON and exit codes. Two findings along the way: like-for-like +requires the same policy bundle on both arms (a tokens-only server is +default-deny by design — the harness encodes this), and inline execution's +unbound-param matches-all vs the invoke path's hard error is a cross-path +asymmetry, filed as #207 and pinned (not repaired) by the matrix. + ### Phase 2 — One wire-DTO crate Move the HTTP request/response types and the single `engine result → DTO` diff --git a/docs/dev/testing.md b/docs/dev/testing.md index f2b33de..866af69 100644 --- a/docs/dev/testing.md +++ b/docs/dev/testing.md @@ -7,7 +7,7 @@ This file is the always-on map of the test surface. **Consult it before every ta | Crate | Path | Style | |---|---|---| | `omnigraph` (engine) | `crates/omnigraph/tests/` | Integration tests (28 files), fixture-driven, share `tests/helpers/mod.rs` | -| `omnigraph-cli` | `crates/omnigraph-cli/tests/` | Per-area suites (post-modularization): `cli_cluster.rs` (cluster command surface + operator-actor cascade), `cli_cluster_e2e.rs` (spawned-binary lifecycle compositions — lost-state re-import recovery, out-of-band drift, graph-root destruction, multi-graph mixed-disposition convergence), `cli_data.rs` (load/read/change/branch/commit/export/snapshot/policy/embed/maintenance + operator format cascade), `cli_schema_config.rs` (init/config, schema plan/apply, RFC-008 deprecation warnings + `config migrate` + strict mode), `cli_queries.rs`, `system_local.rs` (full-cycle cluster lifecycle with a spawned `--cluster` server, applied-policy enforcement over HTTP, keyed-credential auth, operator aliases), `system_remote.rs`; share `tests/support/mod.rs` (hermetic `OMNIGRAPH_HOME` by default) | +| `omnigraph-cli` | `crates/omnigraph-cli/tests/` | Per-area suites (post-modularization): `cli_cluster.rs` (cluster command surface + operator-actor cascade), `cli_cluster_e2e.rs` (spawned-binary lifecycle compositions — lost-state re-import recovery, out-of-band drift, graph-root destruction, multi-graph mixed-disposition convergence), `cli_data.rs` (load/read/change/branch/commit/export/snapshot/policy/embed/maintenance + operator format cascade), `cli_schema_config.rs` (init/config, schema plan/apply, RFC-008 deprecation warnings + `config migrate` + strict mode), `cli_queries.rs`, `parity_matrix.rs` (RFC-009 Phase 1: the embedded-vs-remote referee — every forked verb run against both arms with matched Cedar policy and the same actor, scrubbed-JSON + exit-code equality; divergences are pinned in its `KNOWN_DIVERGENCES` ledger, never silently repaired), `system_local.rs` (full-cycle cluster lifecycle with a spawned `--cluster` server, applied-policy enforcement over HTTP, keyed-credential auth, operator aliases), `system_remote.rs`; share `tests/support/mod.rs` (hermetic `OMNIGRAPH_HOME` by default) | | `omnigraph-cluster` | mostly in-source `#[cfg(test)] mod tests`; `tests/failpoints.rs` (feature-gated); `tests/s3_cluster.rs` (bucket-gated full lifecycle on object storage) | Cluster config parser, local JSON state diff, state CAS/lock handling/recovery, read-only validate/plan/status plus explicit refresh/import graph observations, config-only apply (content-addressed payload publish, disposition gating, composite-digest convergence, idempotent re-apply), catalog payload verification (status read-only, refresh drift + self-heal), failpoint crash-mid-apply / CAS-race coverage, Stage 4A graph creation (create executor, recovery sidecars + sweep rows, create crash windows), Stage 4B schema apply (migration previews in plan, schema executor, schema-apply sweep classification, schema crash windows), Stage 4C gated deletes (digest-bound approvals, delete executor + tombstones, delete sweep rows, delete crash windows), and 5A policy binding metadata (applies_to in the applied revision, binding-change diffing + convergence, pre-5A backfill), and the 5B serving-snapshot read API (converged read, refusal rows) | | `omnigraph-server` | `crates/omnigraph-server/tests/` | Per-area suites (post-modularization): `auth_policy.rs`, `data_routes.rs`, `schema_routes.rs`, `stored_queries.rs`, `multi_graph.rs` (cluster-mode boot — converged serving, policy binding wiring, boot refusals — + the concurrent branch-ops matrix), `boot_settings.rs` (mode inference, PolicySource), `s3.rs` (bucket-gated: single-graph serving + config-free `--cluster s3://` boot), `openapi.rs` (OpenAPI drift / regeneration); share `tests/support/mod.rs` | | `omnigraph-compiler` | mostly in-source `#[cfg(test)] mod tests` | Parser, type-checker, IR lowering, lint |