From a09045028fcfebc02c39bb9777ed1da22c6ed56d Mon Sep 17 00:00:00 2001 From: Andrew Altshuler Date: Mon, 15 Jun 2026 14:30:58 +0300 Subject: [PATCH] feat(cli)!: unify graph selection under --graph; --cluster is a global scope; remove --cluster-graph (#241) RFC-011: --graph is the single graph selector across server and cluster scopes; --cluster becomes a global scope primitive; --cluster-graph removed. Maintenance dispatch unified through resolve_scope. Wrong-address guard validates each scope flag against the verb it can consume. --- crates/omnigraph-cli/src/cli.rs | 37 ++-- crates/omnigraph-cli/src/client.rs | 4 +- crates/omnigraph-cli/src/helpers.rs | 51 ++++- crates/omnigraph-cli/src/main.rs | 136 +++---------- crates/omnigraph-cli/src/planes.rs | 89 ++++++--- crates/omnigraph-cli/src/scope.rs | 187 +++++++++++++++--- crates/omnigraph-cli/tests/cli_cluster.rs | 16 +- crates/omnigraph-cli/tests/cli_data.rs | 55 +++++- .../omnigraph-cli/tests/cli_schema_config.rs | 2 +- docs/user/cli/reference.md | 15 +- docs/user/clusters/index.md | 9 +- docs/user/operations/maintenance.md | 2 +- 12 files changed, 398 insertions(+), 205 deletions(-) diff --git a/crates/omnigraph-cli/src/cli.rs b/crates/omnigraph-cli/src/cli.rs index ec0da08..56bfd3a 100644 --- a/crates/omnigraph-cli/src/cli.rs +++ b/crates/omnigraph-cli/src/cli.rs @@ -37,9 +37,11 @@ pub(crate) struct Cli { #[arg(long, global = true, value_name = "NAME|URL")] pub(crate) server: Option, - /// Graph id on a multi-graph `--server` (appends `/graphs/` to - /// the server url). Requires --server. - #[arg(long, global = true, value_name = "GRAPH_ID", requires = "server")] + /// Select a graph within a multi-graph scope: on a `--server` it appends + /// `/graphs/` to the server url; on a `--cluster` it picks which + /// cluster graph to maintain. Rejected on a single-graph address (a + /// positional URI / `--store`). + #[arg(long, global = true, value_name = "GRAPH_ID")] pub(crate) graph: Option, /// Select a named scope bundle (RFC-011) from `profiles:` in @@ -56,6 +58,14 @@ pub(crate) struct Cli { #[arg(long, global = true, value_name = "URI")] pub(crate) store: Option, + /// Address a cluster-managed graph's storage for maintenance (RFC-011): + /// a cluster directory or storage-root URI — named via `clusters:` in + /// ~/.omnigraph/config.yaml, or a literal `file://`/`s3://` root. Pair + /// with `--graph ` to select the graph. Used by optimize / repair / + /// cleanup; exclusive with a positional URI / `--store` / `--server`. + #[arg(long, global = true, value_name = "DIR|URI")] + pub(crate) cluster: Option, + #[command(subcommand)] pub(crate) command: Command, } @@ -239,13 +249,6 @@ pub(crate) enum Command { uri: Option, #[arg(long)] config: Option, - /// Cluster directory or storage-root URI; with --cluster-graph, resolves - /// the graph's storage URI from the served cluster state. - #[arg(long, conflicts_with = "uri", requires = "cluster_graph")] - cluster: Option, - /// Graph id within --cluster. - #[arg(long, requires = "cluster")] - cluster_graph: Option, #[arg(long)] json: bool, }, @@ -255,13 +258,6 @@ pub(crate) enum Command { uri: Option, #[arg(long)] config: Option, - /// Cluster directory or storage-root URI; with --cluster-graph, resolves - /// the graph's storage URI from the served cluster state. - #[arg(long, conflicts_with = "uri", requires = "cluster_graph")] - cluster: Option, - /// Graph id within --cluster. - #[arg(long, requires = "cluster")] - cluster_graph: Option, /// Publish verified maintenance drift. Without this flag, repair only /// previews what it would do. #[arg(long)] @@ -279,13 +275,6 @@ pub(crate) enum Command { uri: Option, #[arg(long)] config: Option, - /// Cluster directory or storage-root URI; with --cluster-graph, resolves - /// the graph's storage URI from the served cluster state. - #[arg(long, conflicts_with = "uri", requires = "cluster_graph")] - cluster: Option, - /// Graph id within --cluster. - #[arg(long, requires = "cluster")] - cluster_graph: Option, /// Number of recent versions to keep per table. Either `--keep` or /// `--older-than` (or both) must be set. #[arg(long)] diff --git a/crates/omnigraph-cli/src/client.rs b/crates/omnigraph-cli/src/client.rs index 5c427f2..81d3934 100644 --- a/crates/omnigraph-cli/src/client.rs +++ b/crates/omnigraph-cli/src/client.rs @@ -100,7 +100,7 @@ impl GraphClient { let scope = crate::scope::resolve_scope( &crate::operator::load_operator_config()?, crate::planes::Capability::Any, - crate::scope::ScopeFlags { profile, store, server, graph, uri }, + crate::scope::ScopeFlags { profile, store, server, cluster: None, graph, uri }, )?; let (server, graph, uri) = ( scope.server.as_deref(), @@ -147,7 +147,7 @@ impl GraphClient { let scope = crate::scope::resolve_scope( &crate::operator::load_operator_config()?, crate::planes::Capability::Any, - crate::scope::ScopeFlags { profile, store, server, graph, uri }, + crate::scope::ScopeFlags { profile, store, server, cluster: None, graph, uri }, )?; let (server, graph, uri) = ( scope.server.as_deref(), diff --git a/crates/omnigraph-cli/src/helpers.rs b/crates/omnigraph-cli/src/helpers.rs index d49d17f..d0632d7 100644 --- a/crates/omnigraph-cli/src/helpers.rs +++ b/crates/omnigraph-cli/src/helpers.rs @@ -539,12 +539,49 @@ pub(crate) fn resolve_local_uri( Ok(resolve_local_graph(config, cli_uri, operation)?.uri) } -/// Resolve a storage-plane verb's address to a direct storage URI (RFC-010 -/// Slice 3). `--cluster --cluster-graph ` resolves the graph's -/// storage URI from the **served cluster state** (the truth a `--cluster` -/// server serves); otherwise the ordinary positional-URI path. -/// clap enforces both-or-neither and exclusion with `uri`, so the mismatched -/// arm is defensive. +/// Resolve a maintenance verb's (optimize/repair/cleanup) address to a direct +/// storage URI through the one RFC-011 scope path. Every primitive funnels +/// here: a positional URI, `--store`, `--cluster --graph `, a +/// `--profile` cluster binding, or operator defaults — all resolved at the +/// `Direct` capability (so a server scope is rejected, a cluster scope is +/// allowed), then mapped to a storage URI by `resolve_storage_uri`. +pub(crate) async fn resolve_maintenance_uri( + config: &OmnigraphConfig, + profile: Option<&str>, + store: Option<&str>, + cluster: Option<&str>, + graph: Option<&str>, + cli_uri: Option, + operation: &str, +) -> Result { + let scope = scope::resolve_scope( + &operator::load_operator_config()?, + planes::Capability::Direct, + scope::ScopeFlags { + profile, + store, + server: None, + cluster, + graph, + uri: cli_uri, + }, + )?; + resolve_storage_uri( + config, + scope.uri, + scope.cluster.as_deref(), + scope.cluster_graph.as_deref(), + operation, + ) + .await +} + +/// Map a resolved direct address to a storage URI: a cluster scope +/// (`--cluster --graph `, or a `--profile` cluster binding) +/// resolves the graph's storage URI from the **served cluster state** (the +/// truth a `--cluster` server serves); otherwise the ordinary positional-URI +/// path. The scope resolver guarantees a cluster scope always carries a graph, +/// so the mismatched arm is defensive. pub(crate) async fn resolve_storage_uri( config: &OmnigraphConfig, cli_uri: Option, @@ -555,7 +592,7 @@ pub(crate) async fn resolve_storage_uri( match (cluster, cluster_graph) { (Some(cluster), Some(graph_id)) => resolve_cluster_graph_uri(cluster, graph_id).await, (None, None) => resolve_local_uri(config, cli_uri, operation), - _ => bail!("--cluster and --cluster-graph must be given together"), + _ => bail!("internal error: a cluster scope was resolved without a graph id"), } } diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index 606db96..9734573 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -790,46 +790,18 @@ async fn main() -> Result<()> { print_policy_explain(&decision, &actor, &request); } }, - Command::Optimize { - uri, - config, - cluster, - cluster_graph, - json, - } => { + Command::Optimize { uri, config, json } => { let config = load_cli_config(config.as_ref())?; - let uri = if uri.is_some() || cluster.is_some() { - resolve_storage_uri( - &config, - uri, - cluster.as_deref(), - cluster_graph.as_deref(), - "optimize", - ) - .await? - } else { - // RFC-011: no explicit per-command address — consult the scope - // (a --profile cluster binding, --store, or operator defaults). - let scope = scope::resolve_scope( - &operator::load_operator_config()?, - planes::Capability::Direct, - scope::ScopeFlags { - profile: cli.profile.as_deref(), - store: cli.store.as_deref(), - server: None, - graph: cli.graph.as_deref(), - uri: None, - }, - )?; - resolve_storage_uri( - &config, - scope.uri, - scope.cluster.as_deref(), - scope.cluster_graph.as_deref(), - "optimize", - ) - .await? - }; + let uri = resolve_maintenance_uri( + &config, + cli.profile.as_deref(), + cli.store.as_deref(), + cli.cluster.as_deref(), + cli.graph.as_deref(), + uri, + "optimize", + ) + .await?; let db = Omnigraph::open(&uri).await?; let stats = db.optimize().await?; if json { @@ -865,44 +837,21 @@ async fn main() -> Result<()> { Command::Repair { uri, config, - cluster, - cluster_graph, confirm, force, json, } => { let config = load_cli_config(config.as_ref())?; - let uri = if uri.is_some() || cluster.is_some() { - resolve_storage_uri( - &config, - uri, - cluster.as_deref(), - cluster_graph.as_deref(), - "repair", - ) - .await? - } else { - // RFC-011: no explicit per-command address — consult the scope. - let scope = scope::resolve_scope( - &operator::load_operator_config()?, - planes::Capability::Direct, - scope::ScopeFlags { - profile: cli.profile.as_deref(), - store: cli.store.as_deref(), - server: None, - graph: cli.graph.as_deref(), - uri: None, - }, - )?; - resolve_storage_uri( - &config, - scope.uri, - scope.cluster.as_deref(), - scope.cluster_graph.as_deref(), - "repair", - ) - .await? - }; + let uri = resolve_maintenance_uri( + &config, + cli.profile.as_deref(), + cli.store.as_deref(), + cli.cluster.as_deref(), + cli.graph.as_deref(), + uri, + "repair", + ) + .await?; let db = Omnigraph::open(&uri).await?; let stats = db .repair(omnigraph::db::RepairOptions { confirm, force }) @@ -979,45 +928,22 @@ async fn main() -> Result<()> { Command::Cleanup { uri, config, - cluster, - cluster_graph, keep, older_than, confirm, json, } => { let config = load_cli_config(config.as_ref())?; - let uri = if uri.is_some() || cluster.is_some() { - resolve_storage_uri( - &config, - uri, - cluster.as_deref(), - cluster_graph.as_deref(), - "cleanup", - ) - .await? - } else { - // RFC-011: no explicit per-command address — consult the scope. - let scope = scope::resolve_scope( - &operator::load_operator_config()?, - planes::Capability::Direct, - scope::ScopeFlags { - profile: cli.profile.as_deref(), - store: cli.store.as_deref(), - server: None, - graph: cli.graph.as_deref(), - uri: None, - }, - )?; - resolve_storage_uri( - &config, - scope.uri, - scope.cluster.as_deref(), - scope.cluster_graph.as_deref(), - "cleanup", - ) - .await? - }; + let uri = resolve_maintenance_uri( + &config, + cli.profile.as_deref(), + cli.store.as_deref(), + cli.cluster.as_deref(), + cli.graph.as_deref(), + uri, + "cleanup", + ) + .await?; let older_than_dur = older_than.as_deref().map(parse_duration_arg).transpose()?; diff --git a/crates/omnigraph-cli/src/planes.rs b/crates/omnigraph-cli/src/planes.rs index c289daa..792cab4 100644 --- a/crates/omnigraph-cli/src/planes.rs +++ b/crates/omnigraph-cli/src/planes.rs @@ -177,35 +177,78 @@ pub(crate) fn command_label(cmd: &Command) -> &'static str { } } -/// Reject the data-plane addressing flags (`--server`/`--graph`) on any verb -/// that does not live on the data plane. This replaces the old silent-ignore -/// — e.g. `optimize --server prod` previously dropped `--server` and tried to -/// resolve a default target, failing (if at all) with an unrelated message. -/// Now it fails with one honest, declared error. RFC-010 Slice 1. +/// The verbs that address an existing graph through a cluster scope +/// (`--cluster --graph `): the storage-maintenance commands. +/// `init` is storage-plane too but *creates* a graph (cluster graphs are born +/// from `cluster apply`, not `init`), and `schema plan` / `lint` take a +/// positional URI — none consume cluster addressing, so the guard rejects +/// `--cluster`/`--graph` on them rather than silently dropping the flag. +pub(crate) fn accepts_cluster_addressing(cmd: &Command) -> bool { + matches!( + cmd, + Command::Optimize { .. } | Command::Repair { .. } | Command::Cleanup { .. } + ) +} + +/// Reject a scope-addressing flag (`--server`/`--cluster`/`--graph`) on a verb +/// that cannot consume it, rather than silently dropping it (the old behavior: +/// e.g. `optimize --server prod` dropped `--server` and failed later with an +/// unrelated message). Each flag has a distinct valid surface: +/// - `--server` → served-graph scopes (`any`/`served`); +/// - `--cluster` → the cluster-maintenance verbs (optimize/repair/cleanup); +/// - `--graph` → any multi-graph scope: a served scope *or* a cluster one. +/// RFC-010 Slice 1, generalized for RFC-011 cluster addressing. pub(crate) fn guard_addressing(cli: &Cli) -> Result<()> { - if cli.server.is_none() && cli.graph.is_none() { + if cli.server.is_none() && cli.cluster.is_none() && cli.graph.is_none() { return Ok(()); } let capability = command_capability(&cli.command); - if capability.accepts_server_addressing() { - return Ok(()); - } let label = command_label(&cli.command); - let how = match capability { - Capability::Direct => match cli.command { - Command::Init { .. } => "Pass a storage URI.", - _ => "Pass a storage URI, or --cluster --cluster-graph .", + let cluster_ok = accepts_cluster_addressing(&cli.command); + + if cli.server.is_some() && !capability.accepts_server_addressing() { + bail!( + "`{label}` is a {} command; --server addresses a served graph and does not apply.{}", + capability.describe(), + remediation(capability, &cli.command), + ); + } + if cli.cluster.is_some() && !cluster_ok { + bail!( + "`{label}` is a {} command; --cluster addresses a cluster-managed graph for \ + maintenance (optimize/repair/cleanup) and does not apply.{}", + capability.describe(), + remediation(capability, &cli.command), + ); + } + if cli.graph.is_some() && !(capability.accepts_server_addressing() || cluster_ok) { + bail!( + "`{label}` is a {} command; --graph selects a graph within a server or cluster \ + scope and does not apply.{}", + capability.describe(), + remediation(capability, &cli.command), + ); + } + Ok(()) +} + +/// The "what to do instead" tail for a wrong-address error, by capability. +/// Includes its own leading space when non-empty so the caller appends it +/// directly — an empty tail (the served-addressing capabilities, which only +/// reach this fn for a misplaced `--cluster`/`--graph`) leaves no trailing space. +fn remediation(capability: Capability, cmd: &Command) -> &'static str { + match capability { + Capability::Direct => match cmd { + Command::Init { .. } => " Pass a storage URI.", + Command::Optimize { .. } | Command::Repair { .. } | Command::Cleanup { .. } => { + " Pass a storage URI, or --cluster --graph ." + } + _ => " Pass a storage URI.", }, - Capability::Control => "It operates on a cluster (pass --config ).", - Capability::Local => "It does not address a graph.", - Capability::Any | Capability::Served => { - unreachable!("served-addressing capabilities returned early") - } - }; - bail!( - "`{label}` is a {} command; --server/--graph address a served graph and do not apply. {how}", - capability.describe() - ); + Capability::Control => " It operates on a cluster (pass --config ).", + Capability::Local => " It does not address a graph.", + Capability::Any | Capability::Served => "", + } } #[cfg(test)] diff --git a/crates/omnigraph-cli/src/scope.rs b/crates/omnigraph-cli/src/scope.rs index 4349231..1adcc07 100644 --- a/crates/omnigraph-cli/src/scope.rs +++ b/crates/omnigraph-cli/src/scope.rs @@ -42,6 +42,7 @@ pub(crate) struct ScopeFlags<'a> { pub(crate) profile: Option<&'a str>, pub(crate) store: Option<&'a str>, pub(crate) server: Option<&'a str>, + pub(crate) cluster: Option<&'a str>, pub(crate) graph: Option<&'a str>, pub(crate) uri: Option, } @@ -56,17 +57,49 @@ pub(crate) fn resolve_scope( capability: Capability, flags: ScopeFlags<'_>, ) -> Result { - // `--store` is its own way to address a graph; combining it with a positional - // URI or `--server` is a contradiction, not a silent precedence. - if flags.store.is_some() && (flags.uri.is_some() || flags.server.is_some()) { + // At most one explicit scope primitive may address a command — a positional + // URI, `--store`, `--server`, or `--cluster` are mutually exclusive ways to + // name the graph. Combining them is a contradiction, not a silent precedence. + let primitives: Vec<&str> = [ + flags.uri.as_deref().map(|_| "a positional URI"), + flags.store.map(|_| "--store"), + flags.server.map(|_| "--server"), + flags.cluster.map(|_| "--cluster"), + ] + .into_iter() + .flatten() + .collect(); + if primitives.len() > 1 { bail!( - "--store is exclusive with a positional URI and --server — pick one way to \ - address the graph" + "{} are mutually exclusive — pick one way to address the graph", + primitives.join(" and ") ); } - // 1. Any explicit address wins; reproduce today's behavior untouched. - // `--store` is an explicit store URI — fold it into `uri`. + + // 1a. `--cluster` is the cluster scope primitive (maintenance): resolve its + // root + select the graph with `--graph`. + if let Some(cluster) = flags.cluster { + return scope_from_binding( + op, + capability, + ScopeBinding::Cluster(cluster.to_string()), + flags.graph.map(str::to_string), + "--cluster", + ); + } + + // 1b. Any other explicit address wins; reproduce today's behavior untouched. + // `--store` is an explicit store URI — fold it into `uri`. if flags.uri.is_some() || flags.server.is_some() || flags.store.is_some() { + // `--graph` selects within a multi-graph scope; a bare positional URI / + // `--store` is already a single graph, so a stray `--graph` is an error + // rather than a silently-dropped flag. + if flags.graph.is_some() && flags.server.is_none() { + bail!( + "--graph selects a graph within a server or cluster scope; a positional \ + URI / --store is already a single graph" + ); + } return Ok(ResolvedScope { server: flags.server.map(str::to_string), graph: flags.graph.map(str::to_string), @@ -128,8 +161,8 @@ fn scope_from_binding( if capability == Capability::Direct { bail!( "this command needs direct storage access, but {source} resolves a \ - server scope; name storage explicitly with --store (or a \ - --cluster/--cluster-graph for a managed graph)" + server scope; name storage explicitly with --store (or \ + --cluster --graph for a managed graph)" ); } Ok(ResolvedScope { @@ -146,21 +179,25 @@ fn scope_from_binding( direct access" ); } - // A cluster binding is a config name (resolved against `clusters:`) - // or a literal root URI. - let root = if let Some(root) = op.cluster_root(&cluster) { - root.to_string() - } else if cluster.contains("://") { - cluster - } else { + // A cluster value is a config name (resolved against `clusters:`) + // or a literal root: an `s3://`/`file://` URI or a local cluster + // directory. Only a configured name is rewritten; anything else is + // passed through to the cluster-state resolver verbatim, so a bare + // directory path keeps working as it did for per-command `--cluster`. + let root = op + .cluster_root(&cluster) + .map(str::to_string) + .unwrap_or(cluster); + // A cluster holds many graphs; maintenance addresses one at a time. + let Some(graph) = graph else { bail!( - "unknown cluster '{cluster}' ({source}); define it under `clusters:` \ - in operator config, or use a literal root URI" + "{source} resolves a cluster scope; pass --graph to select which \ + graph to maintain" ); }; Ok(ResolvedScope { cluster: Some(root), - cluster_graph: graph, + cluster_graph: Some(graph), ..Default::default() }) } @@ -192,6 +229,7 @@ mod tests { profile: None, store: None, server: None, + cluster: None, graph: None, uri: None, } @@ -230,7 +268,7 @@ mod tests { } #[test] - fn store_is_exclusive_with_positional_uri_and_server() { + fn scope_primitives_are_mutually_exclusive() { let op = OperatorConfig::default(); for flags in [ ScopeFlags { @@ -243,9 +281,93 @@ mod tests { server: Some("prod"), ..flags() }, + ScopeFlags { + cluster: Some("./brain"), + uri: Some("file://other.omni".into()), + ..flags() + }, + ScopeFlags { + cluster: Some("./brain"), + server: Some("prod"), + ..flags() + }, ] { - let err = resolve_scope(&op, Capability::Any, flags).unwrap_err().to_string(); - assert!(err.contains("--store is exclusive"), "{err}"); + let err = resolve_scope(&op, Capability::Direct, flags) + .unwrap_err() + .to_string(); + assert!(err.contains("mutually exclusive"), "{err}"); + } + } + + #[test] + fn cluster_flag_resolves_root_and_graph_for_maintenance() { + let op = cfg("clusters:\n brain:\n root: s3://acme/brain\n"); + let scope = resolve_scope( + &op, + Capability::Direct, + ScopeFlags { + cluster: Some("brain"), + graph: Some("knowledge"), + ..flags() + }, + ) + .unwrap(); + assert_eq!(scope.cluster.as_deref(), Some("s3://acme/brain")); + assert_eq!(scope.cluster_graph.as_deref(), Some("knowledge")); + } + + #[test] + fn cluster_flag_accepts_a_literal_root_uri() { + let op = OperatorConfig::default(); + let scope = resolve_scope( + &op, + Capability::Direct, + ScopeFlags { + cluster: Some("s3://bucket/clusters/brain"), + graph: Some("knowledge"), + ..flags() + }, + ) + .unwrap(); + assert_eq!(scope.cluster.as_deref(), Some("s3://bucket/clusters/brain")); + assert_eq!(scope.cluster_graph.as_deref(), Some("knowledge")); + } + + #[test] + fn cluster_scope_without_a_graph_is_a_loud_error() { + let op = cfg("clusters:\n brain:\n root: s3://acme/brain\n"); + let err = resolve_scope( + &op, + Capability::Direct, + ScopeFlags { + cluster: Some("brain"), + ..flags() + }, + ) + .unwrap_err() + .to_string(); + assert!(err.contains("--graph "), "{err}"); + } + + #[test] + fn graph_on_a_bare_store_or_uri_is_rejected() { + let op = OperatorConfig::default(); + for flags in [ + ScopeFlags { + uri: Some("graph.omni".into()), + graph: Some("knowledge"), + ..flags() + }, + ScopeFlags { + store: Some("s3://b/g.omni"), + graph: Some("knowledge"), + ..flags() + }, + ] { + let err = resolve_scope(&op, Capability::Any, flags) + .unwrap_err() + .to_string(); + assert!(err.contains("already a single graph"), "{err}"); } } @@ -294,6 +416,27 @@ mod tests { assert_eq!(scope.cluster_graph.as_deref(), Some("knowledge")); } + #[test] + fn profile_cluster_scope_with_graph_override() { + // The deferral closed by this slice: a `--graph` flag overrides a + // profile cluster's default_graph, exactly as it does for a server scope. + let op = cfg( + "clusters:\n brain:\n root: s3://acme/brain\nprofiles:\n admin:\n cluster: brain\n default_graph: knowledge\n", + ); + let scope = resolve_scope( + &op, + Capability::Direct, + ScopeFlags { + profile: Some("admin"), + graph: Some("archive"), + ..flags() + }, + ) + .unwrap(); + assert_eq!(scope.cluster.as_deref(), Some("s3://acme/brain")); + assert_eq!(scope.cluster_graph.as_deref(), Some("archive")); // flag beats profile default + } + #[test] fn server_scope_on_maintenance_verb_errors() { let op = cfg("defaults:\n server: prod\nservers:\n prod:\n url: https://x\n"); diff --git a/crates/omnigraph-cli/tests/cli_cluster.rs b/crates/omnigraph-cli/tests/cli_cluster.rs index 9205b84..29a08f8 100644 --- a/crates/omnigraph-cli/tests/cli_cluster.rs +++ b/crates/omnigraph-cli/tests/cli_cluster.rs @@ -975,7 +975,7 @@ fn optimize_resolves_a_cluster_graph_by_id() { .arg("optimize") .arg("--cluster") .arg(temp.path()) - .arg("--cluster-graph") + .arg("--graph") .arg("knowledge") .arg("--json"), ); @@ -994,7 +994,7 @@ fn optimize_unknown_cluster_graph_id_errors() { .arg("optimize") .arg("--cluster") .arg(temp.path()) - .arg("--cluster-graph") + .arg("--graph") .arg("does-not-exist") .arg("--json"), ); @@ -1006,8 +1006,10 @@ fn optimize_unknown_cluster_graph_id_errors() { } #[test] -fn cluster_flag_requires_cluster_graph() { - // clap enforces both-or-neither. +fn cluster_without_graph_demands_a_graph_selector() { + // A cluster holds many graphs; `--cluster` alone can't pick one. The scope + // resolver demands `--graph ` (replacing the old `--cluster-graph` + // requirement) before it ever touches cluster state. let out = output_failure( cli() .arg("optimize") @@ -1017,8 +1019,8 @@ fn cluster_flag_requires_cluster_graph() { ); let stderr = String::from_utf8_lossy(&out.stderr); assert!( - stderr.contains("cluster-graph") || stderr.contains("required"), - "expected --cluster to require --cluster-graph; got: {stderr}" + stderr.contains("--graph "), + "expected --cluster to demand --graph; got: {stderr}" ); } @@ -1076,7 +1078,7 @@ fn optimize_by_cluster_works_when_catalog_payloads_are_degraded() { .arg("optimize") .arg("--cluster") .arg(temp.path()) - .arg("--cluster-graph") + .arg("--graph") .arg("knowledge") .arg("--json"), ); diff --git a/crates/omnigraph-cli/tests/cli_data.rs b/crates/omnigraph-cli/tests/cli_data.rs index fc5db0a..8896912 100644 --- a/crates/omnigraph-cli/tests/cli_data.rs +++ b/crates/omnigraph-cli/tests/cli_data.rs @@ -165,12 +165,63 @@ fn optimize_with_server_flag_errors_wrong_plane() { let stderr = String::from_utf8_lossy(&output.stderr); assert!( stderr.contains("`optimize` is a direct (storage-native) command") - && stderr.contains("--server/--graph address a served graph and do not apply") - && stderr.contains("Pass a storage URI, or --cluster --cluster-graph ."), + && stderr.contains("--server addresses a served graph and does not apply") + && stderr.contains("Pass a storage URI, or --cluster --graph ."), "wrong-capability guard message not found; got: {stderr}" ); } +#[test] +fn wrong_address_guard_message_has_no_trailing_space() { + // The remediation tail is empty for served-addressing capabilities, so a + // misplaced --cluster on a data verb must not leave "… does not apply. " + // with a dangling space (error text is observable contract). NO_COLOR keeps + // the assertion off ANSI styling. + let output = output_failure( + cli() + .env("NO_COLOR", "1") + .arg("query") + .arg("--cluster") + .arg("./brain") + .arg("-e") + .arg("query q { Person { id } }"), + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("and does not apply."), + "expected the wrong-address message; got: {stderr}" + ); + assert!( + !stderr.contains("and does not apply. "), + "trailing space after the message; got: {stderr}" + ); +} + +#[test] +fn graph_flag_on_a_positional_uri_errors() { + // RFC-011: `--graph` selects within a multi-graph scope (a server or + // cluster). A bare positional URI is already a single graph, so pairing it + // with `--graph` is a loud error, not a silently-dropped flag. (The guard + // lets `--graph` reach a data verb; the scope resolver is what rejects it.) + let temp = tempdir().unwrap(); + let graph = graph_path(temp.path()); + init_graph(&graph); + let output = output_failure( + cli() + .arg("query") + .arg(&graph) + .arg("--graph") + .arg("knowledge") + .arg("-e") + .arg("query q { Person { id } }"), + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("already a single graph"), + "expected --graph-on-positional-URI rejection; got: {stderr}" + ); +} + #[test] fn optimize_with_remote_target_errors_storage_plane() { // RFC-010 Slice 1: a maintenance verb pointed at a remote URI fails loudly diff --git a/crates/omnigraph-cli/tests/cli_schema_config.rs b/crates/omnigraph-cli/tests/cli_schema_config.rs index 0b6eca9..8c5b1f3 100644 --- a/crates/omnigraph-cli/tests/cli_schema_config.rs +++ b/crates/omnigraph-cli/tests/cli_schema_config.rs @@ -121,7 +121,7 @@ fn schema_plan_with_server_flag_errors_wrong_plane() { let stderr = String::from_utf8_lossy(&output.stderr); assert!( stderr.contains("`schema plan` is a direct (storage-native) command") - && stderr.contains("Pass a storage URI, or --cluster --cluster-graph ."), + && stderr.contains("Pass a storage URI."), "schema plan wrong-capability message not found; got: {stderr}" ); } diff --git a/docs/user/cli/reference.md b/docs/user/cli/reference.md index 3da502a..85b3435 100644 --- a/docs/user/cli/reference.md +++ b/docs/user/cli/reference.md @@ -34,18 +34,18 @@ Every command declares the **capability** it needs — what it requires to reach - **`any`** — `query`, `mutate`, `load`, `ingest`, `branch *`, `snapshot`, `export`, `commit *`, `schema show`, `schema apply`. Run against a graph **served (via a server) or embedded (direct against a store)**: accept a positional `file://`/`s3://` URI, `--server ` (+ `--graph ` for multi-graph servers), `--store `, or `--profile `. A remote server is addressed with `--server` — a positional `http(s)://` URI does **not** dispatch to one. - **`served`** — `graphs list`. Requires a server (accepts `--server` / `--profile`). -- **`direct`** — `init`, `optimize`, `repair`, `cleanup`, `schema plan`, `queries validate`, `lint`. Need **direct storage access** (`file://` / `s3://`), never through a server. They accept a positional `URI`, but **not** `--server` / `--graph`, and a remote (`http(s)://`) URI is rejected. `optimize` / `repair` / `cleanup` also accept **`--cluster --cluster-graph `**, which resolves the graph's storage URI from the served cluster state (so you needn't know the `/graphs/.omni` layout). +- **`direct`** — `init`, `optimize`, `repair`, `cleanup`, `schema plan`, `queries validate`, `lint`. Need **direct storage access** (`file://` / `s3://`), never through a server. They accept a positional `URI`, but **not** `--server`, and a remote (`http(s)://`) URI is rejected. `optimize` / `repair` / `cleanup` additionally accept **`--cluster --graph `** (`--cluster` is a cluster directory or storage-root URI, named via `clusters:` in `~/.omnigraph/config.yaml` or a literal root), which resolves the graph's storage URI from the served cluster state (so you needn't know the `/graphs/.omni` layout). `--graph` is the one graph selector across all scopes — on these three verbs it picks the cluster graph; on the other `direct` verbs it does not apply. - **`control`** — `cluster *`. Operates on a cluster directory via `--config `. - **`local`** — `policy *`, `embed`, `login`, `logout`, `config`, `version`, `queries list`. Address no graph. These restrictions are enforced and reported, not silent: -- A served-graph flag (`--server` / `--graph`) on a verb that doesn't reach a graph through a server fails loudly, e.g.: ``optimize is a direct (storage-native) command; --server/--graph address a served graph and do not apply. Pass a storage URI, or --cluster --cluster-graph .`` +- A scope flag on a verb that can't consume it fails loudly rather than being silently dropped — `--server` outside a served scope, `--cluster` outside the maintenance verbs, or `--graph` where no multi-graph scope applies, e.g.: ``optimize is a direct (storage-native) command; --server addresses a served graph and does not apply. Pass a storage URI, or --cluster --graph .`` - A `direct` verb pointed at a remote URI fails loudly, e.g.: ``optimize is a direct (storage-native) command and needs direct storage access; the resolved target is a remote server (https://…). Pass the graph's file:// or s3:// URI.`` - A data verb pointed at a positional `http(s)://` URI fails loudly: ``a remote graph must be addressed with --server — a positional (or --uri) http(s):// URL no longer dispatches to a server.`` - `init` into an **established cluster's** storage layout (`/graphs/.omni` where `` holds `__cluster/state.json`) is refused — graphs in a cluster are created by `cluster apply` (which records ledger / recovery / approvals), not `init`. -To maintain a server-backed graph, run the `direct` verbs from a host with storage access against the graph's storage URI (a positional URI, or `--cluster … --cluster-graph …`), out-of-band from the serving process — there are no server routes for `optimize` / `repair` / `cleanup` by design. +To maintain a server-backed graph, run the `direct` verbs from a host with storage access against the graph's storage URI (a positional URI, or `--cluster … --graph …`), out-of-band from the serving process — there are no server routes for `optimize` / `repair` / `cleanup` by design. `omnigraph --help` lists commands with a **capability legend** at the bottom (any / served / direct / control / local). @@ -102,13 +102,14 @@ resolves its scope fresh, there is no sticky "current" mode. - `--store ` addresses a single graph's storage directly (ad-hoc / break-glass). - A `cluster`-bound profile reaches `optimize` / `repair` / `cleanup` for a managed graph (resolving its storage root from `clusters:`), the same as - `--cluster --cluster-graph `. + `--cluster --graph `. A `--graph` flag overrides the profile's default. - A `server`-bound scope on a maintenance verb, or a `cluster`-bound scope on a data verb, is rejected with a message pointing at the right addressing. -`--target` and the positional-`http(s)://`→remote dispatch have been **removed**; -the remaining legacy surfaces (`--cluster-graph`, `omnigraph.yaml`'s `cli.graph` -default) still work and an explicit address always wins. +`--target`, `--cluster-graph`, and the positional-`http(s)://`→remote dispatch +have been **removed** (`--graph` is now the one graph selector across server and +cluster scopes); `omnigraph.yaml`'s `cli.graph` default still works and an +explicit address always wins. #### Credentials keyed by server name diff --git a/docs/user/clusters/index.md b/docs/user/clusters/index.md index 053d5a1..bbaf033 100644 --- a/docs/user/clusters/index.md +++ b/docs/user/clusters/index.md @@ -258,10 +258,11 @@ operation — it runs out-of-band, with direct storage access, against the graph roots. Address a cluster graph by name instead of hand-typing its storage path: ```bash -omnigraph optimize --cluster ./company-brain --cluster-graph knowledge -omnigraph cleanup --cluster ./company-brain --cluster-graph knowledge --keep 10 --confirm -# --cluster also takes the storage-root URI directly (config-free): -omnigraph optimize --cluster s3://bucket/clusters/company-brain --cluster-graph knowledge +omnigraph optimize --cluster ./company-brain --graph knowledge +omnigraph cleanup --cluster ./company-brain --graph knowledge --keep 10 --confirm +# --cluster also takes the storage-root URI directly (config-free), and a +# `clusters:` name from ~/.omnigraph/config.yaml: +omnigraph optimize --cluster s3://bucket/clusters/company-brain --graph knowledge ``` The graph's storage URI is resolved from the **served cluster state** (the same diff --git a/docs/user/operations/maintenance.md b/docs/user/operations/maintenance.md index bf7a81c..668d41c 100644 --- a/docs/user/operations/maintenance.md +++ b/docs/user/operations/maintenance.md @@ -1,6 +1,6 @@ # Maintenance: Optimize, Repair & Cleanup -**Addressing.** `optimize`, `repair`, and `cleanup` are **direct** (storage-native) CLI commands: they run with direct storage access against a positional `file://`/`s3://` URI or **`--cluster --cluster-graph `** (which resolves the graph's storage URI from the served cluster state, so you needn't know the `/graphs/.omni` layout). They never run through a server, and reject `--server` / `--graph` or a remote (`http(s)://`) URI with a declared error. There are no server routes for them by design — to maintain a server-backed graph, run them out-of-band against the graph's storage URI. See the *Command capabilities* section of [cli-reference.md](../cli/reference.md). +**Addressing.** `optimize`, `repair`, and `cleanup` are **direct** (storage-native) CLI commands: they run with direct storage access against a positional `file://`/`s3://` URI or **`--cluster --graph `** (which resolves the graph's storage URI from the served cluster state, so you needn't know the `/graphs/.omni` layout). They never run through a server, and reject `--server` or a remote (`http(s)://`) URI with a declared error. There are no server routes for them by design — to maintain a server-backed graph, run them out-of-band against the graph's storage URI. See the *Command capabilities* section of [cli-reference.md](../cli/reference.md). ## `optimize` — non-destructive