From d2340f19e983379c314b1a57d3f9c0ef37e3294c Mon Sep 17 00:00:00 2001 From: Andrew Altshuler Date: Mon, 15 Jun 2026 23:11:42 +0300 Subject: [PATCH] feat(cli)!: schema apply refuses a cluster-managed graph (RFC-011 D10) (#253) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `omnigraph schema apply` against a cluster-managed graph's storage root bypassed the cluster ledger/recovery/approvals. Mirror `init`'s refusal: on the embedded (direct-store) path, if the resolved URI is inside a cluster (`cluster_root_for_graph_uri`), bail and point at `cluster apply`. The served (`--server`) path is unaffected — it addresses a server, not a storage root. `schema plan`/`show` (read-only) are untouched. Two e2e tests injected "out-of-band drift" via this exact CLI path; since the CLI now refuses it, they inject drift via a direct engine `apply_schema` against the storage root instead — a faithful control-plane bypass, which is what out-of-band drift is. New regression: `schema_apply_refuses_a_cluster_managed_graph_and_signposts_cluster_apply`. Co-authored-by: Claude Opus 4.8 --- crates/omnigraph-cli/src/main.rs | 18 ++++++++ crates/omnigraph-cli/tests/cli_cluster.rs | 41 +++++++++++++++++++ crates/omnigraph-cli/tests/cli_cluster_e2e.rs | 34 +++++++-------- crates/omnigraph-cli/tests/system_local.rs | 27 +++++------- docs/user/cli/reference.md | 2 +- 5 files changed, 89 insertions(+), 33 deletions(-) diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index d9c5720..1817aaa 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -383,6 +383,24 @@ async fn main() -> Result<()> { cli.store.as_deref(), ) .await?; + // RFC-011 Decision 10: a graph managed by a cluster evolves via + // `cluster apply` (ledger/recovery/approvals), not a direct + // `schema apply` against its storage root — that would bypass the + // ledger. Mirrors `init`'s refusal. Only the embedded path can + // address a storage root; a served apply (`--server`) is the + // server's concern. + if !client.is_remote() { + if let Some(root) = + omnigraph_cluster::cluster_root_for_graph_uri(client.uri()).await + { + bail!( + "`{}` is inside cluster `{root}`. A graph in a cluster evolves via \ + `cluster apply` (which records ledger, recovery, and approvals), not \ + `schema apply`. Update the schema in cluster.yaml and run `cluster apply`.", + client.uri() + ); + } + } let schema_source = fs::read_to_string(&schema)?; // The embedded (direct-store) arm carries no stored-query // registry — the registry is cluster-owned (RFC-011), so a diff --git a/crates/omnigraph-cli/tests/cli_cluster.rs b/crates/omnigraph-cli/tests/cli_cluster.rs index bd2fbfc..e35a54d 100644 --- a/crates/omnigraph-cli/tests/cli_cluster.rs +++ b/crates/omnigraph-cli/tests/cli_cluster.rs @@ -1039,6 +1039,47 @@ fn init_refuses_a_cluster_managed_path_and_signposts_cluster_apply() { assert!(!temp.path().join("graphs").join("sneaky.omni").exists()); } +#[test] +fn schema_apply_refuses_a_cluster_managed_graph_and_signposts_cluster_apply() { + // RFC-011 Decision 10: a direct `schema apply` against a cluster-managed + // graph's storage root would bypass the ledger/recovery/approvals, so it is + // refused and points at `cluster apply` (mirrors `init`'s refusal). + let temp = applied_knowledge_cluster(); + // A schema that WOULD change the graph (adds `bio`) — so the no-mutation + // assertion below is meaningful, not a no-op re-apply. + fs::write( + temp.path().join("people_v2.pg"), + "node Person {\n name: String @key\n age: I32?\n bio: String?\n}\n", + ) + .unwrap(); + let out = output_failure( + cli() + .arg("schema") + .arg("apply") + .arg("--schema") + .arg(temp.path().join("people_v2.pg")) + .arg("--store") + .arg(temp.path().join("graphs").join("knowledge.omni")), + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("cluster apply"), + "schema apply against a cluster-managed graph should signpost `cluster apply`; got: {stderr}" + ); + // And it bailed BEFORE mutating: the live schema still lacks `bio`. + let show = output_success( + cli() + .arg("schema") + .arg("show") + .arg(temp.path().join("graphs").join("knowledge.omni")), + ); + assert!( + !stdout_string(&show).contains("bio"), + "the refused apply must not have changed the live schema; got: {}", + stdout_string(&show) + ); +} + #[test] fn init_outside_a_cluster_still_works() { // Regression guard: ordinary init (no cluster layout) is unaffected. diff --git a/crates/omnigraph-cli/tests/cli_cluster_e2e.rs b/crates/omnigraph-cli/tests/cli_cluster_e2e.rs index 36b476a..35ded58 100644 --- a/crates/omnigraph-cli/tests/cli_cluster_e2e.rs +++ b/crates/omnigraph-cli/tests/cli_cluster_e2e.rs @@ -3,6 +3,7 @@ use std::fs; +use omnigraph::db::Omnigraph; use tempfile::tempdir; mod support; @@ -236,27 +237,28 @@ fn cluster_e2e_out_of_band_schema_drift_then_apply_converges_it() { let apply = cluster_json(temp.path(), "apply"); assert_eq!(apply["converged"], true, "{apply}"); - // Out-of-band: the live graph evolves, cluster.yaml stays put. - fs::write( - temp.path().join("people_v2.pg"), - r#" + // Out-of-band: the live graph evolves while cluster.yaml stays put. RFC-011 + // D10 makes the CLI `schema apply` refuse a cluster-managed graph, so this + // simulates a true bypass — a direct engine apply against the storage root, + // exactly the drift the control plane must still detect and converge. + let people_v2 = r#" node Person { name: String @key age: I32? bio: String? } -"#, - ) - .unwrap(); - output_success( - cli() - .arg("schema") - .arg("apply") - .arg(temp.path().join("graphs/knowledge.omni")) - .arg("--schema") - .arg(temp.path().join("people_v2.pg")) - .arg("--json"), - ); +"#; + tokio::runtime::Runtime::new().unwrap().block_on(async { + let db = Omnigraph::open( + temp.path() + .join("graphs/knowledge.omni") + .to_string_lossy() + .as_ref(), + ) + .await + .unwrap(); + db.apply_schema(people_v2).await.unwrap(); + }); // Drift is visible... let refresh = cluster_json(temp.path(), "refresh"); diff --git a/crates/omnigraph-cli/tests/system_local.rs b/crates/omnigraph-cli/tests/system_local.rs index c548ac5..77f1cb6 100644 --- a/crates/omnigraph-cli/tests/system_local.rs +++ b/crates/omnigraph-cli/tests/system_local.rs @@ -3,6 +3,7 @@ mod support; use std::env; use std::fs; +use omnigraph::db::Omnigraph; use reqwest::blocking::Client; use serde_json::Value; @@ -2090,22 +2091,16 @@ fn local_cluster_full_lifecycle_declare_serve_evolve_delete() { } // Out-of-band drift: the live graph evolves behind the cluster's back; - // refresh observes it, apply converges it back to the declared schema. - std::fs::write( - dir.join("rogue.pg"), - "\nnode Person {\n name: String @key\n bio: String?\n rogue: String?\n}\n", - ) - .unwrap(); - let output = cli() - .arg("schema") - .arg("apply") - .arg(dir.join("graphs/knowledge.omni")) - .arg("--schema") - .arg(dir.join("rogue.pg")) - .arg("--json") - .output() - .unwrap(); - assert!(output.status.success(), "out-of-band schema apply failed"); + // refresh observes it, apply converges it back to the declared schema. RFC-011 + // D10 makes the CLI `schema apply` refuse a cluster-managed graph, so a true + // bypass is a direct engine apply against the storage root. + let rogue_pg = "\nnode Person {\n name: String @key\n bio: String?\n rogue: String?\n}\n"; + tokio::runtime::Runtime::new().unwrap().block_on(async { + let db = Omnigraph::open(dir.join("graphs/knowledge.omni").to_string_lossy().as_ref()) + .await + .unwrap(); + db.apply_schema(rogue_pg).await.unwrap(); + }); let refresh = cluster_cli(dir, &["refresh"]); assert_eq!( refresh["resource_statuses"]["schema.knowledge"]["status"], diff --git a/docs/user/cli/reference.md b/docs/user/cli/reference.md index 2429bdf..2c628fb 100644 --- a/docs/user/cli/reference.md +++ b/docs/user/cli/reference.md @@ -18,7 +18,7 @@ Top-level command families and subcommands. Graph-targeting commands accept a po | `export` | dump to JSONL on stdout (`--type T`, `--table K` filters) | | `branch create \| list \| delete \| merge` | branching ops | | `commit list \| show` | inspect commit graph | -| `schema plan \| apply \| show (alias: get)` | migrations | +| `schema plan \| apply \| show (alias: get)` | migrations. `apply` refuses a cluster-managed graph (one whose storage is inside a cluster) and points at `cluster apply` — those graphs evolve through the cluster ledger, not a direct apply | | `lint` (alias: `check`) | offline / graph-backed query validation. Replaces `query lint` / `query check`, which are kept as deprecated argv-level shims that print a one-line warning and rewrite to `omnigraph lint` | | `cluster validate \| plan \| apply \| approve \| status \| refresh \| import \| force-unlock` | declarative cluster control plane. `validate` checks a local `cluster.yaml` folder and referenced schema/query/policy files; `plan` diffs it against local JSON state at `__cluster/state.json`, annotates dispositions, and embeds real schema-migration previews; `apply` converges the cluster — stored-query/policy catalog writes (content-addressed under `__cluster/resources/`), graph creates, schema updates (soft drops only; `--as` records the actor), and graph deletes behind a digest-bound approval from `cluster approve --as ` (`apply`/`approve` default the actor from `~/.omnigraph/config.yaml`'s `operator.actor` when `--as` is omitted); what apply converges is what an `omnigraph-server --cluster ` deployment serves on its next restart (`--cluster` is the server's only boot source — RFC-011 cluster-only); `status` reads the state ledger; `refresh`/`import` explicitly update local JSON state from read-only graph observations; `force-unlock ` manually removes a held local state lock by exact id | | `optimize` | non-destructive Lance compaction (skips tables with `Blob` columns or uncovered drift; `--json` reports `skipped`) |