diff --git a/crates/omnigraph-cli/src/cli.rs b/crates/omnigraph-cli/src/cli.rs index feb08e8..7b976b4 100644 --- a/crates/omnigraph-cli/src/cli.rs +++ b/crates/omnigraph-cli/src/cli.rs @@ -56,6 +56,12 @@ pub(crate) enum Command { #[arg(long)] json: bool, }, + /// Legacy-config tooling (RFC-008): split omnigraph.yaml into its + /// two destinations. + Config { + #[command(subcommand)] + command: ConfigCommand, + }, /// Remove a named server's stored credential. Idempotent. Logout { name: String, @@ -681,3 +687,20 @@ impl CliLoadMode { } } +#[derive(Debug, Subcommand)] +pub(crate) enum ConfigCommand { + /// Propose (and with --write, apply) the RFC-008 split of a legacy + /// omnigraph.yaml: team half -> a ready-to-review cluster.yaml, + /// personal half -> ~/.omnigraph/config.yaml (key-level merge, + /// existing entries always win). Touches nothing without --write. + Migrate { + /// Path to the legacy omnigraph.yaml (default: ./omnigraph.yaml) + #[arg(long)] + config: Option, + /// Apply the split instead of only printing it + #[arg(long)] + write: bool, + #[arg(long)] + json: bool, + }, +} diff --git a/crates/omnigraph-cli/src/helpers.rs b/crates/omnigraph-cli/src/helpers.rs index a48f2e4..67fb6ea 100644 --- a/crates/omnigraph-cli/src/helpers.rs +++ b/crates/omnigraph-cli/src/helpers.rs @@ -677,69 +677,6 @@ pub(crate) fn normalize_legacy_alias_uri( (Some(candidate), alias_args) } -pub(crate) fn scaffold_config_if_missing(uri: &str) -> Result<()> { - let path = inferred_config_path(uri)?; - if path.exists() { - return Ok(()); - } - - fs::write( - path, - format!( - "\ -project: - name: Omnigraph Project - -graphs: - local: - uri: {} - # bearer_token_env: OMNIGRAPH_BEARER_TOKEN - -server: - graph: local - bind: 127.0.0.1:8080 - -cli: - graph: local - branch: main - output_format: table - table_max_column_width: 80 - table_cell_layout: truncate - -query: - roots: - - queries - - . - -aliases: - # owner: - # command: read - # query: context.gq - # name: decision_owner - # args: [slug] - # graph: local - # branch: main - # format: kv - # - # attach_trace: - # command: change - # query: mutations.gq - # name: attach_trace - # args: [decision_slug, trace_slug] - # graph: local - # branch: main - -# auth: -# env_file: ./.env.omni -# -# policy: -# file: ./policy.yaml -", - yaml_string(uri), - ), - )?; - Ok(()) -} pub(crate) fn inferred_config_path(uri: &str) -> Result { if uri.contains("://") { diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index 4306b67..8178e65 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 migrate; mod operator; mod read_format; @@ -73,6 +74,42 @@ async fn main() -> Result<()> { }; let http_client = build_http_client()?; match cli.command { + Command::Config { command } => match command { + ConfigCommand::Migrate { config, write, json } => { + let path = migrate::legacy_config_path(config.as_ref()); + if !path.exists() { + bail!( + "no legacy config at '{}' — nothing to migrate", + path.display() + ); + } + let legacy = load_config(Some(&path))?; + let report = migrate::build_report(&legacy, &path); + if write { + let legacy_dir = path + .parent() + .filter(|parent| !parent.as_os_str().is_empty()) + .unwrap_or(std::path::Path::new(".")) + .to_path_buf(); + let written = migrate::apply_report(&report, &legacy_dir)?; + if json { + print_json(&serde_json::json!({ + "report": report, + "written": written, + }))?; + } else { + print!("{}", migrate::render_report(&report)); + for line in written { + println!("wrote: {line}"); + } + } + } else if json { + print_json(&report)?; + } else { + print!("{}", migrate::render_report(&report)); + } + } + }, Command::Login { name, token, json } => { let token = match token { Some(token) => token, @@ -116,7 +153,6 @@ async fn main() -> Result<()> { omnigraph::db::InitOptions { force }, ) .await?; - scaffold_config_if_missing(&uri)?; println!("initialized {}", uri); } Command::Load { diff --git a/crates/omnigraph-cli/src/migrate.rs b/crates/omnigraph-cli/src/migrate.rs new file mode 100644 index 0000000..3891061 --- /dev/null +++ b/crates/omnigraph-cli/src/migrate.rs @@ -0,0 +1,408 @@ +//! `omnigraph config migrate` (RFC-008 stage 2): split a legacy +//! `omnigraph.yaml` into its two destinations — the team half as a +//! ready-to-review `cluster.yaml` proposal, the personal half merged into +//! `~/.omnigraph/config.yaml` — and name what's obsolete. The command is +//! the completeness test of RFC-008's migration map: any key it cannot +//! place is a bug in the RFC. +//! +//! Touches nothing without `--write`. Referenced `.gq`/policy files are +//! never moved; manual steps are printed instead. + +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +use color_eyre::Result; +use color_eyre::eyre::eyre; +use omnigraph_server::OmnigraphConfig; +use serde::Serialize; + +use crate::operator; + +#[derive(Debug, Serialize)] +pub(crate) struct MigrateReport { + pub(crate) source: String, + /// The ready-to-review cluster.yaml text (None when the legacy file + /// declares nothing team-shaped). + pub(crate) cluster_yaml: Option, + /// Operator keys to merge: dotted key -> YAML value text. + pub(crate) operator_merge: BTreeMap, + /// Keys with no destination, and why. + pub(crate) dropped: Vec, + /// Steps the command will not do for you. + pub(crate) manual_steps: Vec, +} + +#[derive(Debug, Serialize)] +pub(crate) struct DroppedKey { + pub(crate) key: String, + pub(crate) reason: String, +} + +/// Classify a parsed legacy config into the report. Pure — no I/O. +pub(crate) fn build_report(config: &OmnigraphConfig, source: &Path) -> MigrateReport { + let mut dropped = Vec::new(); + let mut manual_steps = Vec::new(); + let mut operator_merge: BTreeMap = BTreeMap::new(); + + // ---- personal half ---- + if let Some(actor) = &config.cli.actor { + operator_merge.insert("operator.actor".into(), actor.clone()); + } + if let Some(format) = config.cli.output_format { + operator_merge.insert( + "defaults.output".into(), + serde_yaml::to_string(&format).unwrap_or_default().trim().to_string(), + ); + } + if let Some(width) = config.cli.table_max_column_width { + operator_merge.insert("defaults.table_max_column_width".into(), width.to_string()); + } + if let Some(layout) = config.cli.table_cell_layout { + operator_merge.insert( + "defaults.table_cell_layout".into(), + serde_yaml::to_string(&layout).unwrap_or_default().trim().to_string(), + ); + } + if config.cli.graph.is_some() { + dropped.push(DroppedKey { + key: "cli.graph".into(), + reason: "no operator default-target yet — address graphs explicitly via --target/--server (RFC-002 locator territory)".into(), + }); + } + if config.cli.branch.is_some() { + dropped.push(DroppedKey { + key: "cli.branch".into(), + reason: "pass --branch explicitly".into(), + }); + } + + // Remote graphs with a token env become operator servers (the keyed + // chain replaces invented env-var names). + for (name, target) in &config.graphs { + if target.uri.starts_with("http://") || target.uri.starts_with("https://") { + operator_merge.insert(format!("servers.{name}.url"), target.uri.clone()); + if target.bearer_token_env.is_some() { + manual_steps.push(format!( + "store the '{name}' token in the keyed chain: echo $TOKEN | omnigraph login {name} (replaces bearer_token_env)" + )); + } + } + } + if config.auth.env_file.is_some() { + manual_steps.push( + "auth.env_file keeps working during the window; prefer `omnigraph login ` per server going forward".into(), + ); + } + + // Legacy aliases split: content -> catalog stored query, binding -> + // operator alias referencing the name. + for (name, alias) in &config.aliases { + let query_name = alias.name.clone().unwrap_or_else(|| name.clone()); + operator_merge.insert( + format!("aliases.{name}"), + format!( + "{{ server: TODO-server-name, graph: {}, query: {query_name}, args: [{}] }}", + alias.graph.as_deref().unwrap_or("TODO-graph-id"), + alias.args.join(", ") + ), + ); + manual_steps.push(format!( + "alias '{name}': move its query content ('{}') into the cluster checkout's queries/ so '{query_name}' becomes a catalog stored query", + alias.query + )); + } + + // ---- team half ---- + let has_team_content = !config.graphs.is_empty() + || !config.queries.is_empty() + || config.policy.file.is_some() + || config.server.policy.file.is_some(); + let cluster_yaml = has_team_content.then(|| { + let mut out = String::from("version: 1\n"); + if let Some(name) = &config.project.name { + out.push_str(&format!("metadata:\n name: {name}\n")); + } + out.push_str("# storage: s3://bucket/prefix # or omit: this folder is the root\n"); + if !config.graphs.is_empty() || !config.queries.is_empty() { + out.push_str("graphs:\n"); + } + // Single-graph top-level queries belong to a graph the legacy file + // never named; propose one. + if !config.queries.is_empty() && config.graphs.is_empty() { + out.push_str(" default: # TODO: pick the graph id\n schema: # TODO: path to this graph's .pg schema\n queries: queries/\n"); + } + for (name, target) in &config.graphs { + out.push_str(&format!(" {name}:\n")); + out.push_str(" schema: # TODO: path to this graph's .pg schema\n"); + if !target.queries.is_empty() { + out.push_str(" queries: queries/ # move the .gq files here\n"); + } + out.push_str(&format!( + " # legacy root: {} — the cluster manages graph roots under its storage; run `omnigraph cluster import` after reviewing\n", + target.uri + )); + } + let mut policies: Vec<(String, String, String)> = Vec::new(); + if let Some(file) = &config.policy.file { + policies.push(("default".into(), file.clone(), "graph. # TODO: bind".into())); + } + if let Some(file) = &config.server.policy.file { + policies.push(("server".into(), file.clone(), "cluster".into())); + } + for (name, target) in &config.graphs { + if let Some(file) = &target.policy.file { + policies.push((name.clone(), file.clone(), format!("graph.{name}"))); + } + } + if !policies.is_empty() { + out.push_str("policies:\n"); + for (name, file, binding) in policies { + out.push_str(&format!( + " {name}:\n file: {file}\n applies_to: [{binding}]\n" + )); + } + } + out + }); + + if !config.query.roots.is_empty() { + dropped.push(DroppedKey { + key: "query.roots".into(), + reason: "obsolete — cluster query discovery (queries: ) replaced it".into(), + }); + } + if config.server.bind.is_some() || config.server.graph.is_some() { + dropped.push(DroppedKey { + key: "server.bind / server.graph".into(), + reason: "deployment runtime — pass --bind / target flags or env".into(), + }); + } + if config.project.name.is_some() && cluster_yaml.is_none() { + dropped.push(DroppedKey { + key: "project.name".into(), + reason: "the cluster's metadata.name is the deployment label".into(), + }); + } + + MigrateReport { + source: source.display().to_string(), + cluster_yaml, + operator_merge, + dropped, + manual_steps, + } +} + +pub(crate) fn render_report(report: &MigrateReport) -> String { + let mut out = format!("migration plan for {}\n", report.source); + if let Some(cluster) = &report.cluster_yaml { + out.push_str("\n== team half -> cluster.yaml (ready to review) ==\n"); + out.push_str(cluster); + } + if !report.operator_merge.is_empty() { + out.push_str("\n== personal half -> ~/.omnigraph/config.yaml ==\n"); + for (key, value) in &report.operator_merge { + out.push_str(&format!(" {key}: {value}\n")); + } + } + if !report.dropped.is_empty() { + out.push_str("\n== no destination ==\n"); + for dropped in &report.dropped { + out.push_str(&format!(" {} — {}\n", dropped.key, dropped.reason)); + } + } + if !report.manual_steps.is_empty() { + out.push_str("\n== manual steps ==\n"); + for step in &report.manual_steps { + out.push_str(&format!(" - {step}\n")); + } + } + out.push_str("\n(nothing written; pass --write to apply the operator merge and emit cluster.yaml)\n"); + out +} + +/// `--write`: merge the personal half into the operator config (key-level, +/// existing entries always win; the prior file is backed up) and write the +/// team half to cluster.yaml in the legacy config's directory (or +/// cluster.yaml.proposed when one already exists). +pub(crate) fn apply_report(report: &MigrateReport, legacy_dir: &Path) -> Result> { + let mut written = Vec::new(); + + if !report.operator_merge.is_empty() { + let dir = operator::operator_dir() + .ok_or_else(|| eyre!("no home directory resolvable for the operator config"))?; + std::fs::create_dir_all(&dir)?; + let path = dir.join(operator::OPERATOR_CONFIG_FILE); + let existing_text = std::fs::read_to_string(&path).unwrap_or_default(); + let mut mapping: serde_yaml::Mapping = if existing_text.trim().is_empty() { + serde_yaml::Mapping::new() + } else { + serde_yaml::from_str(&existing_text) + .map_err(|err| eyre!("operator config '{}' does not parse: {err}", path.display()))? + }; + let mut merged_any = false; + for (dotted, value_text) in &report.operator_merge { + if merge_dotted_if_absent(&mut mapping, dotted, value_text)? { + merged_any = true; + } + } + if merged_any { + if !existing_text.is_empty() { + let backup = path.with_extension("yaml.bak"); + std::fs::write(&backup, &existing_text)?; + written.push(format!("backed up prior operator config to {}", backup.display())); + } + let rendered = serde_yaml::to_string(&mapping)?; + let tmp = path.with_extension(format!("yaml.tmp.{}", std::process::id())); + std::fs::write(&tmp, &rendered)?; + std::fs::rename(&tmp, &path)?; + written.push(format!("merged personal keys into {}", path.display())); + } else { + written.push("operator config already carries every personal key (nothing merged)".into()); + } + } + + if let Some(cluster) = &report.cluster_yaml { + let target = legacy_dir.join("cluster.yaml"); + let target = if target.exists() { + legacy_dir.join("cluster.yaml.proposed") + } else { + target + }; + std::fs::write(&target, cluster)?; + written.push(format!("wrote team-half proposal to {}", target.display())); + } + + Ok(written) +} + +/// Set `a.b.c` in the mapping only when absent; returns whether it wrote. +fn merge_dotted_if_absent( + mapping: &mut serde_yaml::Mapping, + dotted: &str, + value_text: &str, +) -> Result { + let value: serde_yaml::Value = + serde_yaml::from_str(value_text).unwrap_or(serde_yaml::Value::String(value_text.into())); + let parts: Vec<&str> = dotted.split('.').collect(); + let mut current = mapping; + for part in &parts[..parts.len() - 1] { + let key = serde_yaml::Value::String((*part).into()); + let entry = current + .entry(key) + .or_insert_with(|| serde_yaml::Value::Mapping(serde_yaml::Mapping::new())); + current = entry + .as_mapping_mut() + .ok_or_else(|| eyre!("operator config key '{dotted}' collides with a non-mapping"))?; + } + let leaf = serde_yaml::Value::String(parts[parts.len() - 1].into()); + if current.contains_key(&leaf) { + return Ok(false); + } + current.insert(leaf, value); + Ok(true) +} + +pub(crate) fn legacy_config_path(explicit: Option<&PathBuf>) -> PathBuf { + explicit.cloned().unwrap_or_else(|| PathBuf::from("omnigraph.yaml")) +} + +#[cfg(test)] +mod tests { + use super::*; + use omnigraph_server::config::load_config; + + fn full_legacy_fixture(dir: &Path) -> PathBuf { + let path = dir.join("omnigraph.yaml"); + std::fs::write( + &path, + r#" +project: { name: brain } +graphs: + prod: + uri: https://graph.example.com + bearer_token_env: PROD_TOKEN + policy: { file: ./prod.policy.yaml } + queries: + find: { file: ./find.gq } + local: + uri: /tmp/local.omni +server: { bind: "0.0.0.0:9999", policy: { file: ./server.policy.yaml } } +auth: { env_file: .env.omni } +cli: + graph: prod + branch: main + actor: act-me + output_format: json + table_max_column_width: 40 +query: { roots: ["."] } +aliases: + triage: { command: query, query: ./triage.gq, name: weekly_triage, args: [since], graph: prod } +policy: { file: ./top.policy.yaml } +queries: + top_q: { file: ./top.gq } +"#, + ) + .unwrap(); + path + } + + /// The RFC-008 completeness contract: every top-level key of the + /// legacy schema must appear in the report somewhere (team half, + /// operator merge, dropped, or manual steps). + #[test] + fn every_legacy_key_is_classified() { + let dir = tempfile::tempdir().unwrap(); + let path = full_legacy_fixture(dir.path()); + let config = load_config(Some(&path)).unwrap(); + let report = build_report(&config, &path); + let rendered = render_report(&report); + + let serialized = + serde_yaml::to_value(OmnigraphConfig::default()).expect("default serializes"); + for key in serialized.as_mapping().unwrap().keys() { + let key = key.as_str().unwrap(); + assert!( + rendered.contains(key) + || report.operator_merge.keys().any(|k| k.contains(key)) + || matches!(key, "graphs" | "queries" | "policy" | "project") + && report.cluster_yaml.is_some(), + "legacy key '{key}' is unclassified — fix the RFC-008 map: {rendered}" + ); + } + + // spot checks on each section + assert_eq!(report.operator_merge["operator.actor"], "act-me"); + assert_eq!(report.operator_merge["defaults.output"], "json"); + assert_eq!( + report.operator_merge["servers.prod.url"], + "https://graph.example.com" + ); + assert!(report.operator_merge["aliases.triage"].contains("query: weekly_triage")); + let cluster = report.cluster_yaml.as_deref().unwrap(); + assert!(cluster.contains("version: 1")); + assert!(cluster.contains("name: brain")); + assert!(cluster.contains(" prod:")); + assert!(cluster.contains("applies_to: [cluster]")); + assert!(cluster.contains("applies_to: [graph.prod]")); + assert!(report.dropped.iter().any(|d| d.key == "query.roots")); + assert!(report.dropped.iter().any(|d| d.key.contains("server.bind"))); + assert!( + report + .manual_steps + .iter() + .any(|s| s.contains("omnigraph login prod")) + ); + } + + #[test] + fn merge_dotted_never_clobbers_existing() { + let mut mapping: serde_yaml::Mapping = + serde_yaml::from_str("operator:\n actor: keep-me\n").unwrap(); + assert!(!merge_dotted_if_absent(&mut mapping, "operator.actor", "new").unwrap()); + assert!(merge_dotted_if_absent(&mut mapping, "defaults.output", "json").unwrap()); + let text = serde_yaml::to_string(&mapping).unwrap(); + assert!(text.contains("keep-me") && !text.contains("new")); + assert!(text.contains("output: json")); + } +} diff --git a/crates/omnigraph-cli/src/operator.rs b/crates/omnigraph-cli/src/operator.rs index 16f5550..fb8658d 100644 --- a/crates/omnigraph-cli/src/operator.rs +++ b/crates/omnigraph-cli/src/operator.rs @@ -91,6 +91,10 @@ pub(crate) struct OperatorIdentity { pub(crate) struct OperatorDefaults { /// Default read output format, below every more-specific source. pub(crate) output: Option, + /// Table rendering preferences (below the legacy cli.table_* keys + /// during the RFC-008 window). + pub(crate) table_max_column_width: Option, + pub(crate) table_cell_layout: Option, #[serde(flatten)] unknown: serde_yaml::Mapping, } diff --git a/crates/omnigraph-cli/src/output.rs b/crates/omnigraph-cli/src/output.rs index 04df60a..964307b 100644 --- a/crates/omnigraph-cli/src/output.rs +++ b/crates/omnigraph-cli/src/output.rs @@ -716,10 +716,7 @@ pub(crate) fn print_read_output( render_read( output, format, - &ReadRenderOptions { - max_column_width: config.table_max_column_width(), - cell_layout: config.table_cell_layout(), - }, + &resolve_table_render_options(config), )? ); Ok(()) @@ -873,3 +870,21 @@ pub(crate) fn finish_logout( } Ok(()) } + +/// Table prefs cascade (RFC-007/008): legacy cli.table_* (window) > +/// operator defaults.table_* > built-in. +pub(crate) fn resolve_table_render_options(config: &OmnigraphConfig) -> ReadRenderOptions { + let operator = crate::operator::load_operator_config().unwrap_or_default(); + ReadRenderOptions { + max_column_width: config + .cli + .table_max_column_width + .or(operator.defaults.table_max_column_width) + .unwrap_or(80), + cell_layout: config + .cli + .table_cell_layout + .or(operator.defaults.table_cell_layout) + .unwrap_or_default(), + } +} diff --git a/crates/omnigraph-cli/tests/cli_cluster.rs b/crates/omnigraph-cli/tests/cli_cluster.rs index bfadf40..3b2eed3 100644 --- a/crates/omnigraph-cli/tests/cli_cluster.rs +++ b/crates/omnigraph-cli/tests/cli_cluster.rs @@ -949,3 +949,4 @@ graphs: let leaked = b.to_string(); assert!(!leaked.contains("phantom") && !leaked.contains("9999"), "{leaked}"); } + diff --git a/crates/omnigraph-cli/tests/cli_schema_config.rs b/crates/omnigraph-cli/tests/cli_schema_config.rs index a0dac0a..3e2a2b9 100644 --- a/crates/omnigraph-cli/tests/cli_schema_config.rs +++ b/crates/omnigraph-cli/tests/cli_schema_config.rs @@ -36,7 +36,8 @@ fn init_creates_graph_successfully_on_missing_local_directory() { assert!(stdout.contains("initialized")); assert!(graph.join("_schema.pg").exists()); assert!(graph.join("__manifest").exists()); - assert!(temp.path().join("omnigraph.yaml").exists()); + // RFC-008 stage 3: init no longer scaffolds the legacy config file. + assert!(!temp.path().join("omnigraph.yaml").exists()); } #[test] @@ -498,3 +499,114 @@ fn graphs_list_against_local_uri_errors_with_remote_only_message() { "expected 'remote multi-graph server URL' rejection in stderr; got:\n{stderr}" ); } + +/// RFC-008 stage 1: loading a legacy omnigraph.yaml emits the per-key +/// deprecation block (the migration map applied to THIS file), suppressible +/// via OMNIGRAPH_SUPPRESS_YAML_DEPRECATION. +#[test] +fn legacy_config_load_warns_per_key_and_suppression_silences() { + let temp = tempdir().unwrap(); + fs::write( + temp.path().join("omnigraph.yaml"), + "cli:\n actor: act-x\ngraphs:\n g:\n uri: /tmp/never-opened\n", + ) + .unwrap(); + + // `graphs list --json` loads the config and exits without touching the + // graph URI. + let output = cli() + .current_dir(temp.path()) + .arg("graphs") + .arg("list") + .arg("--json") + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("deprecated (RFC-008)") && stderr.contains("`cli.actor` -> `operator.actor`"), + "{stderr}" + ); + assert!(stderr.contains("config migrate"), "{stderr}"); + + let output = cli() + .current_dir(temp.path()) + .env("OMNIGRAPH_SUPPRESS_YAML_DEPRECATION", "1") + .arg("graphs") + .arg("list") + .arg("--json") + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(!stderr.contains("deprecated (RFC-008)"), "{stderr}"); +} + +/// RFC-008 stage 2: `config migrate` proposes the split read-only, applies +/// it with --write (operator merge never clobbers; cluster.yaml emitted), +/// and a second --write is idempotent. +#[test] +fn config_migrate_splits_legacy_config() { + let temp = tempdir().unwrap(); + fs::write( + temp.path().join("omnigraph.yaml"), + "graphs:\n prod:\n uri: https://graph.example.com\n bearer_token_env: PROD_TOKEN\ncli:\n actor: act-me\n output_format: json\npolicy:\n file: ./top.policy.yaml\n", + ) + .unwrap(); + let operator_home = tempfile::tempdir().unwrap(); + fs::write( + operator_home.path().join("config.yaml"), + "operator:\n actor: act-existing\n", + ) + .unwrap(); + + // Read-only proposal: names both halves, writes nothing. + let output = cli() + .current_dir(temp.path()) + .env("OMNIGRAPH_HOME", operator_home.path()) + .env("OMNIGRAPH_SUPPRESS_YAML_DEPRECATION", "1") + .arg("config") + .arg("migrate") + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("team half -> cluster.yaml"), "{stdout}"); + assert!(stdout.contains("operator.actor: act-me"), "{stdout}"); + assert!(stdout.contains("omnigraph login prod"), "{stdout}"); + assert!(!temp.path().join("cluster.yaml").exists()); + + // --write: cluster.yaml lands; the existing operator actor is KEPT. + let output = cli() + .current_dir(temp.path()) + .env("OMNIGRAPH_HOME", operator_home.path()) + .env("OMNIGRAPH_SUPPRESS_YAML_DEPRECATION", "1") + .arg("config") + .arg("migrate") + .arg("--write") + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + let cluster = fs::read_to_string(temp.path().join("cluster.yaml")).unwrap(); + assert!(cluster.contains("version: 1") && cluster.contains(" prod:"), "{cluster}"); + let operator_text = + fs::read_to_string(operator_home.path().join("config.yaml")).unwrap(); + assert!(operator_text.contains("act-existing"), "{operator_text}"); + assert!(!operator_text.contains("act-me"), "existing keys win: {operator_text}"); + assert!(operator_text.contains("output: json"), "{operator_text}"); + assert!( + operator_text.contains("url: https://graph.example.com"), + "{operator_text}" + ); + + // Second --write: cluster.yaml exists -> proposal file, no clobber. + let output = cli() + .current_dir(temp.path()) + .env("OMNIGRAPH_HOME", operator_home.path()) + .env("OMNIGRAPH_SUPPRESS_YAML_DEPRECATION", "1") + .arg("config") + .arg("migrate") + .arg("--write") + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + assert!(temp.path().join("cluster.yaml.proposed").exists()); +} diff --git a/crates/omnigraph-server/src/config.rs b/crates/omnigraph-server/src/config.rs index 52bac2e..ea7ce30 100644 --- a/crates/omnigraph-server/src/config.rs +++ b/crates/omnigraph-server/src/config.rs @@ -549,7 +549,9 @@ fn load_config_in( }); let mut config = if let Some(path) = &config_path { - serde_yaml::from_str::(&fs::read_to_string(path)?)? + let text = fs::read_to_string(path)?; + warn_yaml_deprecation_once(path, &text); + serde_yaml::from_str::(&text)? } else { OmnigraphConfig::default() }; @@ -563,6 +565,74 @@ fn load_config_in( Ok(config) } +/// RFC-008 stage 1: suppress the legacy-config deprecation warning +/// (one process), for CI logs during the deprecation window. +pub const SUPPRESS_YAML_DEPRECATION_ENV: &str = "OMNIGRAPH_SUPPRESS_YAML_DEPRECATION"; + +/// RFC-008's migration map (the "Where every key goes" table), applied to +/// the keys actually present in a loaded file — never a generic banner. +/// Keys are `(yaml pointer, destination)`; the pointer is matched against +/// the file's real top-level/nested keys. +const YAML_DEPRECATION_MAP: &[(&str, &str)] = &[ + ("graphs", "cluster.yaml `graphs:` (team surface) — or flags/env for the zero-config tier"), + ("queries", "the cluster catalog (`.gq` discovery in cluster.yaml)"), + ("policy", "cluster.yaml `policies:` + `applies_to` bindings"), + ("server", "flags/env (`--bind`); meaningless under cluster boot"), + ("auth", "the operator credentials chain (`omnigraph login `)"), + ("aliases", "operator `aliases:` (bindings) + catalog stored queries (content)"), + ("query", "obsolete — cluster query discovery replaced `query.roots`"), + ("project", "cluster.yaml `metadata.name`"), + ("cli.actor", "`operator.actor` in ~/.omnigraph/config.yaml"), + ("cli.output_format", "`defaults.output` in ~/.omnigraph/config.yaml"), + ("cli.table_max_column_width", "`defaults.table_max_column_width` in ~/.omnigraph/config.yaml"), + ("cli.table_cell_layout", "`defaults.table_cell_layout` in ~/.omnigraph/config.yaml"), + ("cli.graph", "explicit `--target`/`--server` (no operator default-target yet)"), + ("cli.branch", "explicit `--branch`"), +]; + +/// Emit the per-key deprecation block once per process when a legacy +/// `omnigraph.yaml` is actually loaded. `omnigraph config migrate` +/// produces the split these lines describe. +fn warn_yaml_deprecation_once(path: &Path, text: &str) { + static WARNED: std::sync::OnceLock<()> = std::sync::OnceLock::new(); + if env::var_os(SUPPRESS_YAML_DEPRECATION_ENV).is_some() { + return; + } + let lines = yaml_deprecation_lines(text); + if lines.is_empty() { + return; + } + WARNED.get_or_init(|| { + eprintln!( + "warning: '{}' is deprecated (RFC-008) — its keys have new homes; run `omnigraph config migrate` for the split, set {SUPPRESS_YAML_DEPRECATION_ENV}=1 to silence:", + path.display() + ); + for line in &lines { + eprintln!(" {line}"); + } + }); +} + +fn yaml_deprecation_lines(text: &str) -> Vec { + let Ok(mapping) = serde_yaml::from_str::(text) else { + return Vec::new(); + }; + let present = |pointer: &str| -> bool { + match pointer.split_once('.') { + None => mapping.contains_key(pointer), + Some((outer, inner)) => mapping + .get(outer) + .and_then(|value| value.as_mapping()) + .is_some_and(|nested| nested.contains_key(inner)), + } + }; + YAML_DEPRECATION_MAP + .iter() + .filter(|(pointer, _)| present(pointer)) + .map(|(pointer, destination)| format!("`{pointer}` -> {destination}")) + .collect() +} + fn absolute_base_dir(cwd: &Path, path: &Path) -> Result { let path = if path.is_absolute() { path.to_path_buf() @@ -608,6 +678,22 @@ mod tests { assert_eq!(config.cli.actor.as_deref(), Some("act-env")); } + #[test] + fn yaml_deprecation_lines_name_present_keys_only() { + let lines = super::yaml_deprecation_lines( + "graphs:\n g:\n uri: /tmp/x\ncli:\n actor: a\n branch: main\n", + ); + let joined = lines.join("\n"); + assert!(joined.contains("`graphs` ->"), "{joined}"); + assert!(joined.contains("`cli.actor` -> `operator.actor`"), "{joined}"); + assert!(joined.contains("`cli.branch` ->"), "{joined}"); + assert!(!joined.contains("`aliases`"), "{joined}"); + assert!(!joined.contains("`cli.output_format`"), "{joined}"); + + assert!(super::yaml_deprecation_lines("").is_empty()); + assert!(super::yaml_deprecation_lines("not: [valid").is_empty()); + } + #[test] fn load_config_reads_yaml_defaults_from_current_dir() { let temp = tempdir().unwrap(); diff --git a/docs/dev/rfc-008-deprecate-omnigraph-yaml.md b/docs/dev/rfc-008-deprecate-omnigraph-yaml.md index d496df8..a59be52 100644 --- a/docs/dev/rfc-008-deprecate-omnigraph-yaml.md +++ b/docs/dev/rfc-008-deprecate-omnigraph-yaml.md @@ -112,22 +112,26 @@ Two placements worth defending: Per Hyrum's Law (the repo's own deny-list: shipped observable behavior is contract), retirement is staged, loud, and tooled: -1. **Warn.** Loading `omnigraph.yaml` emits a one-line deprecation notice +1. **Warn** *(landed)*. Loading `omnigraph.yaml` emits a one-line deprecation notice naming the replacement for each key actually present in the file (not a generic banner — the migration map above, applied to *your* file). Suppressible per-process (`OMNIGRAPH_SUPPRESS_YAML_DEPRECATION=1`) for CI logs during the window. -2. **Migrate.** `omnigraph config migrate` reads an existing +2. **Migrate** *(landed)*. `omnigraph config migrate` reads an existing `omnigraph.yaml` and writes the split: the team half as a ready-to-review `cluster.yaml` (+ moves query/policy files into the checkout layout), the personal half merged into `~/.omnigraph/config.yaml` — printing a diff-style summary and touching nothing without `--write`. The command is the test of the migration map's completeness: any key it cannot place is a bug in this RFC. -3. **Stop scaffolding.** `omnigraph init` stops generating - `omnigraph.yaml` (it currently scaffolds one into cwd — the source of - the test-pollution bug). `omnigraph cluster init` (new, small) scaffolds - a minimal `cluster.yaml` instead. +3. **Stop scaffolding** *(landed)*. `omnigraph init` stops generating + `omnigraph.yaml` (it scaffolded one into cwd — the source of the + test-pollution bug). **No replacement scaffold**: a minimal + `cluster.yaml` is five lines; a generator would be a second copy of the + schema to keep in sync, producing a file that is unusable until + hand-edited anyway (Terraform has no config scaffolder either). New + users copy from the cluster quick-start; migrants get a ready-to-review + `cluster.yaml` from `config migrate`. 4. **Opt-in strict.** `OMNIGRAPH_NO_LEGACY_CONFIG=1` turns the warning into an error — for teams that finished migrating and want regressions caught. 5. **Remove at the next major.** Loading the file becomes an error pointing diff --git a/docs/user/cli-reference.md b/docs/user/cli-reference.md index b113ef3..62e3e0d 100644 --- a/docs/user/cli-reference.md +++ b/docs/user/cli-reference.md @@ -8,7 +8,7 @@ Top-level command families and subcommands. Graph-targeting commands accept a po | Command | Purpose | |---|---| -| `init` | `--schema ` → initialize a graph (also scaffolds `omnigraph.yaml` if missing) | +| `init` | `--schema ` → initialize a graph (no longer scaffolds `omnigraph.yaml` — RFC-008; start cluster configs from the [cluster.md](cluster.md) quick-start or `config migrate`) | | `load` | bulk load a branch, local or remote (`--mode overwrite\|append\|merge` is **required** — overwrite is destructive, so there is no default). Without `--from` the target branch must exist; `--from ` forks a missing `--branch` from `` first | | `ingest` | deprecated alias of `load --from ` (defaults: `--from main --mode merge`); prints a one-line warning to stderr | | `query` (alias: `read`) | run named read query; source via `--query `, `-e`/`--query-string `, or `--alias ` (exactly one). `read` is the deprecated previous name and prints a one-line warning to stderr | @@ -19,6 +19,7 @@ Top-level command families and subcommands. Graph-targeting commands accept a po | `commit list \| show` | inspect commit graph | | `schema plan \| apply \| show (alias: get)` | migrations | | `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` | +| `config migrate` | propose (or `--write`: apply) the RFC-008 split of a legacy `omnigraph.yaml` — team half → ready-to-review `cluster.yaml`, personal half → `~/.omnigraph/config.yaml` (key-level merge, existing entries win), plus dropped-key reasons and manual steps | | `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 the per-operator `omnigraph.yaml`'s `cli.actor` when `--as` is omitted; nothing else in that file affects cluster commands); what apply converges is what an `omnigraph-server --cluster ` deployment serves on its next restart (omnigraph.yaml deployments are unaffected); `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`) | | `repair [--confirm] [--force]` | preview or explicitly publish uncovered manifest/head drift. `--confirm` heals verified maintenance drift and exits non-zero if suspicious/unverifiable drift is refused; `--force --confirm` publishes suspicious/unverifiable drift after operator review | @@ -104,6 +105,12 @@ operator server use the legacy chain alone. ## `omnigraph.yaml` schema (legacy combined file) +> **Deprecated (RFC-008).** Loading this file prints a per-key notice +> naming each present key's new home (suppress in CI with +> `OMNIGRAPH_SUPPRESS_YAML_DEPRECATION=1`); `omnigraph config migrate` +> produces the split. The file keeps working through the deprecation +> window. + ```yaml project: { name } graphs: