From 304ac5ec23f20e11af00c2af71940f0db7baf249 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Thu, 4 Jun 2026 21:38:25 +0200 Subject: [PATCH] refactor(config,cli,server): rename the `server:` config block to `serve:` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces the `serve:` host-role block in its RFC-002 shape — `graphs:` is a served-set list, replacing the single `server.graph` scalar. The legacy `server:` block is folded into `serve:` under the legacy schema and rejected under `version: 1` (pointing at the new spelling, noting the scalar->list change). Serving a true subset (`serve.graphs` with more than one entry) is rejected with a route-unification hint; one entry (single-graph) and none (serve all) preserve today's behavior. Renames the accessors (server_* -> serve_*) and repoints the server boot call sites; migrates the init scaffold to `serve:`. `servers:` (the remote endpoint map) is deliberately unaffected — the key scan is exact-match. --- crates/omnigraph-cli/src/main.rs | 5 +- crates/omnigraph-config/src/lib.rs | 137 +++++++++++++++++++++++++++-- crates/omnigraph-server/src/lib.rs | 15 ++-- 3 files changed, 138 insertions(+), 19 deletions(-) diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index 1e8c468..c537d36 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -1579,8 +1579,9 @@ graphs: storage: {} # bearer_token_env: OMNIGRAPH_BEARER_TOKEN -server: - graph: local +serve: + graphs: + - local bind: 127.0.0.1:8080 defaults: diff --git a/crates/omnigraph-config/src/lib.rs b/crates/omnigraph-config/src/lib.rs index 5c2acd8..e388d48 100644 --- a/crates/omnigraph-config/src/lib.rs +++ b/crates/omnigraph-config/src/lib.rs @@ -220,6 +220,26 @@ pub struct ServerDefaults { pub policy: PolicySettings, } +/// Host-role serving config (`serve:`) — RFC-002. Supersedes the legacy +/// `server:` block (a single `graph:` scalar), which is folded into this at +/// load under the legacy schema; under `version: 1` `server:` is rejected and +/// `serve:` is authoritative. `serve:` is v1-only syntax with no legacy spelling, +/// so it is always strict (`deny_unknown_fields`), like `storage:`/`servers:`. +/// +/// `graphs` is the served set. Today only 0 or 1 entries are honored — one ⇒ +/// single-graph mode, none ⇒ serve every embedded graph — matching the shipped +/// behavior of the old `server.graph` selector. Serving a true subset (>1 +/// entry) is rejected until route unification lands. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct Serve { + #[serde(default)] + pub graphs: Vec, + pub bind: Option, + #[serde(default)] + pub policy: PolicySettings, +} + #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct AuthDefaults { pub env_file: Option, @@ -328,6 +348,14 @@ pub struct OmnigraphConfig { pub servers: BTreeMap, #[serde(default, rename = "graphs")] pub graphs: BTreeMap, + /// Host-role serving config (`serve:`) — RFC-002. The legacy `server:` block + /// (below) is folded into this under the legacy schema; under `version: 1` + /// `server:` is rejected (see [`legacy_top_level_keys`]). Reads go through + /// the `serve_*` accessors. + #[serde(default)] + pub serve: Serve, + /// Legacy spelling of `serve:` (no `version:`); rejected under v1, folded + /// into `serve` at load. Do not read directly — use the `serve_*` accessors. #[serde(default)] pub server: ServerDefaults, #[serde(default)] @@ -372,6 +400,7 @@ impl Default for OmnigraphConfig { project: ProjectConfig::default(), servers: BTreeMap::new(), graphs: BTreeMap::new(), + serve: Serve::default(), server: ServerDefaults::default(), auth: AuthDefaults::default(), defaults: CliDefaults::default(), @@ -411,12 +440,12 @@ impl OmnigraphConfig { self.defaults.graph.as_deref() } - pub fn server_graph_name(&self) -> Option<&str> { - self.server.graph.as_deref() + pub fn serve_graph_name(&self) -> Option<&str> { + self.serve.graphs.first().map(String::as_str) } - pub fn server_bind(&self) -> &str { - self.server.bind.as_deref().unwrap_or("127.0.0.1:8080") + pub fn serve_bind(&self) -> &str { + self.serve.bind.as_deref().unwrap_or("127.0.0.1:8080") } pub fn resolve_target_name<'a>( @@ -527,7 +556,7 @@ impl OmnigraphConfig { pub fn resolve_policy_tooling_graph_selection(&self) -> Result> { self.resolve_graph_selection( self.default_graph_name() - .or_else(|| self.server_graph_name()), + .or_else(|| self.serve_graph_name()), ) } @@ -592,9 +621,9 @@ impl OmnigraphConfig { } /// Resolve the server-level policy file path (used by management - /// endpoints). Returns `None` if `server.policy.file` is not set. - pub fn resolve_server_policy_file(&self) -> Option { - self.server + /// endpoints). Returns `None` if `serve.policy.file` is not set. + pub fn resolve_serve_policy_file(&self) -> Option { + self.serve .policy .file .as_deref() @@ -746,6 +775,32 @@ impl OmnigraphConfig { } } + /// Fold the legacy `server:` block into the canonical `serve:` (legacy + /// schema only — under `version: 1` `server:` is rejected at load, so the + /// fold is a no-op there) and validate the served set. The legacy single + /// `graph:` scalar maps to a one-element `graphs:` list. Serving a true + /// subset (more than one graph) is rejected until route unification. + fn normalize_serve(&mut self) -> Result<()> { + if self.version.is_none() { + let legacy = std::mem::take(&mut self.server); + if legacy.graph.is_some() || legacy.bind.is_some() || legacy.policy.file.is_some() { + self.serve = Serve { + graphs: legacy.graph.into_iter().collect(), + bind: legacy.bind, + policy: legacy.policy, + }; + } + } + if self.serve.graphs.len() > 1 { + bail!( + "`serve.graphs` lists {} graphs, but serving a subset is not yet \ + supported (it lands with route unification); list at most one graph", + self.serve.graphs.len() + ); + } + Ok(()) + } + /// Fill each graph entry's `uri` from `storage`/`server` and validate the /// locator shape (RFC-002 §1.2/§3), so existing `uri`-readers see a /// resolved base URI and invalid combinations fail at load. @@ -934,6 +989,7 @@ fn load_config_in(cwd: &Path, config_path: Option<&PathBuf>) -> Result Option<&'static str> { match key { "project" => Some("remove it; it has no effect under `version: 1`"), "cli" => Some("rename to `defaults:`"), + "server" => Some("rename to `serve:` (note `graph:` becomes the `graphs:` list)"), _ => None, } } @@ -1483,6 +1540,70 @@ cli: ); } + #[test] + fn version_one_rejects_legacy_server_key() { + let err = load_yaml_err( + "version: 1\ngraphs:\n local:\n storage: ./demo.omni\nserver:\n graph: local\n", + ); + assert!( + err.contains("server") && err.contains("rename to `serve:`"), + "v1 must reject `server:` and point at `serve:`: {err}" + ); + } + + #[test] + fn version_one_honors_serve_block() { + let config = load_yaml( + "version: 1\ngraphs:\n local:\n storage: ./demo.omni\n\ + serve:\n graphs:\n - local\n bind: 0.0.0.0:9000\n", + ); + assert_eq!(config.serve_graph_name(), Some("local")); + assert_eq!(config.serve_bind(), "0.0.0.0:9000"); + assert!(config.deprecation_warnings().is_empty()); + } + + #[test] + fn servers_plural_accepted_under_v1() { + // `servers:` (the remote endpoint map) is one letter from the rejected + // `server:`; the exact-match key scan must not flag it. + let config = load_yaml( + "version: 1\nservers:\n prod:\n endpoint: https://og.example\n\ + graphs:\n prod:\n server: prod\n", + ); + assert!(config.servers.contains_key("prod")); + } + + #[test] + fn serve_graphs_multi_entry_rejected_until_route_unification() { + let err = load_yaml_err( + "version: 1\ngraphs:\n a:\n storage: ./a.omni\n b:\n storage: ./b.omni\n\ + serve:\n graphs:\n - a\n - b\n", + ); + assert!( + err.contains("serve.graphs") && err.contains("subset"), + "serving >1 graph must be rejected with a route-unification hint: {err}" + ); + } + + #[test] + fn legacy_server_block_folds_into_serve_and_warns() { + // No `version:` ⇒ the legacy `server:` block (scalar `graph:`) is folded + // into `serve` (one-element `graphs:`), honored, and flagged. + let config = load_yaml( + "graphs:\n local:\n uri: ./demo.omni\nserver:\n graph: local\n bind: 0.0.0.0:9000\n", + ); + assert_eq!(config.serve_graph_name(), Some("local")); + assert_eq!(config.serve_bind(), "0.0.0.0:9000"); + assert!( + config + .deprecation_warnings() + .iter() + .any(|w| w.contains("server") && w.contains("serve")), + "legacy `server:` 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 diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index 8d10d2d..7056b03 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -921,7 +921,7 @@ pub fn load_server_settings( for warning in config.deprecation_warnings() { warn!("{warning}"); } - let bind = cli_bind.unwrap_or_else(|| config.server_bind().to_string()); + let bind = cli_bind.unwrap_or_else(|| config.serve_bind().to_string()); // Either `--unauthenticated` or `OMNIGRAPH_UNAUTHENTICATED=1` flips // this. Treat any non-empty, non-"0"/"false" string as truthy — // standard 12-factor "any value is true" reading of the env var. @@ -948,7 +948,7 @@ pub fn load_server_settings( // `resolve_target_uri` precedence. let has_cli_uri = cli_uri.is_some(); let has_cli_target = cli_target.is_some(); - let has_server_graph = config.server_graph_name().is_some(); + let has_server_graph = config.serve_graph_name().is_some(); let has_graphs_map = !config.graphs.is_empty(); let has_explicit_config = config_path.is_some(); @@ -961,7 +961,7 @@ pub fn load_server_settings( let selected: Option<&str> = if has_cli_uri { None } else { - cli_target.as_deref().or_else(|| config.server_graph_name()) + cli_target.as_deref().or_else(|| config.serve_graph_name()) }; // omnigraph-server serves embedded graphs only — refuse a remote target // (`server:`/remote `uri:`) before resolving or opening it. @@ -971,11 +971,8 @@ pub fn load_server_settings( selected, selected.or(cli_uri.as_deref()).unwrap_or(""), )?; - let raw_uri = config.resolve_target_uri( - cli_uri, - cli_target.as_deref(), - config.server_graph_name(), - )?; + let raw_uri = + config.resolve_target_uri(cli_uri, cli_target.as_deref(), config.serve_graph_name())?; let uri = normalize_root_uri(&raw_uri).wrap_err_with(|| { format!("normalize single-graph URI '{raw_uri}' from server settings") })?; @@ -1044,7 +1041,7 @@ pub fn load_server_settings( let config_path = config_path .cloned() .expect("has_explicit_config implies config_path is Some"); - let server_policy_file = config.resolve_server_policy_file(); + let server_policy_file = config.resolve_serve_policy_file(); ServerConfigMode::Multi { graphs, config_path,