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

feat(config): OMNIGRAPH_NO_LEGACY_CONFIG strict mode (RFC-008 stage 4)

Browse source 
Opt-in: with the env set, loading a legacy omnigraph.yaml is a hard
error pointing at config migrate — the regression guard for migrated
teams (a stray legacy file would otherwise silently outrank operator
config during the window) and the rehearsal for stage 5's removal.
Strict refuses the FILE, never its absence: flag-less invocations on
migrated setups are untouched. Inert unless set.

The RFC's stages-1-3-then-4 release gap collapsed honestly: no version
boundary was crossed between them, so all four ship in the same release
(noted in the RFC).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
aaltshuler 2026-06-12 00:03:10 +03:00
parent 108d2defa6
commit 4c50170c77
4 changed files with 95 additions and 21 deletions
Download patch file Download diff file Expand all files Collapse all files

36
crates/omnigraph-cli/tests/cli_schema_config.rs
View file

@ -610,3 +610,39 @@ fn config_migrate_splits_legacy_config() {
assert!(output.status.success(), "{output:?}");
assert!(temp.path().join("cluster.yaml.proposed").exists());
}
/// RFC-008 stage 4: OMNIGRAPH_NO_LEGACY_CONFIG refuses a present legacy
/// file (pointing at config migrate) but changes nothing on migrated
/// setups with no file.
#[test]
fn strict_mode_refuses_legacy_file_but_not_its_absence() {
let temp = tempdir().unwrap();
fs::write(temp.path().join("omnigraph.yaml"), "cli:\n actor: a\n").unwrap();
let output = cli()
.current_dir(temp.path())
.env("OMNIGRAPH_NO_LEGACY_CONFIG", "1")
.arg("graphs")
.arg("list")
.arg("--json")
.output()
.unwrap();
assert!(!output.status.success());
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(
stderr.contains("OMNIGRAPH_NO_LEGACY_CONFIG") && stderr.contains("config migrate"),
"{stderr}"
);
// Migrated setup (no file): strict mode is a no-op — a config-loading
// command that tolerates empty defaults succeeds.
let clean = tempdir().unwrap();
let output = cli()
.current_dir(clean.path())
.env("OMNIGRAPH_NO_LEGACY_CONFIG", "1")
.arg("queries")
.arg("list")
.arg("--json")
.output()
.unwrap();
assert!(output.status.success(), "{output:?}");
}

74
crates/omnigraph-server/src/config.rs
View file

