From b47e3bbc709a1a3e8603da54460a7f77d56fcf06 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Sat, 30 May 2026 21:53:07 +0200 Subject: [PATCH] Route registry selection through one shared query_entries_for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "which queries: block applies for graph X" rule existed twice — the server boot path and the CLI's registry_entries — and had already drifted: the CLI carried an unreachable unwrap_or_else fallback the server lacked. Add OmnigraphConfig::query_entries_for(graph: Option<&str>) as the single definition (named graph -> its per-graph block; otherwise top-level) and route all three sites through it: server single mode, server multi-graph loop, and the CLI. The CLI's dead fallback arm is deleted; CLI and server now resolve identically by construction. No behavior change. Extends the config round-trip test to pin the selector, including the unknown-name -> top-level fallback the deleted CLI arm covered. --- crates/omnigraph-cli/src/main.rs | 14 +++++--------- crates/omnigraph-server/src/config.rs | 22 ++++++++++++++++++++++ crates/omnigraph-server/src/lib.rs | 10 ++++++---- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index bb4906e..a0fb3b5 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -1679,19 +1679,15 @@ struct QueriesListOutput { queries: Vec, } -/// Resolve the stored-query registry entries for the selected graph, -/// mirroring the server: a named graph in `graphs:` uses its per-graph -/// `queries:`; otherwise the top-level `queries:` (single-graph mode). +/// Resolve the stored-query registry entries for the selected graph via +/// the same `query_entries_for` the server boot uses, so the CLI +/// validates exactly the block the server would load. A `--target` +/// overrides the configured default graph. fn registry_entries<'a>( config: &'a OmnigraphConfig, target: Option<&str>, ) -> &'a std::collections::BTreeMap { - match target.or_else(|| config.cli_graph_name()) { - Some(name) if config.graphs.contains_key(name) => config - .target_query_entries(name) - .unwrap_or_else(|| config.query_entries()), - _ => config.query_entries(), - } + config.query_entries_for(target.or_else(|| config.cli_graph_name())) } fn load_registry_or_report(config: &OmnigraphConfig, target: Option<&str>) -> Result { diff --git a/crates/omnigraph-server/src/config.rs b/crates/omnigraph-server/src/config.rs index aa697eb..6e2eae0 100644 --- a/crates/omnigraph-server/src/config.rs +++ b/crates/omnigraph-server/src/config.rs @@ -301,6 +301,20 @@ impl OmnigraphConfig { self.graphs.get(target_name).map(|target| &target.queries) } + /// The stored-query registry entries that apply for a graph + /// selection — the single definition of "which `queries:` block + /// governs graph X", shared by server boot and the CLI so the two + /// can't drift. A named graph present in `graphs:` uses its + /// per-graph block; everything else (no selection, or a name that is + /// not a known graph, e.g. a bare URI) falls back to the top-level + /// block (single-graph mode). + pub fn query_entries_for(&self, graph: Option<&str>) -> &BTreeMap { + match graph { + Some(name) if self.graphs.contains_key(name) => &self.graphs[name].queries, + _ => &self.queries, + } + } + /// Resolve a stored-query `.gq` file path (from a registry entry), /// relative to the config's `base_dir`. Mirrors policy-file /// resolution; the registry loader calls this to turn each entry's @@ -595,6 +609,14 @@ queries: // Top-level registry (single-graph mode). assert_eq!(config.query_entries().len(), 1); + // The shared selector resolves the same blocks the server boot + // and the CLI use: a known graph → its per-graph block; no + // selection or an unknown name → the top-level block (the latter + // pins the behavior of the CLI's now-deleted fallback arm). + assert_eq!(config.query_entries_for(Some("prod")).len(), 2); + assert_eq!(config.query_entries_for(None).len(), 1); + assert_eq!(config.query_entries_for(Some("nonexistent")).len(), 1); + // Path resolution joins against base_dir, like policy files. assert_eq!( config.resolve_query_file(&find_user.file), diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index 3cc26d5..7c6f93c 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -910,7 +910,7 @@ pub fn load_server_settings( // Single mode uses the top-level `queries:` (mirrors top-level // `policy.file`). Load + identity-check now (no engine needed); // the schema type-check happens when the engine opens. - let queries = QueryRegistry::load(&config, config.query_entries()) + let queries = QueryRegistry::load(&config, config.query_entries_for(None)) .map_err(|errs| color_eyre::eyre::eyre!(format_registry_load_errors(&uri, &errs)))?; ServerConfigMode::Single { uri, @@ -939,9 +939,11 @@ pub fn load_server_settings( let uri = normalize_root_uri(&raw_uri).wrap_err_with(|| { format!("normalize URI '{raw_uri}' for graph '{name}' in omnigraph.yaml") })?; - // Per-graph `queries:`. Load + identity-check now; the schema - // type-check happens when this graph's engine opens. - let queries = QueryRegistry::load(&config, &target.queries) + // Per-graph `queries:`, selected through the shared + // `query_entries_for` so server and CLI resolve identically. + // Load + identity-check now; the schema type-check happens + // when this graph's engine opens. + let queries = QueryRegistry::load(&config, config.query_entries_for(Some(name.as_str()))) .map_err(|errs| color_eyre::eyre::eyre!(format_registry_load_errors(name, &errs)))?; graphs.push(GraphStartupConfig { graph_id: name.clone(),