apunkt/omnigraph
1
0
Fork 0
mirror of https://github.com/ModernRelay/omnigraph.git synced 2026-06-09 01:35:18 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

refactor(config,cli): rename the cli: config block to defaults: under v1

Browse source 
`defaults:` is the canonical CLI/client-defaults block. The legacy spelling
`cli:` is accepted as a serde alias and honored under the legacy schema, but
rejected under `version: 1` (pointing at the new spelling) and flagged by a
deprecation warning. Generalizes the version-gated key scan into
`legacy_top_level_keys`, which now drives both the v1 rejection and the legacy
warnings via a shared migration-hint table. Renames the config accessors
(cli_* -> default_*) and repoints the CLI call sites; migrates the init
scaffold, the example config, and the shared test helpers to `defaults:`.
This commit is contained in:
Ragnor Comerford 2026-06-04 21:31:38 +02:00
parent 5f693ac646
commit 56ff5eb9ec
No known key found for this signature in database
4 changed files with 146 additions and 72 deletions
Download patch file Download diff file Expand all files Collapse all files

26
crates/omnigraph-cli/src/main.rs
View file

@ -869,7 +869,7 @@ async fn open_local_db_with_policy(graph: &ResolvedCliGraph) -> Result<Omnigraph
/// footgun guard intentionally denies — silent bypass via "I forgot the
/// actor" is what the guard prevents.
fn resolve_cli_actor<'a>(cli_as: Option<&'a str>, config: &'a OmnigraphConfig) -> Option<&'a str> {
cli_as.or(config.cli.actor.as_deref())
cli_as.or(config.defaults.actor.as_deref())
}
fn resolve_policy_tests_path(context: &ResolvedPolicyContext) -> PathBuf {
@ -890,7 +890,7 @@ fn resolve_remote_bearer_token(
explicit_target: Option<&str>,
) -> Result<Option<String>> {
let scoped_env =
config.graph_bearer_token_env(explicit_uri, explicit_target, config.cli_graph_name());
config.graph_bearer_token_env(explicit_uri, explicit_target, config.default_graph_name());
let mut env_names = Vec::new();
if let Some(name) = scoped_env {
env_names.push(name.to_string());
@ -962,7 +962,7 @@ fn resolve_uri(
cli_uri: Option<String>,
cli_target: Option<&str>,
) -> Result<String> {
config.resolve_target_uri(cli_uri, cli_target, config.cli_graph_name())
config.resolve_target_uri(cli_uri, cli_target, config.default_graph_name())
}
fn resolve_cli_graph(
@ -975,7 +975,7 @@ fn resolve_cli_graph(
} else {
cli_target
.map(str::to_string)
.or_else(|| config.cli_graph_name().map(str::to_string))
.or_else(|| config.default_graph_name().map(str::to_string))
};
config.resolve_graph_selection(selected.as_deref())?;
let locator = config.resolve_graph(cli_uri.as_deref(), cli_target)?;
@ -1056,7 +1056,7 @@ fn resolve_branch(
) -> String {
cli_branch
.or(alias_branch)
.or_else(|| config.cli.branch.clone())
.or_else(|| config.defaults.branch.clone())
.unwrap_or_else(|| default_branch.to_string())
}
@ -1072,7 +1072,7 @@ fn resolve_read_target(
Ok(read_target_from_cli(
cli_branch
.or(alias_branch)
.or_else(|| config.cli.branch.clone()),
.or_else(|| config.defaults.branch.clone()),
cli_snapshot,
))
}
@ -1520,7 +1520,7 @@ fn resolve_read_format(
} else {
cli_format
.or(alias_format)
.unwrap_or_else(|| config.cli_output_format())
.unwrap_or_else(|| config.default_output_format())
}
}
@ -1583,7 +1583,7 @@ server:
graph: local
bind: 127.0.0.1:8080
cli:
defaults:
graph: local
branch: main
output_format: table
@ -1716,7 +1716,7 @@ async fn execute_query_lint(
}
let has_graph_target =
cli_uri.is_some() || cli_target.is_some() || config.cli_graph_name().is_some();
cli_uri.is_some() || cli_target.is_some() || config.default_graph_name().is_some();
if !has_graph_target {
bail!("query lint requires --schema <schema.pg> or a resolvable graph target");
}
@ -1814,7 +1814,7 @@ fn resolve_registry_selection_for_list(
) -> Result<Option<String>> {
let selected = target
.map(str::to_string)
.or_else(|| config.cli_graph_name().map(str::to_string));
.or_else(|| config.default_graph_name().map(str::to_string));
if let Some(name) = selected.as_deref() {
config.resolve_graph_selection(Some(name))?;
return Ok(selected);
@ -2828,7 +2828,7 @@ async fn main() -> Result<()> {
|| alias_config
.and_then(|alias| alias.graph.as_deref())
.is_some()
|| config.cli_graph_name().is_some();
|| config.default_graph_name().is_some();
let (legacy_uri, alias_args) =
normalize_legacy_alias_uri(legacy_uri, target_available, alias_name, alias_args);
let uri = uri.or(legacy_uri);
@ -2914,7 +2914,7 @@ async fn main() -> Result<()> {
|| alias_config
.and_then(|alias| alias.graph.as_deref())
.is_some()
|| config.cli_graph_name().is_some();
|| config.default_graph_name().is_some();
let (legacy_uri, alias_args) =
normalize_legacy_alias_uri(legacy_uri, target_available, alias_name, alias_args);
let uri = uri.or(legacy_uri);
@ -3517,7 +3517,7 @@ policy:
}
#[test]
fn graph_identity_resolve_cli_graph_named_target_uses_graph_key_not_project_name_or_uri() {
fn graph_identity_resolve_default_graph_named_target_uses_graph_key_not_project_name_or_uri() {
let temp = tempdir().unwrap();
let config_path = temp.path().join("omnigraph.yaml");
fs::write(

4
crates/omnigraph-cli/tests/support/mod.rs
View file

@ -123,7 +123,7 @@ version: 1
graphs:
local:
storage: {}
cli:
defaults:
graph: local
branch: main
query:
@ -145,7 +145,7 @@ servers:
graphs:
dev:
server: dev
cli:
defaults:
graph: dev
branch: main
query:

186
crates/omnigraph-config/src/lib.rs
View file

@ -332,8 +332,12 @@ pub struct OmnigraphConfig {
pub server: ServerDefaults,
#[serde(default)]
pub auth: AuthDefaults,
#[serde(default)]
pub cli: CliDefaults,
/// CLI/client defaults (`defaults:`) — RFC-002. The legacy spelling `cli:`
/// is accepted via a serde alias and honored under the legacy schema; under
/// `version: 1` it is rejected at load (see [`legacy_top_level_keys`]). All
/// reads go through the `default_*` accessors.
#[serde(default, alias = "cli")]
pub defaults: CliDefaults,
#[serde(default)]
pub query: QueryDefaults,
#[serde(default)]
@ -353,6 +357,12 @@ pub struct OmnigraphConfig {
/// when no `omnigraph.yaml` exists.
#[serde(skip)]
loaded_from_file: bool,
/// Top-level legacy keys present in the loaded file that v1 renames or
/// removes (see [`legacy_top_level_keys`]). Populated by `load_config_in`
/// from a raw scan; drives the legacy deprecation warnings. Empty under
/// `version: 1` (those keys are rejected at load) and for the built-in default.
#[serde(skip)]
legacy_keys: Vec<String>,
}
impl Default for OmnigraphConfig {
@ -364,13 +374,14 @@ impl Default for OmnigraphConfig {
graphs: BTreeMap::new(),
server: ServerDefaults::default(),
auth: AuthDefaults::default(),
cli: CliDefaults::default(),
defaults: CliDefaults::default(),
query: QueryDefaults::default(),
aliases: BTreeMap::new(),
policy: PolicySettings::default(),
queries: BTreeMap::new(),
base_dir: PathBuf::new(),
loaded_from_file: false,
legacy_keys: Vec::new(),
}
}
}
@ -380,24 +391,24 @@ impl OmnigraphConfig {
&self.base_dir
}
pub fn cli_branch(&self) -> &str {
self.cli.branch.as_deref().unwrap_or("main")
pub fn default_branch(&self) -> &str {
self.defaults.branch.as_deref().unwrap_or("main")
}
pub fn cli_output_format(&self) -> ReadOutputFormat {
self.cli.output_format.unwrap_or_default()
pub fn default_output_format(&self) -> ReadOutputFormat {
self.defaults.output_format.unwrap_or_default()
}
pub fn table_max_column_width(&self) -> usize {
self.cli.table_max_column_width.unwrap_or(80)
self.defaults.table_max_column_width.unwrap_or(80)
}
pub fn table_cell_layout(&self) -> TableCellLayout {
self.cli.table_cell_layout.unwrap_or_default()
self.defaults.table_cell_layout.unwrap_or_default()
}
pub fn cli_graph_name(&self) -> Option<&str> {
self.cli.graph.as_deref()
pub fn default_graph_name(&self) -> Option<&str> {
self.defaults.graph.as_deref()
}
pub fn server_graph_name(&self) -> Option<&str> {
@ -514,7 +525,10 @@ impl OmnigraphConfig {
}
pub fn resolve_policy_tooling_graph_selection(&self) -> Result<Option<&str>> {
self.resolve_graph_selection(self.cli_graph_name().or_else(|| self.server_graph_name()))
self.resolve_graph_selection(
self.default_graph_name()
.or_else(|| self.server_graph_name()),
)
}
/// The policy file that applies for a graph selection — the policy
@ -618,14 +632,13 @@ impl OmnigraphConfig {
.to_string(),
);
}
// Legacy-only notices for keys that `version: 1` rejects at load (so they
// can only reach here under the lenient legacy schema). `project:` has no
// consumer and is removed in v1.
if self.version.is_none() && self.project.name.is_some() {
warnings.push(
"`project:` has no effect and is removed under `version: 1`; delete it."
.to_string(),
);
// Legacy-only notices for top-level keys that `version: 1` rejects at
// load (so a populated `legacy_keys` only ever occurs under the lenient
// legacy schema). Shares the migration hints with the v1 rejection.
for key in &self.legacy_keys {
if let Some(hint) = legacy_key_migration_hint(key) {
warnings.push(format!("`{key}:` — {hint}"));
}
}
for (name, entry) in &self.graphs {
if entry.storage.is_none() && entry.server.is_none() && !entry.uri.is_empty() {
@ -673,7 +686,7 @@ impl OmnigraphConfig {
});
}
let name = name.or_else(|| self.cli_graph_name()).ok_or_else(|| {
let name = name.or_else(|| self.default_graph_name()).ok_or_else(|| {
color_eyre::eyre::eyre!("URI must be provided via <URI>, --graph, or config")
})?;
@ -865,7 +878,7 @@ fn load_config_in(cwd: &Path, config_path: Option<&PathBuf>) -> Result<Omnigraph
let text = fs::read_to_string(path)?;
let mut unknown: Vec<String> = Vec::new();
let de = serde_yaml::Deserializer::from_str(&text);
let config: OmnigraphConfig =
let mut config: OmnigraphConfig =
serde_ignored::deserialize(de, |key| unknown.push(key.to_string()))?;
// Strictness is a function of the version, decided here — the one place
// the loader holds both the parsed version and the set of ignored fields.
@ -887,7 +900,27 @@ fn load_config_in(cwd: &Path, config_path: Option<&PathBuf>) -> Result<Omnigraph
}
_ => {}
}
reject_legacy_top_level_keys_under_v1(&text, config.version)?;
// Known-but-legacy top-level keys (renamed/removed by v1) are invisible
// to `serde_ignored` because they stay parseable for the legacy schema,
// so scan the raw text for them: reject under v1, record for the legacy
// deprecation warnings otherwise.
let legacy_keys = legacy_top_level_keys(&text);
if config.version == Some(1) && !legacy_keys.is_empty() {
let offenders = legacy_keys
.iter()
.map(|key| {
format!(
"`{key}:` — {}",
legacy_key_migration_hint(key).unwrap_or("")
)
})
.collect::<Vec<_>>()
.join("\n ");
bail!(
"invalid key(s) under `version: 1`:\n {offenders}\n(omit `version:` for the legacy lenient schema)"
);
}
config.legacy_keys = legacy_keys;
config
} else {
OmnigraphConfig::default()
@ -925,34 +958,29 @@ fn absolute_base_dir(cwd: &Path, path: &Path) -> Result<PathBuf> {
/// silently ignored). The set grows as each rename/removal lands.
fn legacy_key_migration_hint(key: &str) -> Option<&'static str> {
match key {
"project" => Some("has no effect and is removed under `version: 1`; delete it"),
"project" => Some("remove it; it has no effect under `version: 1`"),
"cli" => Some("rename to `defaults:`"),
_ => None,
}
}
/// Reject any v1-illegal top-level key (see [`legacy_key_migration_hint`]) when
/// `version: 1` is set. Scans the raw top-level mapping rather than the typed
/// config because the offending keys are known struct fields kept for legacy
/// parsing — rejection keys on the *presence* of the key (an empty `policy: {}`
/// under v1 is a silent no-op and must error too), which only a raw scan can see.
fn reject_legacy_top_level_keys_under_v1(text: &str, version: Option<u32>) -> Result<()> {
if version != Some(1) {
return Ok(());
}
/// The top-level keys present in the raw config text that v1 renames or removes
/// (those with a [`legacy_key_migration_hint`]), sorted. Scans the raw mapping
/// rather than the typed config because these keys are honored under the legacy
/// schema (so they stay parseable) — detection keys on the *presence* of the
/// key (an empty `cli: {}`/`policy: {}` under v1 is a silent no-op and must
/// error too), which only a raw scan can see. Drives both the `version: 1`
/// rejection and the legacy deprecation warnings.
fn legacy_top_level_keys(text: &str) -> Vec<String> {
let mapping: serde_yaml::Mapping = serde_yaml::from_str(text).unwrap_or_default();
let mut offenders: Vec<String> = mapping
let mut keys: Vec<String> = mapping
.keys()
.filter_map(serde_yaml::Value::as_str)
.filter_map(|key| legacy_key_migration_hint(key).map(|hint| format!("`{key}:` — {hint}")))
.filter(|key| legacy_key_migration_hint(key).is_some())
.map(str::to_string)
.collect();
if !offenders.is_empty() {
offenders.sort();
bail!(
"invalid key(s) under `version: 1`:\n {}\n(omit `version:` for the legacy lenient schema)",
offenders.join("\n ")
);
}
Ok(())
keys.sort();
keys
}
#[cfg(test)]
@ -991,13 +1019,13 @@ policy: {}
.unwrap();
let config = load_config_in(temp.path(), None).unwrap();
assert_eq!(config.cli_graph_name(), Some("local"));
assert_eq!(config.cli_branch(), "main");
assert_eq!(config.cli_output_format(), ReadOutputFormat::Kv);
assert_eq!(config.default_graph_name(), Some("local"));
assert_eq!(config.default_branch(), "main");
assert_eq!(config.default_output_format(), ReadOutputFormat::Kv);
assert_eq!(config.table_max_column_width(), 40);
assert_eq!(config.table_cell_layout(), TableCellLayout::Wrap);
assert_eq!(
config.graph_bearer_token_env(None, None, config.cli_graph_name()),
config.graph_bearer_token_env(None, None, config.default_graph_name()),
Some("DEMO_TOKEN")
);
assert_eq!(
@ -1007,7 +1035,7 @@ policy: {}
assert_eq!(
PathBuf::from(
config
.resolve_target_uri(None, None, config.cli_graph_name())
.resolve_target_uri(None, None, config.default_graph_name())
.unwrap()
),
temp.path().join("./demo.omni")
@ -1328,7 +1356,7 @@ cli:
config.graph_bearer_token_env(
Some("https://override.example.com"),
None,
config.cli_graph_name()
config.default_graph_name()
),
None
);
@ -1336,7 +1364,7 @@ cli:
config.graph_bearer_token_env(
Some("https://override.example.com"),
Some("demo"),
config.cli_graph_name()
config.default_graph_name()
),
Some("DEMO_TOKEN")
);
@ -1347,15 +1375,15 @@ cli:
let temp = tempdir().unwrap();
fs::write(
temp.path().join("omnigraph.yaml"),
"version: 1\ngraphs:\n local:\n uri: ./demo.omni\ncli:\n graph: local\n",
"version: 1\ngraphs:\n local:\n uri: ./demo.omni\ndefaults:\n graph: local\n",
)
.unwrap();
let config = load_config_in(temp.path(), None).unwrap();
assert_eq!(config.cli_graph_name(), Some("local"));
assert_eq!(config.default_graph_name(), Some("local"));
assert_eq!(
PathBuf::from(
config
.resolve_target_uri(None, None, config.cli_graph_name())
.resolve_target_uri(None, None, config.default_graph_name())
.unwrap()
),
temp.path().join("./demo.omni")
@ -1390,18 +1418,18 @@ cli:
"graphs:\n local:\n uri: ./demo.omni\n\
future_top_level_key: whatever\ncli:\n graph: local\n",
);
assert_eq!(config.cli_graph_name(), Some("local"));
assert_eq!(config.default_graph_name(), Some("local"));
}
#[test]
fn version_one_rejects_unknown_nested_field() {
// `version: 1` is strict at ALL depths via serde_ignored, not only at the
// structs that carry their own `deny_unknown_fields`: a typo inside a
// nested block such as `cli:` must error, naming the offending key
// nested block such as `defaults:` must error, naming the offending key
// (honored-or-rejected, RFC-002 §3).
let err = load_yaml_err(
"version: 1\ngraphs:\n local:\n uri: ./demo.omni\n\
cli:\n graph: local\n outout_format: kv\n",
defaults:\n graph: local\n outout_format: kv\n",
);
assert!(
err.contains("outout_format") || err.contains("unknown config field"),
@ -1409,6 +1437,52 @@ cli:
);
}
#[test]
fn version_one_rejects_legacy_cli_key() {
// `cli:` is renamed to `defaults:` under v1; writing it is a hard error
// that points at the new spelling (a known struct field via the `cli`
// alias, so only the raw key scan catches it).
let err = load_yaml_err(
"version: 1\ngraphs:\n local:\n storage: ./demo.omni\ncli:\n graph: local\n",
);
assert!(
err.contains("cli") && err.contains("rename to `defaults:`"),
"v1 must reject `cli:` and point at `defaults:`: {err}"
);
}
#[test]
fn version_one_honors_defaults_block() {
let config = load_yaml(
"version: 1\ngraphs:\n local:\n storage: ./demo.omni\n\
defaults:\n graph: local\n branch: dev\n output_format: kv\n",
);
assert_eq!(config.default_graph_name(), Some("local"));
assert_eq!(config.default_branch(), "dev");
assert_eq!(config.default_output_format(), ReadOutputFormat::Kv);
assert!(
config.deprecation_warnings().is_empty(),
"clean v1 `defaults:` config must not warn: {:?}",
config.deprecation_warnings()
);
}
#[test]
fn legacy_cli_key_honored_via_alias_and_warned() {
// No `version:` ⇒ the legacy `cli:` spelling is accepted (serde alias of
// `defaults`) and still honored, but flagged for migration.
let config = load_yaml("graphs:\n local:\n uri: ./demo.omni\ncli:\n graph: local\n");
assert_eq!(config.default_graph_name(), Some("local"));
assert!(
config
.deprecation_warnings()
.iter()
.any(|w| w.contains("cli") && w.contains("defaults")),
"legacy `cli:` must warn to migrate: {:?}",
config.deprecation_warnings()
);
}
#[test]
fn version_one_rejects_legacy_project_key() {
// `version: 1` removes the `project:` block (no consumer). Writing it

2
omnigraph.example.yaml
View file

@ -11,7 +11,7 @@ graphs:
server: prod
bearer_token_env: OMNIGRAPH_BEARER_TOKEN
cli:
defaults:
graph: local
branch: main