@ -531,15 +531,24 @@ pub fn default_config_path() -> PathBuf {
/// uses it for the server; RFC-007 §D1 extends it to the CLI).
pub const CONFIG_PATH_ENV: &str = "OMNIGRAPH_CONFIG";
/// RFC-008 stage 4 — opt-in strict mode: when set, loading a legacy
/// `omnigraph.yaml` is a hard error instead of a warning. For teams that
/// finished migrating and want regressions caught (a stray legacy file
/// would otherwise silently outrank operator config during the window).
/// The rehearsal for stage 5's removal.
pub const NO_LEGACY_CONFIG_ENV: &str = "OMNIGRAPH_NO_LEGACY_CONFIG";
pub fn load_config(config_path: Option<&PathBuf>) -> Result<OmnigraphConfig> {
let env_path = env::var_os(CONFIG_PATH_ENV).map(PathBuf::from);
load_config_in(&env::current_dir()?, config_path, env_path.as_ref())
let strict = env::var_os(NO_LEGACY_CONFIG_ENV).is_some();
load_config_in(&env::current_dir()?, config_path, env_path.as_ref(), strict)
}
fn load_config_in(
cwd: &Path,
config_path: Option<&PathBuf>,
env_path: Option<&PathBuf>,
strict_no_legacy: bool,
) -> Result<OmnigraphConfig> {
// Precedence: explicit --config flag > $OMNIGRAPH_CONFIG > ./omnigraph.yaml.
let explicit_path = config_path.or(env_path).cloned();
@ -549,6 +558,14 @@ fn load_config_in(
});
let mut config = if let Some(path) = &config_path {
if strict_no_legacy {
// Strict refuses the FILE, not its absence — flag-less
// invocations on migrated setups keep working.
bail!(
"legacy config '{}' refused: {NO_LEGACY_CONFIG_ENV} is set (RFC-008 strict mode); run `omnigraph config migrate`, then remove the file — or unset the variable",
path.display()
);
}
let text = fs::read_to_string(path)?;
warn_yaml_deprecation_once(path, &text);
serde_yaml::from_str::<OmnigraphConfig>(&text)?
@ -665,19 +682,38 @@ mod tests {
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();
let config = load_config_in(temp.path(), None, Some(&env_path), false).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();
let config = load_config_in(temp.path(), Some(&flag_path), Some(&env_path), false).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();
let config = load_config_in(temp.path(), None, Some(&env_path), false).unwrap();
assert_eq!(config.cli.actor.as_deref(), Some("act-env"));
}
#[test]
fn strict_mode_refuses_the_file_not_its_absence() {
let temp = tempdir().unwrap();
// No file: strict mode changes nothing (defaults load).
let config = load_config_in(temp.path(), None, None, true).unwrap();
assert!(config.cli.actor.is_none());
// File present: strict refuses with the migrate pointer.
fs::write(temp.path().join("omnigraph.yaml"), "cli:\n actor: a\n").unwrap();
let err = load_config_in(temp.path(), None, None, true).unwrap_err();
let message = err.to_string();
assert!(
message.contains("OMNIGRAPH_NO_LEGACY_CONFIG") && message.contains("config migrate"),
"{message}"
);
// Without strict, the same file loads.
assert!(load_config_in(temp.path(), None, None, false).is_ok());
}
#[test]
fn yaml_deprecation_lines_name_present_keys_only() {
let lines = super::yaml_deprecation_lines(
@ -717,7 +753,7 @@ policy: {}
)
.unwrap();
let config = load_config_in(temp.path(), None, None).unwrap();
let config = load_config_in(temp.path(), None, None, false).unwrap();
assert_eq!(config.cli_graph_name(), Some("local"));
assert_eq!(config.cli_branch(), "main");
assert_eq!(config.cli_output_format(), ReadOutputFormat::Kv);
@ -752,7 +788,7 @@ policy: {}
)
.unwrap();
let config = load_config_in(&child, None, None).unwrap();
let config = load_config_in(&child, None, None, false).unwrap();
assert!(config.graphs.is_empty());
}
@ -776,7 +812,7 @@ policy: {}
"graphs:\n local:\n uri: ./demo.omni\n",
)
.unwrap();
let config = load_config_in(temp.path(), None, None).unwrap();
let config = load_config_in(temp.path(), None, None, false).unwrap();
// A known graph passes through unchanged.
assert_eq!(config.resolve_graph_selection(Some("local")).unwrap(), Some("local"));
@ -799,7 +835,7 @@ policy: {}
"graphs:\n local:\n uri: ./demo.omni\npolicy:\n file: ./top.yaml\n",
)
.unwrap();
let incoherent = load_config_in(temp2.path(), None, None).unwrap();
let incoherent = load_config_in(temp2.path(), None, None, false).unwrap();
let err = incoherent
.resolve_graph_selection(Some("local"))
.unwrap_err()
@ -824,7 +860,7 @@ policy: {}
server:\n graph: local\ncli:\n graph: prod\n",
)
.unwrap();
let config = load_config_in(temp.path(), None, None).unwrap();
let config = load_config_in(temp.path(), None, None, false).unwrap();
assert_eq!(
config.resolve_policy_tooling_graph_selection().unwrap(),
Some("prod")
@ -836,7 +872,7 @@ policy: {}
"graphs:\n local:\n uri: ./local.omni\nserver:\n graph: local\n",
)
.unwrap();
let config = load_config_in(temp.path(), None, None).unwrap();
let config = load_config_in(temp.path(), None, None, false).unwrap();
assert_eq!(
config.resolve_policy_tooling_graph_selection().unwrap(),
Some("local")
@ -844,7 +880,7 @@ policy: {}
let temp = tempdir().unwrap();
fs::write(temp.path().join("omnigraph.yaml"), "policy: {}\n").unwrap();
let config = load_config_in(temp.path(), None, None).unwrap();
let config = load_config_in(temp.path(), None, None, false).unwrap();
assert_eq!(config.resolve_policy_tooling_graph_selection().unwrap(), None);
let temp = tempdir().unwrap();
@ -853,7 +889,7 @@ policy: {}
"graphs:\n local:\n uri: ./local.omni\nserver:\n graph: ghost\n",
)
.unwrap();
let config = load_config_in(temp.path(), None, None).unwrap();
let config = load_config_in(temp.path(), None, None, false).unwrap();
let err = config
.resolve_policy_tooling_graph_selection()
.unwrap_err()
@ -879,7 +915,7 @@ policy: {}
)
.unwrap();
let config = load_config_in(temp.path(), None, None).unwrap();
let config = load_config_in(temp.path(), None, None, false).unwrap();
let resolved = config.resolve_query_path(Path::new("test.gq")).unwrap();
assert_eq!(resolved, temp.path().join("queries").join("test.gq"));
}
@ -896,7 +932,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")), None).unwrap();
load_config_in(&ambient_dir, Some(&config_dir.join("omnigraph.yaml")), None, false).unwrap();
let resolved = config.resolve_query_path(Path::new("local.gq")).unwrap();
assert_eq!(resolved, config_dir.join("local.gq"));
@ -926,7 +962,7 @@ queries:
)
.unwrap();
let config = load_config_in(temp.path(), None, None).unwrap();
let config = load_config_in(temp.path(), None, None, false).unwrap();
// Per-graph registry (multi-graph mode).
let prod = config.target_query_entries("prod").unwrap();
@ -967,7 +1003,7 @@ queries:
policy:\n file: ./prod.yaml\n bare:\n uri: s3://b/bare\n",
)
.unwrap();
let config = load_config_in(temp.path(), None, None).unwrap();
let config = load_config_in(temp.path(), None, None, false).unwrap();
// Named graph with its own policy → per-graph (not top-level).
assert!(
@ -1003,7 +1039,7 @@ queries:
)
.unwrap();
let config = load_config_in(temp.path(), None, None).unwrap();
let config = load_config_in(temp.path(), None, None, false).unwrap();
// Additive: no `queries:` anywhere → empty registries everywhere.
assert!(config.query_entries().is_empty());
assert!(
@ -1023,7 +1059,7 @@ queries:
)
.unwrap();
let config = load_config_in(temp.path(), None, None).unwrap();
let config = load_config_in(temp.path(), None, None, false).unwrap();
assert_eq!(
config.resolve_policy_file().unwrap(),
temp.path().join("policy.yaml")
@ -1046,7 +1082,7 @@ cli:
)
.unwrap();
let config = load_config_in(temp.path(), None, None).unwrap();
let config = load_config_in(temp.path(), None, None, false).unwrap();
assert_eq!(
config.graph_bearer_token_env(
Some("https://override.example.com"),

2
docs/dev/rfc-008-deprecate-omnigraph-yaml.md
View file

@ -132,7 +132,7 @@ contract), retirement is staged, loud, and tooled:
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
4. **Opt-in strict** *(landed — the release gap to stages 1–3 collapsed: no version boundary was crossed between them, so all four ship in the same release)*. `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
at `config migrate`. The `OmnigraphConfig` code path, the dual

4
docs/user/cli-reference.md
View file

@ -109,7 +109,9 @@ operator server use the legacy chain alone.
> 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.
> window. Migrated teams can set `OMNIGRAPH_NO_LEGACY_CONFIG=1` to turn
> any legacy-file load into a hard error (regression guard; the file's
> absence is always fine).
```yaml
project: { name }