diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index ff0239e..91becc6 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -9,6 +9,7 @@ use clap::{Arg, ArgAction, Args, CommandFactory, FromArgMatches, Parser, Subcomm use color_eyre::eyre::{Result, bail}; use omnigraph::db::{Omnigraph, ReadTarget, SnapshotId}; use omnigraph::loader::LoadMode; +use omnigraph::storage::normalize_root_uri; use omnigraph_compiler::query::parser::parse_query; use omnigraph_compiler::schema::parser::parse_schema; use omnigraph_compiler::{ @@ -27,7 +28,7 @@ use omnigraph_server::api::{ use omnigraph_server::queries::{QueryRegistry, check, format_check_breakages}; use omnigraph_server::{ AliasCommand, OmnigraphConfig, PolicyAction, PolicyDecision, PolicyEngine, PolicyRequest, - PolicyTestConfig, ReadOutputFormat, load_config, + PolicyTestConfig, ReadOutputFormat, graph_resource_id_for_selection, load_config, }; use reqwest::Method; use reqwest::header::AUTHORIZATION; @@ -782,6 +783,7 @@ fn load_cli_config(config_path: Option<&PathBuf>) -> Result { struct ResolvedCliGraph { uri: String, selected: Option, + graph_id: String, policy_file: Option, is_remote: bool, } @@ -807,14 +809,12 @@ fn resolve_policy_context(config: &OmnigraphConfig) -> Result.policy.file must be set in omnigraph.yaml" ) })?; - let graph_id = if let Some(name) = &config.project.name { - name.clone() - } else if let Some(selected) = selected.as_deref() { - config - .resolve_target_uri(None, Some(selected), None) - .unwrap_or_else(|_| selected.to_string()) - } else { - policy_graph_id_from_uri(config, None) + let graph_id = match selected.as_deref() { + Some(name) => graph_resource_id_for_selection(Some(name), ""), + None => { + let anonymous_graph_id = anonymous_policy_graph_id(config)?; + graph_resource_id_for_selection(None, &anonymous_graph_id) + } }; Ok(ResolvedPolicyContext { policy_file, @@ -826,32 +826,23 @@ fn resolve_policy_engine(context: &ResolvedPolicyContext) -> Result Result { +fn resolve_policy_engine_for_graph(graph: &ResolvedCliGraph) -> Result { let policy_file = graph.policy_file.as_ref().ok_or_else(|| { color_eyre::eyre::eyre!( "policy.file or graphs..policy.file must be set in omnigraph.yaml" ) })?; - PolicyEngine::load_graph( - policy_file, - &policy_graph_id_from_uri(config, Some(&graph.uri)), - ) + PolicyEngine::load_graph(policy_file, &graph.graph_id) } /// Open a local graph and install the policy resolved for the same graph /// identity that produced the URI. A named graph uses /// `graphs..policy.file`; an explicit positional URI is anonymous and /// uses the legacy top-level `policy.file`. -async fn open_local_db_with_policy( - graph: &ResolvedCliGraph, - config: &OmnigraphConfig, -) -> Result { +async fn open_local_db_with_policy(graph: &ResolvedCliGraph) -> Result { let db = Omnigraph::open(&graph.uri).await?; if graph.policy_file.is_some() { - let engine = Arc::new(resolve_policy_engine_for_graph(config, graph)?); + let engine = Arc::new(resolve_policy_engine_for_graph(graph)?); Ok(db.with_policy(engine as Arc)) } else { Ok(db) @@ -872,17 +863,22 @@ fn resolve_policy_tests_path(context: &ResolvedPolicyContext) -> PathBuf { context.policy_file.with_file_name("policy.tests.yaml") } -fn policy_graph_id_from_uri(config: &OmnigraphConfig, uri: Option<&str>) -> String { - if let Some(name) = &config.project.name { - return name.clone(); +fn normalize_policy_graph_uri(uri: &str) -> Result { + if is_remote_uri(uri) { + Ok(uri.trim_end_matches('/').to_string()) + } else { + Ok(normalize_root_uri(uri)?) } - if let Some(uri) = uri { - return uri.to_string(); - } - config +} + +fn anonymous_policy_graph_id(config: &OmnigraphConfig) -> Result { + let raw_uri = config .resolve_target_uri(None, None, config.server_graph_name()) - .or_else(|_| config.resolve_target_uri(None, None, config.cli_graph_name())) - .unwrap_or_else(|_| "default".to_string()) + .or_else(|_| config.resolve_target_uri(None, None, config.cli_graph_name())); + match raw_uri { + Ok(uri) => normalize_policy_graph_uri(&uri), + Err(_) => Ok("default".to_string()), + } } fn resolve_remote_bearer_token( @@ -980,7 +976,10 @@ fn resolve_cli_graph( }; config.resolve_graph_selection(selected.as_deref())?; let uri = resolve_uri(config, cli_uri, cli_target)?; + let normalized_uri = normalize_policy_graph_uri(&uri)?; + let graph_id = graph_resource_id_for_selection(selected.as_deref(), &normalized_uri); Ok(ResolvedCliGraph { + graph_id, is_remote: is_remote_uri(&uri), policy_file: config.resolve_policy_file_for(selected.as_deref()), selected, @@ -2027,7 +2026,7 @@ async fn execute_change( ) -> Result { let (selected_name, query_params) = select_named_query(query_source, query_name)?; let params = query_params_from_json(&query_params, params_json)?; - let db = open_local_db_with_policy(graph, config).await?; + let db = open_local_db_with_policy(graph).await?; let actor = resolve_cli_actor(cli_as_actor, config); let result = db .mutate_as(branch, query_source, &selected_name, ¶ms, actor) @@ -2258,7 +2257,7 @@ async fn main() -> Result<()> { let graph = resolve_local_graph(&config, uri, target.as_deref(), "load")?; let uri = graph.uri.clone(); let branch = resolve_branch(&config, branch, None, "main"); - let db = open_local_db_with_policy(&graph, &config).await?; + let db = open_local_db_with_policy(&graph).await?; let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config); let result = db .load_file_as(&branch, &data.to_string_lossy(), mode.into(), actor) @@ -2319,7 +2318,7 @@ async fn main() -> Result<()> { ) .await? } else { - let db = open_local_db_with_policy(&graph, &config).await?; + let db = open_local_db_with_policy(&graph).await?; let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config); let result = db .ingest_file_as( @@ -2366,7 +2365,7 @@ async fn main() -> Result<()> { ) .await? } else { - let db = open_local_db_with_policy(&graph, &config).await?; + let db = open_local_db_with_policy(&graph).await?; let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config); db.branch_create_from_as(ReadTarget::branch(&from), &name, actor) .await?; @@ -2439,7 +2438,7 @@ async fn main() -> Result<()> { ) .await? } else { - let db = open_local_db_with_policy(&graph, &config).await?; + let db = open_local_db_with_policy(&graph).await?; let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config); db.branch_delete_as(&name, actor).await?; BranchDeleteOutput { @@ -2481,7 +2480,7 @@ async fn main() -> Result<()> { ) .await? } else { - let db = open_local_db_with_policy(&graph, &config).await?; + let db = open_local_db_with_policy(&graph).await?; let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config); let outcome = db.branch_merge_as(&source, &into, actor).await?; BranchMergeOutput { @@ -2637,7 +2636,7 @@ async fn main() -> Result<()> { ) .await? } else { - let db = open_local_db_with_policy(&graph, &config).await?; + let db = open_local_db_with_policy(&graph).await?; let actor = resolve_cli_actor(cli.as_actor.as_deref(), &config); let registry = load_registry_or_report(&config, graph.selected())?; let registry = (!registry.is_empty()).then_some(registry); @@ -3171,7 +3170,8 @@ mod tests { use super::{ DEFAULT_BEARER_TOKEN_ENV, apply_bearer_token, bearer_token_from_env_file, legacy_change_request_body, load_cli_config, load_env_file_into_process, - normalize_bearer_token, parse_env_assignment, resolve_remote_bearer_token, + normalize_bearer_token, parse_env_assignment, resolve_policy_context, + resolve_cli_graph, resolve_remote_bearer_token, }; use omnigraph_server::load_config; use reqwest::header::AUTHORIZATION; @@ -3431,4 +3431,100 @@ graphs: } } } + + #[test] + fn graph_identity_resolve_policy_context_named_cli_graph_uses_graph_key_not_project_name_or_uri() { + let temp = tempdir().unwrap(); + let config_path = temp.path().join("omnigraph.yaml"); + fs::write( + &config_path, + r#" +project: + name: misleading-project +graphs: + local: + uri: /tmp/local-policy-graph.omni + policy: + file: ./policy.yaml +cli: + graph: local +"#, + ) + .unwrap(); + + let config = load_config(Some(&config_path)).unwrap(); + let context = resolve_policy_context(&config).unwrap(); + assert_eq!(context.graph_id, "local"); + } + + #[test] + fn graph_identity_resolve_cli_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( + &config_path, + r#" +project: + name: misleading-project +graphs: + prod: + uri: s3://bucket/prod-graph/ + policy: + file: ./prod-policy.yaml +"#, + ) + .unwrap(); + + let config = load_config(Some(&config_path)).unwrap(); + let graph = resolve_cli_graph(&config, None, Some("prod")).unwrap(); + assert_eq!(graph.selected(), Some("prod")); + assert_eq!(graph.graph_id, "prod"); + assert_eq!(graph.uri, "s3://bucket/prod-graph/"); + } + + #[test] + fn graph_identity_resolve_cli_graph_positional_uri_uses_anonymous_normalized_uri() { + let temp = tempdir().unwrap(); + let config_path = temp.path().join("omnigraph.yaml"); + fs::write( + &config_path, + r#" +project: + name: misleading-project +graphs: + local: + uri: /tmp/configured-graph.omni + policy: + file: ./policy.yaml +cli: + graph: local +"#, + ) + .unwrap(); + + let config = load_config(Some(&config_path)).unwrap(); + let local_graph_path = temp.path().join("explicit-graph.omni"); + let local_graph = resolve_cli_graph( + &config, + Some(format!("file://{}", local_graph_path.display())), + None, + ) + .unwrap(); + assert_eq!(local_graph.selected(), None); + assert_eq!( + local_graph.graph_id, + local_graph_path.to_string_lossy().as_ref() + ); + assert_eq!(local_graph.policy_file, None); + + let s3_graph = resolve_cli_graph( + &config, + Some("s3://bucket/anonymous-graph/".to_string()), + None, + ) + .unwrap(); + assert_eq!(s3_graph.selected(), None); + assert_eq!(s3_graph.graph_id, "s3://bucket/anonymous-graph"); + assert_eq!(s3_graph.policy_file, None); + } } diff --git a/crates/omnigraph-server/src/config.rs b/crates/omnigraph-server/src/config.rs index 2e83d1a..3bab877 100644 --- a/crates/omnigraph-server/src/config.rs +++ b/crates/omnigraph-server/src/config.rs @@ -9,6 +9,13 @@ use serde::{Deserialize, Serialize}; pub const DEFAULT_CONFIG_FILE: &str = "omnigraph.yaml"; +pub fn graph_resource_id_for_selection( + selected_graph: Option<&str>, + normalized_uri: &str, +) -> String { + selected_graph.unwrap_or(normalized_uri).to_string() +} + #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct ProjectConfig { pub name: Option, @@ -560,7 +567,9 @@ mod tests { use tempfile::tempdir; - use super::{ReadOutputFormat, TableCellLayout, load_config_in}; + use super::{ + ReadOutputFormat, TableCellLayout, graph_resource_id_for_selection, load_config_in, + }; #[test] fn load_config_reads_yaml_defaults_from_current_dir() { @@ -624,6 +633,18 @@ policy: {} assert!(config.graphs.is_empty()); } + #[test] + fn graph_resource_id_for_selection_uses_name_or_anonymous_uri() { + assert_eq!( + graph_resource_id_for_selection(Some("local"), "/tmp/graph.omni"), + "local" + ); + assert_eq!( + graph_resource_id_for_selection(None, "/tmp/graph.omni"), + "/tmp/graph.omni" + ); + } + #[test] fn resolve_graph_selection_validates_membership_and_coherence() { let temp = tempdir().unwrap(); diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index e84f5f7..60ebef3 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -44,7 +44,7 @@ use color_eyre::eyre::{Result, WrapErr, bail}; pub use config::{ AliasCommand, AliasConfig, CliDefaults, DEFAULT_CONFIG_FILE, OmnigraphConfig, PolicySettings, ProjectConfig, QueryDefaults, ReadOutputFormat, ServerDefaults, TableCellLayout, TargetConfig, - load_config, + graph_resource_id_for_selection, load_config, }; use futures::stream; use omnigraph::db::{Omnigraph, ReadTarget}; @@ -956,7 +956,7 @@ pub fn load_server_settings( let policy_file = config.resolve_policy_file_for(selected); let queries = QueryRegistry::load(&config, config.query_entries_for(selected)) .map_err(|errs| color_eyre::eyre::eyre!(format_registry_load_errors(&uri, &errs)))?; - let graph_id = selected.unwrap_or(uri.as_str()).to_string(); + let graph_id = graph_resource_id_for_selection(selected, &uri); ServerConfigMode::Single { uri, graph_id,