From 5c124b1afe2816172d267abb6046024775f64ee5 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Mon, 1 Jun 2026 12:38:08 +0200 Subject: [PATCH] Enforce top-level coherence in the single CLI selection gate queries validate validated graph membership only as a side effect of URI resolution and queries list only via resolve_graph_selection's membership check; neither applied the named-graph/top-level coherence rule server boot enforces, so both gave a false green on a config boot refuses. Fold ensure_top_level_blocks_honored into resolve_graph_selection so it is the single gate that returns only valid + server-coherent selections, and route resolve_selected_graph (queries validate) through it; queries list already calls the gate. A named graph with a populated top-level block now errors in both commands, matching boot. A positional URI stays anonymous (top-level honored), so queries_validate_positional_uri_ignores_default_graph is unaffected. --- crates/omnigraph-cli/src/main.rs | 5 +++ crates/omnigraph-server/src/config.rs | 54 ++++++++++++++++++++++----- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index 8b47691..a3bee2e 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -1698,6 +1698,11 @@ fn resolve_selected_graph( .map(str::to_string) .or_else(|| config.cli_graph_name().map(str::to_string)) }; + // Validate the selection through the single gate (membership + coherence), + // so a positional URI stays anonymous and a named graph is rejected when a + // top-level block would be silently ignored — matching server boot. `list` + // already routes through the same gate; this keeps `validate` in step. + config.resolve_graph_selection(selected.as_deref())?; let uri = resolve_local_uri(config, cli_uri, cli_target, operation)?; Ok((uri, selected)) } diff --git a/crates/omnigraph-server/src/config.rs b/crates/omnigraph-server/src/config.rs index cbb096f..2e83d1a 100644 --- a/crates/omnigraph-server/src/config.rs +++ b/crates/omnigraph-server/src/config.rs @@ -331,16 +331,27 @@ impl OmnigraphConfig { } } - /// Validate a graph selection against `graphs:` — the fallible - /// counterpart to the infallible [`OmnigraphConfig::query_entries_for`]. - /// A known name passes through; an unknown name errors (the **same** - /// message [`OmnigraphConfig::resolve_target_uri`] produces, so a command - /// that opens no URI rejects an unknown `--target` exactly like the - /// URI-resolving commands do); an anonymous selection (`None`) stays - /// anonymous, resolving to the top-level registry downstream. + /// The single CLI gate that turns a raw graph selection into a *validated* + /// one — the fallible counterpart to the infallible + /// [`OmnigraphConfig::query_entries_for`]. Both `queries` subcommands route + /// their selection through here so neither can skip a check the other (or + /// server boot) applies: + /// * a known name passes through, but only after the same coherence check + /// server boot enforces + /// ([`OmnigraphConfig::ensure_top_level_blocks_honored`]) — a named graph + /// with a populated top-level block is rejected; + /// * an unknown name errors with the **same** message + /// [`OmnigraphConfig::resolve_target_uri`] produces, so a command that + /// opens no URI rejects an unknown `--target` exactly like the + /// URI-resolving commands do; + /// * an anonymous selection (`None`, e.g. a bare URI) stays anonymous, + /// resolving to the top-level registry downstream (top-level honored). pub fn resolve_graph_selection<'a>(&self, graph: Option<&'a str>) -> Result> { match graph { - Some(name) if self.graphs.contains_key(name) => Ok(Some(name)), + Some(name) if self.graphs.contains_key(name) => { + self.ensure_top_level_blocks_honored(Some(name))?; + Ok(Some(name)) + } Some(name) => bail!("graph '{}' not found in {}", name, DEFAULT_CONFIG_FILE), None => Ok(None), } @@ -614,7 +625,7 @@ policy: {} } #[test] - fn resolve_graph_selection_validates_membership() { + fn resolve_graph_selection_validates_membership_and_coherence() { let temp = tempdir().unwrap(); fs::write( temp.path().join("omnigraph.yaml"), @@ -633,6 +644,31 @@ policy: {} err.contains("ghost") && err.contains("not found"), "unknown graph must error naming it: {err}" ); + + // Coherence: a named graph plus a populated top-level block is the + // config server boot refuses, so the gate rejects it too (shared rule + // via ensure_top_level_blocks_honored). An anonymous selection still + // passes — top-level is honored when no graph is named. + let temp2 = tempdir().unwrap(); + fs::write( + temp2.path().join("omnigraph.yaml"), + "graphs:\n local:\n uri: ./demo.omni\npolicy:\n file: ./top.yaml\n", + ) + .unwrap(); + let incoherent = load_config_in(temp2.path(), None).unwrap(); + let err = incoherent + .resolve_graph_selection(Some("local")) + .unwrap_err() + .to_string(); + assert!( + err.contains("local") && err.contains("policy.file"), + "named graph + populated top-level block must be rejected, naming both: {err}" + ); + assert_eq!( + incoherent.resolve_graph_selection(None).unwrap(), + None, + "anonymous selection still honors top-level" + ); } #[test]