From be4bd46212de836668c545c16f0d2ce2bbd079da Mon Sep 17 00:00:00 2001 From: aaltshuler Date: Thu, 11 Jun 2026 20:29:02 +0300 Subject: [PATCH 1/2] =?UTF-8?q?feat(cli):=20the=20operator=20config=20surf?= =?UTF-8?q?ace=20=E2=80=94=20identity=20and=20output=20defaults=20(RFC-007?= =?UTF-8?q?=20PR=201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ~/.omnigraph/config.yaml joins the resolution chains as the operator surface: operator.actor becomes the last hop of THE actor chain (--as > legacy cli.actor during the RFC-008 window > operator.actor > none, one implementation for direct-engine and cluster commands alike) and defaults.output joins the read-format cascade below every more-specific source. Discovery honors $OMNIGRAPH_HOME (tilde-expanded, #139 finding 9); an absent file is an empty layer; unknown keys WARN and load (a file written for later slices must not break this CLI); malformed YAML is a loud error. The module is CLI-only — the server never reads operator config (invariant 11 by construction). $OMNIGRAPH_CONFIG becomes a first-class stand-in for --config in load_config (flag > env > ./omnigraph.yaml), one meaning in both binaries. The test harness pins hermeticity: spawned binaries get a nonexistent OMNIGRAPH_HOME by default so no test ever reads the developer's real operator config. New coverage: loader unit tests, the env-precedence matrix on load_config_in, and spawned-binary e2es for the actor chain (operator wins with no flag/legacy key; legacy outranks it; --as wins) and the format cascade. Co-Authored-By: Claude Fable 5 --- crates/omnigraph-cli/src/helpers.rs | 52 +++++- crates/omnigraph-cli/src/main.rs | 19 +- crates/omnigraph-cli/src/operator.rs | 212 ++++++++++++++++++++++ crates/omnigraph-cli/tests/cli_cluster.rs | 67 +++++++ crates/omnigraph-cli/tests/cli_data.rs | 46 +++++ crates/omnigraph-cli/tests/support/mod.rs | 16 +- crates/omnigraph-server/src/config.rs | 69 +++++-- 7 files changed, 445 insertions(+), 36 deletions(-) create mode 100644 crates/omnigraph-cli/src/operator.rs diff --git a/crates/omnigraph-cli/src/helpers.rs b/crates/omnigraph-cli/src/helpers.rs index be356a9..85eb42a 100644 --- a/crates/omnigraph-cli/src/helpers.rs +++ b/crates/omnigraph-cli/src/helpers.rs @@ -3,6 +3,7 @@ //! main.rs in the modularization). use super::*; +use crate::operator; pub(crate) fn ensure_local_graph_parent(uri: &str) -> Result<()> { if !uri.contains("://") { @@ -167,18 +168,40 @@ pub(crate) async fn open_local_db_with_policy(graph: &ResolvedCliGraph) -> Resul } } +/// THE actor chain (RFC-007 §D3) — every command that needs an identity +/// resolves through this one function (one path per concern): +/// `--as` > legacy `cli.actor` in omnigraph.yaml (RFC-008 window) > +/// `operator.actor` in ~/.omnigraph/config.yaml > none. +pub(crate) fn resolve_actor( + cli_as: Option<&str>, + legacy_config_actor: Option<&str>, +) -> Result> { + if let Some(actor) = cli_as { + return Ok(Some(actor.to_string())); + } + if let Some(actor) = legacy_config_actor { + return Ok(Some(actor.to_string())); + } + Ok(operator::load_operator_config()? + .actor() + .map(str::to_string)) +} + pub(crate) fn resolve_cluster_actor(cli_as: Option<&str>) -> Result> { if let Some(actor) = cli_as { return Ok(Some(actor.to_string())); } let config = load_config(None).wrap_err( - "resolving the default actor from the per-operator omnigraph.yaml (pass --as to skip this lookup)", + "resolving the default actor from omnigraph.yaml (pass --as to skip this lookup)", )?; - Ok(config.cli.actor.clone()) + resolve_actor(None, config.cli.actor.as_deref()) } -pub(crate) fn resolve_cli_actor<'a>(cli_as: Option<&'a str>, config: &'a OmnigraphConfig) -> Option<&'a str> { - cli_as.or(config.cli.actor.as_deref()) +pub(crate) fn resolve_cli_actor( + cli_as: Option<&str>, + config: &OmnigraphConfig, +) -> Result> { + resolve_actor(cli_as, config.cli.actor.as_deref()) } pub(crate) fn resolve_policy_tests_path(context: &ResolvedPolicyContext) -> PathBuf { @@ -460,6 +483,9 @@ pub(crate) fn merged_params_json( } } +/// The format cascade (RFC-007 §D3): `--json` > `--format` > alias format > +/// legacy `cli.output_format` (RFC-008 window) > operator `defaults.output` +/// > table. pub(crate) fn resolve_read_format( config: &OmnigraphConfig, cli_format: Option, @@ -467,12 +493,17 @@ pub(crate) fn resolve_read_format( alias_format: Option, ) -> ReadOutputFormat { if json { - ReadOutputFormat::Json - } else { - cli_format - .or(alias_format) - .unwrap_or_else(|| config.cli_output_format()) + return ReadOutputFormat::Json; } + cli_format + .or(alias_format) + .or(config.cli.output_format) + .or_else(|| { + operator::load_operator_config() + .ok() + .and_then(|operator| operator.output()) + }) + .unwrap_or_default() } pub(crate) fn resolve_alias<'a>( @@ -935,7 +966,8 @@ pub(crate) async fn execute_change( let (selected_name, query_params) = select_named_query(query_source, query_name)?; let params = query_params_from_json(&query_params, params_json)?; let db = open_local_db_with_policy(graph).await?; - let actor = resolve_cli_actor(cli_as_actor, config); + let actor = resolve_cli_actor(cli_as_actor, config)?; + let actor = actor.as_deref(); let result = db .mutate_as(branch, query_source, &selected_name, ¶ms, actor) .await?; diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index 0d6ce03..bef111f 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -42,6 +42,7 @@ use serde::de::DeserializeOwned; use serde_json::Value; mod embed; +mod operator; mod read_format; use embed::{EmbedArgs, EmbedOutput, execute_embed}; @@ -129,7 +130,8 @@ async fn main() -> Result<()> { load_output_from_tables(&uri, &branch, mode, &output) } else { let db = open_local_db_with_policy(&graph).await?; - let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config); + let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config)?; + let actor = actor.as_deref(); let result = db .load_file_as( &branch, @@ -196,7 +198,8 @@ async fn main() -> Result<()> { .await? } else { let db = open_local_db_with_policy(&graph).await?; - let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config); + let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config)?; + let actor = actor.as_deref(); let result = db .load_file_as( &branch, @@ -243,7 +246,8 @@ async fn main() -> Result<()> { .await? } else { let db = open_local_db_with_policy(&graph).await?; - let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config); + let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config)?; + let actor = actor.as_deref(); db.branch_create_from_as(ReadTarget::branch(&from), &name, actor) .await?; BranchCreateOutput { @@ -316,7 +320,8 @@ async fn main() -> Result<()> { .await? } else { let db = open_local_db_with_policy(&graph).await?; - let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config); + let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config)?; + let actor = actor.as_deref(); db.branch_delete_as(&name, actor).await?; BranchDeleteOutput { uri: uri.clone(), @@ -358,7 +363,8 @@ async fn main() -> Result<()> { .await? } else { let db = open_local_db_with_policy(&graph).await?; - let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config); + let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config)?; + let actor = actor.as_deref(); let outcome = db.branch_merge_as(&source, &into, actor).await?; BranchMergeOutput { source: source.clone(), @@ -514,7 +520,8 @@ async fn main() -> Result<()> { .await? } else { let db = open_local_db_with_policy(&graph).await?; - let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config); + let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config)?; + let actor = actor.as_deref(); let registry = load_registry_or_report(&config, graph.selected())?; let registry = (!registry.is_empty()).then_some(registry); let label = graph.selected().unwrap_or(&uri).to_string(); diff --git a/crates/omnigraph-cli/src/operator.rs b/crates/omnigraph-cli/src/operator.rs new file mode 100644 index 0000000..bac37b3 --- /dev/null +++ b/crates/omnigraph-cli/src/operator.rs @@ -0,0 +1,212 @@ +//! The operator config surface (RFC-007): `~/.omnigraph/config.yaml` — who +//! the operator IS (identity, ergonomics), never what the system is (that's +//! cluster config) and never a project file (nothing here arrives with a +//! repo checkout). +//! +//! PR-1 scope: `operator.actor` + `defaults.output`. Unknown keys WARN and +//! are preserved-by-ignoring — a file written for a newer CLI (servers, +//! aliases, credentials keys from later slices) must load cleanly on this +//! one. Contrast with `cluster.yaml`, where unknown keys are fatal because +//! they change what a plan means. +//! +//! This module is CLI-only by design: the server never reads operator +//! config (server-side identity comes from bearer auth — invariant 11 +//! holds by construction). + +use std::env; +use std::path::{Path, PathBuf}; + +use color_eyre::Result; +use color_eyre::eyre::eyre; +use serde::Deserialize; + +use omnigraph_server::config::ReadOutputFormat; + +pub(crate) const OPERATOR_HOME_ENV: &str = "OMNIGRAPH_HOME"; +pub(crate) const OPERATOR_DIR: &str = ".omnigraph"; +pub(crate) const OPERATOR_CONFIG_FILE: &str = "config.yaml"; + +#[derive(Debug, Default, Deserialize)] +pub(crate) struct OperatorConfig { + #[serde(default)] + pub(crate) operator: OperatorIdentity, + #[serde(default)] + pub(crate) defaults: OperatorDefaults, + /// Everything this CLI version doesn't know. Warned once at load, + /// otherwise ignored (forward compatibility within the operator layer). + #[serde(flatten)] + unknown: serde_yaml::Mapping, +} + +#[derive(Debug, Default, Deserialize)] +pub(crate) struct OperatorIdentity { + /// Default actor for every `--as` cascade (CLI direct-engine writes and + /// cluster commands alike): `--as` > legacy config actor (RFC-008 + /// window) > this > none. + pub(crate) actor: Option, + #[serde(flatten)] + unknown: serde_yaml::Mapping, +} + +#[derive(Debug, Default, Deserialize)] +pub(crate) struct OperatorDefaults { + /// Default read output format, below every more-specific source. + pub(crate) output: Option, + #[serde(flatten)] + unknown: serde_yaml::Mapping, +} + +impl OperatorConfig { + pub(crate) fn actor(&self) -> Option<&str> { + self.operator.actor.as_deref() + } + + pub(crate) fn output(&self) -> Option { + self.defaults.output + } +} + +/// The operator dir: `$OMNIGRAPH_HOME` if set (tilde-expanded), else +/// `~/.omnigraph`. Returns None when no home directory is resolvable +/// (degenerate environments — the layer is simply absent). +pub(crate) fn operator_dir() -> Option { + if let Some(home_override) = env::var_os(OPERATOR_HOME_ENV) { + let raw = home_override.to_string_lossy().into_owned(); + return Some(expand_tilde(&raw)); + } + env::home_dir().map(|home| home.join(OPERATOR_DIR)) +} + +/// Load the operator layer. Absent file (or unresolvable home) is an empty +/// layer, never an error; a present-but-malformed file is a loud error (the +/// operator owns it and can fix it); unknown keys warn to stderr once. +pub(crate) fn load_operator_config() -> Result { + let Some(dir) = operator_dir() else { + return Ok(OperatorConfig::default()); + }; + load_operator_config_at(&dir.join(OPERATOR_CONFIG_FILE)) +} + +pub(crate) fn load_operator_config_at(path: &Path) -> Result { + let text = match std::fs::read_to_string(path) { + Ok(text) => text, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + return Ok(OperatorConfig::default()); + } + Err(err) => { + return Err(eyre!( + "could not read operator config '{}': {err}", + path.display() + )); + } + }; + let config: OperatorConfig = serde_yaml::from_str(&text).map_err(|err| { + eyre!( + "could not parse operator config '{}': {err}", + path.display() + ) + })?; + for warning in config.unknown_key_warnings() { + eprintln!("warning: {warning} in operator config '{}'", path.display()); + } + Ok(config) +} + +impl OperatorConfig { + fn unknown_key_warnings(&self) -> Vec { + let mut warnings = Vec::new(); + let mut collect = |mapping: &serde_yaml::Mapping, prefix: &str| { + for key in mapping.keys() { + if let Some(name) = key.as_str() { + warnings.push(format!( + "unknown key `{prefix}{name}` (newer CLI feature or typo); ignored" + )); + } + } + }; + collect(&self.unknown, ""); + collect(&self.operator.unknown, "operator."); + collect(&self.defaults.unknown, "defaults."); + warnings + } +} + +/// Expand a leading `~` / `~/` to the home directory (PR #139 finding 9: +/// a literal `./~/…` path silently created a directory named `~`). +pub(crate) fn expand_tilde(raw: &str) -> PathBuf { + if raw == "~" { + return env::home_dir().unwrap_or_else(|| PathBuf::from(raw)); + } + if let Some(rest) = raw.strip_prefix("~/") { + if let Some(home) = env::home_dir() { + return home.join(rest); + } + } + PathBuf::from(raw) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + #[test] + fn absent_file_is_an_empty_layer() { + let dir = tempfile::tempdir().unwrap(); + let config = load_operator_config_at(&dir.path().join("config.yaml")).unwrap(); + assert!(config.actor().is_none()); + assert!(config.output().is_none()); + } + + #[test] + fn parses_identity_and_defaults() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.yaml"); + fs::write( + &path, + "operator:\n actor: act-andrew\ndefaults:\n output: json\n", + ) + .unwrap(); + let config = load_operator_config_at(&path).unwrap(); + assert_eq!(config.actor(), Some("act-andrew")); + assert_eq!(config.output(), Some(ReadOutputFormat::Json)); + } + + #[test] + fn unknown_keys_warn_but_load() { + // A file written for a later slice (servers/aliases) must load + // cleanly today — warn-only forward compatibility. + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.yaml"); + fs::write( + &path, + "operator:\n actor: act-a\n color: green\nservers:\n prod:\n url: https://example.com\naliases: {}\n", + ) + .unwrap(); + let config = load_operator_config_at(&path).unwrap(); + assert_eq!(config.actor(), Some("act-a")); + let warnings = config.unknown_key_warnings(); + assert_eq!(warnings.len(), 3, "{warnings:?}"); + assert!(warnings.iter().any(|w| w.contains("`servers`"))); + assert!(warnings.iter().any(|w| w.contains("`aliases`"))); + assert!(warnings.iter().any(|w| w.contains("`operator.color`"))); + } + + #[test] + fn malformed_yaml_is_a_loud_error() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.yaml"); + fs::write(&path, "operator: [not, a, mapping\n").unwrap(); + let err = load_operator_config_at(&path).unwrap_err(); + assert!(err.to_string().contains("could not parse operator config")); + } + + #[test] + fn expand_tilde_resolves_home_prefix() { + let home = env::home_dir().unwrap(); + assert_eq!(expand_tilde("~"), home); + assert_eq!(expand_tilde("~/x/y"), home.join("x/y")); + assert_eq!(expand_tilde("/abs/path"), PathBuf::from("/abs/path")); + assert_eq!(expand_tilde("rel/path"), PathBuf::from("rel/path")); + } +} diff --git a/crates/omnigraph-cli/tests/cli_cluster.rs b/crates/omnigraph-cli/tests/cli_cluster.rs index be7675a..bfadf40 100644 --- a/crates/omnigraph-cli/tests/cli_cluster.rs +++ b/crates/omnigraph-cli/tests/cli_cluster.rs @@ -726,6 +726,73 @@ fn cluster_apply_uses_cli_actor_from_local_config() { assert_eq!(apply(&["--as", "andrew"]), "andrew", "--as overrides cli.actor"); } +/// RFC-007 PR 1: the operator layer joins the actor chain — +/// `--as` > legacy `cli.actor` (RFC-008 window) > `operator.actor` > none. +#[test] +fn cluster_apply_uses_operator_actor_from_omnigraph_home() { + let temp = tempdir().unwrap(); + write_cluster_config_fixture(temp.path()); + let operator_home = tempdir().unwrap(); + fs::write( + operator_home.path().join("config.yaml"), + "operator:\n actor: act-operator\n", + ) + .unwrap(); + + let output = cli() + .current_dir(temp.path()) + .env("OMNIGRAPH_HOME", operator_home.path()) + .arg("cluster") + .arg("import") + .arg("--config") + .arg(temp.path()) + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + + let apply = |extra: &[&str]| { + let mut command = cli(); + command + .current_dir(temp.path()) + .env("OMNIGRAPH_HOME", operator_home.path()); + for arg in extra { + command.arg(arg); + } + let output = command + .arg("cluster") + .arg("apply") + .arg("--config") + .arg(temp.path()) + .arg("--json") + .output() + .unwrap(); + let json: serde_json::Value = + serde_json::from_str(String::from_utf8_lossy(&output.stdout).trim()).unwrap(); + json["actor"].clone() + }; + + // No --as, no omnigraph.yaml: the operator identity applies. + assert_eq!( + apply(&[]), + "act-operator", + "operator.actor is the no-flag, no-legacy-config default" + ); + // --as still wins over everything. + assert_eq!(apply(&["--as", "andrew"]), "andrew"); + + // A legacy cli.actor (RFC-008 window) outranks the operator layer. + fs::write( + temp.path().join("omnigraph.yaml"), + "cli:\n actor: act-legacy\n", + ) + .unwrap(); + assert_eq!( + apply(&[]), + "act-legacy", + "legacy cli.actor wins over operator.actor during the deprecation window" + ); +} + #[test] fn cluster_approve_uses_cli_actor_fallback() { let temp = tempdir().unwrap(); diff --git a/crates/omnigraph-cli/tests/cli_data.rs b/crates/omnigraph-cli/tests/cli_data.rs index 841bedf..203a7c2 100644 --- a/crates/omnigraph-cli/tests/cli_data.rs +++ b/crates/omnigraph-cli/tests/cli_data.rs @@ -984,6 +984,52 @@ fn read_csv_format_outputs_header_and_row_values() { assert!(stdout.contains("Alice")); } +/// RFC-007 PR 1: the format cascade's operator hop — `defaults.output` in +/// ~/.omnigraph/config.yaml applies when nothing more specific is given, +/// and `--format` still wins over it. +#[test] +fn read_uses_operator_default_output_format() { + let temp = tempdir().unwrap(); + let graph = graph_path(temp.path()); + init_graph(&graph); + load_fixture(&graph); + let operator_home = tempdir().unwrap(); + fs::write( + operator_home.path().join("config.yaml"), + "defaults:\n output: csv\n", + ) + .unwrap(); + + let read = |extra: &[&str]| { + let mut command = cli(); + command + .env("OMNIGRAPH_HOME", operator_home.path()) + .arg("read") + .arg(&graph) + .arg("--query") + .arg(fixture("test.gq")) + .arg("--name") + .arg("get_person") + .arg("--params") + .arg(r#"{"name":"Alice"}"#); + for arg in extra { + command.arg(arg); + } + stdout_string(&output_success(&mut command)) + }; + + let stdout = read(&[]); + assert!( + stdout.lines().next().unwrap().contains("p.name") && stdout.contains("Alice"), + "operator defaults.output: csv applies with no --format: {stdout}" + ); + let stdout = read(&["--format", "jsonl"]); + assert!( + stdout.starts_with('{'), + "--format wins over the operator default: {stdout}" + ); +} + #[test] fn read_jsonl_format_outputs_metadata_header_first() { let temp = tempdir().unwrap(); diff --git a/crates/omnigraph-cli/tests/support/mod.rs b/crates/omnigraph-cli/tests/support/mod.rs index 586bf93..41e46c7 100644 --- a/crates/omnigraph-cli/tests/support/mod.rs +++ b/crates/omnigraph-cli/tests/support/mod.rs @@ -12,12 +12,24 @@ use reqwest::blocking::Client; use serde_json::Value; use tempfile::{TempDir, tempdir}; +/// Hermetic default: point OMNIGRAPH_HOME at a path that exists on no +/// machine, so spawned binaries never read the developer's real +/// ~/.omnigraph/ (an absent operator config is an empty layer). Tests +/// exercising the operator layer override the var explicitly. +pub const HERMETIC_OPERATOR_HOME: &str = "/nonexistent/omnigraph-test-home"; + pub fn cli() -> Command { - Command::cargo_bin("omnigraph").unwrap() + let mut command = Command::cargo_bin("omnigraph").unwrap(); + command.env("OMNIGRAPH_HOME", HERMETIC_OPERATOR_HOME); + command.env_remove("OMNIGRAPH_CONFIG"); + command } pub fn cli_process() -> StdCommand { - StdCommand::new(assert_cmd::cargo::cargo_bin("omnigraph")) + let mut command = StdCommand::new(assert_cmd::cargo::cargo_bin("omnigraph")); + command.env("OMNIGRAPH_HOME", HERMETIC_OPERATOR_HOME); + command.env_remove("OMNIGRAPH_CONFIG"); + command } fn server_process() -> StdCommand { diff --git a/crates/omnigraph-server/src/config.rs b/crates/omnigraph-server/src/config.rs index b308b72..52bac2e 100644 --- a/crates/omnigraph-server/src/config.rs +++ b/crates/omnigraph-server/src/config.rs @@ -526,12 +526,23 @@ pub fn default_config_path() -> PathBuf { PathBuf::from(DEFAULT_CONFIG_FILE) } +/// `OMNIGRAPH_CONFIG` env var: a first-class stand-in for `--config`, one +/// name with one meaning in both binaries (the container entrypoint already +/// uses it for the server; RFC-007 §D1 extends it to the CLI). +pub const CONFIG_PATH_ENV: &str = "OMNIGRAPH_CONFIG"; + pub fn load_config(config_path: Option<&PathBuf>) -> Result { - load_config_in(&env::current_dir()?, config_path) + let env_path = env::var_os(CONFIG_PATH_ENV).map(PathBuf::from); + load_config_in(&env::current_dir()?, config_path, env_path.as_ref()) } -fn load_config_in(cwd: &Path, config_path: Option<&PathBuf>) -> Result { - let explicit_path = config_path.cloned(); +fn load_config_in( + cwd: &Path, + config_path: Option<&PathBuf>, + env_path: Option<&PathBuf>, +) -> Result { + // Precedence: explicit --config flag > $OMNIGRAPH_CONFIG > ./omnigraph.yaml. + let explicit_path = config_path.or(env_path).cloned(); let config_path = explicit_path.or_else(|| { let default_path = cwd.join(DEFAULT_CONFIG_FILE); default_path.exists().then_some(default_path) @@ -575,6 +586,28 @@ mod tests { ReadOutputFormat, TableCellLayout, graph_resource_id_for_selection, load_config_in, }; + #[test] + fn env_config_path_stands_in_for_the_flag_but_loses_to_it() { + let temp = tempdir().unwrap(); + let flag_path = temp.path().join("flag.yaml"); + let env_path = temp.path().join("env.yaml"); + fs::write(&flag_path, "cli:\n actor: act-flag\n").unwrap(); + fs::write(&env_path, "cli:\n actor: act-env\n").unwrap(); + + // $OMNIGRAPH_CONFIG used when no flag… + let config = load_config_in(temp.path(), None, Some(&env_path)).unwrap(); + assert_eq!(config.cli.actor.as_deref(), Some("act-env")); + + // …loses to an explicit --config… + let config = load_config_in(temp.path(), Some(&flag_path), Some(&env_path)).unwrap(); + assert_eq!(config.cli.actor.as_deref(), Some("act-flag")); + + // …and beats the cwd default file. + fs::write(temp.path().join("omnigraph.yaml"), "cli:\n actor: act-cwd\n").unwrap(); + let config = load_config_in(temp.path(), None, Some(&env_path)).unwrap(); + assert_eq!(config.cli.actor.as_deref(), Some("act-env")); + } + #[test] fn load_config_reads_yaml_defaults_from_current_dir() { let temp = tempdir().unwrap(); @@ -598,7 +631,7 @@ policy: {} ) .unwrap(); - let config = load_config_in(temp.path(), None).unwrap(); + let config = load_config_in(temp.path(), None, None).unwrap(); assert_eq!(config.cli_graph_name(), Some("local")); assert_eq!(config.cli_branch(), "main"); assert_eq!(config.cli_output_format(), ReadOutputFormat::Kv); @@ -633,7 +666,7 @@ policy: {} ) .unwrap(); - let config = load_config_in(&child, None).unwrap(); + let config = load_config_in(&child, None, None).unwrap(); assert!(config.graphs.is_empty()); } @@ -657,7 +690,7 @@ policy: {} "graphs:\n local:\n uri: ./demo.omni\n", ) .unwrap(); - let config = load_config_in(temp.path(), None).unwrap(); + let config = load_config_in(temp.path(), None, None).unwrap(); // A known graph passes through unchanged. assert_eq!(config.resolve_graph_selection(Some("local")).unwrap(), Some("local")); @@ -680,7 +713,7 @@ policy: {} "graphs:\n local:\n uri: ./demo.omni\npolicy:\n file: ./top.yaml\n", ) .unwrap(); - let incoherent = load_config_in(temp2.path(), None).unwrap(); + let incoherent = load_config_in(temp2.path(), None, None).unwrap(); let err = incoherent .resolve_graph_selection(Some("local")) .unwrap_err() @@ -705,7 +738,7 @@ policy: {} server:\n graph: local\ncli:\n graph: prod\n", ) .unwrap(); - let config = load_config_in(temp.path(), None).unwrap(); + let config = load_config_in(temp.path(), None, None).unwrap(); assert_eq!( config.resolve_policy_tooling_graph_selection().unwrap(), Some("prod") @@ -717,7 +750,7 @@ policy: {} "graphs:\n local:\n uri: ./local.omni\nserver:\n graph: local\n", ) .unwrap(); - let config = load_config_in(temp.path(), None).unwrap(); + let config = load_config_in(temp.path(), None, None).unwrap(); assert_eq!( config.resolve_policy_tooling_graph_selection().unwrap(), Some("local") @@ -725,7 +758,7 @@ policy: {} let temp = tempdir().unwrap(); fs::write(temp.path().join("omnigraph.yaml"), "policy: {}\n").unwrap(); - let config = load_config_in(temp.path(), None).unwrap(); + let config = load_config_in(temp.path(), None, None).unwrap(); assert_eq!(config.resolve_policy_tooling_graph_selection().unwrap(), None); let temp = tempdir().unwrap(); @@ -734,7 +767,7 @@ policy: {} "graphs:\n local:\n uri: ./local.omni\nserver:\n graph: ghost\n", ) .unwrap(); - let config = load_config_in(temp.path(), None).unwrap(); + let config = load_config_in(temp.path(), None, None).unwrap(); let err = config .resolve_policy_tooling_graph_selection() .unwrap_err() @@ -760,7 +793,7 @@ policy: {} ) .unwrap(); - let config = load_config_in(temp.path(), None).unwrap(); + let config = load_config_in(temp.path(), None, None).unwrap(); let resolved = config.resolve_query_path(Path::new("test.gq")).unwrap(); assert_eq!(resolved, temp.path().join("queries").join("test.gq")); } @@ -777,7 +810,7 @@ policy: {} fs::write(ambient_dir.join("local.gq"), "query ambient { return {} }").unwrap(); let config = - load_config_in(&ambient_dir, Some(&config_dir.join("omnigraph.yaml"))).unwrap(); + load_config_in(&ambient_dir, Some(&config_dir.join("omnigraph.yaml")), None).unwrap(); let resolved = config.resolve_query_path(Path::new("local.gq")).unwrap(); assert_eq!(resolved, config_dir.join("local.gq")); @@ -807,7 +840,7 @@ queries: ) .unwrap(); - let config = load_config_in(temp.path(), None).unwrap(); + let config = load_config_in(temp.path(), None, None).unwrap(); // Per-graph registry (multi-graph mode). let prod = config.target_query_entries("prod").unwrap(); @@ -848,7 +881,7 @@ queries: policy:\n file: ./prod.yaml\n bare:\n uri: s3://b/bare\n", ) .unwrap(); - let config = load_config_in(temp.path(), None).unwrap(); + let config = load_config_in(temp.path(), None, None).unwrap(); // Named graph with its own policy → per-graph (not top-level). assert!( @@ -884,7 +917,7 @@ queries: ) .unwrap(); - let config = load_config_in(temp.path(), None).unwrap(); + let config = load_config_in(temp.path(), None, None).unwrap(); // Additive: no `queries:` anywhere → empty registries everywhere. assert!(config.query_entries().is_empty()); assert!( @@ -904,7 +937,7 @@ queries: ) .unwrap(); - let config = load_config_in(temp.path(), None).unwrap(); + let config = load_config_in(temp.path(), None, None).unwrap(); assert_eq!( config.resolve_policy_file().unwrap(), temp.path().join("policy.yaml") @@ -927,7 +960,7 @@ cli: ) .unwrap(); - let config = load_config_in(temp.path(), None).unwrap(); + let config = load_config_in(temp.path(), None, None).unwrap(); assert_eq!( config.graph_bearer_token_env( Some("https://override.example.com"), From 9427fb510e8ec74303caaf253c6afd0148e35a90 Mon Sep 17 00:00:00 2001 From: aaltshuler Date: Thu, 11 Jun 2026 20:32:04 +0300 Subject: [PATCH 2/2] docs(cli): the two config surfaces + the operator file reference cli-reference.md gains the config-surfaces table (cluster / operator / flags-env, with omnigraph.yaml marked as the legacy combined file per RFC-008) and the operator config.yaml reference; audit.md documents the unified actor chain. Co-Authored-By: Claude Fable 5 --- docs/user/audit.md | 2 +- docs/user/cli-reference.md | 31 ++++++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/docs/user/audit.md b/docs/user/audit.md index 52cecde..845c2e0 100644 --- a/docs/user/audit.md +++ b/docs/user/audit.md @@ -3,5 +3,5 @@ - `Omnigraph::audit_actor_id: Option` is the actor in effect. - `_as` variants of every write API let callers override the actor: `mutate_as`, `load_as`, `branch_merge_as`, `apply_schema_as`, etc. - Actor IDs are persisted on `GraphCommit.actor_id` with split storage in `_graph_commit_actors.lance` (the commit graph is split into `_graph_commits.lance` for the linkage and `_graph_commit_actors.lance` for the actor map). -- HTTP server uses the bearer-token actor automatically; CLI uses the local user / explicit env (no implicit actor). +- HTTP server uses the bearer-token actor automatically. The CLI resolves one actor chain everywhere: `--as` > legacy `cli.actor` in `omnigraph.yaml` > `operator.actor` in `~/.omnigraph/config.yaml` > none (RFC-007). - Pre-v0.4.0 graphs also stored actor IDs on `RunRecord.actor_id` in `_graph_runs.lance` / `_graph_run_actors.lance`. The Run state machine was removed in MR-771; those files are inert post-v0.4.0. The v2→v3 manifest migration sweeps any stale `__run__*` branches on first write-open (MR-770); the inert dataset bytes remain until a `delete_prefix` primitive lands. diff --git a/docs/user/cli-reference.md b/docs/user/cli-reference.md index 74d772f..1dbc1ff 100644 --- a/docs/user/cli-reference.md +++ b/docs/user/cli-reference.md @@ -27,7 +27,36 @@ Top-level command families and subcommands. Graph-targeting commands accept eith | `policy validate \| test \| explain` | Cedar tooling. Selects `cli.graph`, else `server.graph`, else top-level `policy.file` | | `version` / `-v` | print `omnigraph 0.3.x` | -## `omnigraph.yaml` schema +## Config surfaces + +Two config surfaces with single owners (RFC-007/RFC-008), plus a zero-config +tier: + +| Surface | Owner | Location | Declares | +|---|---|---|---| +| Cluster config | the team, in a repo | `cluster.yaml` + checkout ([cluster-config.md](cluster-config.md)) | what the system **is**: graphs, schemas, queries, policies, storage | +| Operator config | one person | `~/.omnigraph/config.yaml` (override dir with `$OMNIGRAPH_HOME`) | who **I** am: identity, ergonomics | +| Flags / env | per invocation | — | everything, explicitly | + +`omnigraph.yaml` (below) is the legacy combined file — fully supported +today, slated for staged deprecation (RFC-008); its keys' future homes are +listed there. + +### `~/.omnigraph/config.yaml` (operator) + +```yaml +operator: + actor: act-andrew # default identity for every --as cascade: + # --as > legacy cli.actor > operator.actor > none +defaults: + output: table # read format default, below --json/--format/alias/legacy +``` + +Absent file = empty layer. Unknown keys warn and load (a file written for a +newer CLI works on an older one). `$OMNIGRAPH_CONFIG=` stands in for +`--config` (the flag wins) in both the CLI and the server. + +## `omnigraph.yaml` schema (legacy combined file) ```yaml project: { name }